WIP: add openperouter deploy-verify test on AWS - #82873
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis pull request configures continuous integration for the OpenPERouter operator. The changes add build image specifications, update the bundle Dockerfile configuration, set the container image pullspec, and implement AWS-based deployment verification that validates operator installation and pod health. ChangesOpenPERouter CI Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AWSCluster
participant OperatorDeployment
participant OpenPERouterResource
participant ControllerDaemonSet
participant RouterDaemonSet
AWSCluster->>OperatorDeployment: Install OpenPERouter operator
OperatorDeployment-->>AWSCluster: Operator ready
AWSCluster->>OpenPERouterResource: Create OpenPERouter resource
OpenPERouterResource->>ControllerDaemonSet: Trigger deployment
OpenPERouterResource->>RouterDaemonSet: Trigger deployment
AWSCluster->>ControllerDaemonSet: Wait and check pod health
AWSCluster->>RouterDaemonSet: Wait and check pod health
ControllerDaemonSet-->>AWSCluster: Health status
RouterDaemonSet-->>AWSCluster: Health status
🚥 Pre-merge checks | ✅ 5 | ❌ 10❌ Failed checks (10 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml`:
- Around line 110-119: Update the rollout verification around the pod readiness
check to validate every DaemonSet’s status.desiredNumberScheduled and
status.numberReady, failing when no targets are scheduled or the ready count is
insufficient. After excluding Completed pods, also reject empty or non-ready
readiness output so pods reporting 0/N READY cannot pass; preserve the existing
diagnostics before exiting.
- Around line 95-105: Bound the DaemonSet creation loops for controller and
router so they cannot run indefinitely before rollout status begins. Add a
finite timeout and failure diagnostics to each wait, while preserving the
existing 300-second oc rollout status timeout for availability and progress.
- Line 65: Update the OO_INSTALL_MODE configuration to AllNamespaces so the
operator-sdk run bundle invocation matches the install mode supported by the
built bundle CSV; leave other CI settings unchanged.
🪄 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: ab710965-ab27-445e-bfd6-74b7714f65df
⛔ 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 (2)
ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yamlci-operator/step-registry/cluster-profiles/cluster-profiles-config.yaml
| echo "### Wait for controller daemonset" | ||
| until oc get daemonset controller -n openshift-openperouter-system &>/dev/null; do | ||
| sleep 5 | ||
| done | ||
| oc rollout status daemonset/controller -n openshift-openperouter-system --timeout=300s | ||
|
|
||
| echo "### Wait for router daemonset" | ||
| until oc get daemonset router -n openshift-openperouter-system &>/dev/null; do | ||
| sleep 5 | ||
| done | ||
| oc rollout status daemonset/router -n openshift-openperouter-system --timeout=300s |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file exists and relevant lines =="
if [ -f ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml ]; then
sed -n '1,140p' ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml | nl -ba | sed -n '80,115p'
else
echo "missing target file"
fi
echo
echo "== kubectl/oc wait help if available =="
command -v oc || true
if command -v oc >/dev/null 2>&1; then
oc wait --help | sed -n '1,160p'
fi
command -v kubectl || true
if command -v kubectl >/dev/null 2>&1; then
kubectl wait --help | sed -n '1,160p'
fi
echo
echo "== search for wait_for_daemonset in repo =="
rg -n "wait_for_daemonset|for=create|oc wait" ci-operator/config/openshift-kni/openperouter ci-operator/config || trueRepository: openshift/release
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate target file =="
git ls-files | grep -F 'ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml' || true
echo
echo "== relevant target lines =="
awk '{printf "%4d\t%s\n", NR, $0}' ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml | sed -n '80,115p'Repository: openshift/release
Length of output: 1879
🌐 Web query:
kubectl wait --for=create Kubernetes condition create support
💡 Result:
Yes, kubectl wait supports the --for=create option to wait for a resource to be created [1][2]. When you use kubectl wait --for=create [resource], the command will wait until the specified resource exists or until the defined timeout is reached [1][2]. This is particularly useful for workflows where you need to ensure a resource exists before proceeding to check its status [3]. Key details include: - Usage: You can combine it with other --for flags. For example, kubectl wait --for=create --for=condition=Ready [resource] will wait for the resource to be created and then for it to reach the Ready condition [1][3]. - Execution Order: If both --for=create and other conditions (like --for=condition=...) are provided, --for=create is always processed first [1][4]. - Label Selectors: The functionality supports label selectors, allowing you to wait for resources that match specific labels to be created [5]. - Default Behavior: If you do not provide --for=create, kubectl wait generally expects the target resource to already exist and may return an error if it cannot be found [5]. This feature was added to improve the reliability of scripts and CI/CD pipelines that depend on resources that might not be immediately available [6][3].
Citations:
- 1: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_wait/
- 2: https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/wait/wait.go
- 3: kubectl wait: Support multiple conditions kubernetes/kubernetes#136855
- 4: https://man.archlinux.org/man/extra/kubectl/kubectl-wait.1.en
- 5: kubectl: fix wait --for=create to work correctly with label selectors kubernetes/kubernetes#128662
- 6: Add --for=create option to kubectl wait kubernetes/kubernetes#125868
Bound both DaemonSet creation waits.
Both until oc get daemonset loops can run indefinitely if the DaemonSet is not created. The 300-second oc rollout status timeout only starts after creation succeeds. Use a bounded creation wait with diagnostics on failure, and keep the rollout timeout for availability/progress.
🤖 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/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml`
around lines 95 - 105, Bound the DaemonSet creation loops for controller and
router so they cannot run indefinitely before rollout status begins. Add a
finite timeout and failure diagnostics to each wait, while preserving the
existing 300-second oc rollout status timeout for availability and progress.
| echo "### Verify all containers are ready" | ||
| NOT_READY=$(oc get pods -n openshift-openperouter-system --no-headers \ | ||
| | grep -v Completed | grep -v "Running" || true) | ||
| if [ -n "$NOT_READY" ]; then | ||
| echo "ERROR: Some pods are not healthy:" | ||
| echo "$NOT_READY" | ||
| oc describe pods -n openshift-openperouter-system | ||
| oc logs -n openshift-openperouter-system -l app=router --all-containers --tail=50 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Locate CI config"
git ls-files | rg 'ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml' || true
echo "## Relevant lines"
if [ -f ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml ]; then
nl -ba ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml | sed -n '80,145p'
fi
echo "## Search rollout/status/health related patterns"
rg -n "rollout status|Verify all containers|openshift-openperouter-system|daemonset|desire|numberReady|desiredNumberScheduled|grep -v Running" ci-operator/config/openshift-kni/openperouter -S || true
echo "## Programmatic probe: current grep behavior for representative statuses"
python3 - <<'PY'
from subprocess import run
cases = {
"empty": "",
"running_ok": "openshift-openperouter-controller-controller-abcde 1/1 2/2 2 20s Running",
"running_unready": "openshift-openperouter-controller-controller-abcde 0/1 2/2 2 20s Running",
"mixed": """openshift-openperouter-controller-controller-abcde 0/1 2/2 2 20s Running
openshift-openperouter-router-router-fghij 1/1 2/2 2 20s Running""".replace("\n", " | "),
}
for name, lines in cases.items():
args = ["grep", "-v", "Completed"]
import pathlib
p = pathlib.Path(f"/tmp/cases-{name}.txt")
p.write_text(lines + "\n")
# Simulate exactly the shell pipeline after file read, no oc/run outside.
print(f"\nCASE {name}:")
print("lines:", repr(lines))
for label, cmd in [
("grep -v Completed", ["grep", "-v", "Completed"]),
("grep -v Running", ["grep", "-v", "Running"]),
("merged grep -v Completed | grep -v Running", ["bash", "-c", "grep -v Completed /tmp/cases-{}.txt | grep -v Running".format(name)]),
]:
proc = run(cmd, text=True, input=lines if label.startswith("merged") else lines, capture_output=True)
print(f"{label} => NOT_READY={bool(proc.stdout.strip())} stdout={proc.stdout.strip()!r}")
PY
echo "## Check git diff stat/name only for context"
git diff --stat || trueRepository: openshift/release
Length of output: 316
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Relevant lines"
if [ -f ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml ]; then
cat -n ci-operator/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml | sed -n '1,160p'
fi
echo "## Search health/status patterns"
rg -n "rollout status|Verify all containers|openshift-openperouter-system|daemonset|desiredNumberScheduled|numberReady|grep -v Running|grep -v Completed" ci-operator/config/openswitch-kni/openperouter ci-operator/config/openshift-kni/openperouter -S || true
echo "## Programmatic probe: current grep behavior for representative statuses"
python3 - <<'PY'
from pathlib import Path
cases = {
"empty": "",
"running_ok": "openshift-openperouter-controller-controller-abcde 1/1 2/2 2 20s Running",
"running_unready": "openshift-openperouter-controller-controller-abcde 0/1 2/2 2 20s Running",
"mixed": """openshift-openperouter-controller-controller-abcde 0/1 2/2 2 20s Running
openshift-openperouter-router-router-fghij 1/1 2/2 2 20s Running""".replace("\n", " | "),
}
for name, lines in cases.items():
print(f"\nCASE {name}:")
print("lines:", repr(lines))
import subprocess
for label, cmd in [
("grep -v Completed", ["grep", "-v", "Completed"]),
("grep -v Running", ["grep", "-v", "Running"]),
]:
proc = subprocess.run(cmd, text=True, input=lines, capture_output=True)
print(f"{label} => stdout={proc.stdout.strip()!r}, exit={proc.returncode}")
PYRepository: openshift/release
Length of output: 8905
Fail when DaemonSet Pods are not Ready or have no targets.
oc rollout status accepts a DaemonSet with desiredNumberScheduled=0, and the pod check only filters by Running; a pod with 0/1 READY passes. Check status.desiredNumberScheduled and status.numberReady for each DaemonSet, then reject empty readiness output after excluding Completed pods.
[low_effort_and_high_reward+]
🤖 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/config/openshift-kni/openperouter/openshift-kni-openperouter-main.yaml`
around lines 110 - 119, Update the rollout verification around the pod readiness
check to validate every DaemonSet’s status.desiredNumberScheduled and
status.numberReady, failing when no targets are scheduled or the ready count is
insufficient. After excluding Completed pods, also reject empty or non-ready
readiness output so pods reporting 0/N READY cannot pass; preserve the existing
diagnostics before exiting.
Add a lightweight test that deploys openperouter via OLM on an AWS cluster and verifies operator, controller, and router pods come up healthy. Uses the optional-operators-ci-operator-sdk-aws workflow following the MetalLB operator-e2e pattern. Changes: - Switch bundle to simple bundle.Dockerfile (no submodule dependency) - Substitute raw CSV pullspecs (quay.io/openperouter/router:main) - Add operator-deploy-verify test with cluster_profile: openshift-org-aws - Add openperouter to aws cluster profile owners - Add cli-operator-sdk base image Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1dec90a to
3c22504
Compare
|
/pj-rehearse |
|
@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: |
|
/pj-rehearse |
|
@oribon: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@oribon: 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
Add a lightweight test that deploys openperouter via OLM on an AWS cluster and verifies operator, controller, and router pods come up healthy. Uses the optional-operators-ci-operator-sdk-aws workflow following the MetalLB operator-e2e pattern.
Changes:
Summary by CodeRabbit
bundle.Dockerfile, use raw CSV pullspecs, and add thecli-operator-sdkbase image.operator-deploy-verifytest that installs OpenPERouter, creates anOpenPERouterresource, and verifies healthy controller and router pods.