CherryPicked: [cnv-4.20] net, BGP: adjust BGP environment to use localnet NAD #3546
Conversation
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
…tQE#2389) Client\Server looks quite common and such naming does not reflect the real intention. The usage traffic_generator.Client does not solve the problem as well. It's crucial to explicitly point out that we work with Tcp traffic and the classes implement TCP client and TCP server in reality. And this clarity is significant for tests also. The commit also includes replacing TimeoutSampler with retry decorator, that leads a little refactoring for classes to align the logic. Retry-enabled `_ensure_is_running()` method is added to `__enter__()`. PodTcpClient already uses a similar implementation. We need to make sure once when entering into the context manager that the client\server process is running, then it is enough to use the atomic method `is_running()` without retry to determine the status of the process. Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
The current implementation of BGP testing is based on some knowledge about the infrastructure: it is expected that the cluster has several NICs, we should know the vlan of the main traffic on the cluster, as well as for the static assignment of IP to an external router, we expect a predefined environment variables. This PR gets rid of these dependencies and allows us to run BGP tests without additional knowledge about the cluster. Main changes: 1) the usage of the NNCP and NAD localnet, the env no longer dependent on secondary NICs. 2) dynamic receiving of IP address for an external router from the main cluster network via DHCP. 3) a condition under which the external router and the UDN VM are guaranteed to live on different cluster nodes, which is vital with the current localnet configuration. Along the way, as necessary: 1) adding `dhcp-client` to net-tools infra image 2) libs: add `namespaces` to anti-affinity and related calls 3) removing BGP sanity check as useless with the current implementation 4) de-quarantining BGP tests 5) doc fixes <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Utility container now includes a DHCP client. * **Tests** * Re-enabled BGP connectivity tests and refactored BGP test infra to use explicit external FRR pod state, module-scoped fixtures, and a localnet network attachment. * Removed legacy NIC-selection fixture and node-network-state helper modules. * **Behavior Changes** * Tier2 tagging now considers BGP-marked tests when no other exclusions apply. * VM anti-affinity can target specific namespaces. * **Documentation** * Simplified BGP test README by removing cluster requirements and env setup. * **Chores** * Removed BGP environment-variable verification from network sanity checks. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
In some rear cases on some clusters the connectivity can be broken between external provider network and UDN VM. The issue should be investigated, the suite is quarantined. Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-4
requested review from
EdDev,
azhivovk,
geetikakay,
vsibirsk and
yossisegev
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
approved these changes
Jan 20, 2026
Contributor
Author
|
/verified 1 fail is expected and will be fixed here, it does not refer the current fixes. |
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:
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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| "swap", | ||
| "numa", | ||
| "bgp", | ||
| ] |
There was a problem hiding this comment.
Test Execution Plan
Run smoke tests: False
The user has already verified the affected test areas:
-m bgp- BGP tests (covers changes to BGP infrastructure, localnet NAD migration, and traffic generator updates)tests/network/localnet/- Localnet tests (covers localnet infrastructure and traffic generator class renames)tests/network/user_defined_network/test_user_defined_network.py- UDN tests (covers traffic generator class renames in UDN context)
These test executions fully cover:
- BGP infrastructure migration from VLAN/macvlan to localnet NAD (
tests/network/bgp/) - Traffic generator class renames (
libs/net/traffic_generator.py: Server→TcpServer, Client→VMTcpClient, etc.) - Pod anti-affinity namespace parameter enhancement (
libs/vm/affinity.py) - Container image updates
Smoke test analysis: No smoke tests depend on the changed networking code (traffic_generator, affinity, BGP/localnet fixtures).
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 670 🏁 Script executed: Length of output: 35124 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 559 🏁 Script executed: Length of output: 36 🏁 Script executed: Length of output: 3525 🧠 Learnings used |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
Manual cherry-pick connected to each other PRs: #2389, #3132, #3445