Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Drilling into:
$ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep 'Ensure.*kube-rbac-proxy\|kube-rbac-proxy due to diff' | grep -B10 'due to diff' | head -n23 | tail -n11
I0309 20:13:30.978171 1 meta.go:22] EnsureObjectMeta after OwnerReferences openshift-machine-config-operator/kube-rbac-proxy-crio (false)
I0309 20:13:32.278156 1 core.go:17] EnsureConfigMap openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278175 1 meta.go:12] EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278182 1 meta.go:14] EnsureObjectMeta after Namespace openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278189 1 meta.go:16] EnsureObjectMeta after Name openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278198 1 meta.go:18] EnsureObjectMeta after Labels openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278207 1 meta.go:20] EnsureObjectMeta after Annotations openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278218 1 meta.go:22] EnsureObjectMeta after OwnerReferences openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278222 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false)
I0309 20:13:32.278226 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true)
I0309 20:13:32.278523 1 core.go:138] Updating ConfigMap openshift-machine-config-operator/kube-rbac-proxy due to diff: &v1.ConfigMap{
…terminal newline The manifests in the release payload have trailing newlines: $ oc adm release extract --to manifests quay.io/openshift-release-dev/ocp-release:4.20.0-x86_64 Extracted release payload from digest sha256:d1dc76522d1e235b97675b28e977cb8c452f47d39c0eb519cde02114925f91d2 created at 2025-10-16T13:35:24Z $ hexdump -C manifests/0000_80_machine-config_04_kube_rbac_proxy_config.yaml | tail -n3 00000200 6e 65 2d 63 6f 6e 66 69 67 2d 6f 70 65 72 61 74 |ne-config-operat| 00000210 6f 72 0a |or.| 00000213 and without this change, the cluster-version operator will fight with the machine-config operator over whether the data gets a trailing newline or not. For example, this 4.20 CI run [1]: $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-6cb5d49c59-4kn58_cluster-version-operator.log | grep -A47 'Updating .*kube-rbac-proxy due to diff' | head -n48 I0307 17:11:18.674086 1 core.go:138] Updating ConfigMap openshift-machine-config-operator/kube-rbac-proxy due to diff: &v1.ConfigMap{ TypeMeta: v1.TypeMeta{ - Kind: "", + Kind: "ConfigMap", - APIVersion: "", + APIVersion: "v1", }, ObjectMeta: v1.ObjectMeta{ ... // 2 identical fields Namespace: "openshift-machine-config-operator", SelfLink: "", - UID: "cd945200-3036-4ce7-9254-124a454a6b20", + UID: "", - ResourceVersion: "31148", + ResourceVersion: "", Generation: 0, - CreationTimestamp: v1.Time{Time: s"2026-03-07 16:41:43 +0000 UTC"}, + CreationTimestamp: v1.Time{}, DeletionTimestamp: nil, DeletionGracePeriodSeconds: nil, ... // 2 identical fields OwnerReferences: {{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: "version", UID: "70a5a04e-5cfd-4253-8ad0-95ab6aecf9a9", ...}}, Finalizers: nil, - ManagedFields: []v1.ManagedFieldsEntry{ - { - Manager: "cluster-version-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:13 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{},"f:metadata":{"f:annotations":{".":{},"f:include.re`..., - }, - { - Manager: "machine-config-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:15 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{"f:config-file.yaml":{}}}`, - }, - }, + ManagedFields: nil, }, Immutable: nil, Data: {"config-file.yaml": "authorization:\n resourceAttributes:\n apiVersion: v1\n reso"...}, BinaryData: nil, } I0307 17:11:18.873545 1 task_graph.go:497] Worker 0: Running job 77 (with 1 tasks) Not clear to me why that CVO diff doesn't highlight the newline divergence in the data portion, but debug logging in [2] highlights it: $ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep -1 'mergeMap' | head -n7 I0309 22:06:40.228975 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:06:40.228981 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:06:40.228996 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) -- I0309 22:07:04.385955 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:07:04.385973 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:07:04.385989 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) We don't need to do anything for 4.21 or later, because the CVO/release manifest was removed [3]. [1]: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696 [2]: openshift/cluster-version-operator#1343 [3]: openshift@645f16c#diff-eb2b2b1188ea07692750735a36a32fd4e0b29d353aaf08d48dccc35f3af7ecc3
…terminal newline The manifests in the release payload have trailing newlines: $ oc adm release extract --to manifests quay.io/openshift-release-dev/ocp-release:4.20.0-x86_64 Extracted release payload from digest sha256:d1dc76522d1e235b97675b28e977cb8c452f47d39c0eb519cde02114925f91d2 created at 2025-10-16T13:35:24Z $ hexdump -C manifests/0000_80_machine-config_04_kube_rbac_proxy_config.yaml | tail -n3 00000200 6e 65 2d 63 6f 6e 66 69 67 2d 6f 70 65 72 61 74 |ne-config-operat| 00000210 6f 72 0a |or.| 00000213 and without this change, the cluster-version operator will fight with the machine-config operator over whether the data gets a trailing newline or not. For example, this 4.20 CI run [1]: $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-6cb5d49c59-4kn58_cluster-version-operator.log | grep -A47 'Updating .*kube-rbac-proxy due to diff' | head -n48 I0307 17:11:18.674086 1 core.go:138] Updating ConfigMap openshift-machine-config-operator/kube-rbac-proxy due to diff: &v1.ConfigMap{ TypeMeta: v1.TypeMeta{ - Kind: "", + Kind: "ConfigMap", - APIVersion: "", + APIVersion: "v1", }, ObjectMeta: v1.ObjectMeta{ ... // 2 identical fields Namespace: "openshift-machine-config-operator", SelfLink: "", - UID: "cd945200-3036-4ce7-9254-124a454a6b20", + UID: "", - ResourceVersion: "31148", + ResourceVersion: "", Generation: 0, - CreationTimestamp: v1.Time{Time: s"2026-03-07 16:41:43 +0000 UTC"}, + CreationTimestamp: v1.Time{}, DeletionTimestamp: nil, DeletionGracePeriodSeconds: nil, ... // 2 identical fields OwnerReferences: {{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: "version", UID: "70a5a04e-5cfd-4253-8ad0-95ab6aecf9a9", ...}}, Finalizers: nil, - ManagedFields: []v1.ManagedFieldsEntry{ - { - Manager: "cluster-version-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:13 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{},"f:metadata":{"f:annotations":{".":{},"f:include.re`..., - }, - { - Manager: "machine-config-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:15 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{"f:config-file.yaml":{}}}`, - }, - }, + ManagedFields: nil, }, Immutable: nil, Data: {"config-file.yaml": "authorization:\n resourceAttributes:\n apiVersion: v1\n reso"...}, BinaryData: nil, } I0307 17:11:18.873545 1 task_graph.go:497] Worker 0: Running job 77 (with 1 tasks) Not clear to me why that CVO diff doesn't highlight the newline divergence in the data portion, but debug logging in [2] highlights it: $ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep -1 'mergeMap' | head -n7 I0309 22:06:40.228975 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:06:40.228981 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:06:40.228996 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) -- I0309 22:07:04.385955 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:07:04.385973 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:07:04.385989 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) We don't need to do anything for 4.21 or later, because the CVO/release manifest was removed [3]. I'm not actually clear why the two manifests/... files exist here, when the install/... manifest comes in before the machine-config operator Deployment. But synchronizing around trailing-newline seems like the smallest possible pivot to deconflict the ConfigMap, so that's the approach I'm taking here. [1]: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696 [2]: openshift/cluster-version-operator#1343 [3]: openshift@645f16c#diff-eb2b2b1188ea07692750735a36a32fd4e0b29d353aaf08d48dccc35f3af7ecc3
…terminal newline The manifests in the release payload have trailing newlines (0a): $ oc adm release extract --to manifests quay.io/openshift-release-dev/ocp-release:4.20.0-x86_64 Extracted release payload from digest sha256:d1dc76522d1e235b97675b28e977cb8c452f47d39c0eb519cde02114925f91d2 created at 2025-10-16T13:35:24Z $ hexdump -C manifests/0000_80_machine-config_04_kube_rbac_proxy_config.yaml | tail -n3 00000200 6e 65 2d 63 6f 6e 66 69 67 2d 6f 70 65 72 61 74 |ne-config-operat| 00000210 6f 72 0a |or.| 00000213 and without this change, the cluster-version operator will fight with the machine-config operator over whether the data gets a trailing newline or not. For example, this 4.20 CI run [1]: $ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696/artifacts/e2e-aws-ovn-serial/gather-extra/artifacts/pods/openshift-cluster-version_cluster-version-operator-6cb5d49c59-4kn58_cluster-version-operator.log | grep -A47 'Updating .*kube-rbac-proxy due to diff' | head -n48 I0307 17:11:18.674086 1 core.go:138] Updating ConfigMap openshift-machine-config-operator/kube-rbac-proxy due to diff: &v1.ConfigMap{ TypeMeta: v1.TypeMeta{ - Kind: "", + Kind: "ConfigMap", - APIVersion: "", + APIVersion: "v1", }, ObjectMeta: v1.ObjectMeta{ ... // 2 identical fields Namespace: "openshift-machine-config-operator", SelfLink: "", - UID: "cd945200-3036-4ce7-9254-124a454a6b20", + UID: "", - ResourceVersion: "31148", + ResourceVersion: "", Generation: 0, - CreationTimestamp: v1.Time{Time: s"2026-03-07 16:41:43 +0000 UTC"}, + CreationTimestamp: v1.Time{}, DeletionTimestamp: nil, DeletionGracePeriodSeconds: nil, ... // 2 identical fields OwnerReferences: {{APIVersion: "config.openshift.io/v1", Kind: "ClusterVersion", Name: "version", UID: "70a5a04e-5cfd-4253-8ad0-95ab6aecf9a9", ...}}, Finalizers: nil, - ManagedFields: []v1.ManagedFieldsEntry{ - { - Manager: "cluster-version-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:13 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{},"f:metadata":{"f:annotations":{".":{},"f:include.re`..., - }, - { - Manager: "machine-config-operator", - Operation: "Update", - APIVersion: "v1", - Time: s"2026-03-07 17:07:15 +0000 UTC", - FieldsType: "FieldsV1", - FieldsV1: s`{"f:data":{"f:config-file.yaml":{}}}`, - }, - }, + ManagedFields: nil, }, Immutable: nil, Data: {"config-file.yaml": "authorization:\n resourceAttributes:\n apiVersion: v1\n reso"...}, BinaryData: nil, } I0307 17:11:18.873545 1 task_graph.go:497] Worker 0: Running job 77 (with 1 tasks) Not clear to me why that CVO diff doesn't highlight the newline divergence in the data portion, but debug logging in [2] highlights it: $ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep -1 'mergeMap' | head -n7 I0309 22:06:40.228975 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:06:40.228981 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:06:40.228996 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) -- I0309 22:07:04.385955 1 core.go:19] EnsureConfigMap after EnsureObjectMeta openshift-machine-config-operator/kube-rbac-proxy (false) I0309 22:07:04.385973 1 meta.go:45] mergeMap update "config-file.yaml" from "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator" to "authorization:\n resourceAttributes:\n apiVersion: v1\n resource: namespace\n subresource: metrics\n namespace: openshift-machine-config-operator\n" (new key? true New value? true) I0309 22:07:04.385989 1 core.go:22] EnsureConfigMap after Data openshift-machine-config-operator/kube-rbac-proxy (true) We don't need to do anything for 4.21 or later, because the CVO/release manifest was removed [3]. I'm not actually clear why the two manifests/... files exist here, when the install/... manifest comes in before the machine-config operator Deployment. But synchronizing around trailing-newline seems like the smallest possible pivot to deconflict the ConfigMap, so that's the approach I'm taking here. [1]: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-openshift-release-main-nightly-4.20-e2e-aws-ovn-serial-1of2/2030318150326685696 [2]: openshift/cluster-version-operator#1343 [3]: openshift@645f16c#diff-eb2b2b1188ea07692750735a36a32fd4e0b29d353aaf08d48dccc35f3af7ecc3
| for k, v := range required { | ||
| if existingV, ok := (*existing)[k]; !ok || v != existingV { | ||
| *modified = true | ||
| klog.V(2).Infof("mergeMap update %q from %q to %q (new key? %t New value? %t)", k, existingV, v, ok, v != existingV) |
There was a problem hiding this comment.
oops, I got the sense wrong for "new key?", should have been !ok. Ah well.
|
openshift/machine-config-operator#5753 up with the fix; no need for this debugging anymore |
No description provided.