Enable Istio Sidecar injection for activator by nikkithurmond · Pull Request #966 · knative/serving

nikkithurmond · 2018-05-24T23:26:44Z

Enable istio sidecar injection for activator.

Fixes #838

Proposed Changes

Enable istio sidecar injection for activator.
Since istio does not support direct pod ip access istio/istio#3398, we need to use FQDN when proxy requests.

See #833 for the history of this PR

Release Note

NONE

nikkithurmond · 2018-05-24T23:56:57Z

/assign mdemirhan
/assign tcnghia
/assign josephburnett

mdemirhan · 2018-05-25T15:06:03Z

Hard coded sleeps are non-portable and a value that works in one environment is unlikely to work in another unless a real large value is chosen. Instead of this, is there a way to call the readiness probe of a pod (if one exists) and decide the readiness based on that? If that pod doesn't have one, then a hard coded sleep as a fallback might seem ok.

Agreed. I'm going to file a github issue to replace this with a call to the readinessProbe.

@mdemirhan Is that good for now?

Yes, sorry for taking this long to respond. I completely missed this one.

nikkithurmond · 2018-05-25T18:09:44Z

/retest

mdemirhan · 2018-05-25T23:14:19Z

/lgtm

tcnghia · 2018-05-25T23:16:20Z

/lgtm
/approve

steuhs · 2018-06-04T18:28:56Z

/test pull-knative-serving-go-coverage

knative-metrics-robot · 2018-06-05T23:08:46Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5%
pkg/activator/revision_test.go	76.1%	79.5%	3.5%

*TestCoverage feature is being tested, do not rely on any info here yet

knative-metrics-robot · 2018-06-05T23:11:53Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5%
pkg/activator/revision_test.go	76.1%	79.5%	3.5%

*TestCoverage feature is being tested, do not rely on any info here yet

nikkithurmond · 2018-06-05T23:12:56Z

/retest

nikkithurmond · 2018-06-06T17:45:20Z

/retest

ZhiminXiang · 2018-06-06T18:08:19Z

By adding this label, seems like Istio sidecar will be injected into all of pods under knative-serving-system (including controller, webhook, etc.) according to my experiment. Is this what we want in this PR?

Probably not. I'll work on it.

I don't know, looks like you enable sidecar injection for an entire namespace https://istio.io/docs/setup/kubernetes/sidecar-injection/#deploying-an-app

I think I might need to disable it on a case by case basis.

There, that should do it.

yea. or maybe we can move activator and autoscaler into a separate namespace? Just a thought :)

I'll leave that for when we make the autoscaler multitenant

https://istio.io/docs/setup/kubernetes/sidecar-injection/#policy suggests that it should be possible to have this governed by an annotation, but I haven't been able to make this work with experimentation.

knative-metrics-robot · 2018-06-06T18:51:22Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5%
pkg/activator/revision_test.go	76.1%	79.5%	3.5%

*TestCoverage feature is being tested, do not rely on any info here yet

mdemirhan · 2018-06-06T19:11:16Z

/lgtm
/approve

nikkithurmond · 2018-06-06T19:13:06Z

/assign @vaikas-google
Assigning to an owner

mdemirhan · 2018-06-06T19:13:40Z

Probably a mistake. Should revert :)

mdemirhan · 2018-06-06T19:13:45Z

Probably a mistake. Should revert :)

Enable istio sidecar injection for activator.

mdemirhan · 2018-06-06T19:16:10Z

/lgtm

knative-metrics-robot · 2018-06-06T19:17:09Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5%
pkg/activator/revision_test.go	76.1%	79.5%	3.5%

*TestCoverage feature is being tested, do not rely on any info here yet

nikkithurmond · 2018-06-07T18:08:35Z

PTAL

/assign @evankanderson
/assign @mattmoor

evankanderson

/lgtm
/approve

evankanderson · 2018-06-08T22:09:48Z

https://istio.io/docs/setup/kubernetes/sidecar-injection/#policy suggests that it should be possible to have this governed by an annotation, but I haven't been able to make this work with experimentation.

google-prow-robot · 2018-06-08T22:34:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: evankanderson, mdemirhan, nikkithurmond, tcnghia

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:

~~OWNERS~~ [evankanderson]

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

knative-metrics-robot · 2018-06-08T22:36:54Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5
pkg/activator/revision_test.go	76.1%	79.5%	3.5

*TestCoverage feature is being tested, do not rely on any info here yet

nikkithurmond · 2018-06-11T18:13:16Z

/retest

nikkithurmond · 2018-06-11T18:15:49Z

