net, udn: Introduce passt migration test#2833
Merged
rnetser merged 3 commits intoRedHatQE:mainfrom Jan 8, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughRename/add UDN binding constants; make udn_primary_network accept a binding and propagate it through udn_vm call sites. Centralize iperf port and client-server context manager into libs/net/traffic_generator, remove local duplicates, add UDN fixtures, and add PASST live-migration connectivity tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-bugs-are-open:passed
Contributor
Author
/verified uv run pytest --verbose -rs -s -o log_cli=true --junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log --leftovers-collector --tb=native --skip-deprecated-api-test --cluster-sanity-skip-check --cluster-sanity-skip-storage-check tests/network/user_defined_network/test_user_defined_network.py /verified uv run pytest --verbose -rs -s -o log_cli=true --junitxml=xunit_results.xml --pytest-log-file=pytest-tests.log --leftovers-collector --tb=native --skip-deprecated-api-test --cluster-sanity-skip-check --cluster-sanity-skip-storage-check -k "localnet" |
rnetser
reviewed
Jan 6, 2026
Contributor
Author
|
/verified uv run pytest $(cat params) tests/network/user_defined_network/test_user_defined_network_passt.py |
openshift-virtualization-qe-bot
approved these changes
Jan 6, 2026
Collaborator
|
/approve |
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:
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}
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 476 🏁 Script executed: Length of output: 1835 🧠 Learnings used |
openshift-virtualization-qe-bot
previously approved these changes
Jan 7, 2026
Currently, the udn VM factory only creates VMs with l2bridge network bindings. Add support for creation of UDN VMs with passt network bindings, in preparation for passt binding udn test. Modify existing UDN vms to explicitly use l2bridge, as there are now 2 supported binding plugins. Signed-off-by: Nir Dothan <ndothan@redhat.com>
client_server_active_connection was placed under localnet but it is generic to network therefore move to network libs. Signed-off-by: Nir Dothan <ndothan@redhat.com>
openshift-virtualization-qe-bot
previously approved these changes
Jan 7, 2026
Introduce passt based seamless migration tests similar to those that are currently testing l2bridge binding. Move common udn fixtures to conftest.py wait_for_ready_vm_with_restart: In order to be able to wait for external dependencies to materialize without specifically checking them, add a function that waits for READY condition. If the condition is not met within the time period restart the VM and wait again for the condition. Restarting the VM is required in scenarios that VM/VMI does not know how to wait for dependencies and is stuck in either pending or failed state. Assited by Cursor AI. Signed-off-by: Nir Dothan <ndothan@redhat.com>
openshift-virtualization-qe-bot
approved these changes
Jan 7, 2026
Contributor
Author
|
/verified |
yossisegev
approved these changes
Jan 7, 2026
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: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 299 🏁 Script executed: Length of output: 389 🏁 Script executed: Length of output: 4465 🏁 Script executed: Length of output: 1741 🧠 Learnings used |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @libs/net/traffic_generator.py:
- Line 1: Replace uses of typing.Generator with collections.abc.Generator:
update import statements so any "from typing import Generator" becomes "from
collections.abc import Generator" and adjust type annotations in
functions/classes that reference Generator (e.g., any parameters or return types
annotated as Generator) to use the new import; ensure imports are de-duplicated
and run static type checks to confirm compatibility with Python 3.9+.
- Around line 194-229: The function client_server_active_connection currently
annotates its return as Generator[tuple[VMTcpClient, TcpServer], None, None];
replace this with the simpler Iterator[tuple[VMTcpClient, TcpServer]] (and add
the import from typing if missing) so the annotation only expresses that the
context yields a sequence of client/server tuples without the unnecessary
explicit send/return None parameters.
In @tests/network/user_defined_network/conftest.py:
- Around line 9-15: Add a Google-style docstring to the pytest fixture
udn_namespace in conftest.py that describes what the fixture provides (a test
namespace created via create_ns), its scope (module), the side effects (creates
and yields a Kubernetes Namespace resource and any teardown behavior performed
by create_ns), and the return value (what is yielded). Place the docstring
immediately above the udn_namespace function definition and follow Google
docstring sections like Args, Yields, and Side effects so readers understand the
setup/teardown and what callers receive.
- Around line 18-32: Add a concise docstring to the
namespaced_layer2_user_defined_network fixture describing its purpose and side
effects: state that it creates a Layer2UserDefinedNetwork named "layer2-udn" in
udn_namespace with role "Primary", a /24 subnet, Persistent IPAM, uses
admin_client, waits for the "NetworkAllocationSucceeded" condition to be TRUE
via udn.wait_for_condition, and yields the udn for tests; place the docstring
immediately above the fixture definition so future readers know what the fixture
configures and what tests can expect.
- Around line 35-37: The fixture udn_affinity_label lacks a docstring and should
use the yield pattern for consistency; update the pytest fixture function
udn_affinity_label to include a one-line docstring describing its purpose (e.g.,
"Provide a UDN affinity label for tests") and change the implementation to yield
the affinity.new_label(key_prefix="udn") instead of returning it (no teardown
required), keeping the fixture scope="module".
azhivovk
approved these changes
Jan 7, 2026
Collaborator
|
/approve |
Collaborator
|
@nirdothan please address and resolve the comments |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
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
12 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.
Short description:
passtnetwork binding has been reintroduced into openshift virtualization since release 4.20 as tech preview.Introduce two seamless migration tests that are identical to the existing
l2bridgebinding seamless migration tests.Summary by CodeRabbit
New Features
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.