net, upgrade: add dedicated NIC bridge localnet connectivity test - #5798
net, upgrade: add dedicated NIC bridge localnet connectivity test#5798servolkov wants to merge 1 commit into
Conversation
|
/wip |
|
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 (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthroughThe upgrade fixtures attach dedicated-NIC localnet interfaces to both VMs and provide separate IPv4/IPv6 pools. Connectivity tests validate default and dedicated-NIC bridges before and after upgrades. ChangesDedicated-NIC fixture and VM wiring
Before- and after-upgrade connectivity tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (5 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 |
servolkov
changed the title
openshift-virtualization-qe-bot-4
changed the title
|
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
servolkov
changed the title
openshift-virtualization-qe-bot
removed
the
tox:verify-tc-requirement-polarion:running
|
D/S test |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:failed
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
|
change: reorder tests in the file |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
![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 `@tests/network/upgrade/test_localnet_connectivity.py`:
- Around line 4-5: Add the CNV-89537 Jira URL to the module-level docstring in
tests/network/upgrade/test_localnet_connectivity.py, using the existing
traceability format and preserving the current connectivity test documentation.
🪄 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: 16603150-95e8-4615-8c67-70c1c15a9f97
📒 Files selected for processing (1)
tests/network/upgrade/test_localnet_connectivity.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
|
/verified |
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='5798'
echo '=== PR metadata ==='
gh pr view "$pr" --repo "$repo" --json baseRefOid,headRefOid,files \
--jq '{baseRefOid, headRefOid, files: [.files[].path]}'
echo '=== Current PR diff ==='
gh pr diff "$pr" --repo "$repo" --patch
echo '=== Changed test and fixture definitions ==='
ast-grep outline tests/network/upgrade/conftest.py --items all --type function
ast-grep outline tests/network/upgrade/test_localnet_connectivity.py --items all --type function
echo '=== Changed fixture definitions and all consumers ==='
rg -n -C4 '\b(localnet_running_vms_upgrade|vm_localnet_upgrade_[ab]|nncp_dedicated_nic_bridge_localnet_upgrade|cudn_dedicated_nic_bridge_localnet_upgrade|ipv[46]_dedicated_nic_bridge_localnet_address_pool_upgrade)\b' tests
echo '=== Affected module tests and markers ==='
rg -n -C3 '`@pytest`\.mark\.(smoke|gating)|^def test_|localnet_running_vms_upgrade|BEFORE_UPGRADE' \
tests/network/upgrade/test_localnet_connectivity.py
echo '=== Marker files ==='
rg -l '`@pytest`\.mark\.smoke' tests/ | head -50
rg -l '`@pytest`\.mark\.gating' tests/ | head -50
echo '=== Marker use in the affected hierarchy ==='
rg -n -C2 '`@pytest`\.mark\.(smoke|gating)' \
tests/network/upgrade/conftest.py \
tests/network/conftest.py \
tests/conftest.py 2>/dev/null || trueLength of output: 40674 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='RedHatQE/openshift-virtualization-tests'
pr='5798'
base='718a7789a3a88a99fbe50776c82aec9c2e21fba6'
head='37595e4f8e24febfb06cf4011857f0c88e36bbb1'
path='tests/network/upgrade/conftest.py'
line=$(
git diff --unified=0 "$base" "$head" -- "$path" |
awk '
/^@@/ {
if (match($0, /\+([0-9]+)/, fields)) line = fields[1]
next
}
/^\+/ && !/^\+\+\+/ { print line; exit }
/^[^+-]/ { line++ }
'
)
test -n "$line"
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False.** The changed paths contain no `utilities/` or `libs/` symbols. `tests/network/upgrade/test_localnet_connectivity.py` and its `conftest.py` hierarchy have no verified `smoke` dependency.
**Run gating tests: False.** The affected tests have no `gating` marker.
**Affected tests to run**
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_after_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_after_upgrade`
All four tests use `localnet_running_vms_upgrade` → `vm_localnet_upgrade_a` / `vm_localnet_upgrade_b` → `cudn_dedicated_nic_bridge_localnet_upgrade` → `nncp_dedicated_nic_bridge_localnet_upgrade`. The fixture chain now creates and validates both localnet interfaces.
**Real tests (cluster required)**
Dedicated-NIC path, on a cluster with a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'dedicated_nic_bridge_localnet_connectivity'`
Expected: Both IPv4 and IPv6 TCP connections use `LOCALNET_OVS_BRIDGE_INTERFACE` before and after the upgrade.
Default-bridge regression:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'default_bridge_localnet_connectivity'`
Expected: Both IPv4 and IPv6 TCP connections use `LOCALNET_BR_EX_INTERFACE` before and after the upgrade.
No-secondary-NIC collection safety, on a cluster without a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py --collect-only`
Expected: Collection succeeds. The dedicated-NIC fixture must handle unavailable secondary-NIC hardware without an indexing or fixture-setup error.
EOF
)
gh api --method POST "repos/$repo/pulls/$pr/comments" \
-f body="$body" \
-f commit_id="$head" \
-f path="$path" \
-F line="$line" \
-f side='RIGHT' \
--jq '{id, path, line, side}'Length of output: 2922 You are interacting with an AI system. |
rnetser
left a comment
rnetser
left a comment
There was a problem hiding this comment.
Code Review
Found 4 issue(s) in this PR:
💡 Suggestions (4)
| File | Line | Issue |
|---|---|---|
tests/network/upgrade/test_localnet_connectivity.py |
8 | [WARNING] Module-level Preconditions not updated for the new dedicated-NIC b |
tests/network/upgrade/test_localnet_connectivity.py |
43 | [WARNING] @pytest.mark.single_nic was removed from the default-bridge test |
tests/network/upgrade/test_localnet_connectivity.py |
72 | [SUGGESTION] Per AGENTS.md: "When using @pytest.mark.dependency, a comment |
tests/network/upgrade/test_localnet_connectivity.py |
133 | [SUGGESTION] Same as above — missing dependency comment. |
Review generated by pi
Assisted-by: PI (claude-opus-4-6-1m)
…test Extend localnet upgrade tests to cover connectivity over a dedicated NIC bridge. VMs now carry two localnet interfaces (br-ex + dedicated NIC bridge). Tests verify TCP connectivity over both bridges before and after upgrade. Signed-off-by: Sergei Volkov <sevolkov@redhat.com> Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/approve |
|
/verified |
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='5798'
echo '=== PR metadata and changed paths ==='
gh pr view "$pr" --repo "$repo" --json baseRefOid,headRefOid,files \
--jq '{baseRefOid, headRefOid, files: [.files[].path]}'
echo '=== Current diff summary ==='
gh pr diff "$pr" --repo "$repo" --name-only
echo '=== Current source diff ==='
gh pr diff "$pr" --repo "$repo" --patch
echo '=== Changed fixture and test outlines ==='
ast-grep outline tests/network/upgrade/conftest.py --items all --type function 2>/dev/null || true
ast-grep outline tests/network/upgrade/test_localnet_connectivity.py --items all --type function 2>/dev/null || true
echo '=== Fixture definitions and consumers ==='
rg -n -C4 '\b(localnet_running_vms_upgrade|vm_localnet_upgrade_[ab]|nncp_dedicated_nic_bridge_localnet_upgrade|cudn_dedicated_nic_bridge_localnet_upgrade|ipv[46]_dedicated_nic_bridge_localnet_address_pool_upgrade)\b' tests || true
echo '=== Affected tests and markers ==='
rg -n -C3 '`@pytest`\.mark\.(smoke|gating)|^def test_|localnet_running_vms_upgrade|BEFORE_UPGRADE' \
tests/network/upgrade/test_localnet_connectivity.py || true
echo '=== Smoke and gating marker-file checks ==='
printf 'Smoke marker files:\n'
rg -l '`@pytest`\.mark\.smoke' tests/ | head -50 || true
printf 'Gating marker files:\n'
rg -l '`@pytest`\.mark\.gating' tests/ | head -50 || true
echo '=== Markers in affected fixture hierarchy ==='
rg -n -C2 '`@pytest`\.mark\.(smoke|gating)' \
tests/network/upgrade/conftest.py \
tests/network/conftest.py \
tests/conftest.py 2>/dev/null || trueLength of output: 42280 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='RedHatQE/openshift-virtualization-tests'
pr='5798'
base='08a920590e28a072d2c86d28dcdaa7d4c112ef61'
head='05e54ae653b08424c279d369c16a8c63bcc38436'
path='tests/network/upgrade/conftest.py'
line=$(
git diff --unified=0 "$base" "$head" -- "$path" |
awk '
/^@@/ {
if (match($0, /\+([0-9]+)/, fields)) line = fields[1]
next
}
/^\+/ && !/^\+\+\+/ { print line; exit }
/^[^+-]/ { line++ }
'
)
test -n "$line"
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False.** The changed paths contain no `utilities/` or `libs/` symbols. The affected module and its `conftest.py` hierarchy have no verified `smoke` dependency.
**Run gating tests: False.** The affected module has no `gating` marker.
**Affected tests to run**
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_after_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_after_upgrade`
All four tests use `localnet_running_vms_upgrade` → `vm_localnet_upgrade_a` / `vm_localnet_upgrade_b` → `cudn_dedicated_nic_bridge_localnet_upgrade` → `nncp_dedicated_nic_bridge_localnet_upgrade`. This session fixture chain creates both localnet interfaces and validates link-local addresses.
**Real tests (cluster required)**
Dedicated-NIC path, on a cluster with a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'dedicated_nic_bridge_localnet_connectivity'`
Expected: IPv4 and IPv6 TCP connectivity uses `LOCALNET_OVS_BRIDGE_INTERFACE` before and after the upgrade.
Default-bridge regression:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'default_bridge_localnet_connectivity'`
Expected: IPv4 and IPv6 TCP connectivity uses `LOCALNET_BR_EX_INTERFACE` before and after the upgrade.
Collection safety, on a cluster without a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py --collect-only`
Expected: Collection succeeds. The dedicated-NIC fixture must not fail when `hosts_common_available_ports` has no usable secondary NIC.
EOF
)
for attempt in 1 2; do
if gh api --method POST "repos/$repo/pulls/$pr/comments" \
-f body="$body" \
-f commit_id="$head" \
-f path="$path" \
-F line="$line" \
-f side='RIGHT' \
--jq '{id, path, line, side}'; then
exit 0
fi
sleep 2
done
exit 1Length of output: 3108 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='RedHatQE/openshift-virtualization-tests'
pr='5798'
files_json=$(mktemp)
trap 'rm -f "$files_json"' EXIT
gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid' > /tmp/pr_head_oid
head=$(cat /tmp/pr_head_oid)
gh api --paginate "repos/$repo/pulls/$pr/files?per_page=100" > "$files_json"
path=$(jq -r '.[0].filename' "$files_json")
patch=$(jq -r '.[0].patch' "$files_json")
test "$path" = 'tests/network/upgrade/conftest.py'
line=$(
printf '%s\n' "$patch" |
awk '
/^@@/ {
if (match($0, /\+([0-9]+)/, fields)) line = fields[1]
next
}
/^\+/ && !/^\+\+\+/ { print line; exit }
/^[^+-]/ { line++ }
'
)
test -n "$line"
body=$(cat <<'EOF'
## Test Execution Plan
**Run smoke tests: False.** The changed paths contain no `utilities/` or `libs/` symbols. The affected module and its `conftest.py` hierarchy have no verified `smoke` dependency.
**Run gating tests: False.** The affected module has no `gating` marker.
**Affected tests to run**
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_before_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_default_bridge_localnet_connectivity_after_upgrade`
- `tests/network/upgrade/test_localnet_connectivity.py::test_dedicated_nic_bridge_localnet_connectivity_after_upgrade`
All four tests use `localnet_running_vms_upgrade` → `vm_localnet_upgrade_a` / `vm_localnet_upgrade_b` → `cudn_dedicated_nic_bridge_localnet_upgrade` → `nncp_dedicated_nic_bridge_localnet_upgrade`. This session fixture chain creates both localnet interfaces and validates link-local addresses.
**Real tests (cluster required)**
Dedicated-NIC path, on a cluster with a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'dedicated_nic_bridge_localnet_connectivity'`
Expected: IPv4 and IPv6 TCP connectivity uses `LOCALNET_OVS_BRIDGE_INTERFACE` before and after the upgrade.
Default-bridge regression:
`pytest tests/network/upgrade/test_localnet_connectivity.py -k 'default_bridge_localnet_connectivity'`
Expected: IPv4 and IPv6 TCP connectivity uses `LOCALNET_BR_EX_INTERFACE` before and after the upgrade.
Collection safety, on a cluster without a usable secondary NIC:
`pytest tests/network/upgrade/test_localnet_connectivity.py --collect-only`
Expected: Collection succeeds. The dedicated-NIC fixture must not fail when `hosts_common_available_ports` has no usable secondary NIC.
EOF
)
for attempt in 1 2; do
if gh api --method POST "repos/$repo/pulls/$pr/comments" \
-f body="$body" \
-f commit_id="$head" \
-f path="$path" \
-F line="$line" \
-f side='RIGHT' \
--jq '{id, path, line, side}'; then
exit 0
fi
sleep 2
done
exit 1Length of output: 3187
You are interacting with an AI system. |
|
/check-can-merge |
12 participants
What this PR does / why we need it:
Add dedicated NIC bridge localnet connectivity upgrade tests alongside the
existing default bridge (br-ex) tests.
Both VMs now carry two localnet interfaces to avoid creating additional VMs.
The tests verify TCP connectivity over the dedicated NIC bridge before and
after upgrade.
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket: https://redhat.atlassian.net/browse/CNV-89537
Summary by CodeRabbit