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(injection template): correct owner string format #18485

Merged
merged 2 commits into from
Oct 31, 2019
Merged

fix(injection template): correct owner string format #18485

merged 2 commits into from
Oct 31, 2019

Conversation

douglas-reid
Copy link
Contributor

@douglas-reid douglas-reid commented Oct 30, 2019 •
edited by istio-policy-bot
Loading

The ISTIO_META_OWNER field is improperly specified in the injection template. This PR fixes it.

@douglas-reid
fix(injection template): correct owner string format
9db0f34 
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
@douglas-reid douglas-reid requested a review from a team as a code owner October 30, 2019 21:34
@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 Oct 30, 2019
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 30, 2019
@douglas-reid douglas-reid added cherrypick/release-1.4 and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 30, 2019
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 30, 2019
howardjohn
howardjohn approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

What is the right format here? I have seen other similar PRs before, not sure what format this actually expects?

@douglas-reid
Copy link
Contributor Author

@howardjohn i too get a bit lost. i'm going off of the URLS in https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.13/#-strong-read-operations-deployment-v1-apps-strong-

@istio-testing istio-testing merged commit 93567f7 into istio:master Oct 31, 2019
@douglas-reid
Copy link
Contributor Author

@howardjohn did this get CP'd correctly? i don't see the bot info here.

@bianpengyuan
Copy link
Contributor

/cherry-pick release-1.4

@istio-testing
Copy link
Collaborator

@bianpengyuan: #18485 failed to apply on top of branch "release-1.4":

