Add nmstate install step for virt-cluster-validate.#82122
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughThe CI workflow adds an nmstate installation step before Azure and GCP virt-cluster validation. The step installs or reuses nmstate resources, and host-network checks are no longer excluded from must-gather validation. ChangesNmstate validation integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI as virt-cluster-validate
participant Cluster as OpenShift cluster
participant OLM as nmstate operator
participant Checks as Host-network checks
CI->>Cluster: Run install-nmstate step
CI->>OLM: Apply operator manifests
OLM-->>Cluster: Make NMState resource Available
CI->>Checks: Run validation with host-network checks enabled
Possibly related PRs
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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oshoval 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-ref.yaml (1)
5-5: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winIncrease the step timeout to align with script wait times.
The installation script contains multiple sequential wait operations (e.g., waiting for the CSV to appear, waiting for the CSV to succeed, and waiting for the NMState CR to become Available), each using a 5-minute timeout (
TIMEOUT=300).
However, the step itself is configured with a hard timeout of5m0s. If the operator installation takes slightly longer than 5 minutes (which is common due to image pulling and reconciliation), Prow will forcefully terminate the step before the script's internal logic has a chance to finish or time out gracefully.Consider increasing this timeout to
15m0sto safely accommodate the sequential wait loops.🛠️ Proposed fix
- timeout: 5m0s + timeout: 15m0s🤖 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/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-ref.yaml` at line 5, Increase the step timeout from 5m0s to 15m0s in the step configuration so the installation script’s sequential waits can complete without premature termination.
🤖 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/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-ref.yaml`:
- Line 5: Increase the step timeout from 5m0s to 15m0s in the step configuration
so the installation script’s sequential waits can complete without premature
termination.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 73df976f-9686-43be-ad86-f6fe18b3e6cf
📒 Files selected for processing (5)
ci-operator/config/openshift-cnv/virt-cluster-validate/openshift-cnv-virt-cluster-validate-main.yamlci-operator/step-registry/cnv/virt-cluster-validate/install-nmstate/OWNERSci-operator/step-registry/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-commands.shci-operator/step-registry/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-ref.metadata.jsonci-operator/step-registry/cnv/virt-cluster-validate/install-nmstate/cnv-virt-cluster-validate-install-nmstate-ref.yaml
886f62e to
68907de
Compare
|
/pj-rehearse |
|
@oshoval: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Install nmstate before virt-cluster-validate runs so CI can exercise host-network checks instead of skipping them. Assisted-by: Cursor Signed-off-by: Or Shoval <oshoval@redhat.com>
|
/pj-rehearse pull-ci-openshift-cnv-virt-cluster-validate-main-e2e-azure |
|
@oshoval: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@oshoval: requesting more than one rehearsal in one comment is not supported. If you would like to rehearse multiple specific jobs, please separate the job names by a space in a single command. |
|
/pj-rehearse pull-ci-openshift-cnv-virt-cluster-validate-main-e2e-azure pull-ci-openshift-cnv-virt-cluster-validate-main-e2e-gcp |
|
@oshoval: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
| cpu: 100m | ||
| memory: 600Mi | ||
| timeout: 1h0m0s | ||
| - ref: cnv-virt-cluster-validate-install-nmstate |
There was a problem hiding this comment.
Could the operatorhub-subscribe-nmstate-operator step be re-used?
There was a problem hiding this comment.
will check, first need to fix the lanes failures
thx
|
@oshoval: The following tests 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. |
2 participants
Install nmstate before virt-cluster-validate runs so CI can exercise host-network checks instead of skipping them.
Summary by CodeRabbit
Updates OpenShift CNV CI validation to install and set up the
nmstateoperator before runningvirt-cluster-validate. The newcnv-virt-cluster-validate-install-nmstatestep is added to the Azure and GCP e2e pipeline validation flows, and theoc adm must-gatherSKIP_CHECKSlist is adjusted to stop skipping host-network checks (11-host-network). The added install script is responsible for creating/using the required namespaces and waiting for the nmstate CRDs/operator to become available, so CI can exercise host-network validation instead of bypassing it.