OCPNODE-4494: e2e test case for RHCOS upgrade from 9 → 10#31393
OCPNODE-4494: e2e test case for RHCOS upgrade from 9 → 10#31393asahay19 wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@asahay19: This pull request references OCPNODE-4494 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: asahay19 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 |
|
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 (2)
WalkthroughThe upgrade test suite now covers both runc guard behavior and the crun happy path for RHCOS 9-to-10 upgrades. Cleanup is parameterized by the active test pool, and documentation describes both scenarios and their health assertions. ChangesRuntime upgrade test coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UpgradeTest
participant MachineConfigPool
participant WorkerNode
participant ClusterOperator
UpgradeTest->>MachineConfigPool: Create crun upgrade pool
UpgradeTest->>WorkerNode: Label worker and verify RHCOS 9 with crun
UpgradeTest->>MachineConfigPool: Set osImageStream to rhel-10
MachineConfigPool->>WorkerNode: Roll out RHCOS 10 configuration
UpgradeTest->>MachineConfigPool: Assert no RenderDegraded
UpgradeTest->>ClusterOperator: Check DegradedPool does not block upgradeability
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (14 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
openshift-ci
Bot
added
the
ready-for-human-review
|
Scheduling required tests: |
|
@asahay19: 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. |
bitoku
left a comment
bitoku
left a comment
There was a problem hiding this comment.
overall lgtm,
One comment about the style.
There's some functions like assert*, wait*, expect*, verify*.
I don't mind either, but want them to be consistent or follow a rule (for example, expect calls Expect in the function, something like that).
| nodeName = "" | ||
| testPoolName = "" | ||
| cleanupCRC = false |
There was a problem hiding this comment.
nit: I don't think they are needed. They should be initialized when it's defined.
There was a problem hiding this comment.
Dropped redundant BeforeEach resets for nodeName / testPoolName / cleanupCRC in the latest commit
|
|
||
| g.By("Upgrading RHCOS version to RHCOS 10 via osImageStream") | ||
| o.Expect(setPoolOSImageStream(ctx, mcClient, crunRHCOS10UpgradePool, streamRHEL10)).To(o.Succeed()) | ||
| o.Expect(waitForMCP(ctx, mcClient, crunRHCOS10UpgradePool, 45*time.Minute)).To(o.Succeed(), |
There was a problem hiding this comment.
nit: I think 45 mins is too long. It's supposed to be finished much earlier.
There was a problem hiding this comment.
Reduced post-rhel-10 MCP and node waits from 45m to 30m
| "pool should roll out to rhel-10 without runc guard RenderDegraded") | ||
|
|
||
| g.By("Verifying node rolled out to RHCOS 10 with crun and pool stayed healthy") | ||
| o.Expect(waitForNodeRHELMajorVersion(ctx, oc, nodeName, "10", 45*time.Minute)).To(o.Succeed(), |
There was a problem hiding this comment.
nit: I think 45 mins is too long. It's supposed to be finished much earlier.
| if upgradeable != nil && | ||
| upgradeable.Status == configv1.ConditionFalse && | ||
| upgradeable.Reason == degradedPoolUpgradeableReason { | ||
| return fmt.Errorf("ClusterOperator %s Upgradeable=False (reason %s) after crun rhel-10 upgrade: %q", |
There was a problem hiding this comment.
Too specific error message, this may be confusing in the future when someone uses this function.
| return fmt.Errorf("ClusterOperator %s Upgradeable=False (reason %s) after crun rhel-10 upgrade: %q", | |
| return fmt.Errorf("ClusterOperator %s Upgradeable=False (reason %s): %q", |
There was a problem hiding this comment.
Generalized assertMachineConfigNotBlockedByDegradedPool error message in the latest commit
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-disruptive-longrunning-techpreview-1of2 periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-disruptive-longrunning-techpreview-2of2 |
|
@bitoku: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/07d107d0-8112-11f1-980f-e0e7979d9599-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-disruptive-longrunning-techpreview-2of2 |
|
@asahay19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/bc3277f0-81d4-11f1-91a5-e78e2058153d-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-disruptive-longrunning-techpreview-1of2 |
|
@asahay19: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/cab5adb0-81d4-11f1-9135-6cfa88222756-0 |
|
Scheduling required tests: |
|
/retest |
3 participants
Summary
Adds the crun happy-path e2e test for RHCOS 9 → 10
osImageStreamupgrades, complementing the runc guard test merged in #31266 (UC6A).On OCP 5.0–5.2 clusters with dual OS streams and crun as the default runtime, moving a pool from
rhel-9torhel-10should succeed without triggering the runc-on-RHCOS-10 guard. This test validates that recommended upgrade path.What the test does
MachineConfigPoolpinned torhel-9(no runcContainerRuntimeConfig)rhel-10RenderDegraded/Degradedco/machine-configis notUpgradeable=False(reasonDegradedPool)Changes
test/extended/node/runc_upgrade_cases.go— newIt+ helpers; sharedAfterEachparameterized by pool nametest/extended/node/runc_upgrade_cases.md— documents both UC6A and UC5/UC14Epic: OCPNODE-4013
Locally executed on Openshift cluster 5.0, it got executed successfully:
Summary by CodeRabbit
Tests
crun.crun“happy path” upgrade scenario and strengthened assertions around rollout health, node reboot behavior, and runtime preservation.runcguard andcrunscenarios.Documentation