net, tests, udn: Add VM-to-pod connectivity test - #5940
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughChangesUDN tests now create a running pod in the UDN namespace. The suite resolves the pod’s default network IP and verifies VM-to-pod connectivity with three pings and a one-minute timeout. UDN connectivity tests
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This PR adds localized VM-to-pod connectivity coverage for Primary User Defined Networks, with no actionable merge-blocking risk remaining beyond normal checks and review. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
📌 Additional InformationCustom Commands:
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
Anatw
changed the title
|
@coderabbitai, please review this PR. |
|
✅ Action performedReview finished.
|
![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 `@libs/net/udn.py`:
- Around line 50-53: Update the helper containing the network-status parsing so
the value returned by json.loads is explicitly validated or narrowed to the
expected typed structure before accessing udn_entry["ips"][0]. Ensure the final
return is statically proven to be a str, satisfying strict mypy no-any-return
while preserving the existing network-status selection behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 00347232-07f7-48fe-9d01-29c529c8e7ab
📒 Files selected for processing (3)
libs/net/udn.pytests/network/user_defined_network/conftest.pytests/network/user_defined_network/test_user_defined_network.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
|
@coderabbitai - Please review this PR (we discussed your single comment. Do you lgtm?) |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:passed
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5940 published |
fa43ae8 to
488ca3f
Compare
|
Change: rename lookup_udn_pod_ip -> lookup_default_pod_ip to fit implementation and move i a local helper file. |
openshift-virtualization-qe-bot
removed
the
tox:verify-tc-requirement-polarion:passed
|
change: Added missing tests/network/user_defined_network/libudn.py |
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 `@tests/network/user_defined_network/conftest.py`:
- Line 22: Replace the incorrect multiprocessing.dummy Namespace import with
ocp_resources.namespace.Namespace in conftest.py, so the udn_namespace
annotation and its name attribute use the OpenShift resource type yielded by
create_ns.
- Line 5: Add postponed annotation evaluation at the top of the module, then
move the DynamicClient import into the existing TYPE_CHECKING block so it is
available for type checking without runtime import errors.
In `@tests/network/user_defined_network/test_user_defined_network.py`:
- Around line 84-87: Add the required Jira traceability docstring to
TestPrimaryUdn.test_vm_to_pod_connectivity_on_udn, using the CNV-51404 URL and
skip-jira-utils-check marker. Keep the existing test behavior 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c873e10-975d-44f1-91a7-c598267915ec
📒 Files selected for processing (3)
libs/net/udn.pytests/network/user_defined_network/conftest.pytests/network/user_defined_network/test_user_defined_network.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
|
Change: fix incorrect import. |
|
Change: add Jira link. |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5940 published |
|
/verified Tested on net-awax-5 cnv-tests-runner-containerized/6223 |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5940"
echo '=== PR changed files and first patch lines ==='
gh pr diff "$pr" --repo "$repo" --name-only
gh pr diff "$pr" --repo "$repo" --patch | sed -n '1,360p'
echo '=== Changed source outlines ==='
for file in \
libs/net/udn.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/libudn.py \
tests/network/user_defined_network/test_user_defined_network.py
do
if [ -f "$file" ]; then
echo "--- $file ---"
ast-grep outline "$file" --items all || true
fi
done
echo '=== Modified Python definitions in PR diff ==='
gh pr diff "$pr" --repo "$repo" --unified=0 | grep -E '^[+-][[:space:]]*(async[[:space:]]+)?def[[:space:]]' || true
echo '=== Helper definitions, imports, calls, fixtures, and tests ==='
rg -n -A28 -B8 \
'def _lookup_default_pod_ip\b|def lookup_default_pod_ip\b|def _lookup_udn_pod_ip\b|def udn_pod\b|def test_vm_to_pod_connectivity_on_udn\b|lookup_default_pod_ip\s*\(|_lookup_default_pod_ip\s*\(|udn_pod\b' \
libs tests/network/user_defined_network
echo '=== Direct caller search across tests and conftest ==='
rg -n -C3 \
'\b(_lookup_default_pod_ip|lookup_default_pod_ip|_lookup_udn_pod_ip|lookup_udn_pod_ip)\s*\(' \
tests || true
echo '=== Fixture cleanup and context-manager paths ==='
rg -n -C4 \
'yield from|`@pytest`\.fixture|`@contextmanager`|lookup_default_pod_ip|_lookup_default_pod_ip' \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/libudn.py \
tests/conftest.py 2>/dev/null || true
echo '=== Smoke marker files ==='
rg -l '`@pytest`\.mark\.smoke' tests/ || true
echo '=== Gating marker files ==='
rg -l '`@pytest`\.mark\.gating' tests/ || true
echo '=== Markers in the affected UDN test module ==='
rg -n -C3 '`@pytest`\.mark\.(smoke|gating)|test_vm_to_pod_connectivity_on_udn|class Test' \
tests/network/user_defined_network/test_user_defined_network.py
echo '=== Conftest files in hierarchy ==='
for file in \
conftest.py \
tests/conftest.py \
tests/network/conftest.py \
tests/network/user_defined_network/conftest.py
do
if [ -f "$file" ]; then
echo "--- $file ---"
rg -n -C3 'lookup_default_pod_ip|_lookup_default_pod_ip|udn_pod|autouse' "$file" || true
fi
doneLength of output: 112555 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5940"
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False** — The smoke-marked test files do not import or reach `lookup_default_pod_ip` or the `udn_pod` fixture. The UDN `conftest.py` is not in a smoke test parent hierarchy.
**Run gating tests: False** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` is not gating-marked. The gating test in the same module uses only `server` and `client`; it does not use `udn_pod` or `lookup_default_pod_ip`.
**Affected tests to run**
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` (ipv4, single_nic)
**Real tests (cluster required)**
Error/fix path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn`
Expected: the class-scoped `udn_pod` fixture creates a pod in the Primary UDN namespace, `lookup_default_pod_ip` selects the `default` network-status entry, and `vma_udn` can ping that IP.
Happy path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_basic_connectivity_between_udn_vms`
Expected: existing VM-to-VM connectivity in the same Primary UDN namespace remains successful.
EOF
)
payload=$(jq -n \
--arg commit_id "$head_sha" \
--arg body "$body" \
'{
commit_id: $commit_id,
event: "COMMENT",
comments: [{
path: "libs/net/udn.py",
line: 1,
side: "RIGHT",
body: $body
}]
}')
gh api \
--method POST \
"repos/$repo/pulls/$pr/reviews" \
--input - <<<"$payload" \
--jq '{id, state, html_url, comments: [.comments[] | {path, line, side, body}]}'Length of output: 2404 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5940"
existing_count=$(gh api --paginate \
"repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '[.[] | select(.user.login == "coderabbitai[bot]" or .user.login == "coderabbitai") | select(.path == "libs/net/udn.py" and .line == 1 and (.body | contains("## Test Execution Plan"))] | length')
if [ "$existing_count" -eq 0 ]; then
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False** — The smoke-marked test files do not import or reach `lookup_default_pod_ip` or the `udn_pod` fixture. The UDN `conftest.py` is not in a smoke test parent hierarchy.
**Run gating tests: False** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` is not gating-marked. The gating test in the same module uses only `server` and `client`; it does not use `udn_pod` or `lookup_default_pod_ip`.
**Affected tests to run**
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` (ipv4, single_nic)
**Real tests (cluster required)**
Error/fix path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn`
Expected: the class-scoped `udn_pod` fixture creates a pod in the Primary UDN namespace, `lookup_default_pod_ip` selects the `default` network-status entry, and `vma_udn` can ping that IP.
Happy path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_basic_connectivity_between_udn_vms`
Expected: existing VM-to-VM connectivity in the same Primary UDN namespace remains successful.
EOF
)
jq -n \
--arg commit_id "$head_sha" \
--arg body "$body" \
'{commit_id: $commit_id, event: "COMMENT", comments: [{path: "libs/net/udn.py", line: 1, side: "RIGHT", body: $body}]}' \
| gh api --method POST "repos/$repo/pulls/$pr/reviews" --input - --jq '{id, state, html_url}'
else
gh api --paginate \
"repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '.[] | select(.user.login == "coderabbitai[bot]" or .user.login == "coderabbitai") | select(.path == "libs/net/udn.py" and .line == 1 and (.body | contains("## Test Execution Plan"))) | {id, path, line, side}'
fiLength of output: 3342 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5940"
comment_query='[.[] | select(.path == "libs/net/udn.py" and .line == 1 and (.body | contains("## Test Execution Plan")))]'
existing_count=$(gh api --paginate \
"repos/$repo/pulls/$pr/comments?per_page=100" \
--jq "$comment_query | length")
if [ "$existing_count" -gt 0 ]; then
gh api --paginate \
"repos/$repo/pulls/$pr/comments?per_page=100" \
--jq "$comment_query | .[] | {id, path, line, side}"
exit 0
fi
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False** — The smoke-marked test files do not import or reach `lookup_default_pod_ip` or the `udn_pod` fixture. The UDN `conftest.py` is not in a smoke test parent hierarchy.
**Run gating tests: False** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` is not gating-marked. The gating test in the same module uses only `server` and `client`; it does not use `udn_pod` or `lookup_default_pod_ip`.
**Affected tests to run**
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn` (ipv4, single_nic)
**Real tests (cluster required)**
Error/fix path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_vm_to_pod_connectivity_on_udn`
Expected: the class-scoped `udn_pod` fixture creates a pod in the Primary UDN namespace, `lookup_default_pod_ip` selects the `default` network-status entry, and `vma_udn` can ping that IP.
Happy path:
`pytest -s -o log_cli=true -m tier2 --jira --skip-deprecated-api-test tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_basic_connectivity_between_udn_vms`
Expected: existing VM-to-VM connectivity in the same Primary UDN namespace remains successful.
EOF
)
jq -n \
--arg commit_id "$head_sha" \
--arg body "$body" \
'{commit_id: $commit_id, event: "COMMENT", comments: [{path: "libs/net/udn.py", line: 1, side: "RIGHT", body: $body}]}' \
| gh api --method POST "repos/$repo/pulls/$pr/reviews" --input - --jq '{id, state, html_url}'Length of output: 2566
You are interacting with an AI system. |
Test successful connectivity from a VM to a pod, both connected to the UDN. Signed-off-by: Anat Wax <awax@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
|
Change: raise IpNotFound if no ip were assigned to the pod. |
|
/verified net-awax-5 (locally) |
|
/lgtm |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="5940"
echo "=== Current PR metadata and changed files ==="
gh pr view "$pr" --repo "$repo" --json headRefOid,baseRefOid,files \
--jq '{headRefOid, baseRefOid, files: [.files[] | {path, additions, deletions}]}'
echo "=== Current PR patch ==="
gh pr diff "$pr" --repo "$repo" --patch
echo "=== First changed lines on the PR branch ==="
git diff --unified=0 "$(git merge-base HEAD origin/main)" HEAD -- \
| awk '
/^\+\+\+ b\// { file=substr($0, 7) }
/^@@/ {
if (match($0, /\+[0-9]+/)) {
hunk=substr($0, RSTART + 1, RLENGTH - 1)
sub(/,.*/, "", hunk)
print file ": right-side hunk starts at line " hunk
}
}'
echo "=== Outlines ==="
for f in \
libs/net/udn.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/libudn.py \
tests/network/user_defined_network/test_user_defined_network.py
do
test -f "$f" && { echo "--- $f"; ast-grep outline "$f" --items all; }
done
echo "=== Modified definitions and helper call paths ==="
git diff --unified=0 "$(git merge-base HEAD origin/main)" HEAD -- \
| grep -E '^[+-][[:space:]]*(async[[:space:]]+)?def[[:space:]]' || true
rg -n -C4 \
'\b(lookup_default_pod_ip|_lookup_default_pod_ip|lookup_udn_pod_ip|_lookup_udn_pod_ip|udn_pod)\b' \
libs tests || true
echo "=== Fixture cleanup and wrappers ==="
rg -n -C4 \
'yield from|`@pytest`\.fixture|`@contextmanager`|lookup_default_pod_ip|_lookup_default_pod_ip|udn_pod' \
tests/conftest.py tests/network/conftest.py tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/libudn.py 2>/dev/null || true
echo "=== Smoke and gating marker files ==="
rg -l '`@pytest`\.mark\.smoke' tests/ || true
rg -l '`@pytest`\.mark\.gating' tests/ || true
echo "=== Affected UDN module markers and tests ==="
rg -n -C3 \
'`@pytest`\.mark\.(smoke|gating)|def test_|class Test' \
tests/network/user_defined_network/test_user_defined_network.pyLength of output: 7999 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Source outlines ==="
for f in \
libs/net/udn.py \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/libudn.py \
tests/network/user_defined_network/test_user_defined_network.py
do
echo "--- $f"
ast-grep outline "$f" --items all
done
echo "=== Helper calls, fixtures, tests, and same-file callers ==="
rg -n -C5 \
'\b(lookup_default_pod_ip|udn_pod|vma_udn|server|client)\b|`@pytest`\.fixture|yield from|`@contextmanager`|def test_' \
tests/network/user_defined_network libs/net/udn.py
echo "=== Direct imports and calls outside the changed UDN files ==="
rg -n -C3 \
'(from\s+tests\.network\.user_defined_network\.libudn\s+import|lookup_default_pod_ip\s*\()' \
tests libs || true
echo "=== Smoke marker files and their conftest ancestry ==="
smoke_files=$(rg -l '`@pytest`\.mark\.smoke' tests/ || true)
printf '%s\n' "$smoke_files"
while IFS= read -r f; do
test -z "$f" && continue
d=$(dirname "$f")
while :; do
candidate="$d/conftest.py"
if test -f "$candidate"; then
hits=$(rg -n 'lookup_default_pod_ip|udn_pod|tests\.network\.user_defined_network\.libudn' "$candidate" || true)
test -n "$hits" && printf '%s\n%s\n' "--- $candidate" "$hits"
fi
test "$d" = "tests" && break
d=$(dirname "$d")
done
done <<< "$smoke_files"
echo "=== Gating marker files and UDN test marker scope ==="
rg -n -C3 '`@pytest`\.mark\.gating|def test_' tests/network/user_defined_network/test_user_defined_network.py
rg -l '`@pytest`\.mark\.gating' tests/ || true
echo "=== Existing CodeRabbit inline plan on the requested first line ==="
gh api --paginate \
'repos/RedHatQE/openshift-virtualization-tests/pulls/5940/comments?per_page=100' \
--jq '.[] | select((.user.login == "coderabbitai[bot]" or .user.login == "coderabbitai") and .path == "libs/net/udn.py" and .line == 1 and (.body | contains("## Test Execution Plan"))) | {id, commit_id, body}'Length of output: 75581
🧠 Learnings usedYou are interacting with an AI system. |
|
/check-can-merge |
11 participants
What this PR does / why we need it:
Verify that a VM and a pod, both connected to the same Primary User Defined Network, have connectivity over the UDN interfaces.
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-51404
Summary by CodeRabbit
New Features
Tests