WIP: add OpenPERouter baremetal E2E deploy-verify lane - #83156
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
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: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (11)
WalkthroughThe CI configuration updates OpenPerOuter bundle builds and adds a dual-stack Equinix metal end-to-end test. The workflow applies extra-network firewall rules, deploys the operator bundle, creates an ChangesOpenPerOuter bare-metal validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI workflow
participant Host as dev-scripts host
participant SDK as Operator SDK and OLM
participant Cluster as OpenShift cluster
CI->>Host: Apply extra-network SSH reject rules
CI->>SDK: Install Operator SDK and deploy OO_BUNDLE
SDK->>Cluster: Create OpenPERouter namespace and resource
Cluster-->>SDK: Report deployments and daemonsets
SDK-->>CI: Return pod readiness status
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (12 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: oribon 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 |
|
/pj-rehearse |
|
@oribon: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-commands.sh`:
- Line 18: Remove the -x option from the remote shell setup in
ci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-commands.sh:18-18
and
ci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh:14-14,
keeping both scripts at set -euo pipefail by default.
In
`@ci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh`:
- Around line 63-71: Update the pod validation block in the baremetal
openperouter E2E command to inspect each non-completed pod’s Ready condition,
not just its phase. Treat Running pods with a non-ready condition such as 0/1 as
failures, report them through NOT_READY, and print the success message only when
every non-completed pod is Ready.
- Around line 51-56: Update the DaemonSet creation loop in the controller/router
wait flow to enforce a 300-second deadline; if oc get daemonset does not succeed
within that period, print an error and exit nonzero. Preserve the existing
polling interval and rollout status handling after creation.
🪄 Autofix
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: Pro Plus
Run ID: 8375c2c2-6920-4b4d-8844-c4f217b8eb90
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-kni/openperouter/openshift-kni-openperouter-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (9)
ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yamlci-operator/step-registry/baremetalds/openperouter-e2e/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/baremetalds-openperouter-e2e-workflow.yamlci-operator/step-registry/baremetalds/openperouter-e2e/iptables/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-commands.shci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-ref.yamlci-operator/step-registry/baremetalds/openperouter-e2e/test/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.shci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-ref.yaml
b35033d to
5c78f49
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
5c78f49 to
b2556bb
Compare
|
/pj-rehearse |
|
@oribon: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
b2556bb to
479b385
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/pj-rehearse |
|
@oribon: 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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh (1)
68-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCheck Pod phase and Ready condition directly.
The grep accepts a
1/1 Terminatingrow. It also rejects a healthy pod with more than five containers. Query the Pod phase and theReadycondition instead of parsing displayed readiness counts.Proposed fix
-NOT_READY=$(oc get pods -n openshift-openperouter-system --no-headers | grep -v "Completed" | grep -v "1/1\|2/2\|3/3\|4/4\|5/5" || true) +NOT_READY="$(oc get pods -n openshift-openperouter-system \ + -o jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.status.phase}{"\t"}{range .status.conditions[?(@.type=="Ready")]}{.status}{end}{"\n"}{end}' | + awk -F '\t' '$2 != "Succeeded" && ($2 != "Running" || $3 != "True")')"🤖 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/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh` around lines 68 - 76, Replace the readiness-count parsing in the pod verification block with direct checks of each pod’s phase and Ready condition. Update the logic around NOT_READY to identify pods only when phase is not Running or the Ready condition is not True, so terminating pods and healthy pods with any container count are handled correctly.
🤖 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/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh`:
- Around line 20-21: Update the Operator SDK download curl command in the test
setup to fail on HTTP errors and enforce both connection and total request
timeouts, while preserving the existing output path and versioned download URL.
---
Duplicate comments:
In
`@ci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh`:
- Around line 68-76: Replace the readiness-count parsing in the pod verification
block with direct checks of each pod’s phase and Ready condition. Update the
logic around NOT_READY to identify pods only when phase is not Running or the
Ready condition is not True, so terminating pods and healthy pods with any
container count are handled correctly.
🪄 Autofix
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: Pro Plus
Run ID: 2f3b5e68-485b-4dac-9fa3-5930ffc353d9
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-kni/openperouter/openshift-kni-openperouter-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (12)
ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yamlci-operator/step-registry/baremetalds/openperouter-e2e/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/baremetalds-openperouter-e2e-workflow.metadata.jsonci-operator/step-registry/baremetalds/openperouter-e2e/baremetalds-openperouter-e2e-workflow.yamlci-operator/step-registry/baremetalds/openperouter-e2e/iptables/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-commands.shci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-ref.metadata.jsonci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-ref.yamlci-operator/step-registry/baremetalds/openperouter-e2e/test/OWNERSci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.shci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-ref.metadata.jsonci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- ci-operator/step-registry/baremetalds/openperouter-e2e/test/OWNERS
- ci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-ref.yaml
- ci-operator/step-registry/baremetalds/openperouter-e2e/baremetalds-openperouter-e2e-workflow.yaml
- ci-operator/step-registry/baremetalds/openperouter-e2e/OWNERS
- ci-operator/step-registry/baremetalds/openperouter-e2e/iptables/baremetalds-openperouter-e2e-iptables-commands.sh
- ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml
- ci-operator/step-registry/baremetalds/openperouter-e2e/iptables/OWNERS
- ci-operator/step-registry/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-ref.yaml
| curl -sLo /usr/local/bin/operator-sdk \ | ||
| "https://github.com/operator-framework/operator-sdk/releases/download/${OPERATOR_SDK_VERSION}/operator-sdk_linux_amd64" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound and validate the Operator SDK download.
curl -sLo accepts HTTP error responses and has no connection or total timeout. A stalled GitHub request can consume the full 3600-second step timeout. Fail on HTTP errors and set bounded timeouts.
Proposed fix
-curl -sLo /usr/local/bin/operator-sdk \
+curl --fail --silent --show-error --location --retry 3 \
+ --connect-timeout 30 --max-time 300 \
+ -o /usr/local/bin/operator-sdk \
"https://github.com/operator-framework/operator-sdk/releases/download/${OPERATOR_SDK_VERSION}/operator-sdk_linux_amd64"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| curl -sLo /usr/local/bin/operator-sdk \ | |
| "https://github.com/operator-framework/operator-sdk/releases/download/${OPERATOR_SDK_VERSION}/operator-sdk_linux_amd64" | |
| curl --fail --silent --show-error --location --retry 3 \ | |
| --connect-timeout 30 --max-time 300 \ | |
| -o /usr/local/bin/operator-sdk \ | |
| "https://github.com/operator-framework/operator-sdk/releases/download/${OPERATOR_SDK_VERSION}/operator-sdk_linux_amd64" |
🤖 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/baremetalds/openperouter-e2e/test/baremetalds-openperouter-e2e-test-commands.sh`
around lines 20 - 21, Update the Operator SDK download curl command in the test
setup to fail on HTTP errors and enforce both connection and total request
timeouts, while preserving the existing output path and versioned download URL.
Baseline for the openperouter-e2e-metal test. Currently deploys the operator via OLM bundle on a baremetal OCP cluster and verifies all components (operator, webhook, controller, router) come up healthy. The full E2E lane will extend this with containerlab fabric setup and test execution. The bundle image is now built using bundle.Dockerfile rather than bundle.Dockerfile.openshift (like the other telco-5g projects using konflux here). The workflow inserts an iptables pre step before devscripts-setup. EXTRA_NETWORK_NAMES creates additional NICs on each node. The installer's bootstrap gather collects ALL NIC IPs and SSHes to each for log gathering. Since the extra networks are unreachable via SSH, this avoids timeouts on attempts to gather them. This is the reason we unpacked the dev-scripts chain instead of calling it directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
479b385 to
e8ce01b
Compare
|
/pj-rehearse |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@oribon: 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: |
|
@oribon: 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
Baseline for the openperouter-e2e-metal test. Currently deploys the operator via OLM bundle on a baremetal OCP cluster and verifies all components (operator, webhook, controller, router) come up healthy. The full E2E lane will extend this with containerlab fabric setup and test execution.
The bundle image is now built using bundle.Dockerfile rather than bundle.Dockerfile.openshift (like the other telco-5g projects using konflux here).
The workflow inserts an iptables pre step before devscripts-setup. EXTRA_NETWORK_NAMES creates additional NICs on each node. The installer's bootstrap gather collects ALL NIC IPs and SSHes to each for log gathering. Since the extra networks are unreachable via SSH, this avoids timeouts on attempts to gather them.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Signed-off-by: Ori Braunshtein obraunsh@redhat.com
Summary by CodeRabbit
Adds the
openperouter-e2e-metalOpenShift CI lane for OpenPERouter.build05Equinix bare-metal profile.