net, tests, stuntime: Add L2 bridge client migration stuntime scenario#4867
Conversation
|
Warning Review limit reached
More reviews will be available in 39 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors stuntime measurement into a shared helper, adds labels support to the secondary VM builder, introduces L2-bridge NAD and server/client VM fixtures with ip-family support and an active-ping fixture, and updates localnet and L2-bridge tests to use the shared measurement API. ChangesL2 Bridge Stuntime Migration Tests and Shared Library Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
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-4
requested review from
frenzyfriday and
yossisegev
openshift-virtualization-qe-bot-4
requested review from
azhivovk,
nirdothan and
servolkov
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Anatw
force-pushed
the
stuntime_l2_bridge_first_scenario
branch
from
d45e49c to
f5cc635
Compare
Anatw
force-pushed
the
stuntime_l2_bridge_first_scenario
branch
from
f5cc635 to
efe097c
Compare
Anatw
force-pushed
the
stuntime_l2_bridge_first_scenario
branch
from
efe097c to
1c2681f
Compare
|
Clean rebase detected — no code changes compared to previous head ( |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
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:
Your deliverable: Test Execution Plan
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: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get latest commit SHA
gh pr view 4867 --json headRefOid --jq '.headRefOid'
# Get changed files
gh pr view 4867 --json files --jq '.files[].path'
# Get the diff to find the first changed line of the first file
gh pr diff 4867 -- tests/network/l2_bridge/libl2bridge.py | head -30
# Check smoke test files for any imports from changed modules
rg -n "smoke" tests/ --type py -l | head -20Length of output: 1303 🏁 Script executed: #!/bin/bash
# Get the diff for libl2bridge.py
gh pr diff 4867 | grep -A 40 "diff --git a/tests/network/l2_bridge/libl2bridge.py"
# Check smoke test files for imports from libl2bridge or stuntime
rg -n "libl2bridge\|stuntime" tests/virt/node/general/test_container_disk_vm.py tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py tests/storage/cdi_clone/test_clone.py tests/storage/cdi_import/test_import_registry.py tests/storage/cdi_upload/test_upload_virtctl.py tests/storage/cdi_import/test_import_http.py tests/install_upgrade_operators/csv/test_csv.py tests/infrastructure/instance_types/supported_os/test_rhel_os.py 2>/dev/null || echo "No matches found"Length of output: 2839 🏁 Script executed: #!/bin/bash
gh api \
--method POST \
/repos/RedHatQE/openshift-virtualization-tests/pulls/4867/reviews \
--input - <<'EOF'
{
"commit_id": "898daa4a6b4b26c261a6c36f97e501739a8a463f",
"event": "COMMENT",
"comments": [
{
"path": "tests/network/l2_bridge/libl2bridge.py",
"line": 15,
"body": "**Test Execution Plan**\n\n- **Run smoke tests: False** — Verified: no smoke test file imports `libl2bridge`, `stuntime`, or any other changed module.\n\n**Tests to run:**\n\n- `tests/network/l2_bridge/migration_stuntime/test_migration_stuntime.py` — New L2 bridge stuntime tests (IPv4 + IPv6 parametrized, `tier3`).\n- `tests/network/localnet/migration_stuntime/test_migration_stuntime.py::TestMigrationStuntime::test_client_migrates_off_server_node` — Refactored to use shared `measure_stuntime()`; verify behavior is unchanged."
}
]
}
EOFLength of output: 2656 |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4833. Overlapping filestests/network/localnet/migration_stuntime/conftest.py |
Extract Stuntime measurement utilities to tests/network/libs/ to enable reuse for l2_bridge stuntime tests and future network types. Update localnet imports accordingly. Signed-off-by: Anat Wax <awax@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
Move helpers below ContinuousPing class to eliminate the forward reference. Make compute_stuntime private (_compute_stuntime) since it's only called from measure_stuntime. This clarifies the public API surface: callers use measure_stuntime, not the internal helper. Signed-off-by: Anat Wax <awax@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
|
Change: rebase on main (absorb localnet stuntime additions from #4833) |
Implement stuntime scenario over Linux bridge in which client VM migrates off the server node. This scenario will serve as a baseline for future performance testing, mirroring the existing OVN localnet stuntime coverage. Signed-off-by: Anat Wax <awax@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
|
Change: fix flake8 failure - rename ip_family for l2_bridge. |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4867 published |
|
/verified Tested on bm03-tlv2 |
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:
Your deliverable: Test Execution Plan
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: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get latest commit SHA
gh pr view 4867 --json headRefOid --jq '.headRefOid'
# Get changed files
gh pr view 4867 --json files --jq '.files[].path'
# Get diff for libl2bridge.py to find first changed line
gh pr diff 4867 | grep -A 50 "diff --git a/tests/network/l2_bridge/libl2bridge.py"
# Check smoke test files for any imports from changed modules
rg -n "pytest.mark.smoke" tests/ --type py -l | head -20Length of output: 3687 🏁 Script executed: #!/bin/bash
# Verify smoke test files don't import from changed modules
rg -n "libl2bridge\|stuntime\|libs\.vm\.spec" \
tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py \
tests/virt/node/general/test_container_disk_vm.py \
tests/storage/cdi_import/test_import_http.py \
tests/storage/cdi_upload/test_upload_virtctl.py \
tests/storage/cdi_import/test_import_registry.py \
tests/storage/cdi_clone/test_clone.py \
tests/install_upgrade_operators/csv/test_csv.py \
tests/infrastructure/instance_types/supported_os/test_rhel_os.py 2>/dev/null || echo "No matches found"
# Post inline comment on line 15 of libl2bridge.py (the Metadata import change)
gh api \
--method POST \
/repos/RedHatQE/openshift-virtualization-tests/pulls/4867/reviews \
--input - <<'EOF'
{
"commit_id": "36eae7a690783290d6d32be1e058bdbf5094348a",
"event": "COMMENT",
"comments": [
{
"path": "tests/network/l2_bridge/libl2bridge.py",
"line": 15,
"body": "**Test Execution Plan**\n\n- **Run smoke tests: False** — Verified: none of the 8 smoke-marked test files import `libl2bridge`, `stuntime`, or `libs.vm.spec` (confirmed via codebase search on latest commit).\n\n**Tests to run:**\n\n- `tests/network/l2_bridge/migration_stuntime/test_migration_stuntime.py` — New L2 bridge stuntime tests (IPv4 + IPv6 parametrized, `tier3`).\n- `tests/network/localnet/migration_stuntime/test_migration_stuntime.py::TestMigrationStuntime::test_client_migrates_off_server_node` — Refactored to use shared `measure_stuntime()`; verify behavior is unchanged."
}
]
}
EOFLength of output: 3448 |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4962. Overlapping filestests/network/l2_bridge/libl2bridge.py |
|
/retest build-container |
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-4867. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/retest build-container |
9 participants
Short description:
Implement stuntime scenario over Linux bridge in which the client VM migrates off the server node.
More details:
What this PR does / why we need it:
Establish a performance baseline and enable regression detection for Linux bridge live migration connectivity gaps (stuntime), mirroring the existing OVN localnet coverage.
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-84398
Summary by CodeRabbit
Tests
Chores