Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix POD_PORTS when multiple containers #18976

Merged
merged 1 commit into from Nov 22, 2019
Merged

Fix POD_PORTS when multiple containers #18976

merged 1 commit into from Nov 22, 2019

Conversation

howardjohn
Copy link
Member

@howardjohn howardjohn commented Nov 15, 2019 •
edited by istio-policy-bot

Fixes #18594

Whithout this change, POD_PORTS would have an invalid json like

[,{port: 123}] (leading comma)

@howardjohn
Fix POD_PORTS when multiple containers
c2f6e97 
Fixes istio#18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 15, 2019
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 15, 2019
@howardjohn
Copy link
Member Author

Fixes #19096

@howardjohn howardjohn mentioned this pull request Nov 20, 2019
Closed
@howardjohn
Copy link
Member Author

@rshriram please take a look

@istio-testing istio-testing merged commit d4f9846 into istio:master Nov 22, 2019
@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #18976 failed to apply on top of branch "release-1.3":

Using index info to reconstruct a base tree...
M	install/kubernetes/helm/istio/files/injection-template.yaml
A	pkg/kube/inject/inject_test.go
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
A	pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
Auto-merging pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
CONFLICT (content): Merge conflict in pilot/pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
Auto-merging pilot/pkg/kube/inject/inject_test.go
Auto-merging install/kubernetes/helm/istio/files/injection-template.yaml
error: Failed to merge in the changes.
Patch failed at 0001 Fix POD_PORTS when multiple containers

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #18976 failed to apply on top of branch "release-1.4":

Using index info to reconstruct a base tree...
M	install/kubernetes/helm/istio/files/injection-template.yaml
M	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
M	pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
Falling back to patching base and 3-way merge...
Auto-merging pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/two_container.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/ready_only.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/ready_live.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/one_container.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/named_port.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/https-probes.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/hello-readiness.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/hello-probes.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-unset-in-annotation.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
CONFLICT (content): Merge conflict in pkg/kube/inject/testdata/inject/app_probe/hello-probes-with-flag-set-in-annotation.yaml.injected
Auto-merging install/kubernetes/helm/istio/files/injection-template.yaml
error: Failed to merge in the changes.
Patch failed at 0001 Fix POD_PORTS when multiple containers

sdake pushed a commit to sdake/istio that referenced this pull request Dec 1, 2019
@howardjohn
Fix POD_PORTS when multiple containers (istio#18976)
e678f9a 
Fixes istio#18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)
@nicktrav
Copy link
Contributor

nicktrav commented Dec 3, 2019

Hey @howardjohn - reckon we could get this backported into 1.4.x? Seems like this didn't land cleanly on the release-1.4 branch.

@howardjohn
Copy link
Member Author

oof.. I thought we got this in 1.4 and lost track. Thanks. I'll backport now

howardjohn added a commit to howardjohn/istio that referenced this pull request Dec 3, 2019
@howardjohn
Fix POD_PORTS when multiple containers (istio#18976)
d1ee72f 
Fixes istio#18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)

(cherry picked from commit d4f9846)
@howardjohn
Copy link
Member Author

In the meantime the change here is just to the injection template, so a workaround would be to manually modify that

howardjohn added a commit to howardjohn/istio that referenced this pull request Dec 3, 2019
@howardjohn
Fix POD_PORTS when multiple containers (istio#18976)
2600143 
Fixes istio#18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)

(cherry picked from commit d4f9846)
This was referenced Dec 3, 2019
Merged
Merged
howardjohn added a commit to howardjohn/istio-installer that referenced this pull request Dec 3, 2019
@howardjohn
Fix POD_PORTS when multiple containers
7539c86 
Manual CP of istio/istio#18976
@howardjohn howardjohn mentioned this pull request Dec 3, 2019
Merged
istio-testing pushed a commit to istio/installer that referenced this pull request Dec 4, 2019
@howardjohn @istio-testing
Fix POD_PORTS when multiple containers (#585)
fe6547e 
Manual CP of istio/istio#18976
istio-testing pushed a commit to istio-testing/installer that referenced this pull request Dec 4, 2019
@howardjohn @istio-testing
Fix POD_PORTS when multiple containers
74cff7c 
Manual CP of istio/istio#18976
istio-testing pushed a commit that referenced this pull request Dec 4, 2019
@howardjohn @istio-testing
Fix POD_PORTS when multiple containers (#18976) (#19367)
f39630a 
Fixes #18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)

(cherry picked from commit d4f9846)
haines pushed a commit to zencargo/istio-installer that referenced this pull request Dec 5, 2019
@howardjohn @haines
Fix POD_PORTS when multiple containers
b3cfc13 
Manual CP of istio/istio#18976
istio-testing added a commit to istio/installer that referenced this pull request Dec 6, 2019
@istio-testing
Fix POD_PORTS when multiple containers (#586)
2eaff78 
Manual CP of istio/istio#18976
istio-testing pushed a commit that referenced this pull request Dec 10, 2019
@howardjohn @istio-testing
Fix POD_PORTS when multiple containers (#18976) (#19366)
0cfb3fe 
Fixes #18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)

(cherry picked from commit d4f9846)
brian-avery pushed a commit to brian-avery/istio that referenced this pull request Mar 4, 2020
@howardjohn
Fix POD_PORTS when multiple containers (istio#18976) (istio#19366)
8928799 
Fixes istio#18594

Whithout this change, POD_PORTS would have an invalid json like

`[,{port: 123}]` (leading comma)

(cherry picked from commit d4f9846)
antonioberben pushed a commit to antonioberben/istio that referenced this pull request Jan 29, 2024
@naseemkullah @k8s-ci-robot
Update to 1.15.1 (istio#18976)
dbff584 
Signed-off-by: Naseem <naseemkullah@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morvencao morvencao morvencao approved these changes

@rshriram rshriram rshriram approved these changes

@geeknoid geeknoid Awaiting requested review from geeknoid

@linsun linsun Awaiting requested review from linsun

Assignees

@rshriram rshriram

@geeknoid geeknoid

Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

If there is a container that doesn't expose any port, istio-proxy won't start
7 participants
@howardjohn @istio-testing @nicktrav @morvencao @rshriram @googlebot @geeknoid