fix istio-revision-tag-default MutatingWebhookConfigurations is not created during installation#56004
Conversation
|
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test? Courtesy of your friendly test nag. |
…reated during installation Signed-off-by: xin.li <xin.li@daocloud.io>
|
/test integ-ambient-mc |
|
@my-git9: The following test failed, say
DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
| func PreviousInstallExists(ctx context.Context, client kubernetes.Interface) bool { | ||
| mwhs, err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().List(ctx, metav1.ListOptions{ | ||
| LabelSelector: "app=sidecar-injector", | ||
| LabelSelector: "app=sidecar-injector, istio.io/tag=default", |
There was a problem hiding this comment.
So the old logic for this function in 1.23 was the same (no istio.io/tag). However, this logic seemed to work in 1.23...?
Do we understand why? I am worried we have no tests here so may not be quite targeting the right thing.
Note this PreviousInstallExists is also used in two places, and now no longer really means "previous install exists" so we may want to rename it and make sure its appropriate for all usages?
There was a problem hiding this comment.
The reason why this part had problems before is:
- During the installation process, mainfest (including MutatingWebhook) will be installed first, and then it will be detected whether the tag operation needs to be performed, https://github.com/istio/istio/blob/master/operator/pkg/webhook/webhook.go#L42-L43
- In the detection logic, if there is already a MutatingWebhook with the label "app=sidecar-injector" or the revision specified for this installation is default, a tag operation is required:
https://github.com/istio/istio/blob/master/operator/pkg/webhook/webhook.go#L171-L176
https://github.com/istio/istio/blob/master/istioctl/pkg/tag/util.go#L107-L115 - But the problem lies in the first point. Since MutatingWebhook is installed first, the MutatingWebhook installed here It must contain the "app=sidecar-injector" tag, so the second point determines that the MutatingWebhook is filtered by "app=sidecar-injector", which is always true, that is, there is only one effective judgment: whether the revision is default
Another part of the use of the PreviousInstallExists function is in CheckWebhooks: https://github.com/istio/istio/blob/master/operator/pkg/webhook/webhook.go#L78
The function should be to determine whether the revision of the MutatingWebhook of the default tag is default if a tag operation is required, otherwise a warning is issued. According to the above logic, this judgment should have been inaccurate before:
https://github.com/istio/istio/blob/master/operator/pkg/webhook/webhook.go#L144-L156
…reated during installation (istio#56004) Signed-off-by: xin.li <xin.li@daocloud.io>
…reated during installation (istio#56004) Signed-off-by: xin.li <xin.li@daocloud.io>
* upstream/master: remove 1.23 compatibility profile (istio#56023) Create ambient multinetwork flag (istio#55991) krt: make krt and kube client indexes named (istio#55999) Automator: update proxy@master in istio/istio@master (istio#56026) fix istio-revision-tag-default MutatingWebhookConfigurations is not created during installation (istio#56004) doc: fix typo in accesslog test (istio#55117) fix(networking): Add absolute FQDNs (trailing dot) to VirtualHost domains (istio#56008)
Please provide a description of this PR:
fix istio-revision-tag-default MutatingWebhookConfiguration is not created during installation when the revision is not default
Fix #55980