OCPBUGS-85778: Fix NodePool reconciliation failure when updating mirrored immutable ConfigMaps - #8543
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
@vsolanki12: This pull request references Jira Issue OCPBUGS-85778, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR makes the reconciler delete an existing mirrored ConfigMap if it is immutable before recreating it, changes the mirrored ConfigMap mutation to explicitly set Immutable = false, and updates unit tests to expect mutable mirrored ConfigMaps across containerruntime, kubeletconfig, deletion, and immutable-existing scenarios. Sequence Diagram(s)sequenceDiagram
participant NodePoolReconciler
participant KubeAPIServer
participant ConfigMapResource
NodePoolReconciler->>KubeAPIServer: GET mirrored ConfigMap
KubeAPIServer-->>NodePoolReconciler: existing ConfigMap / NotFound
alt existing.Immutable == true and labels match nodePool
NodePoolReconciler->>KubeAPIServer: DELETE existing immutable ConfigMap
KubeAPIServer-->>NodePoolReconciler: DeleteResult
end
NodePoolReconciler->>KubeAPIServer: CreateOrUpdate mirrored ConfigMap (Immutable=false)
KubeAPIServer-->>NodePoolReconciler: CreateOrUpdateResult
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
53511be to
0c716ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hypershift-operator/controllers/nodepool/nto.go`:
- Around line 213-225: The deletion currently removes any immutable ConfigMap at
the target key; restrict it to only remove ConfigMaps that this controller
manages by checking ownership/management metadata before deleting. Update
deleteImmutableConfigMapIfNeeded to verify existingCM is owned/managed by this
controller (e.g., check for an OwnerReference with Controller==true and
Kind=="NodePool" and/or a controller management label you use for mirrored
ConfigMaps such as a "mirrored" or "managed-by" label) and only call
k8sutil.DeleteIfNeeded when that check passes; if ownership/label is absent, log
and return without deleting. Use existingCM.OwnerReferences and
existingCM.Labels in the NodePoolReconciler.deleteImmutableConfigMapIfNeeded
function to implement the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 40fe0a06-ff2c-48c8-8989-7c6b6eef2da0
📒 Files selected for processing (2)
hypershift-operator/controllers/nodepool/nto.gohypershift-operator/controllers/nodepool/nto_test.go
|
/jira refresh |
|
@vsolanki12: This pull request references Jira Issue OCPBUGS-85778, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8543 +/- ##
==========================================
+ Coverage 40.51% 41.28% +0.76%
==========================================
Files 755 755
Lines 93213 93462 +249
==========================================
+ Hits 37768 38586 +818
+ Misses 52742 52145 -597
- Partials 2703 2731 +28
... and 44 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
0c716ea to
f26d6b1
Compare
|
@vsolanki12: This pull request references Jira Issue OCPBUGS-85778, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
✅ Actions performedReview triggered.
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill, vsolanki12 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 |
|
/lgtm |
|
Scheduling tests matching the |
|
All 5 jobs have the identical failure pattern. Here is the consolidated report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll 5 Prow jobs failed with the identical root cause: a CI infrastructure issue where specific container images for OCP releases 4.19, 4.20, and 4.21 were missing from both the primary registry (quay.io) and the proxy registry (quay-proxy.ci.openshift.org). The failures occurred during the release image import phase — before any test code was executed — meaning the PR's code changes were never actually tested. Releases 4.18 and 4.22 imported successfully, indicating this was not a total registry outage but rather specific images that were unavailable or had been garbage-collected. Root CauseCI Infrastructure / Registry Issue — Not a code problem. The ci-operator multi-version test workflow requires importing multiple OCP release payloads (4.18 through 5.0) to validate upgrade and cross-version compatibility. Three specific component images were missing from both
Each image import was retried 6 times before timing out. Since these release imports are a prerequisite for the test workflow, the entire job was aborted before any test step could run. This is a transient infrastructure issue — the images were likely garbage-collected or temporarily unavailable from the CI image mirrors. The PR code changes (ConfigMap reconciliation fix) are completely unrelated to image registry operations. Recommendations
Evidence
|
|
@vsolanki12: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
|
/retest |
|
/verified by @vsolanki12 Before fix: After fix: |
|
@vsolanki12: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@vsolanki12: Jira Issue Verification Checks: Jira Issue OCPBUGS-85778 Jira Issue OCPBUGS-85778 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Fix included in release 5.0.0-0.nightly-2026-06-18-000016 |
|
/jira backport release-4.22,release-4.21,release-4.20 |
|
@vsolanki12: The following backport issues have been created:
Queuing cherrypicks to the requested branches to be created after this PR merges: DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@openshift-ci-robot: new pull request created: #8765 DetailsIn response to this:
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-sigs/prow repository. |
|
@openshift-ci-robot: new pull request created: #8766 DetailsIn response to this:
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-sigs/prow repository. |
|
@openshift-ci-robot: new pull request created: #8767 DetailsIn response to this:
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-sigs/prow repository. |
What this PR does / why we need it:
When multiple NodePools reference the same kubelet ConfigMap and the source ConfigMap is modified after mirroring, the nodepool-controller's NTO reconciliation fails because mirrored ConfigMaps are created with
immutable: true. Kubernetes rejects in-place updates to immutable ConfigMaps, causing NodePools to get stuck inUpdatingConfig=Truewith:This PR:
immutable: trueflag frommutateMirroredConfigso newly created mirrored ConfigMaps are mutabledeleteImmutableConfigMapIfNeededtoreconcileMirroredConfigsto handle migration — existing immutable mirrored ConfigMaps are deleted beforeCreateOrUpdaterecreates them as mutableWhich issue(s) this PR fixes:
Fixes https://issues.redhat.com/browse/OCPBUGS-85778
Special notes for your reviewer:
custom-kubeletConfigMapUpdatingConfig=Truewith immutable field errorChecklist:
Summary by CodeRabbit
Bug Fixes
Tests