net, evpn: add stretched L2 connectivity tests#4709
net, evpn: add stretched L2 connectivity tests#4709servolkov wants to merge 3 commits intoRedHatQE:mainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR extends traffic client support for network namespace execution, renames the external FRR pod's utility container from ChangesEVPN L2 Stretched Connectivity Testing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes The PR combines straightforward constant renaming with significant new EVPN infrastructure. Review effort is driven by: (1) understanding the new EVPN deployment command sequencing and state management via 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 20 minutes and 13 seconds.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-3
requested review from
EdDev,
RoniKishner,
dshchedr,
frenzyfriday,
nirdothan,
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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/net/traffic_generator.py`:
- Around line 175-183: The constructor PodTcpClient.__init__ in
libs/net/traffic_generator.py is missing the explicit return type annotation
required by mypy strict (ANN204); update the method signature for
PodTcpClient.__init__ to include the return type "-> None" (keeping all existing
parameter types unchanged) so the initializer explicitly declares it returns
None.
In `@tests/network/bgp/evpn/conftest.py`:
- Around line 199-223: The evpn_bridge and external_l2_endpoint fixtures
currently return without cleaning up, leaving br0, vxlan-l2, veth-l2-*, and
l2-ep behind; change both fixtures to use yield-based fixtures and perform
teardown after the yield: in evpn_bridge (which calls deploy_evpn_bridge) yield
control and then call the inverse cleanup to delete the bridge and related
interfaces on frr_external_pod, and in external_l2_endpoint (which calls
deploy_evpn_l2_endpoint) yield the EvpnEndpoint and then remove the deployed L2
endpoint, delete vxlan and veth interfaces and any created l2-ep state on
frr_external_pod and worker nodes; ensure you reference and use the same
identifiers (evpn_bridge, external_l2_endpoint, deploy_evpn_bridge,
deploy_evpn_l2_endpoint, frr_external_pod) so teardown targets the exact
resources created.
In `@tests/network/libs/bgp.py`:
- Around line 309-312: _acquire_dhcp_ipv4 currently calls dhclient in
NET_TOOLS_CONTAINER_NAME but runs the subsequent ip lookup with pod.execute()'s
default container, reintroducing a dependency on the FRR image; update the ip
command invocation in _acquire_dhcp_ipv4 to pass
container=NET_TOOLS_CONTAINER_NAME (same as the dhclient call) so iface_info is
collected from the net-tools container, referencing the pod.execute call,
NET_TOOLS_CONTAINER_NAME and iface_name inside the _acquire_dhcp_ipv4 function.
🪄 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: 4519cb2f-c0e2-4e58-a937-4833ca362422
📒 Files selected for processing (6)
libs/net/traffic_generator.pytests/network/bgp/conftest.pytests/network/bgp/evpn/conftest.pytests/network/bgp/evpn/libevpn.pytests/network/bgp/evpn/test_evpn_connectivity.pytests/network/libs/bgp.py
The container running net-tools image in the external FRR pod was misleadingly named "iperf3". Rename to "net-tools", extract NET_TOOLS_CONTAINER_NAME constant, and add container parameter to PodTcpClient for callers to specify the target container. Signed-off-by: Sergei Volkov <sevolkov@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Allow callers to run iperf3 inside a network namespace. Netns wrapping is applied only at execution time, keeping a single _cmd for process matching. Signed-off-by: Sergei Volkov <sevolkov@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Add EVPN L2 endpoint emulation inside the external FRR pod (bridge, VXLAN, veth, netns) and implement stretched L2 connectivity and live migration tests. Reference VM is function-scoped as it is used by a single test, ensuring proper teardown. Signed-off-by: Sergei Volkov <sevolkov@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
servolkov
force-pushed
the
net/evpn-l2-connectivity-tests
branch
from
047d4af to
89eec62
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
|
Change: address bot comments |
|
/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:
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: |
| bind_interface (str): The interface or IP address to bind the client to (optional). | ||
| If not specified, the client will use the default interface. | ||
| container (str): Container name to execute commands in. | ||
| netns (str): Network namespace to run iperf3 in (optional). |
There was a problem hiding this comment.
Test Execution Plan
Run smoke tests: False
No concrete dependency path found from any smoke-marked test to changed files (
libs/net/traffic_generator.py,tests/network/libs/bgp.py).
Tests to Run
tests/network/bgp/evpn/test_evpn_connectivity.py::test_stretched_l2_connectivity_udn_vm_and_external_provider— newly enabled; validates external L2 endpoint TCP connectivitytests/network/bgp/evpn/test_evpn_connectivity.py::test_stretched_l2_connectivity_is_preserved_over_live_migration— newly enabled; validates connectivity survives live migrationtests/network/bgp/test_bgp_connectivity.py—bgp/conftest.pychanged (tcp_client_external_networknow passescontainer=NET_TOOLS_CONTAINER_NAME); all BGP connectivity tests should be re-verified
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4170 🏁 Script executed: Length of output: 41 🏁 Script executed: Length of output: 1735 |
4 participants
What this PR does / why we need it:
Add stretched L2 connectivity tests for EVPN, including live migration verification.
NET_TOOLS_CONTAINER_NAMEconstantcontainerandnetnsparameters toPodTcpClientfor running iperf3 in specific containers and network namespacesWhich issue(s) this PR fixes:
Special notes for reviewer:
vm_evpn_reference) is function-scoped — it is used by a single test, ensuring proper teardown and freeing a node for migrationjira-ticket: https://redhat.atlassian.net/browse/CNV-80611
Assisted-by: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
Tests
Chores