Skip to content

Remove Istio dependency from Eventing (Part - 5) - final cleanup#1119

Merged
knative-prow-robot merged 77 commits into
knative:masterfrom
akashrv:noistiofinal
May 1, 2019
Merged

Remove Istio dependency from Eventing (Part - 5) - final cleanup#1119
knative-prow-robot merged 77 commits into
knative:masterfrom
akashrv:noistiofinal

Conversation

@akashrv
Copy link
Copy Markdown
Contributor

@akashrv akashrv commented Apr 29, 2019

Fixes: #294
Previous PRs for reference:
#1044
#1058
#1074
#1107

Proposed Changes

  • Remove Istio sidecar from all dispatchers
  • Remove Istio sidecar from tests

Release Note

All Knative eventing dispatchers, broker ingress and broker filter service will not have side-cars.
Knative Eventing wouldn't need Istio mesh to function, however it will continue work if the mesh is present.

akashrv and others added 30 commits April 5, 2019 10:13
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 29, 2019
@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented Apr 29, 2019

/assign @Harwayne @vaikas-google

@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented Apr 29, 2019

/retest

Comment thread contrib/gcppubsub/config/gcppubsub.yaml
Comment thread pkg/apis/eventing/v1alpha1/subscription_types.go Outdated
@Harwayne
Copy link
Copy Markdown
Contributor

Will everything work correctly if Istio sidecar injection is enabled? For example, the controller used to opt-out of injection, what happens if the sidecar is present? As might happen if you are running an existing release of Knative eventing, which labels the knative-eventing namespace to forcibly inject the sidecar.

@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented Apr 30, 2019

/retest

@Harwayne
Copy link
Copy Markdown
Contributor

/lgtm
/approve
/hold

Holding until you have verified that this works both via direct installation from scratch and via upgrade from 0.5. Once you have verified that, feel free to cancel the hold yourself.

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Apr 30, 2019
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akashrv, Harwayne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 30, 2019
@Harwayne Harwayne mentioned this pull request May 1, 2019
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label May 1, 2019
@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented May 1, 2019

/lgtm
/approve
/hold

Holding until you have verified that this works both via direct installation from scratch and via upgrade from 0.5. Once you have verified that, feel free to cancel the hold yourself.

Have tested following scenarios:

  1. New gke cluster, install 0.5 (istio, serving, eventing), test each channel, upgrade and test all channels again
  2. New gke cluster, install 0.6 eventing only, test each channel
    basic test - send one single event to each channel and then to default broker and verify that the subscriber received it.

@knative-metrics-robot
Copy link
Copy Markdown

The following is the coverage report on pkg/.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/provisioners/channel_util.go 92.4% 80.0% -12.4
pkg/reconciler/containersource/containersource.go Do not exist 80.2%
pkg/reconciler/containersource/resources/deployment.go Do not exist 100.0%
pkg/reconciler/cronjobsource/resources/receive_adapter.go Do not exist 100.0%
pkg/reconciler/v1alpha1/broker/broker.go 89.8% 88.1% -1.7

@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented May 1, 2019

/retest

Copy link
Copy Markdown
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold cancel

@knative-prow-robot knative-prow-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 1, 2019
@akashrv
Copy link
Copy Markdown
Contributor Author

akashrv commented May 1, 2019

/retest

@knative-prow-robot knative-prow-robot merged commit da72a31 into knative:master May 1, 2019
matzew pushed a commit to matzew/eventing that referenced this pull request Feb 25, 2025
Co-authored-by: Christoph Stäbler <cstabler@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove Istio as a dependency

6 participants