Updating contents of block-runc-on-rhcos10-upgrade - #2071
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
WalkthroughThe enhancement document updates its metadata, adds user personas, records the status-reporting decision, and expands E2E, release, and support-testing guidance for RHCOS 10 upgrades. ChangesRHCOS 10 upgrade validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ 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 |
|
PTAL @cpmeadors @bitoku |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md`:
- Around line 400-402: Update the “Infrastructure Needed [optional]” section to
align with the release strategy: state that no new cluster infrastructure is
required, while explicitly noting that custom payload jobs are still needed;
alternatively remove the stale section.
- Around line 297-302: Update the “Blocked” definition and corresponding
rollback text to state that the blocked condition clears when the target RHCOS
version is no longer RHEL 10 or when runc is no longer effective. Keep the
existing behavior for RHEL 10 with effective runc, including the
Upgradeable=False condition and DegradedPool reason.
- Around line 285-289: Update the RHEL 10 OSImageURL scenarios in the scenario
table to require an OSImageURL value whose effective OSImageStream differs from
the default stream, ensuring the URL-specific validator runs. Apply this
precondition to all URL variants, including the scenarios around the referenced
alternatives, or explicitly identify the validator exercised by each scenario.
- Line 313: Update the mixed drop-in scenario in the table to define observable
outcomes for each alphabetical ordering: when the final effective runtime is
runc, expect the RHEL 10 guard to block and RenderDegraded to be set; when the
final effective runtime is crun, expect the configuration to be accepted and
RenderDegraded not to be set. Retain the coverage of CRI-O’s last-file-wins
semantics.
🪄 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: Pro Plus
Run ID: 5ea9438d-e766-4c0a-a6aa-39b90cb1aff9
📒 Files selected for processing (1)
enhancements/machine-config/block-runc-on-rhcos10-upgrade.md
45fc620 to
f4fc53d
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 `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md`:
- Around line 335-338: Resolve the contradictory skip policy in the e2e suite
description: make clusters missing either the rhel-9 or rhel-10 stream
automatically skip, consistent with the stated dual-stream precondition, or
explicitly define this as a hard validation and remove it from the documented
skip behavior. Update the affected skip-policy text together.
🪄 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: Pro Plus
Run ID: c09567e4-8464-4224-b569-1b8032b254d9
📒 Files selected for processing (1)
enhancements/machine-config/block-runc-on-rhcos10-upgrade.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
enhancements/machine-config/block-runc-on-rhcos10-upgrade.md (2)
326-326: 🎯 Functional Correctness | 🟡 MinorDefine observable outcomes for each last-wins ordering.
Last winsdoes not define an E2E acceptance result. Specify the final effective runtime and expected outcome for each ordering: on RHEL 10, finalruncmust block withRenderDegraded=True, while finalcrunmust succeed withRenderDegraded=False. On RHCOS 9, both runtimes should succeed.🤖 Prompt for 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. In `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md` at line 326, Update the “Multiple drop-ins, last wins” acceptance criteria to specify outcomes for each ordering: on RHEL 10, an effective final runc must block and set RenderDegraded=True, while an effective final crun must succeed with RenderDegraded=False; on RHCOS 9, both effective runtimes must succeed. Keep the coverage tied to DetectRuncInMachineConfig() and CRI-O’s alphabetical last-file-wins behavior.
310-315: 🎯 Functional Correctness | 🟠 MajorMake the blocked-state exit condition consistent with rollback.
The definition says
Upgradeable=Falsepersists until theruncconfiguration is removed. The rollback scenario allows RHCOS 9 with any runtime. State that the block clears when the target is no longer RHEL 10 or whenruncis no longer effective.Also applies to: 325-337
🤖 Prompt for 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. In `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md` around lines 310 - 315, Update the “Blocked” definition and rollback scenario to state that the block clears when the upgrade target is no longer RHEL 10 or when runc is no longer effective, including when the runc configuration is removed. Preserve the existing RHCOS 9 behavior allowing any runtime and keep the Upgradeable=False condition while both blocking conditions remain.
🤖 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.
Duplicate comments:
In `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md`:
- Line 326: Update the “Multiple drop-ins, last wins” acceptance criteria to
specify outcomes for each ordering: on RHEL 10, an effective final runc must
block and set RenderDegraded=True, while an effective final crun must succeed
with RenderDegraded=False; on RHCOS 9, both effective runtimes must succeed.
Keep the coverage tied to DetectRuncInMachineConfig() and CRI-O’s alphabetical
last-file-wins behavior.
- Around line 310-315: Update the “Blocked” definition and rollback scenario to
state that the block clears when the upgrade target is no longer RHEL 10 or when
runc is no longer effective, including when the runc configuration is removed.
Preserve the existing RHCOS 9 behavior allowing any runtime and keep the
Upgradeable=False condition while both blocking conditions remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d4f01eb-ed21-4cda-a845-41861ecc3438
📒 Files selected for processing (1)
enhancements/machine-config/block-runc-on-rhcos10-upgrade.md
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 `@enhancements/machine-config/block-runc-on-rhcos10-upgrade.md`:
- Around line 314-328: Update the blocked-state definition in the
“Succeed”/“Blocked” criteria to remove the requirement that ClusterVersion
reports Upgradeable=False. Keep the MachineConfigPool degradation and
co/machine-config behavior, while explicitly preserving that this guard must not
block the OCP version upgrade.
🪄 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: Pro Plus
Run ID: 4ec2b97e-0cf9-41a2-a699-1a24662b504c
📒 Files selected for processing (1)
enhancements/machine-config/block-runc-on-rhcos10-upgrade.md
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@asahay19: all tests passed! 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. |
1 participant
This PR is regarding Updating/Adding contents to the enhancement proposal for a guard in the MCO render controller that blocks MachineConfigPool rendering when the pool targets RHEL 10 and runc is still configured as the default runtime.
What changed
terms of observable
MachineConfigPool/ClusterOperator/ClusterVersionconditions, a "What is validated" column per scenario, and a note
explaining the
OSImageURLvsOSImageStreamguard-selection precondition(
isOSImageURLOverridden).end-to-end across
machine-config-operator(unit tests + manualvalidation) and
origin(e2e automation + cross-repository payloadvalidation) prior to merge.
automatically (MicroShift, Hypershift, SNO, missing dual OS image
streams) instead of failing on environments where its preconditions don't
hold.
affected by this guard and their concerns.
Summary by CodeRabbit