test 5.0 cri-o revert#80786
Conversation
|
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughTwo OpenShift QE workload scripts are updated to apply RHCOS MachineConfig patches pinning osImageURL across worker, master, and infra node roles, then wait for cluster stabilization before proceeding with workload execution. A configuration file adds the ChangesRHCOS MachineConfig patching for CI workloads
Sequence DiagramssequenceDiagram
participant Script as build-farm script
participant OC as oc CLI
participant Cluster as OpenShift Cluster
Script->>OC: apply MachineConfig List (worker/master/infra osImageURL)
OC->>Cluster: submit MachineConfig resources
Cluster->>Cluster: reconcile MachineConfigs across nodes
Script->>OC: adm wait-for-stable-cluster --minimum-stable-period 5m
OC->>Cluster: poll cluster stability
Cluster-->>OC: cluster stable (5 minutes)
OC-->>Script: return success
Script->>Script: proceed with workload execution
sequenceDiagram
participant Script as workers-infra-workload script
participant OC as oc CLI
participant Cluster as OpenShift Cluster
Script->>OC: apply MachineConfig List (worker/master/infra osImageURL)
OC->>Cluster: submit MachineConfig resources
Cluster->>Cluster: reconcile MachineConfigs across node roles
Script->>OC: adm wait-for-stable-cluster --minimum-stable-period 2m
OC->>Cluster: poll cluster stability
Cluster-->>OC: cluster stable (2 minutes)
OC-->>Script: return success
Script->>Script: resume workload execution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
`@ci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.sh`:
- Around line 553-585: Remove the echo statement containing the developer name
"mcornea" on line 553, as it indicates this is temporary test code in shared
infrastructure. Either remove the entire oc apply block with the MachineConfig
resources for os-layer-custom-worker, os-layer-custom-master, and
os-layer-custom-infra if this is a temporary test, or if this change is intended
to be permanent, remove the developer attribution from the echo statement and
add documentation explaining what the hardcoded osImageURL digest is pinning to
and the rationale for using it.
🪄 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: 57fa0cc8-076e-46c1-ae9a-46f8c3766fba
📒 Files selected for processing (1)
ci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.sh
| echo "mcornea patching rhcos version with cri-o 1.35.2-5.rhaos4.22 and wait" | ||
| # Apply the changes | ||
| oc apply -f- <<EOF | ||
| apiVersion: v1 | ||
| items: | ||
| - apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: MachineConfig | ||
| metadata: | ||
| labels: | ||
| machineconfiguration.openshift.io/role: worker | ||
| name: os-layer-custom-worker | ||
| spec: | ||
| osImageURL: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:81e97c192d3fed112c182dba8c4bbbbb6b1c15dfb3cf9ee4f3585267ba53ef16 | ||
| - apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: MachineConfig | ||
| metadata: | ||
| labels: | ||
| machineconfiguration.openshift.io/role: master | ||
| name: os-layer-custom-master | ||
| spec: | ||
| osImageURL: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:81e97c192d3fed112c182dba8c4bbbbb6b1c15dfb3cf9ee4f3585267ba53ef16 | ||
| - apiVersion: machineconfiguration.openshift.io/v1 | ||
| kind: MachineConfig | ||
| metadata: | ||
| labels: | ||
| machineconfiguration.openshift.io/role: infra | ||
| name: os-layer-custom-infra | ||
| spec: | ||
| osImageURL: quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:81e97c192d3fed112c182dba8c4bbbbb6b1c15dfb3cf9ee4f3585267ba53ef16 | ||
| kind: List | ||
| metadata: | ||
| resourceVersion: "" | ||
| EOF |
There was a problem hiding this comment.
Test/debug code appears to be intended for merge into shared infrastructure.
The echo on line 553 containing "mcornea patching..." indicates this is developer-specific test code. Based on the PR title "test 5.0 cri-o revert", this appears to be a temporary testing change rather than a permanent addition to the step registry.
Key concerns:
- Developer name in echo statement suggests this is not production-ready
- Hardcoded
osImageURLdigest with no documentation explaining what cri-o version or RHCOS content it pins to - This modifies shared QE infrastructure that other jobs depend on
If this is meant to be a temporary test, consider using a separate test workflow or configuration variant instead of modifying the shared step. If this change is intended to be permanent, please remove the developer attribution and add documentation about the pinned image.
🤖 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
`@ci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.sh`
around lines 553 - 585, Remove the echo statement containing the developer name
"mcornea" on line 553, as it indicates this is temporary test code in shared
infrastructure. Either remove the entire oc apply block with the MachineConfig
resources for os-layer-custom-worker, os-layer-custom-master, and
os-layer-custom-infra if this is a temporary test, or if this change is intended
to be permanent, remove the developer attribution from the echo statement and
add documentation explaining what the hardcoded osImageURL digest is pinning to
and the rationale for using it.
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-5.0-nightly-x86-payload-control-plane-6nodes |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-5.0-nightly-x86-payload-control-plane-6nodes |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-5.0-nightly-x86-payload-control-plane-6nodes |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/openshift-qe/build-farm/openshift-qe-build-farm-commands.sh (1)
61-61: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winHarden the stability wait with availability check and explicit timeout.
Line 61 currently waits without a timeout guard, which can make this step hang until the outer job timeout if the cluster never stabilizes (or fail unclearly if the subcommand is missing in a given payload/tooling combo). Consider reusing the repository’s established pattern here.
Proposed patch
-oc adm wait-for-stable-cluster --minimum-stable-period 5m +if ! oc adm wait-for-stable-cluster --help &>/dev/null; then + echo "oc adm wait-for-stable-cluster is not available in this release" + exit 1 +fi +oc adm wait-for-stable-cluster --minimum-stable-period=5m --timeout=30m🤖 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 `@ci-operator/step-registry/openshift-qe/build-farm/openshift-qe-build-farm-commands.sh` at line 61, The oc adm wait-for-stable-cluster command on line 61 lacks a timeout guard and availability check, which can cause the step to hang indefinitely if the cluster never stabilizes or if the subcommand is unavailable. Add an explicit timeout parameter to the wait-for-stable-cluster command to prevent indefinite hangs, include a pre-check to verify cluster availability before waiting, and follow the established timeout and error handling patterns used elsewhere in the openshift-qe-build-farm-commands.sh script for consistency.
🤖 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.
Nitpick comments:
In
`@ci-operator/step-registry/openshift-qe/build-farm/openshift-qe-build-farm-commands.sh`:
- Line 61: The oc adm wait-for-stable-cluster command on line 61 lacks a timeout
guard and availability check, which can cause the step to hang indefinitely if
the cluster never stabilizes or if the subcommand is unavailable. Add an
explicit timeout parameter to the wait-for-stable-cluster command to prevent
indefinite hangs, include a pre-check to verify cluster availability before
waiting, and follow the established timeout and error handling patterns used
elsewhere in the openshift-qe-build-farm-commands.sh script for consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b6684e7d-e302-4233-aefc-9aab31d98b33
📒 Files selected for processing (2)
ci-operator/step-registry/openshift-qe/build-farm/openshift-qe-build-farm-commands.shci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.sh
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa-5.0-nightly-x86-build-farm-114nodes |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Signed-off-by: Marius Cornea <mcornea@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mcornea 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 |
|
[REHEARSALNOTIFIER]
A total of 719 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa-5.0-nightly-x86-build-farm-114nodes |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@mcornea: 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. |
1 participant
Summary by CodeRabbit
This PR adjusts OpenShift CI’s OpenShift QE performance-scale (5.0 nightly) cluster bootstrap to use a pinned OS/RHCOS image and to wait for cluster stability before continuing—supporting a CRI-O-related revert baseline for newer test environments.
What Changed
ci-operator/step-registry/openshift-qe/workers-infra-workload/openshift-qe-workers-infra-workload-commands.shMachineConfigList for worker, master, and infra roles that pinsspec.osImageURLto the intended fixed release image digest.oc adm wait-for-stable-cluster --minimum-stable-period 2m.ci-operator/step-registry/openshift-qe/build-farm/openshift-qe-build-farm-commands.shos-layer-custom-*MachineConfig List for worker/master/infra with the same pinnedosImageURLdigest.oc adm wait-for-stable-cluster --minimum-stable-period 5mbefore proceeding to the benchmark/build-farm workload.ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa-5.0-nightly-x86.yamlBUILD_FARM_EXTRA_FLAGSto include--ignore-health-check(in addition to existing alerting/job-iteration flags).Impact