Refactor nto lanes#79374
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 (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR updates the telcov10n functional compute-nto CI pipeline infrastructure: migrating cluster configuration from hlxcl51/hlxcl52 to helix17 across step manifests, scaling CPU resource requests, simplifying deployment orchestration by removing the cleanup step from deploy flow, and enhancing test artifact collection. ChangesTelcoV10N Compute-NTO Cluster Migration and Orchestration
🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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/telcov10n/functional/compute-nto/eco-gotests/telcov10n-functional-compute-nto-eco-gotests-commands.sh`:
- Around line 113-115: Make the artifact copy of run_gotests.sh best-effort: the
scp command that uses "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh
-> "${ARTIFACT_DIR}/run_gotests.sh" should not abort the whole script under set
-e; change the invocation so failures are trapped and only emit a warning (e.g.,
run scp and if it fails, echo a clear warning including BASTION_IP/ARTIFACT_DIR
and continue), or temporarily disable errexit around that scp invocation and
restore it afterward; ensure the warning includes the scp exit code or message
so troubleshooting is possible.
🪄 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: 8e9d9014-b9cb-4243-887e-3f4a9d9fbc9b
📒 Files selected for processing (6)
ci-operator/config/openshift-kni/eco-ci-cd/openshift-kni-eco-ci-cd-main__compute-nto.yamlci-operator/step-registry/telcov10n/functional/compute-nto/cleanup-old-clusters/telcov10n-functional-compute-nto-cleanup-old-clusters-ref.yamlci-operator/step-registry/telcov10n/functional/compute-nto/config/telcov10n-functional-compute-nto-config-commands.shci-operator/step-registry/telcov10n/functional/compute-nto/eco-gotests/telcov10n-functional-compute-nto-eco-gotests-commands.shci-operator/step-registry/telcov10n/functional/compute-nto/ocp-deploy/telcov10n-functional-compute-nto-ocp-deploy-commands.shci-operator/step-registry/telcov10n/functional/compute-nto/ocp-deploy/telcov10n-functional-compute-nto-ocp-deploy-ref.yaml
💤 Files with no reviewable changes (1)
- ci-operator/step-registry/telcov10n/functional/compute-nto/cleanup-old-clusters/telcov10n-functional-compute-nto-cleanup-old-clusters-ref.yaml
| echo "Copy eco Gotest script to artifacts directory" | ||
| scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /tmp/temp_ssh_key \ | ||
| "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh "${ARTIFACT_DIR}/run_gotests.sh" |
There was a problem hiding this comment.
Make runner-script artifact copy non-fatal.
At Line 114, scp runs under set -e, so a missing/temporarily unreachable bastion or missing remote file can fail the whole step even though this artifact is auxiliary. Please handle this copy as best-effort with a warning.
Suggested patch
echo "Copy eco Gotest script to artifacts directory"
-scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /tmp/temp_ssh_key \
- "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh "${ARTIFACT_DIR}/run_gotests.sh"
+if ! scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i "${PROJECT_DIR}/temp_ssh_key" \
+ "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh "${ARTIFACT_DIR}/run_gotests.sh"; then
+ echo "[WARNING] Could not copy /tmp/gotest/run_gotests.sh from bastion"
+fi📝 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.
| echo "Copy eco Gotest script to artifacts directory" | |
| scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /tmp/temp_ssh_key \ | |
| "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh "${ARTIFACT_DIR}/run_gotests.sh" | |
| echo "Copy eco Gotest script to artifacts directory" | |
| if ! scp -r -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i "${PROJECT_DIR}/temp_ssh_key" \ | |
| "${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh "${ARTIFACT_DIR}/run_gotests.sh"; then | |
| echo "[WARNING] Could not copy /tmp/gotest/run_gotests.sh from bastion" | |
| fi |
🤖 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/telcov10n/functional/compute-nto/eco-gotests/telcov10n-functional-compute-nto-eco-gotests-commands.sh`
around lines 113 - 115, Make the artifact copy of run_gotests.sh best-effort:
the scp command that uses
"${BASTION_USER}@${BASTION_IP}":/tmp/gotest/run_gotests.sh ->
"${ARTIFACT_DIR}/run_gotests.sh" should not abort the whole script under set -e;
change the invocation so failures are trapped and only emit a warning (e.g., run
scp and if it fails, echo a clear warning including BASTION_IP/ARTIFACT_DIR and
continue), or temporarily disable errexit around that scp invocation and restore
it afterward; ensure the warning includes the scp exit code or message so
troubleshooting is possible.
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-21 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-21 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
eifrach
force-pushed
the
refactor_nto_lanes
branch
from
88e8569 to
ae10d74
Compare
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-21 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
eifrach
force-pushed
the
refactor_nto_lanes
branch
from
ae10d74 to
3697852
Compare
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-21 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-20 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse cancel |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse abort |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@eifrach: job(s): , cancel either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-kni-eco-ci-cd-main-compute-nto-e2e-telcov10n-nto-tests-v2-t0-crun-bm-4-20 |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Signed-off-by: Eran Ifrach <eifrach@redhat.com>
eifrach
force-pushed
the
refactor_nto_lanes
branch
from
3697852 to
da9215c
Compare
|
/pj-rehearse ack |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
rehearsals-ack
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eifrach, rdiscala 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 |
|
/pj-rehearse ack |
|
@eifrach: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
@eifrach: 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. |
Signed-off-by: Eran Ifrach <eifrach@redhat.com>
2 participants
Refactor NTO Test Lanes
This PR refactors the TelCo v10n compute-nto (Network Tuning Operator) CI test infrastructure by consolidating cluster configurations and improving artifact collection.
Cluster Configuration Updates
The changes remove references to two older test clusters (
hlxcl51andhlxcl52) and introduce support for a new cluster (helix17) across the compute-nto test pipeline. Both the cleanup and OCP deployment step manifests now include complete credential mounts for the helix17 cluster, covering:The hlxcl15 cluster remains as the primary test target.
Deployment Flow Simplification
The OCP deployment pipeline no longer runs a separate cluster cleanup step before deploying new OCP instances. The cleanup functionality has been removed from the ocp-deploy-commands.sh script, streamlining the deployment workflow.
Resource and Artifact Improvements
/tmp/gotest/run_gotests.sh) from the bastion host into the artifacts directory via scp, improving debuggability and test artifact completenessls -lacommand from the gotest scriptThese changes consolidate the test infrastructure toward a more streamlined configuration focused on helix17 while maintaining hlxcl15 as an active test lane.