net,vm import: added fixtures to import a VM from external cluster - #2970
Merged
Conversation
Contributor
WalkthroughThis PR introduces MTV (multi-tenant virtualization) migration testing infrastructure. Changes include adding MTV namespace fixtures and operator verification in the network sanity flow, reorganizing pytest markers to include MTV operator requirements, creating provider migration test fixtures for orchestrating multi-component MTV workflows, implementing a VSphere provider library, and adding a migration IP persistence test. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
previously approved these changes
Dec 2, 2025
![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: 5
🧹 Nitpick comments (3)
tests/network/conftest.py (1)
221-224: Remove unusednoqadirective.Static analysis indicates that
UFN001is an unknown directive. Thenoqacomment should be removed.@pytest.fixture(scope="session") -def mtv_namespace(admin_client: DynamicClient) -> Namespace: # noqa: UFN001 +def mtv_namespace(admin_client: DynamicClient) -> Namespace: return Namespace(name="openshift-mtv", client=admin_client)tests/network/vm_import/source_provider.py (1)
34-44: Consider logging or raising on status check failure.Both
power_on_vmandpower_off_vmreturn silently if_check_for_vm_statusreturnsTrue, but there's no explicit handling if the retry times out. The@retrydecorator will raiseTimeoutExpiredErroron failure, which may be the intended behavior, but the conditional return is misleading.The
ifstatement is unnecessary since the method just returns anyway:def power_on_vm(self, vm_name: str) -> None: vm = self._get_vm_by_name(name=vm_name) vm.PowerOn() - if self._check_for_vm_status(vm=vm, status=vim.VirtualMachine.PowerState.poweredOn): - return + self._check_for_vm_status(vm=vm, status=vim.VirtualMachine.PowerState.poweredOn) def power_off_vm(self, vm_name: str) -> None: vm = self._get_vm_by_name(name=vm_name) vm.PowerOff() - if self._check_for_vm_status(vm=vm, status=vim.VirtualMachine.PowerState.poweredOff): - return + self._check_for_vm_status(vm=vm, status=vim.VirtualMachine.PowerState.poweredOff)tests/network/vm_import/conftest.py (1)
41-41: Remove unusednoqadirectives.Static analysis indicates that
UFN001is an unknown directive. Thesenoqacomments should be removed from lines 41, 203, and 241.@pytest.fixture(scope="module") -def cudn_layer2(cudn_namespace: Namespace) -> Generator[libcudn.ClusterUserDefinedNetwork]: # noqa: UFN001 +def cudn_layer2(cudn_namespace: Namespace) -> Generator[libcudn.ClusterUserDefinedNetwork]:Similarly for
mtv_migration_plan(line 203) andmtv_migration(line 241).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/network/conftest.py(5 hunks)tests/network/vm_import/conftest.py(1 hunks)tests/network/vm_import/source_provider.py(1 hunks)tests/network/vm_import/test_ip_persitence.py(1 hunks)
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.
Learnt from: SamAlber
Repo: RedHatQE/openshift-virtualization-tests PR: 2507
File: tests/virt/node/general/test_vmi_reset.py:26-29
Timestamp: 2025-11-19T08:13:30.263Z
Learning: In the openshift-virtualization-tests repository, user SamAlber prefers not to define fixture dependencies by chaining fixtures (adding one fixture as a parameter to another). Instead, all fixture dependencies should be explicitly declared as parameters in the test method itself, relying on parameter order to control execution sequence.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:62-87
Timestamp: 2025-08-28T12:34:56.341Z
Learning: The BGP test infrastructure in tests/network/bgp/ currently uses static IP assignment via EXTERNAL_FRR_STATIC_IPV4, but the long-term goal is to transition to DHCP for IP allocation. Additional validation of EXTERNAL_FRR_STATIC_IPV4 in individual fixtures is considered redundant since validation already occurs at the session level.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:65-72
Timestamp: 2025-09-29T20:33:51.007Z
Learning: In tests/virt/node/migration_and_maintenance/conftest.py, the added_vm_cpu_limit fixture doesn't require ResourceEditor as a context manager because it's the final test to modify the vm_for_multifd_test VM before teardown, so restoration of CPU limits is unnecessary overhead as confirmed by maintainer dshchedr.
📚 Learning: 2025-08-28T12:30:40.692Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Applied to files:
tests/network/conftest.pytests/network/vm_import/conftest.py
📚 Learning: 2025-10-27T15:30:06.412Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.
Applied to files:
tests/network/conftest.pytests/network/vm_import/test_ip_persitence.py
📚 Learning: 2025-06-18T09:21:34.315Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.
Applied to files:
tests/network/conftest.pytests/network/vm_import/conftest.py
📚 Learning: 2025-10-08T07:16:46.347Z
Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.
Applied to files:
tests/network/conftest.py
📚 Learning: 2025-08-20T23:57:48.380Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1840
File: tests/virt/node/workload_density/test_swap.py:88-89
Timestamp: 2025-08-20T23:57:48.380Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, CNV installation and health are verified by sanity tests before other tests run, so hco_namespace is guaranteed to exist in the testing environment. Defensive programming against nil hco_namespace scenarios is not needed in fixtures.
Applied to files:
tests/network/conftest.py
📚 Learning: 2025-08-28T11:29:15.768Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:5-5
Timestamp: 2025-08-28T11:29:15.768Z
Learning: In the openshift-virtualization-tests project, DynamicClient is imported from kubernetes.dynamic, not openshift.dynamic. The standard import pattern is `from kubernetes.dynamic import DynamicClient`.
Applied to files:
tests/network/conftest.py
📚 Learning: 2025-09-07T13:16:32.011Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:1-5
Timestamp: 2025-09-07T13:16:32.011Z
Learning: In the openshift-virtualization-tests project utilities/oadp.py, DynamicClient instances are passed as parameters rather than created internally, so kubernetes.config import is not needed for client creation fallback patterns.
Applied to files:
tests/network/conftest.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/network/conftest.pytests/network/vm_import/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/network/conftest.pytests/network/vm_import/test_ip_persitence.pytests/network/vm_import/conftest.py
📚 Learning: 2025-09-02T11:18:02.529Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:319-325
Timestamp: 2025-09-02T11:18:02.529Z
Learning: In tests/network/conftest.py, for the _verify_nmstate_running_pods function, the user rnetser prefers a simple approach of just logging the result from wait_for_pods_running rather than implementing complex logic to distinguish between None and empty list returns.
Applied to files:
tests/network/conftest.py
📚 Learning: 2025-09-29T20:33:51.007Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:65-72
Timestamp: 2025-09-29T20:33:51.007Z
Learning: In tests/virt/node/migration_and_maintenance/conftest.py, the added_vm_cpu_limit fixture doesn't require ResourceEditor as a context manager because it's the final test to modify the vm_for_multifd_test VM before teardown, so restoration of CPU limits is unnecessary overhead as confirmed by maintainer dshchedr.
Applied to files:
tests/network/vm_import/test_ip_persitence.pytests/network/vm_import/conftest.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/network/vm_import/test_ip_persitence.pytests/network/vm_import/conftest.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
tests/network/vm_import/test_ip_persitence.py
📚 Learning: 2025-08-04T15:27:14.175Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1082
Timestamp: 2025-08-04T15:27:14.175Z
Learning: In tests/observability/metrics/conftest.py, the `non_existent_node_windows_vm` fixture is used for tier3 Windows VM testing and must use Windows VMs rather than Linux VMs, even though it adds overhead, because it's specifically testing Windows VM status transition metrics as part of dedicated Windows VM test coverage.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-09-10T23:16:25.845Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py:44-52
Timestamp: 2025-09-10T23:16:25.845Z
Learning: In pytest, fixtures are executed in the order they appear as parameters in the test method signature. For the multifd CPU limit test in tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py, the parameter order (vm_for_multifd_test, added_vm_cpu_limit, migrated_vm_source_pod) ensures CPU limits are applied before migration occurs.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-06-18T09:19:05.769Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/test_vms_metrics.py:129-137
Timestamp: 2025-06-18T09:19:05.769Z
Learning: For Windows VM testing in tests/observability/metrics/test_vms_metrics.py, it's acceptable to have more fixture parameters than typical pylint recommendations when reusing expensive Windows VM fixtures for performance. Windows VMs take a long time to deploy, so reusing fixtures like windows_vm_for_test and adding labels via windows_vm_with_low_bandwidth_migration_policy is preferred over creating separate fixtures that would require additional VM deployments.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-10-30T10:43:48.886Z
Learnt from: josemacassan
Repo: RedHatQE/openshift-virtualization-tests PR: 2300
File: tests/storage/online_resize/conftest.py:94-96
Timestamp: 2025-10-30T10:43:48.886Z
Learning: In tests/storage/online_resize/conftest.py, the `running_rhel_vm` fixture is intentionally designed to only invoke `running_vm(vm=rhel_vm_for_online_resize)` without returning or yielding a value. It's used as a dependency fixture to ensure the VM is running before other fixtures/tests execute, while the actual VM object is accessed via the `rhel_vm_for_online_resize` fixture.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-10-16T12:47:04.521Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 2249
File: tests/install_upgrade_operators/must_gather/test_must_gather.py:428-441
Timestamp: 2025-10-16T12:47:04.521Z
Learning: In openshift-virtualization-tests repository, DataVolumes in the openshift-virtualization-os-images namespace are volatile resources managed by DataImportCron. They can be created/destroyed between must-gather collection listing and file retrieval, requiring FileNotFoundError exception handling in test_crd_resources to skip these volatile resources gracefully while still validating DataVolumes in other namespaces. There is no pytest_generate_tests hook that filters out datavolumes from test parametrization.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-09-12T14:14:28.329Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:182-187
Timestamp: 2025-09-12T14:14:28.329Z
Learning: Network policy tests in the RedHatQE/openshift-virtualization-tests repository don't require unique resource names as there are no parallel runs on the same cluster in their test environment.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-09-15T06:49:53.478Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-09-18T13:51:28.583Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2105
File: tests/cross_cluster_live_migration/conftest.py:96-109
Timestamp: 2025-09-18T13:51:28.583Z
Learning: In MTV (Migration Toolkit for Virtualization) cross-cluster live migration tests, the remote cluster service account requires "cluster-admin" privileges rather than reduced permissions like "cluster-reader" or "view". This is necessary for the MTV functionality to properly perform cross-cluster migration operations.
Applied to files:
tests/network/vm_import/conftest.py
📚 Learning: 2025-05-28T10:50:56.122Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:264-269
Timestamp: 2025-05-28T10:50:56.122Z
Learning: In the openshift-virtualization-tests codebase, cleanup pytest fixtures like `deleted_old_dvs_of_stopped_vms`, `deleted_completed_virt_launcher_source_pod`, and `deleted_old_dvs_of_online_vms` do not require yield statements. These fixtures perform cleanup operations and work correctly without yielding values.
Applied to files:
tests/network/vm_import/conftest.py
🧬 Code graph analysis (3)
tests/network/conftest.py (1)
tests/conftest.py (1)
admin_client(303-307)
tests/network/vm_import/test_ip_persitence.py (3)
utilities/exceptions.py (1)
run(39-45)tests/network/vm_import/conftest.py (1)
mtv_migration(241-252)tests/network/libs/cluster_user_defined_network.py (2)
Condition(96-98)Status(95-98)
tests/network/vm_import/conftest.py (5)
tests/conftest.py (2)
namespace(661-674)admin_client(303-307)libs/net/udn.py (1)
create_udn_namespace(17-26)tests/network/vm_import/source_provider.py (3)
SourceClusterProvider(10-56)power_on_vm(34-38)power_off_vm(40-44)utilities/bitwarden.py (1)
get_cnv_tests_secret_by_name(59-82)tests/network/libs/cluster_user_defined_network.py (3)
ClusterUserDefinedNetwork(67-105)Condition(96-98)Status(95-98)
🪛 Ruff (0.14.7)
tests/network/conftest.py
222-222: Unused noqa directive (unknown: UFN001)
Remove unused noqa directive
(RUF100)
tests/network/vm_import/source_provider.py
51-51: Avoid specifying long messages outside the exception class
(TRY003)
tests/network/vm_import/conftest.py
32-32: Possible hardcoded password assigned to argument: "secret_name"
(S106)
41-41: Unused function argument: cudn_namespace
(ARG001)
41-41: Unused noqa directive (unknown: UFN001)
Remove unused noqa directive
(RUF100)
106-106: Unused function argument: mtv_namespace
(ARG001)
173-173: Unused function argument: cudn_layer2
(ARG001)
203-203: Unused noqa directive (unknown: UFN001)
Remove unused noqa directive
(RUF100)
205-205: Unused function argument: mtv_namespace
(ARG001)
212-212: Unused function argument: forklift_controller_udn_static_ip_patched
(ARG001)
213-213: Unused function argument: source_vm_powered_on
(ARG001)
241-241: Unused noqa directive (unknown: UFN001)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
🔇 Additional comments (5)
tests/network/vm_import/test_ip_persitence.py (1)
6-18: Test implementation looks good for a quarantined migration test.The test correctly uses
xfail(run=False)to quarantine it while migration performance issues are addressed (MTV-3947). The wait condition and timeout are appropriate for a warm migration scenario.tests/network/conftest.py (1)
370-379: MTV verification follows established patterns.The
_verify_mtv_installedfunction correctly follows the same pattern as other verification functions (_verify_sriov,_verify_service_mesh, etc.) - checking for a marker on collected tests and validating the corresponding namespace exists.tests/network/vm_import/source_provider.py (1)
46-51: VM search does not traverse nested folders.The
_get_vm_by_namemethod only searches immediate children ofdc.vmFolder. If VMs are organized in nested folders (a common vSphere practice), they won't be found.Consider using a recursive traversal or the
SearchIndexAPI:def _get_vm_by_name(self, name: str) -> vim.VirtualMachine: - for dc in self.content.rootFolder.childEntity: - for vm in dc.vmFolder.childEntity: - if vm.name == name: - return vm - raise VmNotFoundError(f"VM '{name}' not found") + search_index = self.content.searchIndex + vm = search_index.FindByDnsName(vmSearch=True, dnsName=name) + if vm is None: + # Fallback: search by inventory path or iterate + for dc in self.content.rootFolder.childEntity: + vm = search_index.FindChild(entity=dc.vmFolder, name=name) + if vm: + return vm + if vm is None: + raise VmNotFoundError(f"VM '{name}' not found") + return vmAlternatively, if the test environment uses a flat folder structure, please verify this is intentional.
tests/network/vm_import/conftest.py (2)
30-32: Fixture dependencies ensure proper resource ordering.The unused function arguments flagged by static analysis (e.g.,
cudn_namespaceincudn_layer2,mtv_namespaceinmtv_source_provider) are intentional pytest fixture dependencies that ensure proper creation order. This is a common pattern in this codebase. Based on learnings, this approach is preferred over fixture chaining.
240-252: Verify namespace configuration for Migration and Plan resources.MTV supports Plan resources with
spec.targetNamespacefor specifying where migrated VMs will be created, which may differ from the Plan resource's own namespace. Confirm whether themtv_migrationandmtv_migration_planfixtures are intentionally configured in different namespaces as part of MTV's expected architecture, or if they should align.
servolkov
force-pushed
the
mtv-list-vm
branch
from
December 2, 2025 13:42
b6afa2c to
cc37712
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Contributor
Author
azhivovk
approved these changes
Dec 18, 2025
Contributor
Author
|
/verified |
Contributor
Test execution plan request detailsCRITICAL: You MUST respond with a review comment on the Files Changed tab, NOT as a regular PR comment. 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 inline review comment on Files Changed 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: 1305 🏁 Script executed: Length of output: 1833 🧠 Learnings used |
Anatw
approved these changes
Dec 18, 2025
Collaborator
|
/lgtm |
yossisegev
approved these changes
Dec 23, 2025
EdDev
reviewed
Dec 24, 2025
Signed-off-by: Sergei Volkov <sevolkov@redhat.com>
Contributor
Author
|
Change: the module name alteration |
openshift-virtualization-qe-bot
approved these changes
Dec 24, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/network/provider_migration/libprovider.py (1)
45-50: VM lookup only searches top-level folders.The
_get_vm_by_namemethod iterates overdc.vmFolder.childEntitywhich only finds VMs in the root VM folder of each datacenter. VMs placed in nested folders (subfolders) won't be found. If this is intentional for your test setup, this is fine; otherwise, consider a recursive search.🔎 Optional: Recursive VM search for nested folders
def _get_vm_by_name(self, name: str) -> vim.VirtualMachine: - for dc in self.content.rootFolder.childEntity: - for vm in dc.vmFolder.childEntity: - if vm.name == name: - return vm + def find_vm(folder): + for item in folder.childEntity: + if isinstance(item, vim.VirtualMachine) and item.name == name: + return item + elif isinstance(item, vim.Folder): + result = find_vm(item) + if result: + return result + return None + + for dc in self.content.rootFolder.childEntity: + if hasattr(dc, 'vmFolder'): + vm = find_vm(dc.vmFolder) + if vm: + return vm raise VmNotFoundError(f"VM '{name}' not found")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/network/provider_migration/__init__.pytests/network/provider_migration/conftest.pytests/network/provider_migration/libprovider.pytests/network/provider_migration/test_ip_persistence.py
🧰 Additional context used
🧠 Learnings (51)
📓 Common learnings
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2105
File: tests/cross_cluster_live_migration/conftest.py:96-109
Timestamp: 2025-09-18T13:51:28.583Z
Learning: In MTV (Migration Toolkit for Virtualization) cross-cluster live migration tests, the remote cluster service account requires "cluster-admin" privileges rather than reduced permissions like "cluster-reader" or "view". This is necessary for the MTV functionality to properly perform cross-cluster migration operations.
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.
📚 Learning: 2025-10-27T15:30:06.412Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-06-18T09:21:34.315Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-08-28T12:30:40.692Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-12-15T12:33:06.686Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3024
File: tests/network/connectivity/utils.py:17-17
Timestamp: 2025-12-15T12:33:06.686Z
Learning: In the test suite, ensure the ipv6_network_data fixture returns a factory function (Callable) and that all call sites invoke it to obtain the actual data dict, i.e., use ipv6_network_data() at call sites. This enables future extensibility for configuring secondary interfaces' IP addresses without changing call sites. Apply this pattern to all Python test files under tests.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-10-08T07:16:46.347Z
Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-29T20:33:51.007Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:65-72
Timestamp: 2025-09-29T20:33:51.007Z
Learning: In tests/virt/node/migration_and_maintenance/conftest.py, the added_vm_cpu_limit fixture doesn't require ResourceEditor as a context manager because it's the final test to modify the vm_for_multifd_test VM before teardown, so restoration of CPU limits is unnecessary overhead as confirmed by maintainer dshchedr.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-12-16T15:09:49.597Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T15:09:49.597Z
Learning: In the openshift-virtualization-tests repository, when conftest.py or utilities/bitwarden.py changes affect py_config["os_login_param"], smoke test impact must be determined by: (1) finding all smoke tests using `rg "pytest.mark.smoke"`, (2) checking each for VM creation patterns (VirtualMachineForTests, running_vm, VirtualMachineForTestsFromTemplate), (3) tracing whether running_vm is called with default check_ssh_connectivity=True, which accesses vm.login_params property that reads py_config["os_login_param"][vm.os_flavor]. The dependency chain is: smoke test → VM creation → running_vm → wait_for_ssh_connectivity → vm.login_params → os_login_param. Any smoke test creating VMs with SSH connectivity (the default) depends on os_login_param.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-08-04T15:27:14.175Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1082
Timestamp: 2025-08-04T15:27:14.175Z
Learning: In tests/observability/metrics/conftest.py, the `non_existent_node_windows_vm` fixture is used for tier3 Windows VM testing and must use Windows VMs rather than Linux VMs, even though it adds overhead, because it's specifically testing Windows VM status transition metrics as part of dedicated Windows VM test coverage.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-12-02T13:52:42.259Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 2970
File: tests/network/vm_import/conftest.py:68-82
Timestamp: 2025-12-02T13:52:42.259Z
Learning: In the ForkliftController CRD (forklift.konveyor.io/v1beta1), the spec field `controller_static_udn_ip_addresses` is a string type that accepts the string values "true" or "false" (not Python boolean True/False). It's used to enable or disable static UDN IP address assignment for the controller.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-08T22:06:55.755Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1987
File: tests/virt/node/descheduler/conftest.py:165-170
Timestamp: 2025-09-08T22:06:55.755Z
Learning: VirtualMachineInstanceMigration.get() from ocp-resources expects the namespace object (not the string name) as the namespace parameter, as consistently shown across all usage patterns in the codebase including tests/utils.py, tests/virt/upgrade/utils.py, and utilities/virt.py.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-08T22:06:55.755Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1987
File: tests/virt/node/descheduler/conftest.py:165-170
Timestamp: 2025-09-08T22:06:55.755Z
Learning: In ocp-resources library, the .get() method expects the namespace object (not the string name) when passed as the namespace parameter, as confirmed in VirtualMachineInstanceMigration.get(dyn_client=admin_client, namespace=namespace) usage in tests/virt/node/descheduler/conftest.py.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-08-14T16:39:26.061Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/bgp.py:83-84
Timestamp: 2025-08-14T16:39:26.061Z
Learning: In the FRR BGP integration workflow (libs/net/bgp.py), the FRR namespace (openshift-frr-k8s) is not automatically deleted by the operator when ResourceEditor reverts the Network patch. The namespace must be explicitly deleted in the cleanup code, and this deletion does not require guarding against ResourceNotFoundError since the operator doesn't remove it.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-11-08T07:36:57.616Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-09-15T06:49:53.478Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-06-17T07:45:37.776Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-08-09T01:46:48.039Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:71-85
Timestamp: 2025-08-09T01:46:48.039Z
Learning: In the openshift-virtualization-tests repository, user dshchedr prefers that all test setup fixtures be explicitly declared as parameters in the test method itself rather than chaining fixtures through dependencies. This makes all setup steps visible in one place at the test method level, improving test readability and making dependencies explicit, even if a fixture isn't directly used within another fixture's implementation.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-11-19T08:13:30.263Z
Learnt from: SamAlber
Repo: RedHatQE/openshift-virtualization-tests PR: 2507
File: tests/virt/node/general/test_vmi_reset.py:26-29
Timestamp: 2025-11-19T08:13:30.263Z
Learning: In the openshift-virtualization-tests repository, user SamAlber prefers not to define fixture dependencies by chaining fixtures (adding one fixture as a parameter to another). Instead, all fixture dependencies should be explicitly declared as parameters in the test method itself, relying on parameter order to control execution sequence.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-11-26T16:03:07.813Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:372-387
Timestamp: 2025-11-26T16:03:07.813Z
Learning: In the openshift-virtualization-tests repository, pytest fixtures that declare other fixtures as parameters purely for dependency ordering (without referencing them in the function body) should not be modified to silence Ruff ARG001 warnings. This is an idiomatic pytest pattern for ensuring setup order, and the team prefers to leave such fixtures unchanged rather than adding defensive comments or code to suppress linter warnings.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-06-18T09:19:05.769Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/test_vms_metrics.py:129-137
Timestamp: 2025-06-18T09:19:05.769Z
Learning: For Windows VM testing in tests/observability/metrics/test_vms_metrics.py, it's acceptable to have more fixture parameters than typical pylint recommendations when reusing expensive Windows VM fixtures for performance. Windows VMs take a long time to deploy, so reusing fixtures like windows_vm_for_test and adding labels via windows_vm_with_low_bandwidth_migration_policy is preferred over creating separate fixtures that would require additional VM deployments.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-08-20T23:43:28.117Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-12-16T14:00:59.076Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-12-22T15:56:00.157Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-12-16T14:06:22.391Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:06:22.391Z
Learning: In the openshift-virtualization-tests repository, when posting test execution plan inline review comments using GitHub API, the full test execution plan content must go in the `comments[].body` field (which appears on Files Changed tab), NOT in the top-level `body` field (which appears in PR discussion thread). The top-level `body` field should be omitted or left empty to avoid posting redundant comments in the PR discussion thread.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-28T14:43:07.181Z
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-28T14:43:07.181Z
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-06-23T19:28:20.281Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:24-64
Timestamp: 2025-06-23T19:28:20.281Z
Learning: In OpenShift Virtualization mass machine type transition tests, the machine type glob pattern "pc-q35-rhel8.*.*" is intentionally hard-coded in the kubevirt_api_lifecycle_automation_job as it's used only once for this specific test case, with plans to update it in the future if the job needs to support other machine types.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-09-15T05:54:21.047Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:219-225
Timestamp: 2025-09-15T05:54:21.047Z
Learning: In dual-stack OpenShift clusters, the ordering of IPs in clusterIPs has semantic meaning - the first IP indicates the cluster's primary IP family. If the first IP is IPv6, it means it's an IPv6-primary dual-stack cluster. Network policy tests should respect this ordering and skip IPv6-primary clusters rather than attempting to find IPv4 alternatives from the clusterIPs list.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-10-30T10:43:48.886Z
Learnt from: josemacassan
Repo: RedHatQE/openshift-virtualization-tests PR: 2300
File: tests/storage/online_resize/conftest.py:94-96
Timestamp: 2025-10-30T10:43:48.886Z
Learning: In tests/storage/online_resize/conftest.py, the `running_rhel_vm` fixture is intentionally designed to only invoke `running_vm(vm=rhel_vm_for_online_resize)` without returning or yielding a value. It's used as a dependency fixture to ensure the VM is running before other fixtures/tests execute, while the actual VM object is accessed via the `rhel_vm_for_online_resize` fixture.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-05-28T10:50:56.122Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:264-269
Timestamp: 2025-05-28T10:50:56.122Z
Learning: In the openshift-virtualization-tests codebase, cleanup pytest fixtures like `deleted_old_dvs_of_stopped_vms`, `deleted_completed_virt_launcher_source_pod`, and `deleted_old_dvs_of_online_vms` do not require yield statements. These fixtures perform cleanup operations and work correctly without yielding values.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-06-18T09:15:25.436Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1178-1180
Timestamp: 2025-06-18T09:15:25.436Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_vm_metric_1` fixture is intentionally designed to stop the VM and leave it in that state - it does not need to restart the VM afterward as this is the desired behavior for the tests that use it.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-11-05T01:28:39.322Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 2509
File: tests/virt/cluster/aaq/conftest.py:0-0
Timestamp: 2025-11-05T01:28:39.322Z
Learning: In pytest fixtures within the openshift-virtualization-tests repo, when using `yield from` to delegate to a generator function for setup/teardown (like `update_hco_memory_overcommit`), the delegated function should be a plain generator function without the `contextmanager` decorator. Using `contextmanager` causes a TypeError because it wraps the generator in a `_GeneratorContextManager` object, which is not iterable. The correct pattern is: remove `contextmanager` and use `yield from plain_generator_function(...)` in the fixture.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-10-16T12:47:04.521Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 2249
File: tests/install_upgrade_operators/must_gather/test_must_gather.py:428-441
Timestamp: 2025-10-16T12:47:04.521Z
Learning: In openshift-virtualization-tests repository, DataVolumes in the openshift-virtualization-os-images namespace are volatile resources managed by DataImportCron. They can be created/destroyed between must-gather collection listing and file retrieval, requiring FileNotFoundError exception handling in test_crd_resources to skip these volatile resources gracefully while still validating DataVolumes in other namespaces. There is no pytest_generate_tests hook that filters out datavolumes from test parametrization.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-09-12T08:10:48.874Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:16-16
Timestamp: 2025-09-12T08:10:48.874Z
Learning: Network policy tests that create different types of pods (client pod, server pod, existing component pods) for connectivity testing can run on SNO, as they don't require multiple replicas of the same component to function properly.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-09-12T14:14:28.329Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:182-187
Timestamp: 2025-09-12T14:14:28.329Z
Learning: Network policy tests in the RedHatQE/openshift-virtualization-tests repository don't require unique resource names as there are no parallel runs on the same cluster in their test environment.
Applied to files:
tests/network/provider_migration/conftest.py
📚 Learning: 2025-12-22T16:27:40.244Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:40.244Z
Learning: For PRs that remove tests, rely on pytest --collect-only to verify the test discovery results (which tests are selected/deselected) and ensure the removal is clean and the test module remains functional. Full test execution is not required for test deletion PRs. This guideline applies to test files anywhere under the tests/ directory (e.g., tests/network/upgrade/test_upgrade_network.py) and should be used for similar test-deletion scenarios across the repository.
Applied to files:
tests/network/provider_migration/conftest.pytests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-11-25T01:56:54.902Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 2838
File: .github/workflows/net-utils-builder-staging.yml:37-37
Timestamp: 2025-11-25T01:56:54.902Z
Learning: In the openshift-virtualization-tests repository, when renaming container images that are built and used by GitHub Actions workflows, the changes must be done sequentially: first merge the workflow files (.github/workflows/) that update the image name in the CI/CD pipelines, then update the code references (like constants.py and manifest files) in a follow-up PR. This prevents the old workflow from running with mismatched image names during the transition.
Applied to files:
tests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-12-16T20:11:03.645Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.
Applied to files:
tests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
tests/network/provider_migration/test_ip_persistence.py
📚 Learning: 2025-12-10T10:57:12.630Z
Learnt from: geetikakay
Repo: RedHatQE/openshift-virtualization-tests PR: 3050
File: tests/infrastructure/golden_images/update_boot_source/test_ssp_data_import_crons.py:277-279
Timestamp: 2025-12-10T10:57:12.630Z
Learning: In tests/infrastructure/golden_images/update_boot_source/test_ssp_data_import_crons.py, the test_all_datasources_support_vm_creation method uses broad Exception catching intentionally to collect all VM creation failures across different data sources for comprehensive reporting, rather than letting specific exceptions bubble up and stop the test early.
Applied to files:
tests/network/provider_migration/test_ip_persistence.pytests/network/provider_migration/libprovider.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Applied to files:
tests/network/provider_migration/libprovider.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.
Applied to files:
tests/network/provider_migration/libprovider.py
🧬 Code graph analysis (1)
tests/network/provider_migration/test_ip_persistence.py (1)
tests/network/provider_migration/conftest.py (2)
mtv_migration_to_cudn_ns(249-257)mtv_migration_plan_to_cudn_ns(213-245)
🪛 Ruff (0.14.10)
tests/network/provider_migration/conftest.py
31-31: Possible hardcoded password assigned to argument: "secret_name"
(S106)
183-183: Unused function argument: cudn_layer2_for_mtv_import
(ARG001)
221-221: Unused function argument: forklift_controller_udn_static_ip_patched
(ARG001)
222-222: Unused function argument: source_vm_powered_on
(ARG001)
tests/network/provider_migration/test_ip_persistence.py
12-12: Unused function argument: mtv_migration_to_cudn_ns
(ARG001)
tests/network/provider_migration/libprovider.py
50-50: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (14)
tests/network/provider_migration/conftest.py (8)
1-27: LGTM on imports and constants.The imports are well-organized and the constants are appropriately typed with
Final. TheVDDK_IMAGEversion 8.0.1 is correctly pinned for reproducibility.
39-59: Good teardown ordering for CUDN cleanup.The explicit
cudn_namespace.clean_up()call before the CUDN context manager exits is the correct approach to ensure pods are detached before the network resource is deleted. The comment on line 58 explains the reasoning well.
62-71: LGTM on source VM lifecycle management.The fixture correctly powers on the VM during setup and powers off during teardown, ensuring the source VM is in the proper state for migration. The unused
yieldpattern is idiomatic pytest for ordering fixtures. Based on learnings, unused fixture arguments for dependency ordering should not be flagged.
74-90: LGTM on ForkliftController patching.The
ResourceEditorcontext manager pattern correctly applies the patch and automatically restores the original state on teardown. Based on learnings, the string value"true"forcontroller_static_udn_ip_addressesis correct per CRD schema requirements.
111-131: Cross-namespace secret reference may cause Provider creation failure.The
mtv_source_provideris created incudn_namespace(line 120), butsource_hypervisor_secretis created inmtv_namespace_scope_session(line 99). The Provider references the secret by name and namespace (lines 123-124), which should work if the Provider CRD supports cross-namespace secret references. However, if the secret namespace differs from the provider namespace and the Provider controller doesn't have permissions or doesn't support this, it may fail to retrieve credentials.Please verify that the Forklift Provider CRD supports cross-namespace secret references, or confirm this works in your testing environment.
179-209: Unused fixture parametercudn_layer2_for_mtv_importis for dependency ordering.The static analysis flagged
cudn_layer2_for_mtv_importas unused (ARG001), but this is an idiomatic pytest pattern for ensuring the CUDN network is created before the NetworkMap. Based on learnings, fixtures that declare other fixtures as parameters purely for dependency ordering should not be modified to silence linter warnings.
212-245: LGTM on migration plan fixture.The plan fixture correctly aggregates all dependencies (storage map, network map, providers, controller patch, source VM powered on) and configures a cold migration with static IP preservation. The unused fixture parameters (
forklift_controller_udn_static_ip_patched,source_vm_powered_on) are dependency ordering fixtures per the idiomatic pytest pattern.
248-257: LGTM on migration fixture.The Migration resource is correctly bound to the plan by name and namespace, establishing the orchestration relationship for the MTV workflow.
tests/network/provider_migration/libprovider.py (4)
1-7: LGTM on imports and exception class.The imports are minimal and appropriate.
VmNotFoundErrorprovides a clear custom exception for VM lookup failures.
10-32: LGTM on SourceHypervisorProvider context manager.The class correctly implements the context manager protocol with
SmartConnectfor vSphere authentication andDisconnectfor cleanup. Disabling SSL certificate validation is appropriate for test environments.
34-43: LGTM on power on/off methods.The methods correctly handle VM power state transitions with status verification and IP address confirmation (for power on). Past review comments confirmed that
PowerOn()andPowerOff()are valid pyVmomi convenience methods.
52-60: LGTM on retry-enabled status checks.The static methods correctly use the
@retrydecorator with appropriate timeouts (120s) and sleep intervals (5s) for waiting on VM power state changes and IP address assignment.tests/network/provider_migration/test_ip_persistence.py (2)
1-11: LGTM on test markers and quarantine configuration.The test is correctly marked with
mtvfor operator requirement checking, includes the Polarion ID, and is properly quarantined withxfail(run=False)due to the known long migration time issue (MTV-3947).
12-18: Fixturemtv_migration_to_cudn_nsis required for triggering the migration.The static analysis flags
mtv_migration_to_cudn_nsas unused (ARG001), but this fixture is essential—it creates theMigrationresource that actually triggers the migration workflow. Without it, thePlanwould remain in READY state but never execute. The test correctly waits on the Plan's SUCCEEDED condition, which reflects the migration outcome. Based on learnings, this is the idiomatic pytest pattern for dependency ordering.
EdDev
approved these changes
Dec 24, 2025
EdDev
left a comment
EdDev
left a comment
Contributor
There was a problem hiding this comment.
Thank you, looks great.
/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: |
Contributor
Author
|
/retest build-container |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1381 🏁 Script executed: Length of output: 1861 🧠 Learnings used |
Collaborator
|
/approve |
Contributor
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
This was referenced Jan 5, 2026
This was referenced Jan 20, 2026
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
9 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:
Adding fixtures to import a VM from external cluster + simple source cluster provider to operate VMs.
More details:
The following functionality has been added:
Special notes for reviewer:
This is an initial PR. Upcoming changes:
SourceHypervisorProvider(perhaps we should just copy the existing provider from the MTV team or put it in a separate library).jira-ticket: https://issues.redhat.com/browse/CNV-72413, https://issues.redhat.com/browse/CNV-72414
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.