Applying: refresh golden files
Using index info to reconstruct a base tree...
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
M	pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected
M	pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected
M	pkg/kube/inject/testdata/inject/auth.yaml.injected
M	pkg/kube/inject/testdata/inject/cronjob.yaml.injected
M	pkg/kube/inject/testdata/inject/daemonset.yaml.injected
M	pkg/kube/inject/testdata/inject/deploymentconfig-multi.yaml.injected
M	pkg/kube/inject/testdata/inject/deploymentconfig.yaml.injected
M	pkg/kube/inject/testdata/inject/enable-core-dump.yaml.injected
M	pkg/kube/inject/testdata/inject/format-duration.yaml.injected
M	pkg/kube/inject/testdata/inject/frontend.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-always.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-config-map-name.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-ignore.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-mtls-not-ready.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-multi.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-namespace.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-never.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-proxy-override.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-template-in-values.yaml.injected
M	pkg/kube/inject/testdata/inject/hello-tproxy.yaml.injected
M	pkg/kube/inject/testdata/inject/hello.yaml.injected
M	pkg/kube/inject/testdata/inject/job.yaml.injected
M	pkg/kube/inject/testdata/inject/kubevirtInterfaces.yaml.injected
M	pkg/kube/inject/testdata/inject/kubevirtInterfaces_list.yaml.injected
M	pkg/kube/inject/testdata/inject/list-frontend.yaml.injected
M	pkg/kube/inject/testdata/inject/list.yaml.injected
M	pkg/kube/inject/testdata/inject/multi-init.yaml.injected
M	pkg/kube/inject/testdata/inject/pod.yaml.injected
M	pkg/kube/inject/testdata/inject/replicaset.yaml.injected
M	pkg/kube/inject/testdata/inject/replicationcontroller.yaml.injected
M	pkg/kube/inject/testdata/inject/statefulset.yaml.injected
M	pkg/kube/inject/testdata/inject/status_annotations.yaml.injected
M	pkg/kube/inject/testdata/inject/status_params.yaml.injected
M	pkg/kube/inject/testdata/inject/traffic-annotations-empty-includes.yaml.injected
M	pkg/kube/inject/testdata/inject/traffic-annotations-wildcards.yaml.injected
M	pkg/kube/inject/testdata/inject/traffic-annotations.yaml.injected
M	pkg/kube/inject/testdata/inject/traffic-params-empty-includes.yaml.injected
M	pkg/kube/inject/testdata/inject/traffic-params.yaml.injected
M	pkg/kube/inject/testdata/webhook/TestWebhookInject_injectorAnnotations.patch
M	pkg/kube/inject/testdata/webhook/daemonset.yaml.injected
M	pkg/kube/inject/testdata/webhook/deploymentconfig-multi.yaml.injected
M	pkg/kube/inject/testdata/webhook/deploymentconfig.yaml.injected
M	pkg/kube/inject/testdata/webhook/frontend.yaml.injected
M	pkg/kube/inject/testdata/webhook/hello-config-map-name.yaml.injected
M	pkg/kube/inject/testdata/webhook/hello-mtls-not-ready.yaml.injected
M	pkg/kube/inject/testdata/webhook/hello-multi.yaml.injected
M	pkg/kube/inject/testdata/webhook/hello-probes.yaml.injected
M	pkg/kube/inject/testdata/webhook/job.yaml.injected
M	pkg/kube/inject/testdata/webhook/list-frontend.yaml.injected
M	pkg/kube/inject/testdata/webhook/list.yaml.injected
M	pkg/kube/inject/testdata/webhook/replicaset.yaml.injected
M	pkg/kube/inject/testdata/webhook/replicationcontroller.yaml.injected
M	pkg/kube/inject/testdata/webhook/resource_annotations.yaml.injected
M	pkg/kube/inject/testdata/webhook/statefulset.yaml.injected
M	pkg/kube/inject/testdata/webhook/status_annotations.yaml.injected
M	pkg/kube/inject/testdata/webhook/traffic-annotations-empty-includes.yaml.injected
M	pkg/kube/inject/testdata/webhook/traffic-annotations-wildcards.yaml.injected
M	pkg/kube/inject/testdata/webhook/traffic-annotations.yaml.injected
M	pkg/kube/inject/testdata/webhook/user-volume.yaml.injected
Falling back to patching base and 3-way merge...
Auto-merging pkg/kube/inject/testdata/webhook/user-volume.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/traffic-annotations.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/traffic-annotations-wildcards.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/traffic-annotations-empty-includes.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/status_annotations.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/statefulset.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/resource_annotations.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/replicationcontroller.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/replicaset.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/list.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/list-frontend.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/job.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/hello-probes.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/hello-multi.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/hello-mtls-not-ready.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/hello-config-map-name.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/frontend.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/deploymentconfig.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/deploymentconfig-multi.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/daemonset.yaml.injected
Auto-merging pkg/kube/inject/testdata/webhook/TestWebhookInject_injectorAnnotations.patch
Auto-merging pkg/kube/inject/testdata/inject/traffic-params.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/traffic-params-empty-includes.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/traffic-annotations.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/traffic-annotations-wildcards.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/traffic-annotations-empty-includes.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/status_params.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/status_annotations.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/statefulset.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/replicationcontroller.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/replicaset.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/pod.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/multi-init.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/list.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/list-frontend.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/kubevirtInterfaces_list.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/kubevirtInterfaces.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/job.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-tproxy.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-template-in-values.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-proxy-override.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-never.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-namespace.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-multi.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-mtls-not-ready.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-ignore.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-config-map-name.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/hello-always.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/frontend.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/format-duration.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/enable-core-dump.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/deploymentconfig.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/deploymentconfig-multi.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/daemonset.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/cronjob.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/auth.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/auth.non-default-service-account.yaml.injected
Auto-merging pkg/kube/inject/testdata/inject/auth.cert-dir.yaml.injected
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
error: Failed to merge in the changes.
Patch failed at 0002 refresh golden files

In response to this:

/cherry-pick release-1.4

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.

@bianpengyuan
Copy link
Contributor

That label does not quite seem to work now. The command could make it do the work.

@bianpengyuan
Copy link
Contributor

But seems like we need a manual cherry-pick...

