Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Explicitly disallow istio sidecar injection for certain workloads #1476

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

yolocs
Copy link
Member

@yolocs yolocs commented Jul 16, 2020

Fixes #1474

Proposed Changes

Release Note

Explicitly disallow istio sidecar injection for controller/webhook/broker fanout/retry

Docs

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yolocs

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

The pull request process is described here

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

@googlebot googlebot added the cla: yes (override cla status due to multiple authors bug) label Jul 16, 2020
@knative-test-reporter-robot

The following jobs failed:

Test name Triggers Retries
pull-google-knative-gcp-integration-tests 2020-07-16 23:13:00.693 +0000 UTC 1/3

Automatically retrying due to test flakiness...
/test pull-google-knative-gcp-integration-tests

@knative-test-reporter-robot

The following jobs failed:

Test name Triggers Retries
pull-google-knative-gcp-wi-tests 2020-07-16 23:13:00.693 +0000 UTC 1/3

Automatically retrying due to test flakiness...
/test pull-google-knative-gcp-wi-tests

@yolocs
Copy link
Member Author

yolocs commented Jul 17, 2020

/retest

3 similar comments
@yolocs
Copy link
Member Author

yolocs commented Jul 17, 2020

/retest

@yolocs
Copy link
Member Author

yolocs commented Jul 17, 2020

/retest

@yolocs
Copy link
Member Author

yolocs commented Jul 17, 2020

/retest

@grac3gao-zz
Copy link
Contributor

/lgtm

Copy link
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.

For my own edification, which of our infrastructure pieces should have the side car?

Only ingress? Why not fanout/retry, which send to other possibly side-car'ed Pods?

@yolocs
Copy link
Member Author

yolocs commented Jul 20, 2020

@Harwayne Fanout/retry are pull based so no inbound policy enforcement is needed. We haven't clearly identified how to use istio for outbound traffic to other sidecar'ed pods (there are several possibilities though). So to minimize impact, I choose to disable sidecar for fanout/retry at the moment. You think it's a better idea to enable them now?

@yolocs
Copy link
Member Author

yolocs commented Jul 20, 2020

/retest

1 similar comment
@yolocs
Copy link
Member Author

yolocs commented Jul 20, 2020

/retest

@knative-prow-robot knative-prow-robot merged commit 778934f into google:master Jul 20, 2020
@yolocs yolocs deleted the b/disablesidecar branch July 20, 2020 22:18
@upodroid
Copy link

Hi @yolocs

I need to have the istio sidecars enabled for all these workloads, where do I set the allowIstioSidecar: true value?

Thank you

@yolocs
Copy link
Member Author

yolocs commented Sep 17, 2020

@upodroid The parameter is internal. E.g. https://github.com/google/knative-gcp/blob/master/pkg/reconciler/brokercell/brokercell.go#L238

If you can create an issue, I will help make the change :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Istio sidecar inject should be explicitly disabled on some workloads
7 participants