/retest

nikkithurmond · 2018-06-11T18:18:54Z

/retest

nikkithurmond · 2018-06-11T18:19:24Z

/test pull-knative-serving-integration-tests

nikkithurmond · 2018-06-11T18:39:27Z

/test pull-knative-serving-integration-tests

knative-metrics-robot · 2018-06-11T18:57:46Z

The following is the coverage report on pkg/. Say /test pull-knative-serving-go-coverage to run the coverage report again

File	Old Coverage	New Coverage	Delta
pkg/activator/revision.go	76.1%	79.5%	3.5
pkg/activator/revision_test.go	76.1%	79.5%	3.5

*TestCoverage feature is being tested, do not rely on any info here yet

google-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 24, 2018

nikkithurmond mentioned this pull request May 24, 2018

Enable Istio Sidecar injection for activator #833

Closed

google-prow-robot assigned josephburnett, mdemirhan and tcnghia May 24, 2018

mdemirhan suggested changes May 25, 2018

View reviewed changes

nikkithurmond force-pushed the prototyping branch 2 times, most recently from 007c0e1 to 9932ea4 Compare May 25, 2018 17:32

mdemirhan approved these changes May 25, 2018

View reviewed changes

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label May 25, 2018

josephburnett approved these changes Jun 1, 2018

View reviewed changes

iterating

b839f91

nikkithurmond force-pushed the prototyping branch from 9932ea4 to 5ed60a6 Compare June 5, 2018 22:54

google-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 5, 2018

nikkithurmond force-pushed the prototyping branch 2 times, most recently from 0db67c1 to 24afbca Compare June 5, 2018 23:08

ZhiminXiang reviewed Jun 6, 2018

View reviewed changes

nikkithurmond force-pushed the prototyping branch from 24afbca to 179a0b9 Compare June 6, 2018 18:49

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2018

google-prow-robot assigned vaikas Jun 6, 2018

mdemirhan suggested changes Jun 6, 2018

View reviewed changes

Enable Istio Sidecar injection for activator

2f81380

Enable istio sidecar injection for activator.

nikkithurmond force-pushed the prototyping branch from 179a0b9 to 2f81380 Compare June 6, 2018 19:15

google-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2018

mdemirhan approved these changes Jun 6, 2018

View reviewed changes

google-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2018

ZhiminXiang mentioned this pull request Jun 6, 2018

Inject Istio sidecar into autoscaler #1080

Merged

google-prow-robot assigned evankanderson and mattmoor Jun 7, 2018

evankanderson approved these changes Jun 8, 2018

View reviewed changes

google-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2018

google-prow-robot merged commit bcc86d3 into knative:master Jun 11, 2018

nikkithurmond deleted the prototyping branch June 11, 2018 22:46

nak3 mentioned this pull request Aug 13, 2019

No longer auto-installing maistra on openshift [SRVKS-196] openshift-knative/knative-serving-operator#52

Merged

nak3 mentioned this pull request Nov 20, 2019

Service without selector does not work with mTLS when switching the endpoint istio/istio#17837

Closed

Conversation

nikkithurmond commented May 24, 2018

Uh oh!

nikkithurmond commented May 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nikkithurmond commented May 25, 2018

Uh oh!

mdemirhan commented May 25, 2018

Uh oh!

tcnghia commented May 25, 2018

Uh oh!

steuhs commented Jun 4, 2018

Uh oh!

knative-metrics-robot commented Jun 5, 2018

Uh oh!

knative-metrics-robot commented Jun 5, 2018

Uh oh!

nikkithurmond commented Jun 5, 2018

Uh oh!

nikkithurmond commented Jun 6, 2018

Uh oh!

ZhiminXiang Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knative-metrics-robot commented Jun 6, 2018

Uh oh!

mdemirhan commented Jun 6, 2018

Uh oh!

nikkithurmond commented Jun 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdemirhan commented Jun 6, 2018

Uh oh!

knative-metrics-robot commented Jun 6, 2018

Uh oh!

nikkithurmond commented Jun 7, 2018

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

google-prow-robot commented Jun 8, 2018

Uh oh!

knative-metrics-robot commented Jun 8, 2018

Uh oh!

nikkithurmond commented Jun 11, 2018

Uh oh!

nikkithurmond commented Jun 11, 2018

Uh oh!

nikkithurmond commented Jun 11, 2018

Uh oh!

nikkithurmond commented Jun 11, 2018

Uh oh!

ZhiminXiang Jun 6, 2018 •

edited

Loading