OCPBUGS-68350: In OCB to check when a image is removed the old build is triggered again and the MC should start updating directly and no new MOSB should be triggred#6150
Conversation
Signed-off-by: Eric <ebragg@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@eric200428: This pull request references Jira Issue OCPBUGS-68350, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an early-return in reconcilePoolChange: when the pool's rendered configuration name is unchanged and the current-build annotation is present, the function logs at verbosity 4 and returns nil, skipping MachineOSBuild trigger/reuse logic. ChangesRendered Configuration Change Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 10❌ Failed checks (10 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eric200428 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@eric200428: This pull request references Jira Issue OCPBUGS-68350, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
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 `@pkg/controller/build/reconciler.go`:
- Around line 1393-1397: The early-return that checks if oldRendered ==
newRendered must not run when missingAnnotation is true; update the logic around
the oldRendered/newRendered comparison to first evaluate missingAnnotation (or
move the missingAnnotation assignment earlier) and only return early when
oldRendered == newRendered AND missingAnnotation is false so the code still
proceeds to the build creation/continue path when missingAnnotation is true
(refer to the variables oldRendered, newRendered, missingAnnotation and the
subsequent build creation/continue logic in this reconciler function).
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4345e3c2-140f-4d45-be70-d0942d96d51f
📒 Files selected for processing (1)
pkg/controller/build/reconciler.go
|
@eric200428: 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. |
-Problem
When a MachineOSBuild (MOSB) image is deleted from the registry and automatically rebuilt, reapplying the same MachineConfig (MC) incorrectly triggers a new MOSB instead of directly updating the
MachineConfigPool (MCP) with the rebuilt image. The expected behavior is that the MCP should update immediately using the existing rebuilt MOSB without creating an unnecessary new build.
- What I did
Add an early check in reconcilePoolChange to skip builds when the rendered MachineConfig hasn't changed. Allowing the MachineConfigPool to update directly with the existing rebuilt image
- How to verify it
Steps:
Expected:
- Description for the changelog
Fixed an issue where reapplying an unchanged MachineConfig would incorrectly trigger a new MachineOSBuild after an image rebuild. The MachineConfigPool now updates directly when the rendered configuration hasn't changed, preventing unnecessary builds.
Summary by CodeRabbit