net: Introduce dual-stream udn tests#4786
Conversation
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesUDN Mixed-OS Migration Connectivity
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-2
requested review from
Anatw,
EdDev,
dshchedr,
nirdothan,
orelmisan,
rnetser,
servolkov and
yossisegev
azhivovk
force-pushed
the
new_rhcos9-10_tests_udn
branch
from
30236fa to
38f8418
Compare
|
Change: Drop localnet changes |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/wip |
openshift-virtualization-qe-bot-3
changed the title
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Change: fixtures' scope module -> class (scope should be minimal and specified as possible) |
servolkov
left a comment
servolkov
left a comment
There was a problem hiding this comment.
Thanks, only small ones from my side:
|
Apply changes after CR:
|
![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/user_defined_network/rhel9_rhel10_cluster/conftest.py`:
- Around line 4-6: Move the annotation-only imports in conftest.py behind a
TYPE_CHECKING guard so they are not loaded at runtime; the symbols to relocate
include DynamicClient, Namespace, Layer2UserDefinedNetwork, TcpServer,
VMTcpClient, and BaseVirtualMachine. Update the relevant type-hint usage in the
fixture/helper definitions to work with deferred or quoted annotations, and if
any annotations are still evaluated at runtime, add from __future__ import
annotations or quote those references so the module imports cleanly without
extra collection-time work or cycle risk.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65067b0d-ea10-443f-81f3-70316ae7877c
📒 Files selected for processing (2)
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.pytests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py
|
Change: use |
|
/verified Regression due to udn_vm refactoring: |
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="4786"
echo "== PR metadata =="
gh pr view "$pr" --repo "$repo" --json number,baseRefName,headRefName,headRefOid,url,title
echo
echo "== Changed files in PR order =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate \
| jq -r '.[] | [.filename, .status] | `@tsv`'
base_branch=$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq '.baseRefName')
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')
echo
echo "== Fetch base branch and compute merge-base =="
git fetch --no-tags origin "$base_branch":"refs/remotes/origin/$base_branch" >/dev/null 2>&1 || true
merge_base=$(git merge-base "origin/$base_branch" HEAD)
echo "BASE_BRANCH=$base_branch"
echo "MERGE_BASE=$merge_base"
echo "HEAD_SHA=$head_sha"
echo
echo "== Diff summary (merge-base...HEAD) =="
git diff --stat "$merge_base"...HEAD
echo
echo "== First changed file patch from GitHub API =="
first_file=$(gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r '.[0].filename')
echo "FIRST_FILE=$first_file"
gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate \
| jq -r --arg f "$first_file" '.[] | select(.filename==$f) | .patch'
echo
echo "== Modified defs in tests/network/libs/vm_factory.py =="
git diff --unified=0 "$merge_base"...HEAD -- tests/network/libs/vm_factory.py | sed -n '1,220p'
echo
git diff --unified=0 "$merge_base"...HEAD -- tests/network/libs/vm_factory.py | grep -nE '^[+-].*(def |class )' || true
echo
echo "== AST outline of tests/network/libs/vm_factory.py =="
ast-grep outline tests/network/libs/vm_factory.py --view expanded || true
echo
echo "== Relevant slices: tests/network/libs/vm_factory.py =="
sed -n '1,260p' tests/network/libs/vm_factory.py
echo
echo "== Relevant slices: tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py =="
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py
echo
echo "== Relevant slices: tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py =="
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py
echo
echo "== Direct callers/importers of udn_vm in tests and conftest =="
rg -n --glob 'tests/**/*.py' '\budn_vm\b' tests
echo
echo "== Same-file callers / affinity handling in vm_factory =="
rg -n 'affinity|template_labels|anti_affinity_namespaces|udn_vm\s*\(' tests/network/libs/vm_factory.py
echo
echo "== Smoke-marked test files =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.smoke' tests || true
echo
echo "== Gating-marked test files =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.gating' tests || true
echo
echo "== UDN RHEL9/RHEL10 references =="
rg -n --glob 'tests/**/*.py' 'rhel9_rhel10_cluster|mixed_os_nodes|RHCOS9_WORKER_LABEL|client_server_active_connection|active_tcp_connections' tests
echo
echo "== Parent conftest chain for affected test directory =="
for f in \
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py \
tests/network/user_defined_network/conftest.py \
tests/network/conftest.py \
tests/conftest.py
do
if [[ -f "$f" ]]; then
echo "--- $f"
rg -n '\budn_vm\b|client_server_active_connection|active_tcp_connections|new_node_affinity|RHCOS9_WORKER_LABEL|affinity' "$f" || true
fi
doneLength of output: 37157 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
base_branch=$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq '.baseRefName')
git fetch --no-tags origin "$base_branch":"refs/remotes/origin/$base_branch" >/dev/null 2>&1 || true
merge_base=$(git merge-base "origin/$base_branch" HEAD)
echo "== Exact added lines in first file =="
git diff --unified=0 "$merge_base"...HEAD -- tests/network/libs/vm_factory.py
echo
echo "== udn_vm call sites with surrounding context =="
for f in \
tests/network/user_defined_network/test_user_defined_network.py \
tests/network/user_defined_network/test_user_defined_network_passt.py \
tests/network/user_defined_network/ip_specification/conftest.py \
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py \
tests/network/provider_migration/conftest.py \
tests/network/bgp/conftest.py \
tests/network/bgp/evpn/conftest.py
do
echo "--- $f"
sed -n '1,260p' "$f" | nl -ba | sed -n '1,260p'
echo
done
echo
echo "== Search for udn_vm calls passing affinity= =="
rg -nUP --glob 'tests/**/*.py' 'udn_vm\s*\([^)]*affinity\s*=' tests || true
echo
echo "== Search for udn_vm calls passing template_labels= =="
rg -nUP --glob 'tests/**/*.py' 'udn_vm\s*\([^)]*template_labels\s*=' tests || true
echo
echo "== Search for udn_vm calls passing both affinity= and template_labels= =="
python - <<'PY'
import pathlib, re
for path in pathlib.Path("tests").rglob("*.py"):
text = path.read_text()
for m in re.finditer(r'udn_vm\s*\((.*?)\)', text, re.S):
call = m.group(1)
if 'affinity=' in call and 'template_labels=' in call:
line = text[:m.start()].count('\n') + 1
print(f"{path}:{line}")
PY
echo
echo "== Gating markers in user_defined_network test file =="
rg -n -C2 '`@pytest`\.mark\.gating|def test_|class ' tests/network/user_defined_network/test_user_defined_network.py
echo
echo "== Parent conftest chain for direct caller files =="
for f in \
tests/network/user_defined_network/test_user_defined_network.py \
tests/network/user_defined_network/test_user_defined_network_passt.py \
tests/network/provider_migration/test_ip_persistence.py \
tests/network/bgp/evpn/test_evpn_connectivity.py
do
dir=$(dirname "$f")
echo "### caller=$f"
while true; do
cf="$dir/conftest.py"
if [[ -f "$cf" ]]; then
echo "--- $cf"
rg -n '\budn_vm\b|def .*vm|`@pytest`\.fixture|yield|client_server_active_connection|active_tcp_connections|new_node_affinity|template_labels|affinity' "$cf" || true
fi
[[ "$dir" == "." || "$dir" == "tests" ]] && break
dir=$(dirname "$dir")
done
done
echo
echo "== Smoke files overlapping network subtree =="
rg -n --glob 'tests/network/**/*.py' '`@pytest`\.mark\.smoke' tests || true
echo
echo "== PR review threads / existing comments on first file =="
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r '.[] | select(.path=="tests/network/libs/vm_factory.py") | [.id, .path, .line, .side, (.body|gsub("\n";" ")|.[0:140])] | `@tsv`'Length of output: 5324 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
echo "== udn_vm call inventory (affinity/template_labels flags) =="
python - <<'PY'
import pathlib, re
for path in sorted(pathlib.Path("tests").rglob("*.py")):
text = path.read_text()
for m in re.finditer(r'udn_vm\s*\((.*?)\)', text, re.S):
call = m.group(1)
line = text[:m.start()].count('\n') + 1
affinity = 'affinity=' in call
labels = 'template_labels=' in call
print(f"{path}:{line}\taffinity={affinity}\ttemplate_labels={labels}")
PY
echo
for f in \
tests/network/user_defined_network/test_user_defined_network.py \
tests/network/user_defined_network/test_user_defined_network_passt.py \
tests/network/user_defined_network/ip_specification/conftest.py \
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py \
tests/network/provider_migration/conftest.py \
tests/network/bgp/conftest.py \
tests/network/bgp/evpn/conftest.py
do
if [[ -f "$f" ]]; then
echo "== $f =="
rg -n -C3 '\budn_vm\s*\(|`@pytest`\.mark\.(gating|smoke|polarion|mixed_os_nodes|ipv4)|def test_|class |`@pytest`\.fixture' "$f" || true
echo
fi
done
echo "== Direct references to fixtures introduced in this PR =="
rg -n -C2 '\budn_server_vm\b|\budn_client_vm\b|\budn_active_tcp_connection\b' tests/network/user_defined_network/rhel9_rhel10_cluster
echo
echo "== Network smoke-marked files =="
rg -n --glob 'tests/network/**/*.py' '`@pytest`\.mark\.smoke' tests || true
echo
echo "== Gating-marked files that also reference udn_vm directly =="
python - <<'PY'
import pathlib, re
for path in sorted(pathlib.Path("tests").rglob("*.py")):
text = path.read_text()
if '`@pytest.mark.gating`' in text and re.search(r'\budn_vm\b', text):
print(path)
PY
echo
echo "== Existing PR comments on tests/network/libs/vm_factory.py =="
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r '.[] | select(.path=="tests/network/libs/vm_factory.py") | [.id, .line, .side, (.body|gsub("\n";" ")|.[0:160])] | `@tsv`'Length of output: 27918 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Consumers of affected BGP/EVPN fixtures =="
for sym in vm_cudn tcp_server_cudn_vm vm_evpn_target vm_evpn_reference; do
echo "-- $sym"
rg -n --glob 'tests/**/*.py' "\\b${sym}\\b" tests/network/bgp tests/network/bgp/evpn || true
echo
done
echo "== Test files under affected directories =="
fd -t f '^test_.*\.py$' tests/network/bgp tests/network/bgp/evpn tests/network/user_defined_network/ip_specification tests/network/user_defined_network
echo
echo "== Any existing smoke/gating markers in those candidate files =="
for f in $(fd -t f '^test_.*\.py$' tests/network/bgp tests/network/bgp/evpn tests/network/user_defined_network/ip_specification tests/network/user_defined_network); do
echo "--- $f"
rg -n '`@pytest`\.mark\.(gating|smoke|polarion|ipv4|mixed_os_nodes)' "$f" || true
doneLength of output: 11253 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
path="tests/network/libs/vm_factory.py"
line="8"
side="RIGHT"
me="$(gh api user --jq '.login')"
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
existing_id="$(
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r --arg me "$me" --arg path "$path" --argjson line "$line" --arg side "$side" '
.[] | select(.user.login == $me and .path == $path and .line == $line and .side == $side) | .id
' \
| head -n1
)"
if [[ -n "${existing_id:-}" ]]; then
exit 0
fi
body=$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures → `tests/network/libs/vm_factory.py::udn_vm` (`template_labels` anti-affinity path).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py` — direct coverage of the new `affinity=` path via `udn_server_vm` / `udn_client_vm`
- `tests/network/user_defined_network/test_user_defined_network.py` (includes gating coverage)
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: the VM is created on an RHCOS 9 node via explicit `affinity=`, migration to RHCOS 10 succeeds after `set_template_affinity(...)`, and the TCP session survives.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: the existing `template_labels`-driven anti-affinity path still schedules correctly and the TCP session survives migration.
EOF
)
gh api "repos/$repo/pulls/$pr/comments" \
-X POST \
-f body="$body" \
-f commit_id="$head_sha" \
-f path="$path" \
-F line="$line" \
-f side="$side"Length of output: 2936 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
body=$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures → `tests/network/libs/vm_factory.py::udn_vm` (`template_labels` anti-affinity path).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py` — direct coverage of the new `affinity=` path via `udn_server_vm` / `udn_client_vm`
- `tests/network/user_defined_network/test_user_defined_network.py` (includes gating coverage)
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: the VM is created on an RHCOS 9 node via explicit `affinity=`, migration to RHCOS 10 succeeds after `set_template_affinity(...)`, and the TCP session survives.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: the existing `template_labels`-driven anti-affinity path still schedules correctly and the TCP session survives migration.
EOF
)
gh pr comment "$pr" --repo "$repo" --body "$body"Length of output: 2295 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
body_file="$(mktemp)"
cat > "$body_file" <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures → `tests/network/libs/vm_factory.py::udn_vm` (`template_labels` anti-affinity path).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py` — direct coverage of the new `affinity=` path via `udn_server_vm` / `udn_client_vm`
- `tests/network/user_defined_network/test_user_defined_network.py` (includes gating coverage)
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: the VM is created on an RHCOS 9 node via explicit `affinity=`, migration to RHCOS 10 succeeds after `set_template_affinity(...)`, and the TCP session survives.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: the existing `template_labels`-driven anti-affinity path still schedules correctly and the TCP session survives migration.
EOF
gh api "repos/$repo/issues/$pr/comments" \
-X POST \
--input "$body_file"Length of output: 2502
Test Execution Plan
Real tests (cluster required) Error path (the fix): Happy path (regression): |
|
/check-can-merge |
1 similar comment
|
/check-can-merge |
The new primary UDN tests need to pin VMs to specific RHCOS node types during live migration, requiring explicit node affinity support in udn_vm. Add affinity parameter. template_labels and affinity are independent: labels are always applied when provided; affinity takes precedence over the auto-generated pod anti-affinity from template_labels, but does not suppress the labels. Signed-off-by: Asia Khromov <azhivovk@redhat.com> Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The l2bridge UDN binding depends on components that may behave differently across RHCOS versions. These tests verify that an active TCP connection over a primary UDN is preserved when the server VM migrates between RHCOS 9 and RHCOS 10 nodes in both directions. Tests cover IPv4 only - IPv6 is not yet officially supported in primary UDN. Signed-off-by: Asia Khromov <azhivovk@redhat.com> Assisted-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Change after CR: Apply affinity when given since there's no point of overriding default affinity when |
|
/verified New dual-stream tests passed Last change produces the same logic as before |
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="4786"
base_branch="$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq '.baseRefName')"
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
git fetch --no-tags origin "$base_branch":"refs/remotes/origin/$base_branch" >/dev/null 2>&1 || true
merge_base="$(git merge-base "origin/$base_branch" HEAD)"
# Discover current PR file order from GitHub and capture the first changed file.
first_file="$(gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r '.[0].filename')"
# Determine the first added/changed line on the RIGHT side for the first file.
first_right_line="$(
gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate \
| jq -r --arg f "$first_file" '
.[] | select(.filename==$f) | .patch
' \
| python - <<'PY'
import sys, re
patch = sys.stdin.read().splitlines()
new_line = None
for line in patch:
if line.startswith('@@'):
m = re.search(r'\+(\d+)(?:,(\d+))?', line)
if m:
new_line = int(m.group(1))
continue
if new_line is None:
continue
if line.startswith('+') and not line.startswith('+++'):
print(new_line)
raise SystemExit(0)
if line.startswith('-') and not line.startswith('---'):
continue
new_line += 1
raise SystemExit("No added line found in first file patch")
PY
)"
# Fresh evidence for the current diff.
echo "BASE_BRANCH=$base_branch"
echo "MERGE_BASE=$merge_base"
echo "HEAD_SHA=$head_sha"
echo "FIRST_FILE=$first_file"
echo "FIRST_RIGHT_LINE=$first_right_line"
echo "== Current diff summary =="
git diff --stat "$merge_base"...HEAD
echo "== Current zero-context diff for first file =="
git diff --unified=0 "$merge_base"...HEAD -- "$first_file"
echo "== Current first file outline =="
ast-grep outline "$first_file" --view expanded || true
echo "== Current first file slice =="
sed -n '1,260p' "$first_file"
echo "== Current changed test files =="
gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r '.[] | [.filename, .status] | `@tsv`'
echo "== Direct callers of udn_vm in tests =="
rg -n --glob 'tests/**/*.py' '\budn_vm\s*\(' tests
echo "== Same-file references to udn_vm/affinity/template_labels in first file =="
rg -n '^\s*def\s+udn_vm\b|\baffinity\b|\btemplate_labels\b|\banti_affinity_namespaces\b' "$first_file"
echo "== New dual-stream conftest =="
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py
echo "== New dual-stream test file =="
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py
echo "== Existing IPv4 UDN test file =="
sed -n '1,260p' tests/network/user_defined_network/test_user_defined_network.py
echo "== UDN passt test file =="
sed -n '1,220p' tests/network/user_defined_network/test_user_defined_network_passt.py
echo "== IP specification test file =="
sed -n '1,220p' tests/network/user_defined_network/ip_specification/test_ip_specification.py
echo "== BGP connectivity test file =="
sed -n '1,220p' tests/network/bgp/test_bgp_connectivity.py
echo "== EVPN connectivity test file =="
sed -n '1,240p' tests/network/bgp/evpn/test_evpn_connectivity.py
echo "== Caller conftests =="
for f in \
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py \
tests/network/provider_migration/conftest.py \
tests/network/bgp/conftest.py \
tests/network/bgp/evpn/conftest.py
do
if [[ -f "$f" ]]; then
echo "--- $f"
rg -n -C2 '\budn_vm\s*\(|`@pytest`\.fixture|yield|template_labels|affinity|new_node_affinity|client_server_active_connection|active_tcp_connections' "$f" || true
fi
done
echo "== Smoke-marked files =="
rg -l '`@pytest`\.mark\.smoke' tests || true
echo "== Gating-marked files =="
rg -l '`@pytest`\.mark\.gating' tests || true
# Build the updated plan body. Use specific affected tests where possible.
body_file="$(mktemp)"
cat > "$body_file" <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — I did not verify any smoke-marked test or parent `conftest.py` dependency path that reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures in `tests/network/user_defined_network/conftest.py` → `tests/network/libs/vm_factory.py::udn_vm` (existing `template_labels` path touched by this change).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos9`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_server_live_migration`
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: when explicit `affinity=` is passed into `udn_vm`, it is applied to the VM template, the server migrates from an RHCOS 9 worker to an RHCOS 10 worker after `set_template_affinity(...)`, and the TCP connection stays established.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: existing callers that rely on the default `template_labels` anti-affinity path still schedule correctly and preserve the TCP connection during migration.
EOF
# Find an existing inline comment by coderabbitai[bot] on the same path/line, update if found; otherwise create it.
existing_id="$(
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r --arg path "$first_file" --argjson line "$first_right_line" '
.[] | select(.user.login=="coderabbitai[bot]" and .path==$path and .line==$line and .side=="RIGHT") | .id
' \
| head -n1
)"
if [[ -n "${existing_id:-}" ]]; then
gh api "repos/$repo/pulls/comments/$existing_id" \
-X PATCH \
--input "$body_file" >/dev/null
else
gh api "repos/$repo/pulls/$pr/comments" \
-X POST \
-f commit_id="$head_sha" \
-f path="$first_file" \
-F line="$first_right_line" \
-f side="RIGHT" \
--input "$body_file" >/dev/null
fiLength of output: 7520 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
base_branch="$(gh pr view "$pr" --repo "$repo" --json baseRefName --jq '.baseRefName')"
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
git fetch --no-tags origin "$base_branch":"refs/remotes/origin/$base_branch" >/dev/null 2>&1 || true
merge_base="$(git merge-base "origin/$base_branch" HEAD)"
first_file="$(gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r '.[0].filename')"
patch="$(
gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate \
| jq -r --arg f "$first_file" '.[] | select(.filename==$f) | .patch // ""'
)"
echo "FIRST_FILE=$first_file"
echo "== Patch for first changed file =="
printf '%s\n' "$patch"
read -r comment_side comment_line < <(
PATCH_CONTENT="$patch" python - <<'PY'
import os, re, sys
patch = os.environ["PATCH_CONTENT"].splitlines()
old_line = new_line = None
for line in patch:
if line.startswith('@@'):
m = re.search(r'-(\d+)(?:,\d+)? \+(\d+)(?:,\d+)?', line)
if not m:
continue
old_line = int(m.group(1))
new_line = int(m.group(2))
continue
if old_line is None or new_line is None:
continue
if line.startswith('+') and not line.startswith('+++'):
print("RIGHT", new_line)
sys.exit(0)
if line.startswith('-') and not line.startswith('---'):
print("LEFT", old_line)
sys.exit(0)
old_line += 1
new_line += 1
sys.exit("Unable to determine first changed line/side from patch")
PY
)
echo "COMMENT_SIDE=$comment_side"
echo "COMMENT_LINE=$comment_line"
echo "== Current diff summary =="
git diff --stat "$merge_base"...HEAD
echo "== Current zero-context diff for first file =="
git diff --unified=0 "$merge_base"...HEAD -- "$first_file"
echo "== Modified symbol(s) in libs file(s) =="
git diff --unified=0 "$merge_base"...HEAD -- tests/network/libs/vm_factory.py | grep -nE '^[+-].*(def |class )' || true
echo "== Direct callers of udn_vm in tests/conftest =="
rg -n --glob 'tests/**/*.py' '\budn_vm\s*\(' tests
echo "== Gating-marked files =="
rg -l '`@pytest`\.mark\.gating' tests || true
echo "== Smoke-marked files =="
rg -l '`@pytest`\.mark\.smoke' tests || true
echo "== Relevant slices =="
sed -n '1,260p' tests/network/libs/vm_factory.py
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py
sed -n '1,260p' tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py
sed -n '1,260p' tests/network/user_defined_network/test_user_defined_network.py
sed -n '1,220p' tests/network/user_defined_network/test_user_defined_network_passt.py
sed -n '1,220p' tests/network/user_defined_network/ip_specification/test_ip_specification.py
sed -n '1,220p' tests/network/bgp/test_bgp_connectivity.py
sed -n '1,240p' tests/network/bgp/evpn/test_evpn_connectivity.py
echo "== Relevant conftests =="
for f in \
tests/network/user_defined_network/conftest.py \
tests/network/user_defined_network/rhel9_rhel10_cluster/conftest.py \
tests/network/user_defined_network/ip_specification/conftest.py \
tests/network/bgp/conftest.py \
tests/network/bgp/evpn/conftest.py \
tests/network/provider_migration/conftest.py
do
if [[ -f "$f" ]]; then
echo "--- $f"
rg -n -C2 '\budn_vm\s*\(|`@pytest`\.fixture|yield|template_labels|affinity|new_node_affinity|client_server_active_connection|active_tcp_connections|vma_udn|vmb_udn|client|server' "$f" || true
fi
done
body_file="$(mktemp)"
cat > "$body_file" <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures in `tests/network/user_defined_network/conftest.py` → `tests/network/libs/vm_factory.py::udn_vm` (the existing `template_labels` path that this refactor preserves).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos9`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_server_live_migration`
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: when explicit `affinity=` is passed into `udn_vm`, it is applied as-is to the VM template, migration from an RHCOS 9 worker to an RHCOS 10 worker succeeds after `set_template_affinity(...)`, and the TCP connection remains established.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: existing callers that rely on the default `template_labels` anti-affinity path still schedule correctly and preserve the TCP connection during migration.
EOF
existing_id="$(
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r --arg path "$first_file" '
.[] | select(.user.login=="coderabbitai[bot]" and .path==$path) | .id
' \
| tail -n1
)"
if [[ -n "${existing_id:-}" ]]; then
gh api "repos/$repo/pulls/comments/$existing_id" \
-X PATCH \
--input "$body_file" >/dev/null
else
gh api "repos/$repo/pulls/$pr/comments" \
-X POST \
-f commit_id="$head_sha" \
-f path="$first_file" \
-F line="$comment_line" \
-f side="$comment_side" \
--input "$body_file" >/dev/null
fiLength of output: 9261 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid')"
first_file="$(gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r '.[0].filename')"
patch="$(gh api "repos/$repo/pulls/$pr/files?per_page=100" --paginate | jq -r --arg f "$first_file" '.[] | select(.filename==$f) | .patch // ""')"
meta_file="$(mktemp)"
PATCH_CONTENT="$patch" python - <<'PY' > "$meta_file"
import os, re, sys
patch = os.environ["PATCH_CONTENT"].splitlines()
old_line = new_line = None
for line in patch:
if line.startswith('@@'):
m = re.search(r'-(\d+)(?:,\d+)? \+(\d+)(?:,\d+)?', line)
if not m:
continue
old_line = int(m.group(1))
new_line = int(m.group(2))
continue
if old_line is None or new_line is None:
continue
if line.startswith('+') and not line.startswith('+++'):
print("RIGHT")
print(new_line)
sys.exit(0)
if line.startswith('-') and not line.startswith('---'):
print("LEFT")
print(old_line)
sys.exit(0)
old_line += 1
new_line += 1
sys.exit("Unable to determine first changed line/side from patch")
PY
comment_side="$(sed -n '1p' "$meta_file")"
comment_line="$(sed -n '2p' "$meta_file")"
body="$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures in `tests/network/user_defined_network/conftest.py` → `tests/network/libs/vm_factory.py::udn_vm` (the existing `template_labels` path that this refactor preserves).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos9`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_server_live_migration`
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: when explicit `affinity=` is passed into `udn_vm`, it is applied as-is to the VM template, migration from an RHCOS 9 worker to an RHCOS 10 worker succeeds after `set_template_affinity(...)`, and the TCP connection remains established.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: existing callers that rely on the default `template_labels` anti-affinity path still schedule correctly and preserve the TCP connection during migration.
EOF
)"
existing_id="$(
gh api "repos/$repo/pulls/$pr/comments?per_page=100" --paginate \
| jq -r --arg path "$first_file" '
.[] | select(.user.login=="coderabbitai[bot]" and .path==$path) | .id
' \
| tail -n1
)"
if [[ -n "${existing_id:-}" ]]; then
gh api "repos/$repo/pulls/comments/$existing_id" \
-X PATCH \
-f body="$body"
else
gh api "repos/$repo/pulls/$pr/comments" \
-X POST \
-f body="$body" \
-f commit_id="$head_sha" \
-f path="$first_file" \
-F line="$comment_line" \
-f side="$comment_side"
fiLength of output: 4805 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="RedHatQE/openshift-virtualization-tests"
pr="4786"
body="$(cat <<'EOF'
**Test Execution Plan**
- **Run smoke tests: False** — no verified smoke-marked test or parent `conftest.py` dependency path reaches `tests/network/libs/vm_factory.py::udn_vm`.
- **Run gating tests: True** — `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)` → `client` / `server` fixtures → `vma_udn` / `vmb_udn` fixtures in `tests/network/user_defined_network/conftest.py` → `tests/network/libs/vm_factory.py::udn_vm` (the existing `template_labels` path that this refactor preserves).
- **Affected tests to run**
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
- `tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos9`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration` `(gating)`
- `tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_server_live_migration`
- `tests/network/user_defined_network/test_user_defined_network_passt.py`
- `tests/network/user_defined_network/ip_specification/test_ip_specification.py`
- `tests/network/bgp/test_bgp_connectivity.py`
- `tests/network/bgp/evpn/test_evpn_connectivity.py`
**Real tests (cluster required)**
Error path (the fix):
`pytest tests/network/user_defined_network/rhel9_rhel10_cluster/test_connectivity.py::TestConnectivity::test_connectivity_preserved_during_server_migration_to_rhcos10`
Expected: when explicit `affinity=` is passed into `udn_vm`, it is applied as-is to the VM template, migration from an RHCOS 9 worker to an RHCOS 10 worker succeeds after `set_template_affinity(...)`, and the TCP connection remains established.
Happy path (regression):
`pytest tests/network/user_defined_network/test_user_defined_network.py::TestPrimaryUdn::test_connectivity_is_preserved_during_client_live_migration`
Expected: existing callers that rely on the default `template_labels` anti-affinity path still schedule correctly and preserve the TCP connection during migration.
EOF
)"
existing_id="$(
gh api "repos/$repo/issues/$pr/comments?per_page=100" --paginate \
| jq -r '
.[]
| select(.user.login=="coderabbitai[bot]" and (.body | startswith("**Test Execution Plan**")))
| .id
' \
| tail -n1
)"
if [[ -n "${existing_id:-}" ]]; then
gh api "repos/$repo/issues/comments/$existing_id" \
-X PATCH \
-f body="$body"
else
gh api "repos/$repo/issues/$pr/comments" \
-X POST \
-f body="$body"
fiLength of output: 3444
CRITICAL — I could not post the requested comment to
That rule exists to ensure the plan is actually attached to the PR, but the current integration permissions prevent it. Test Execution Plan
Affected tests to run
Real tests (cluster required) Error path (the fix): Happy path (regression): |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
10 participants
Short description:
New dual-stream tests: UDN scenarios
This PR adds two tests that verify TCP connection survives migration in both directions (RHCOS 9 -> RHCOS 10 and back). VMs are scheduled using node affinity on the RHCOS 9 worker label rather than hostnames, so the tests are resilient to node replacement.
More details:
STD: #4569
What this PR does / why we need it:
Added automation for new dual-stream tests
Which issue(s) this PR fixes: -
Special notes for reviewer:
Follow-up implementation of new tests of primary UDN which is supported in tests only for IPv4.
Due to low availability of dual-stream BMs, the test implemented with IPv4 only. IPv6 will be added in a follow-up.
jira-ticket: https://redhat.atlassian.net/browse/CNV-86194
Summary by CodeRabbit