net, evpn: add source provider migration test#4817
Open
servolkov wants to merge 3 commits into
Open
Conversation
|
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. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds optional MAC fields to VM and EVPN models, threads MAC through L2 endpoint deploy/teardown and command generation, introduces a vm_source_provider fixture configuring a VM with the external endpoint MAC/IPs, and implements a last-ordered migration test that deploys an explicit-MAC L2 endpoint and validates EVPN connectivity. ChangesMAC Address Configuration for EVPN L2 Endpoints
🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
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 |
openshift-virtualization-qe-bot-5
requested review from
EdDev,
azhivovk,
dshchedr,
frenzyfriday,
orelmisan,
rnetser and
yossisegev
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/bgp/evpn/test_evpn_connectivity.py`:
- Around line 26-31: The test imports assert_evpn_workloads_connectivity but it
does not exist in libevpn.py, causing ImportError; either add an implementation
named assert_evpn_workloads_connectivity in tests.network.bgp.evpn.libevpn that
performs the expected connectivity assertions (mirror behavior of
evpn_workloads_active_connections and any teardown hooks used by the test) or
remove the import and calls to assert_evpn_workloads_connectivity in the test;
implement the function with the same signature the test expects and ensure it
calls/uses evpn_workloads_active_connections, deploy_evpn_l2_endpoint, and
teardown_evpn_l2_endpoint as needed to assert connectivity.
🪄 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: 5fa7a612-5442-43f4-b25f-3443a5bcc428
📒 Files selected for processing (4)
libs/vm/spec.pytests/network/bgp/evpn/conftest.pytests/network/bgp/evpn/libevpn.pytests/network/bgp/evpn/test_evpn_connectivity.py
Contributor
Author
|
/wip |
openshift-virtualization-qe-bot-5
changed the title
yossisegev
approved these changes
May 28, 2026
Contributor
Author
|
/check-can-merge |
Contributor
Author
|
/retest tox |
Contributor
Author
|
/retest pre-commit.ci |
|
No pre-commit.ci configured for this repository |
Clean up VLAN/VNI bridge mappings when removing L2 endpoints, allowing re-deployment with different parameters in the same session. Signed-off-by: Sergei Volkov <sevolkov@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Emulates migration of an external L2 workload into the cluster
as a CUDN VM, preserving its IP and MAC addresses. The old endpoint
is torn down and re-created with new IPs while the VM takes over
the original identity.
test_source_provider_migration uses @pytest.mark.order("last")
because it tears down the shared L2 endpoint, which would break
other tests if run earlier.
Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
Contributor
Author
|
Rebase to get proper labels |
Contributor
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/bgp/evpn/conftest.py`:
- Line 292: The code replaces spec.template.spec.domain.devices with a new
Devices(...) which discards defaults from base_vmspec() (e.g., boot disks);
instead preserve existing Devices and only update its interfaces: fetch
spec.template.spec.domain.devices (create a Devices if None) and set/replace its
interfaces list with [iface] or append iface as needed so other fields (disks,
boot order) from base_vmspec() remain intact; adjust the assignment around
Devices/iface usage in this block to mutate devices.interfaces rather than
overwrite spec.template.spec.domain.devices.
🪄 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: 74b0a5da-cfa6-4f3c-b9ca-d66cb0ef1a0f
📒 Files selected for processing (4)
libs/vm/spec.pytests/network/bgp/evpn/conftest.pytests/network/bgp/evpn/libevpn.pytests/network/bgp/evpn/test_evpn_connectivity.py
Contributor
Author
|
/verified |
Contributor
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: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current PR head SHA and first changed line of first file
echo "=== PR head SHA ==="
gh api "repos/RedHatQE/openshift-virtualization-tests/pulls/4817" --jq '.head.sha'
echo "=== PR files with patches ==="
gh api "repos/RedHatQE/openshift-virtualization-tests/pulls/4817/files" --jq '.[] | {filename: .filename, patch: .patch}' 2>/dev/null | head -60
echo "=== Verify smoke tests have no dependency on Interface or EVPN fixtures ==="
rg -n "pytest.mark.smoke" --type py -l | head -20Length of output: 11533 🏁 Script executed: #!/bin/bash
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
"repos/RedHatQE/openshift-virtualization-tests/pulls/4817/reviews" \
--input - <<'EOF'
{
"commit_id": "7483cce650b641153136cede383b9bb8229801c0",
"event": "COMMENT",
"comments": [
{
"path": "libs/vm/spec.py",
"line": 80,
"side": "RIGHT",
"body": "## Test Execution Plan\n\n**Run smoke tests: False**\n> Smoke test files (tests/virt/, tests/storage/, tests/infrastructure/, tests/install_upgrade_operators/) have no confirmed import or fixture dependency path to the changed `Interface` dataclass or any EVPN fixture/utility.\n\n### Tests to run\n\n- `tests/network/bgp/evpn/test_evpn_connectivity.py` — Run the full EVPN connectivity suite. `conftest.py` changes affect `external_l2_endpoint` teardown (now passes `vni=EVPN_MAC_VRF_VNI`) and add the new `vm_source_provider` fixture (module-scoped impact). `libevpn.py` signature changes to `teardown_evpn_l2_endpoint` (new required `vni` parameter) and `deploy_evpn_l2_endpoint` (new `mac_address` parameter) affect all tests using L2 endpoints.\n- `tests/network/bgp/evpn/test_evpn_connectivity.py::test_source_provider_migration` — New test introduced in this PR; validates full source-provider migration flow: L2 endpoint teardown → VM identity takeover (IP+MAC preserved) → new L2 endpoint deployment → end-to-end connectivity assertion."
}
]
}
EOFLength of output: 3217 |
yossisegev
approved these changes
May 28, 2026
Anatw
approved these changes
May 28, 2026
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5041. Overlapping filestests/network/bgp/evpn/test_evpn_connectivity.py |
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4954. Overlapping filestests/network/bgp/evpn/libevpn.py |
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5014. Overlapping fileslibs/vm/spec.py |
qinqon
suggested changes
Jun 4, 2026
Emulates migration of an external L2 workload into the cluster
as a CUDN VM, preserving its IP and MAC addresses. The old endpoint
is torn down and re-created with new IPs while the VM takes over
the original identity.
test_source_provider_migration uses @pytest.mark.order("last")
because it tears down the shared L2 endpoint, which would break
other tests if run earlier.
Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
Contributor
Author
|
/verified |
Contributor
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
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: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current PR head SHA
echo "=== PR head SHA ==="
gh api "repos/RedHatQE/openshift-virtualization-tests/pulls/4817" --jq '.head.sha'
echo "=== PR files with patches ==="
gh api "repos/RedHatQE/openshift-virtualization-tests/pulls/4817/files" --jq '.[] | {filename: .filename, patch: .patch}'
echo "=== Check callers of Interface from libs/vm/spec.py in tests/ ==="
rg -n "from libs.vm.spec import|from libs\.vm\.spec" --type py -l | head -30
echo "=== Check gating tests ==="
rg -l "`@pytest.mark.gating`" --type py | head -30
echo "=== Check smoke tests ==="
rg -l "`@pytest.mark.smoke`" --type py | head -30
echo "=== Check if any gating test imports Interface from libs/vm/spec ==="
rg -n "from libs.vm.spec import" --type py -l | xargs -I{} grep -l "gating" {} 2>/dev/null | head -10
echo "=== Check if EVPN tests have gating mark ==="
rg -n "pytest.mark.gating" tests/network/bgp/evpn/ --type py | head -10Length of output: 14969 🏁 Script executed: #!/bin/bash
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
"repos/RedHatQE/openshift-virtualization-tests/pulls/4817/reviews" \
--input - <<'EOF'
{
"commit_id": "b827d09f389303c6c3fed89351556d674225fe66",
"event": "COMMENT",
"comments": [
{
"path": "libs/vm/spec.py",
"line": 80,
"side": "RIGHT",
"body": "## Test Execution Plan\n\n**Run smoke tests: False**\n> Verified: none of the smoke-marked files (`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py`, `tests/virt/node/general/test_container_disk_vm.py`, `tests/storage/cdi_upload/test_upload_virtctl.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`) import `Interface` from `libs/vm/spec.py` or any EVPN fixture/utility.\n\n**Run gating tests: False**\n> Verified: no gating-marked test imports `Interface` from `libs/vm/spec.py`; EVPN test module has no `@pytest.mark.gating` marks.\n\n### Affected tests to run\n\n- `tests/network/bgp/evpn/test_evpn_connectivity.py` — Run the full EVPN connectivity suite. `conftest.py` changes update `external_l2_endpoint` fixture teardown (now passes `vni=EVPN_MAC_VRF_VNI` to `teardown_evpn_l2_endpoint`) and add the new module-scoped `vm_source_provider` fixture. `libevpn.py` signature changes: `teardown_evpn_l2_endpoint` gains a required `vni` parameter; `deploy_evpn_l2_endpoint` gains optional `mac_address` — both affect all tests using L2 endpoints.\n- `tests/network/bgp/evpn/test_evpn_connectivity.py::test_source_provider_migration` — New test introduced in this PR; validates full source-provider migration flow: L2 endpoint teardown → VM identity takeover (IP+MAC preserved) → new L2 endpoint deployment → end-to-end connectivity assertion."
}
]
}
EOFLength of output: 3645 🧠 Learnings used |
qinqon
approved these changes
Jun 4, 2026
Collaborator
|
/approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Add source provider migration test for EVPN.
allowing re-deployment with different parameters in the same test session.
macAddressfield toInterfacedataclass for explicit MAC assignment.deploy_evpn_l2_endpoint.test_source_provider_migration: emulates migration of an external L2workload into the cluster as a CUDN VM, preserving its IP and MAC addresses.
The old endpoint is torn down, a VM takes over the original identity, then a
new L2 endpoint is deployed with different IPs to verify connectivity.
Which issue(s) this PR fixes:
Special notes for reviewer:
test_source_provider_migrationuses@pytest.mark.order("last")because ittears down the shared L2 endpoint, which would break other tests if run earlier.
random_ipv6_address()generates addresses with a random 7th hextet,but OVN IPAM caps /64 allocation to 65536 addresses.
Replaced with a low-offset hardcoded address. Only affects IPs requested via
network.kubevirt.io/addresses; external netns endpoints are unaffected.The known bug: https://redhat.atlassian.net/browse/OCPBUGS-86810
known limitation — currently only one L2 endpoint can exist at a time. A follow-up
will reorganise fixtures to separate VLAN lifecycle from endpoints, enabling
independent concurrent endpoints.
jira-ticket: https://redhat.atlassian.net/browse/CNV-87077
Summary by CodeRabbit
New Features
Tests
Bug Fixes