Fix #10380: Remove hardcoded sidecar template for istioctl kube-inject by mathspanda · Pull Request #10830 · istio/istio

mathspanda · 2019-01-09T14:25:51Z

Fix issue #10380

Remove hardcoded sidecar template in kube-inject.
Modify some tests to read sidecar template from install/kubernetes/helm.
Remove part of meaningless tests for injecting.

Signed-off-by: mathspanda mathspanda826@gmail.com

istio-testing · 2019-01-09T14:26:06Z

Hi @mathspanda. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mathspanda · 2019-01-09T14:26:27Z

Previous pr #10690 is closed, for wrong merge branch.

istioctl/cmd/istioctl/kubeinject.go

pilot/pkg/kube/inject/inject_test.go

mathspanda · 2019-01-10T02:26:23Z

/assign @incfly @mandarjog

incfly

Thanks for the change.

pilot/pkg/kube/inject/inject.go

incfly · 2019-01-14T17:40:10Z

pilot/pkg/kube/inject/inject_test.go

Does this mean we don't have a debugMode option in helm template to control the Docker image used?

Yes. We do not hava option debugMode in helm template. Do we need to add one?

It'll be nice if you can add one, but also totally fine to do it later as separate PR.

pilot/pkg/kube/inject/mesh.go

pilot/pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected

Makefile

mathspanda · 2019-01-18T09:49:23Z

@incfly PTAL.

stale · 2019-02-01T11:32:21Z

This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

howardjohn · 2019-04-10T14:59:57Z

/lgtm

howardjohn · 2019-04-10T15:00:45Z

/assign @geeknoid @costinm

linsun · 2019-04-11T02:22:00Z

/approve

can you also check if there is any doc we need to update?

mathspanda · 2019-04-11T03:29:26Z

/test istio-unit-tests

howardjohn · 2019-04-11T03:32:04Z

Tests seem to be consistently failing on my PR too, not sure why they are failing every time now and didn't fail in post submit

hzxuzhonghu · 2019-04-11T06:43:11Z

The unit test failure caused by certificates expired. I have a fix #13233

geeknoid · 2019-04-11T14:11:58Z

/lgtm
/approve

geeknoid · 2019-04-11T14:12:33Z

/test istio-unit-tests

howardjohn · 2019-04-11T15:09:03Z

@mathspanda if you didnt already you need to rebase to pick up the fix for the unit tests

mathspanda · 2019-04-11T15:36:43Z

yes, I have rebased to pick up #13233 .

howardjohn · 2019-04-11T16:50:53Z

/lgtm

howardjohn · 2019-04-11T16:51:06Z

/lgtm

istio-testing · 2019-04-11T16:51:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:51:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:51:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:51:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:51:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:52:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

howardjohn · 2019-04-11T16:52:19Z

/test istio-integ-k8s-tests

Test seems stuck? Maybe related to github issues going on currently

istio-testing · 2019-04-11T16:53:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

istio-testing · 2019-04-11T16:55:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geeknoid, howardjohn, incfly, linsun, mathspanda

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~~ [geeknoid,linsun]

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

howardjohn · 2019-04-11T17:35:24Z

/test e2e-dashboard
/test e2e-simpleTests
/test e2e-bookInfoTests-trustdomain
/test e2e-mixer-no_auth

istio-testing · 2019-04-11T19:09:02Z

@mathspanda: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/istio-pilot-multicluster-e2e.sh	`aec8729`	link	`/test istio-pilot-multicluster-e2e`
prow/istio-integ-k8s-tests.sh	`aec8729`	link	`/test istio-integ-k8s-tests`

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

istio-testing requested review from geeknoid, kyessenov, nmittler and vadimeisenbergibm January 9, 2019 14:25

istio-testing added the needs-ok-to-test label Jan 9, 2019

googlebot added the cla: yes label Jan 9, 2019

mathspanda changed the title ~~Fix #10380: Remove hardcoded sidecar template for istioctl kube-inject~~ [WIP] Fix #10380: Remove hardcoded sidecar template for istioctl kube-inject Jan 9, 2019

istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jan 9, 2019

golangcibot reviewed Jan 9, 2019

View reviewed changes

istioctl/cmd/istioctl/kubeinject.go Outdated Show resolved Hide resolved

pilot/pkg/kube/inject/inject_test.go Outdated Show resolved Hide resolved

mathspanda force-pushed the rm-inject-tpl branch 5 times, most recently from 3011937 to b242f83 Compare January 9, 2019 16:33

mathspanda changed the title ~~[WIP] Fix #10380: Remove hardcoded sidecar template for istioctl kube-inject~~ Fix #10380: Remove hardcoded sidecar template for istioctl kube-inject Jan 10, 2019

istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jan 10, 2019

istio-testing assigned incfly and mandarjog Jan 10, 2019

incfly suggested changes Jan 14, 2019

View reviewed changes

mathspanda force-pushed the rm-inject-tpl branch 6 times, most recently from 8f45894 to c17111b Compare January 18, 2019 09:12

mathspanda added 4 commits April 11, 2019 22:42

Remove the hardcoded sidecar template for

a886f7b

Remove deprecated flags in istioctl kube-inject

fbf11be

update testdata after rebase

7644bfa

add rule for kubeinject.go in codecov.threshold

aec8729

mathspanda mentioned this pull request Apr 12, 2019

Add debugMode option in sidecar helm template #13263

Closed

esnible mentioned this pull request Jun 21, 2019

istioctl kube-inject w/o cluster has additional required arguments in 1.2 #15061

Closed

douglas-reid mentioned this pull request Aug 15, 2019

Bad merge reflected in kube inject golden file #16328

Closed

Conversation

mathspanda commented Jan 9, 2019

Uh oh!

istio-testing commented Jan 9, 2019

Uh oh!

mathspanda commented Jan 9, 2019

Uh oh!

Uh oh!

Uh oh!

mathspanda commented Jan 10, 2019

Uh oh!

incfly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

incfly Jan 14, 2019

Choose a reason for hiding this comment

Uh oh!

mathspanda Jan 15, 2019

Choose a reason for hiding this comment

Uh oh!

incfly Feb 4, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mathspanda commented Jan 18, 2019

Uh oh!

stale bot commented Feb 1, 2019

Uh oh!

howardjohn commented Apr 10, 2019

Uh oh!

howardjohn commented Apr 10, 2019

Uh oh!

linsun commented Apr 11, 2019

Uh oh!

mathspanda commented Apr 11, 2019

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

hzxuzhonghu commented Apr 11, 2019

Uh oh!

geeknoid commented Apr 11, 2019

Uh oh!

geeknoid commented Apr 11, 2019

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

mathspanda commented Apr 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

howardjohn commented Apr 11, 2019

Uh oh!

istio-testing commented Apr 11, 2019

Uh oh!

Reviewers

Assignees

mathspanda commented Apr 11, 2019 •

edited

Loading