DPF: Run network tests directly from Prow - #82742
Conversation
Signed-off-by: Jose Castillo Lema <josecastillolema@gmail.com>
gitleaks.version: 8.30.0
gitleaks.check-secrets: ENABLED
WalkthroughThe network test jobs now load a shared kubeconfig and ChangesDPF network test execution
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CIJob
participant KubeconfigLoader
participant NetworkTestStep
participant DPFCluster
participant TrafficFlowTests
CIJob->>KubeconfigLoader: Load kubeconfig and .env
KubeconfigLoader->>NetworkTestStep: Provide shared credentials
NetworkTestStep->>DPFCluster: Verify access and select worker-dpu nodes
NetworkTestStep->>TrafficFlowTests: Run make run-traffic-flow-tests
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 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 |
openshift-ci
Bot
added
the
approved
openshift-merge-bot
Bot
added
the
rehearsals-ack
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh`:
- Around line 4-7: Set KUBECONFIG to the DPF kubeconfig at
${SHARED_DIR}/kubeconfig before the first oc invocation in the DPF Kubernetes
traffic flow test step. Update the shell flow around the initial “Verifying
cluster access...” and oc get nodes commands, matching the sibling
test-full-suite step’s explicit kubeconfig export so subsequent node selection
targets the correct cluster.
- Around line 9-12: Update the TFT_SERVER_NODE and TFT_CLIENT_NODE setup to
assign the command results separately, then validate both variables are
non-empty before running make run-traffic-flow-tests. Exit immediately with an
appropriate error when fewer than two worker-dpu nodes are available, while
preserving the existing node-selection and logging behavior.
🪄 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: Pro Plus
Run ID: 922859e1-c17f-4a87-833d-3f6535e94b5d
📒 Files selected for processing (4)
ci-operator/config/rh-ecosystem-edge/openshift-dpf/rh-ecosystem-edge-openshift-dpf-main.yamlci-operator/config/rh-ecosystem-edge/openshift-dpf/rh-ecosystem-edge-openshift-dpf-release-4.22.yamlci-operator/step-registry/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.shci-operator/step-registry/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-ref.yaml
| echo "=== Run DPF Kubernetes Traffic Flow Tests ===" | ||
|
|
||
| echo "Setting up SSH access to DPF hypervisor: ${REMOTE_HOST}" | ||
| echo "Verifying cluster access..." | ||
| oc get nodes |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Export KUBECONFIG before you use oc.
The step never sets KUBECONFIG, so oc uses the default CI cluster credentials instead of the DPF cluster kubeconfig written by dpf-hypervisor-load-kubeconfig. The ref.yaml documentation states that this step consumes ${SHARED_DIR}/kubeconfig, and the sibling step ci-operator/step-registry/dpf/hypervisor/test-full-suite/dpf-hypervisor-test-full-suite-commands.sh (line 6) exports it explicitly. Without the export, oc get nodes queries the wrong cluster and the node selection below silently yields empty values.
🐛 Proposed fix
echo "=== Run DPF Kubernetes Traffic Flow Tests ==="
+export KUBECONFIG="${SHARED_DIR}/kubeconfig"
+
echo "Verifying cluster access..."
oc get nodes📝 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 "=== Run DPF Kubernetes Traffic Flow Tests ===" | |
| echo "Setting up SSH access to DPF hypervisor: ${REMOTE_HOST}" | |
| echo "Verifying cluster access..." | |
| oc get nodes | |
| echo "=== Run DPF Kubernetes Traffic Flow Tests ===" | |
| export KUBECONFIG="${SHARED_DIR}/kubeconfig" | |
| echo "Verifying cluster access..." | |
| oc get nodes |
🤖 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/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh`
around lines 4 - 7, Set KUBECONFIG to the DPF kubeconfig at
${SHARED_DIR}/kubeconfig before the first oc invocation in the DPF Kubernetes
traffic flow test step. Update the shell flow around the initial “Verifying
cluster access...” and oc get nodes commands, matching the sibling
test-full-suite step’s explicit kubeconfig export so subsequent node selection
targets the correct cluster.
| export TFT_SERVER_NODE=$(oc get nodes --no-headers | grep worker-dpu | awk 'NR==1 {print $1}') | ||
| echo "TFT_SERVER_NODE: ${TFT_SERVER_NODE}" | ||
| export TFT_CLIENT_NODE=$(oc get nodes --no-headers | grep worker-dpu | awk 'NR==2 {print $1}') | ||
| echo "TFT_CLIENT_NODE: ${TFT_CLIENT_NODE}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how other DPF steps select worker-dpu nodes.
fd -t f -e sh . ci-operator/step-registry/dpf | xargs rg -n -C3 'worker-dpu|TFT_SERVER_NODE|TFT_CLIENT_NODE'Repository: openshift/release
Length of output: 2110
🏁 Script executed:
#!/bin/bash
set -u
printf 'Repository files around DPF step registry:\n'
git ls-files 'ci-operator/step-registry/dpf/**/*-commands.sh' 2>/dev/null | sed -n '1,80p'
printf '\nFile header and node selection lines:\n'
FILE="ci-operator/step-registry/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh"
if [ -f "$FILE" ]; then
wc -l "$FILE"
cat -n "$FILE" | sed -n '1,40p'
else
printf 'MISSING: %s\n' "$FILE"
fi
printf '\nAll worker-dpu references in tracked files:\n'
rg -n -C3 'worker-dpu|TFT_SERVER_NODE|TFT_CLIENT_NODE' . --glob '!*.png' --glob '!*.jpg' --glob '!*.svg' --glob '!node_modules/**' 2>/dev/null | sed -n '1,220p'Repository: openshift/release
Length of output: 3145
🏁 Script executed:
#!/bin/bash
set -u
printf 'Shell pipeline/substitution behavior probe:\n'
tmpdir="$(mktemp -d)"
cd "$tmpdir" || exit 1
cat >probe.sh <<'SH'
set -euo pipefail
export TFT_SERVER_NODE=$(false | true | true; printf 'ok')
echo "TFT_SERVER_NODE='${TFT_SERVER_NODE}'"
echo "shell exited"
SH
if bash probe.sh 2>/tmp/err.txt; then
printf 'script succeeded\n'
else
printf 'script failed\n'
fi
cat /tmp/err.txt 2>/dev/null || true
cat >probe2.sh <<'SH'
set -euo pipefail
export TFT_SERVER_NODE=$(grep worker-dpu /dev/null | awk 'NR==1 {print $1}')
echo "TFT_SERVER_NODE='${TFT_SERVER_NODE}'"
echo "shell exited"
SH
if bash probe2.sh 2>/tmp/err2.txt; then
printf 'script succeeded\n'
else
printf 'script failed\n'
fi
cat /tmp/err2.txt 2>/dev/null || true
printf '\nCluster metadata references in step registry configs/data:\n'
rg -n -i -C2 'dpu|worker-dpu' ci-operator/step-registry/dpf ci-operator 2>/dev/null | sed -n '1,240p'Repository: openshift/release
Length of output: 359
Validate the DPU node selection before running traffic-flow tests.
export VAR=$(...) masks the pipeline exit status, so this can finish with empty TFT_SERVER_NODE or TFT_CLIENT_NODE when fewer than two worker-dpu nodes are returned by oc get nodes. Set the variables separately and exit fast if either one is empty, before make run-traffic-flow-tests.
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 9-9: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 11-11: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 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/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh`
around lines 9 - 12, Update the TFT_SERVER_NODE and TFT_CLIENT_NODE setup to
assign the command results separately, then validate both variables are
non-empty before running make run-traffic-flow-tests. Exit immediately with an
appropriate error when fewer than two worker-dpu nodes are available, while
preserving the existing node-selection and logging behavior.
Source: Linters/SAST tools
Signed-off-by: Jose Castillo Lema <josecastillolema@gmail.com>
gitleaks.version: 8.30.0
gitleaks.check-secrets: ENABLED
|
/pj-rehearse network-access-allowed |
|
@mukrishn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-rh-ecosystem-edge-openshift-dpf-main-network-tests |
|
@josecastillolema: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/assign @wabouhamad |
Signed-off-by: Jose Castillo Lema <josecastillolema@gmail.com>
gitleaks.version: 8.30.0
gitleaks.check-secrets: ENABLED
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: josecastillolema 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/dpf/hypervisor/load-kubeconfig/dpf-hypervisor-load-kubeconfig-commands.sh (1)
49-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse an argument array and quote the
scpoperands.
SSH_OPTSis intentionally split into multiple arguments, but the current unquoted expansions also allow word splitting and glob expansion inREMOTE_HOSTandLAST_OPENSHIFT_DPF. DefineSSH_OPTSas an array and quote the remote source path. Apply the same array form to the other SSH and SCP calls.Proposed fix
-SSH_OPTS="-i /tmp/id_rsa -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR -o ConnectTimeout=30 -o ServerAliveInterval=10 -o ServerAliveCountMax=3 -o BatchMode=yes" +SSH_OPTS=( + -i /tmp/id_rsa + -o StrictHostKeyChecking=no + -o UserKnownHostsFile=/dev/null + -o LogLevel=ERROR + -o ConnectTimeout=30 + -o ServerAliveInterval=10 + -o ServerAliveCountMax=3 + -o BatchMode=yes +) ... -scp ${SSH_OPTS} root@${REMOTE_HOST}:${LAST_OPENSHIFT_DPF}/.env "${SHARED_DIR}/.env" +scp "${SSH_OPTS[@]}" "root@${REMOTE_HOST}:${LAST_OPENSHIFT_DPF}/.env" "${SHARED_DIR}/.env"🤖 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/dpf/hypervisor/load-kubeconfig/dpf-hypervisor-load-kubeconfig-commands.sh` at line 49, Update the SSH/SCP invocations in the load-kubeconfig script to use an argument array for SSH_OPTS, preserving intentional option splitting while preventing word splitting and glob expansion elsewhere. Apply the array form consistently to all SSH and SCP calls, and quote the REMOTE_HOST and LAST_OPENSHIFT_DPF-based operands, including the remote source path in the shown scp command.Source: Linters/SAST tools
🤖 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/dpf/hypervisor/load-kubeconfig/dpf-hypervisor-load-kubeconfig-commands.sh`:
- Line 49: Update the SSH/SCP invocations in the load-kubeconfig script to use
an argument array for SSH_OPTS, preserving intentional option splitting while
preventing word splitting and glob expansion elsewhere. Apply the array form
consistently to all SSH and SCP calls, and quote the REMOTE_HOST and
LAST_OPENSHIFT_DPF-based operands, including the remote source path in the shown
scp command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: eda4f423-d9f8-470f-b37a-894eb53440e9
📒 Files selected for processing (2)
ci-operator/step-registry/dpf/hypervisor/load-kubeconfig/dpf-hypervisor-load-kubeconfig-commands.shci-operator/step-registry/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/dpf/hypervisor/network-tests/dpf-hypervisor-network-tests-commands.sh
openshift-merge-bot
Bot
removed
the
rehearsals-ack
|
[REHEARSALNOTIFIER]
The following jobs are not rehearsable without the
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse network-access-allowed |
|
@linoyaslan: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-rh-ecosystem-edge-openshift-dpf-main-network-tests |
|
@josecastillolema: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-rh-ecosystem-edge-openshift-dpf-main-network-tests |
|
@josecastillolema: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@josecastillolema: 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. |
4 participants
This will allow to test the openshift-dpf image and be able to
/test networkPRs in openshift-dpf that makes changes to the networking testing.Summary by CodeRabbit
/test networkfor OpenShift DPF pull requests that modify networking tests..envfile before network tests.worker-dpunodes.