douglas-reid added a commit to douglas-reid/istio that referenced this pull request Nov 5, 2019
@douglas-reid
[release-1.4] manual cherry-pick of istio#18485
7002e2f
@douglas-reid douglas-reid mentioned this pull request Nov 5, 2019
Merged
istio-testing pushed a commit that referenced this pull request Nov 5, 2019
@douglas-reid @istio-testing
[release-1.4] manual cherry-pick of #18485 (#18649)
bb05b57
sdake pushed a commit to sdake/istio that referenced this pull request Dec 1, 2019
@douglas-reid
fix(injection template): correct owner string format (istio#18485)
c68f416 
* fix(injection template): correct owner string format

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* refresh golden files
douglas-reid added a commit that referenced this pull request Dec 11, 2019
@douglas-reid
fix(gateway): Correct ISTIO_META_OWNER env var
be4a456 
Missing the 's' in 'apis'. This update matches the changes to the injection-template.yaml in #18485.
@douglas-reid douglas-reid mentioned this pull request Dec 11, 2019
Merged
douglas-reid added a commit to istio/installer that referenced this pull request Dec 11, 2019
@douglas-reid
fix(templates): Correct OWNER urls.
b17abb2 
This PR is meant to cover fixes such as:

- istio/istio#18485
- istio/istio#19513
@douglas-reid douglas-reid mentioned this pull request Dec 11, 2019
Merged
istio-testing pushed a commit that referenced this pull request Dec 11, 2019
@douglas-reid @istio-testing
fix(gateway): Correct ISTIO_META_OWNER env var (#19513)
ca44345 
Missing the 's' in 'apis'. This update matches the changes to the injection-template.yaml in #18485.
istio-testing pushed a commit to istio-testing/istio that referenced this pull request Dec 11, 2019
@douglas-reid @istio-testing
fix(gateway): Correct ISTIO_META_OWNER env var
0198cff 
Missing the 's' in 'apis'. This update matches the changes to the injection-template.yaml in istio#18485.
istio-testing added a commit that referenced this pull request Dec 11, 2019
@istio-testing
fix(gateway): Correct ISTIO_META_OWNER env var (#19515)
1fc5ec0 
Missing the 's' in 'apis'. This update matches the changes to the injection-template.yaml in #18485.
istio-testing pushed a commit to istio/installer that referenced this pull request Dec 11, 2019
@douglas-reid @istio-testing
fix(templates): Correct OWNER urls. (#603)
d8d29ff 
* fix(templates): Correct OWNER urls.

This PR is meant to cover fixes such as:

- istio/istio#18485
- istio/istio#19513

* Update injection-template.yaml

* Update deployment.yaml
istio-testing pushed a commit to istio-testing/installer that referenced this pull request Dec 11, 2019
@douglas-reid @istio-testing
fix(templates): Correct OWNER urls.
c9498d2 
This PR is meant to cover fixes such as:

- istio/istio#18485
- istio/istio#19513
istio-testing added a commit to istio/installer that referenced this pull request Dec 11, 2019
@istio-testing
[release-1.4] fix(templates): Correct OWNER urls. (#604)
c53b065 
* fix(templates): Correct OWNER urls.

This PR is meant to cover fixes such as:

- istio/istio#18485
- istio/istio#19513

* Update injection-template.yaml

* Update deployment.yaml
brian-avery pushed a commit to brian-avery/istio that referenced this pull request Mar 4, 2020
@istio-testing
fix(gateway): Correct ISTIO_META_OWNER env var (istio#19515)
78afb53 
Missing the 's' in 'apis'. This update matches the changes to the injection-template.yaml in istio#18485.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howardjohn howardjohn howardjohn approved these changes

@geeknoid geeknoid Awaiting requested review from geeknoid

@linsun linsun Awaiting requested review from linsun

@rshriram rshriram Awaiting requested review from rshriram

Assignees
No one assigned
Labels
area/extensions and telemetry 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.

None yet
6 participants
@douglas-reid @bianpengyuan @istio-testing @howardjohn @googlebot @istio-policy-bot