WIP: Client all test #2965
Open
WIP: Client all test #2965
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
/wip |
WalkthroughMade DynamicClient explicit across networking code and tests: many constructors and helper functions now require a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
Contributor
|
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-3
requested review from
EdDev,
OhadRevah,
akri3i and
duyanyan
openshift-virtualization-qe-bot-2
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2965 +/- ##
=======================================
Coverage 98.28% 98.28%
=======================================
Files 25 25
Lines 2157 2157
=======================================
Hits 2120 2120
Misses 37 37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openshift-virtualization-qe-bot-3
requested review from
dshchedr,
geetikakay,
kgoldbla and
kshvaika
Contributor
Author
|
/build-and-push-container |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
This change makes the client mandatory for all NAD-related resources. Updated fixtures so that all calls to openshift-python-wrapper NAD resources explicitly pass a client. Signed-off-by: Anat Wax <[email protected]>
Make NNCP-related classes take a mandatory DynamicClient and update all NNCP-related fixtures to pass an admin_client explicitly (no implicit default clients). Signed-off-by: Anat Wax <[email protected]>
This change makes the client mandatory for all CUDN-related resources. Updated fixtures so that all calls to openshift-python-wrapper CUDN resources explicitly pass a client. Signed-off-by: Anat Wax <[email protected]>
Updated class instance and fixtures - all calls to openshift-python-wrapper resource should be updated to pass client. Signed-off-by: Anat Wax <[email protected]>
Updated all OCP wrapper class instantiations across the network segment with the relevant client parameter. Signed-off-by: Anat Wax <[email protected]>
Updated class instance and fixtures - all calls to openshift-python-wrapper resource should be updated to pass client. Signed-off-by: Anat Wax <[email protected]>
Make create_custom_template_from_url take a mandatory DynamicClient and update all call sites to pass an admin_client explicitly (no implicit default clients). Signed-off-by: Anat Wax <[email protected]>
Updated VM fixtures - all calls to openshift-python-wrapper resource should be updated to pass client. Signed-off-by: Anat Wax <[email protected]>
Anatw
force-pushed
the
client_all_test
branch
from
4d87db9 to
37192c2
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
approved these changes
Dec 4, 2025
Contributor
Author
|
/build-and-push-container |
Contributor
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-2965 published |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/conftest.py (1)
1786-1833: Fix undefinedcnv_current_versionindetermine_upgrade_streamand clarify version assumptionsIn
determine_upgrade_stream(Lines 1810‑1833):
- The
ValueErrorraised for non-upgrade scenarios referencescnv_current_version, which is not defined in this function, and will trigger aNameErrorinstead of the intendedValueErrorwhentarget_cnv_version <= current_cnv_version.You can fix this by using the function parameters (or the parsed versions) in the error message:
def determine_upgrade_stream(current_version, target_version): @@ elif HOTFIX_STR in current_version: # if we reach here, this is an upgrade out of hotfix to next z-stream return UpgradeStreams.Z_STREAM else: if target_cnv_version <= current_cnv_version: # Upgrade only if a newer CNV version is requested raise ValueError( - f"Cannot upgrade to older/identical versions," - f"current: {cnv_current_version} target: {target_cnv_version}" + f"Cannot upgrade to older/identical versions, " + f"current: {current_version} target: {target_version}" )Also, this helper assumes
current_versionandtarget_versionare non-empty, valid version strings. Ifcnv_current_versioncan be"CNV not yet installed."orNone, orcnv_target_versionis unset, consider either guarding against that incnv_upgrade_streambefore calling this helper or documenting that precondition.
🧹 Nitpick comments (7)
tests/infrastructure/sap/test_sap_hana_vm.py (1)
343-358: Refactor cleanup: use explicit loop instead of list comprehension.The admin_client propagation looks correct and aligns with the broader PR pattern. However, line 358 uses a list comprehension for side effects (calling
clean_up()), which is non-idiomatic Python. List comprehensions should be used for building lists, not for executing side effects.Apply this diff to improve code clarity:
yield nads_list - [nad.clean_up() for nad in nads_list] + for nad in nads_list: + nad.clean_up()tests/conftest.py (1)
586-655: Admin client wiring for NIC/SRIOV fixtures looks correct; address unusedworkers_utility_pods
- Using
admin_clientexplicitly innodes_active_nics(Line 587) viaNodeNetworkState(name=node.name, client=admin_client)and inworker_nodes_ipv4_false_secondary_nics,sriov_namespace, andmac_poolis consistent with how other ocp_resources wrappers are instantiated and should behave as before, just with an explicit client.- In
sriov_node_policy(Lines 1075‑1089), the newclient=admin_clientargument passed intocreate_sriov_node_policyis also consistent withutilities.network.create_sriov_node_policy’s signature.Ruff’s ARG001 for
workers_utility_podsbeing unused (Line 1079) can be fixed without changing fixture wiring by explicitly marking it as used:@pytest.fixture(scope="session") def sriov_node_policy( admin_client, sriov_unused_ifaces, sriov_nodes_states, - workers_utility_pods, + workers_utility_pods, sriov_namespace, ): + # Ensure workers_utility_pods fixture is executed even though it is not used directly here. + _ = workers_utility_pods yield from create_sriov_node_policy( nncp_name="test-sriov-policy", namespace=sriov_namespace.name, sriov_iface=sriov_unused_ifaces[0], sriov_nodes_states=sriov_nodes_states, sriov_resource_name="sriov_net", client=admin_client, )Also applies to: 965-1098
tests/network/service_mesh/conftest.py (1)
46-154: Service mesh resources now consistently use explicit clients; mark unused fixture dependency
- The test wrapper classes
GatewayForTests,DestinationRuleForTests,VirtualServiceForTests, andPeerAuthenticationForTestsnow take aclientparameter and passclient=clientto their respectiveocp_resourcesbase classes. This aligns them with other resource wrappers and looks correct.- Fixtures such as
httpbin_deployment_service_mesh,httpbin_service_account_service_mesh,httpbin_service_service_mesh,server_deployment_v1/v2,server_service_service_mesh,gateway_service_mesh,virtual_service_mesh_service,destination_rule_service_mesh, andpeer_authentication_strict_service_meshnow receiveunprivileged_clientand pass it asclient=unprivileged_clientwhen creating resources. That matches howunprivileged_clientis provided intests/conftest.pyand should behave as intended.Ruff’s ARG001 warning for the unused
httpbin_service_account_service_meshfixture argument inhttpbin_service_service_mesh(Line 223) can be resolved without changing fixture wiring by explicitly marking it as used:@pytest.fixture(scope="class") def httpbin_service_service_mesh( - unprivileged_client, httpbin_deployment_service_mesh, httpbin_service_account_service_mesh + unprivileged_client, httpbin_deployment_service_mesh, httpbin_service_account_service_mesh ): + # Ensure service account fixture is executed, even if not referenced directly. + _ = httpbin_service_account_service_mesh with ServiceMeshDeploymentService( namespace=httpbin_deployment_service_mesh.namespace, app_name=httpbin_deployment_service_mesh.app_name, port=httpbin_deployment_service_mesh.service_port, client=unprivileged_client, ) as sv: yield svAlso applies to: 191-347, 416-421
tests/network/localnet/conftest.py (1)
44-61: Localnet NNCP/CUDN fixtures correctly useadmin_client; silence unusednamespace_localnet_1
nncp_localnet(Lines 44‑61) now takesadmin_client: DynamicClientand passesclient=admin_clientintolibnncp.NodeNetworkConfigurationPolicy, which matches its constructor signature.cudn_localnet,cudn_localnet_no_vlan,cudn_localnet_ovs_bridge, andcudn_localnet_ovs_bridge_jumbo_framenow passclient=admin_clienttolocalnet_cudn, which in turn forwards the client toClusterUserDefinedNetwork. This is consistent with the library’s expectations.nncp_localnet_on_secondary_node_nicandnncp_localnet_on_secondary_node_nic_with_jumbo_framecorrectly threadclient=admin_clientintocreate_nncp_localnet_on_secondary_node_nic, including the MTU parameter for the jumbo-frame variant.For
cudn_localnet_no_vlan(Lines 110‑118), Ruff’s ARG001 on the seemingly unusednamespace_localnet_1can be addressed while preserving fixture dependency by explicitly referencing it:@pytest.fixture(scope="module") def cudn_localnet_no_vlan( admin_client: DynamicClient, namespace_localnet_1: Namespace, ) -> Generator[libcudn.ClusterUserDefinedNetwork]: - with localnet_cudn( + # Ensure namespace_localnet_1 is created and labeled before creating the CUDN. + _ = namespace_localnet_1 + with localnet_cudn( name=LOCALNET_BR_EX_NETWORK_NO_VLAN, match_labels=LOCALNET_TEST_LABEL, physical_network_name=LOCALNET_BR_EX_NETWORK, client=admin_client, ) as cudn: cudn.wait_for_status_success() yield cudnAlso applies to: 92-121, 215-229, 361-385, 388-403
tests/network/kubemacpool/test_kubemacpool.py (1)
127-134:test_kmp_downnow usesunprivileged_client; explicitly markkmp_downas used
- Passing
client=unprivileged_clientintoVirtualMachineForTests(Line 133) is consistent with other tests and with the unprivileged client fixture semantics.To satisfy Ruff’s ARG001 about the unused
kmp_downargument (Line 131) while preserving the dependency on thekmp_downfixture, you can explicitly reference it:@pytest.mark.sno @pytest.mark.polarion("CNV-4405") @pytest.mark.single_nic @pytest.mark.s390x def test_kmp_down(unprivileged_client, namespace, kmp_down): - with pytest.raises(ApiException): - with VirtualMachineForTests(name="kmp-down-vm", namespace=namespace.name, client=unprivileged_client): - return + # Ensure kmp_down fixture is executed, even if its value is not used directly. + _ = kmp_down + with pytest.raises(ApiException): + with VirtualMachineForTests(name="kmp-down-vm", namespace=namespace.name, client=unprivileged_client): + returntests/network/general/test_bridge_marker.py (1)
22-33: Consider adding explicit client parameter for consistency.The
create_bridge_attached_vm_for_bridge_markerhelper does not accept or pass aclientparameter toVirtualMachineForTests, while the similarmulti_bridge_attached_vmifixture (line 99) explicitly usesunprivileged_client. This inconsistency could be addressed in a follow-up.@contextmanager -def create_bridge_attached_vm_for_bridge_marker(namespace, bridge_marker_bridge_network): +def create_bridge_attached_vm_for_bridge_marker(namespace, bridge_marker_bridge_network, client=None): networks = {bridge_marker_bridge_network.name: bridge_marker_bridge_network.name} name = _get_name(suffix="bridge-vm") with VirtualMachineForTests( namespace=namespace.name, name=name, networks=networks, interfaces=sorted(networks.keys()), body=fedora_vm_body(name=name), + client=client, ) as vm: yield vmutilities/network.py (1)
177-223: Consider consistent parameter placement across similar classes.The
clientparameter appears afterbridge_namehere, but in the parent classBridgeNodeNetworkConfigurationPolicyit appears afterstp_config. While not incorrect, consistent positioning (e.g., always immediately after required positional parameters or always in the same relative position) would improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
libs/net/netattachdef.py(2 hunks)tests/conftest.py(9 hunks)tests/infrastructure/sap/test_sap_hana_vm.py(3 hunks)tests/install_upgrade_operators/must_gather/conftest.py(1 hunks)tests/network/bgp/conftest.py(3 hunks)tests/network/bond/test_bond_modes.py(9 hunks)tests/network/bond/test_l2_bridge_over_bond.py(7 hunks)tests/network/conftest.py(6 hunks)tests/network/connectivity/conftest.py(13 hunks)tests/network/dry_run/test_dry_run_kubemacpool.py(1 hunks)tests/network/flat_overlay/conftest.py(6 hunks)tests/network/flat_overlay/utils.py(1 hunks)tests/network/general/test_bridge_marker.py(2 hunks)tests/network/general/test_cnv_tuning_regression.py(1 hunks)tests/network/general/test_ip_family_services.py(2 hunks)tests/network/jumbo_frame/conftest.py(1 hunks)tests/network/jumbo_frame/test_bond.py(8 hunks)tests/network/jumbo_frame/test_bridge.py(4 hunks)tests/network/kubemacpool/conftest.py(16 hunks)tests/network/kubemacpool/test_kubemacpool.py(1 hunks)tests/network/l2_bridge/conftest.py(7 hunks)tests/network/l2_bridge/test_bridge_nic_hot_plug.py(7 hunks)tests/network/l2_bridge/test_ovs_bridge.py(2 hunks)tests/network/l2_bridge/utils.py(2 hunks)tests/network/libs/cluster_user_defined_network.py(3 hunks)tests/network/libs/nodenetworkconfigurationpolicy.py(3 hunks)tests/network/localnet/conftest.py(8 hunks)tests/network/localnet/liblocalnet.py(6 hunks)tests/network/macspoof/conftest.py(2 hunks)tests/network/migration/test_migration.py(3 hunks)tests/network/nmstate/conftest.py(4 hunks)tests/network/nmstate/test_connectivity_after_nmstate_changes.py(2 hunks)tests/network/service_mesh/conftest.py(11 hunks)tests/network/sriov/conftest.py(4 hunks)tests/network/upgrade/conftest.py(2 hunks)tests/network/upgrade/utils.py(1 hunks)tests/network/user_defined_network/test_user_defined_network.py(1 hunks)tests/network/utils.py(5 hunks)tests/storage/cdi_import/conftest.py(1 hunks)tests/virt/cluster/common_templates/windows/test_windows_custom_options.py(1 hunks)tests/virt/node/high_performance_vm/test_numa.py(1 hunks)tests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.py(1 hunks)tests/virt/node/migration_and_maintenance/utils.py(1 hunks)utilities/network.py(19 hunks)utilities/ssp.py(2 hunks)utilities/unittests/test_ssp.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/storage/cdi_import/conftest.py
- tests/virt/node/high_performance_vm/test_numa.py
- tests/network/l2_bridge/test_bridge_nic_hot_plug.py
- tests/virt/cluster/common_templates/windows/test_windows_custom_options.py
- tests/network/sriov/conftest.py
- tests/network/connectivity/conftest.py
- tests/network/nmstate/test_connectivity_after_nmstate_changes.py
- tests/network/macspoof/conftest.py
- tests/network/upgrade/utils.py
- tests/network/flat_overlay/conftest.py
- tests/network/jumbo_frame/test_bridge.py
- tests/network/general/test_ip_family_services.py
- tests/network/user_defined_network/test_user_defined_network.py
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
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: 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: 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: 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: 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.
Learnt from: SiboWang1997
Repo: RedHatQE/openshift-virtualization-tests PR: 1566
File: tests/global_config_x86_64.py:29-29
Timestamp: 2025-07-25T01:59:02.180Z
Learning: In the openshift-virtualization-tests repo, the s390x architecture still relies on Fedora 41 images, while only x86_64 is updated to Fedora 42 in PR #1566.
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`.
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`.
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.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1249-1254
Timestamp: 2025-08-06T13:57:51.928Z
Learning: User rnetser verified that all calls to get_infrastructure() function use the admin_client parameter, confirming that signature changes requiring this parameter don't cause breaking changes in the openshift-virtualization-tests codebase.
📚 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/flat_overlay/utils.pytests/network/libs/nodenetworkconfigurationpolicy.pytests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/network/libs/cluster_user_defined_network.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/l2_bridge/test_ovs_bridge.pyutilities/network.pytests/network/kubemacpool/conftest.pytests/network/utils.pytests/network/kubemacpool/test_kubemacpool.py
📚 Learning: 2025-08-06T13:57:51.928Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1249-1254
Timestamp: 2025-08-06T13:57:51.928Z
Learning: User rnetser verified that all calls to get_infrastructure() function use the admin_client parameter, confirming that signature changes requiring this parameter don't cause breaking changes in the openshift-virtualization-tests codebase.
Applied to files:
tests/network/flat_overlay/utils.pytests/network/upgrade/conftest.pytests/network/jumbo_frame/test_bond.pytests/network/general/test_cnv_tuning_regression.pytests/infrastructure/sap/test_sap_hana_vm.pytests/virt/node/migration_and_maintenance/utils.pytests/network/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/network/nmstate/conftest.pytests/conftest.pytests/network/localnet/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/bond/test_bond_modes.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/l2_bridge/test_ovs_bridge.pytests/network/kubemacpool/conftest.pytests/network/utils.pytests/network/general/test_bridge_marker.pytests/network/bgp/conftest.pytests/network/kubemacpool/test_kubemacpool.pytests/network/migration/test_migration.pytests/network/service_mesh/conftest.py
📚 Learning: 2025-08-06T13:57:34.740Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.
Applied to files:
tests/network/upgrade/conftest.pytests/network/jumbo_frame/test_bond.pytests/network/general/test_cnv_tuning_regression.pytests/infrastructure/sap/test_sap_hana_vm.pytests/virt/node/migration_and_maintenance/utils.pytests/network/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/network/nmstate/conftest.pytests/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/bond/test_bond_modes.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/l2_bridge/test_ovs_bridge.pytests/network/kubemacpool/conftest.pytests/network/utils.pytests/network/general/test_bridge_marker.pytests/network/kubemacpool/test_kubemacpool.pytests/network/migration/test_migration.pytests/network/service_mesh/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/upgrade/conftest.pytests/network/jumbo_frame/test_bond.pytests/network/general/test_cnv_tuning_regression.pytests/network/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/network/bond/test_l2_bridge_over_bond.pytests/network/nmstate/conftest.pytests/conftest.pytests/network/localnet/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/bond/test_bond_modes.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/kubemacpool/conftest.pytests/network/general/test_bridge_marker.pytests/network/bgp/conftest.pytests/network/migration/test_migration.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/upgrade/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/infrastructure/sap/test_sap_hana_vm.pytests/network/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/network/nmstate/conftest.pytests/conftest.pytests/network/localnet/conftest.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/kubemacpool/conftest.pytests/network/general/test_bridge_marker.pytests/network/bgp/conftest.pytests/network/kubemacpool/test_kubemacpool.pytests/network/service_mesh/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/upgrade/conftest.pytests/network/jumbo_frame/test_bond.pytests/network/general/test_cnv_tuning_regression.pytests/network/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/network/bond/test_l2_bridge_over_bond.pytests/conftest.pytests/network/localnet/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/l2_bridge/test_ovs_bridge.pytests/network/general/test_bridge_marker.pytests/network/bgp/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/libs/nodenetworkconfigurationpolicy.pytests/network/libs/cluster_user_defined_network.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/general/test_cnv_tuning_regression.pytests/network/nmstate/conftest.pytests/conftest.pytests/network/localnet/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/general/test_bridge_marker.pytests/network/bgp/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/l2_bridge/conftest.pytests/install_upgrade_operators/must_gather/conftest.pytests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/conftest.pytests/network/localnet/conftest.pytests/network/conftest.pytests/network/kubemacpool/conftest.pytests/network/bgp/conftest.pytests/network/kubemacpool/test_kubemacpool.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.
Applied to files:
tests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.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/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/conftest.pytests/network/localnet/conftest.pytests/network/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/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.pytests/conftest.pytests/network/localnet/conftest.pytests/network/jumbo_frame/conftest.pytests/network/conftest.pytests/network/bgp/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/conftest.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/conftest.pytests/network/dry_run/test_dry_run_kubemacpool.pytests/network/general/test_bridge_marker.pytests/network/migration/test_migration.pytests/network/service_mesh/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/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/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/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/kubemacpool/conftest.py
📚 Learning: 2025-06-23T19:19:31.961Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:83-97
Timestamp: 2025-06-23T19:19:31.961Z
Learning: In OpenShift Virtualization mass machine type transition tests, the kubevirt_api_lifecycle_automation_job requires cluster-admin privileges to function properly, as confirmed by the test maintainer akri3i.
Applied to files:
tests/network/kubemacpool/conftest.pytests/network/kubemacpool/test_kubemacpool.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/kubemacpool/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/utils.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/general/test_bridge_marker.py
📚 Learning: 2025-08-28T12:34:56.341Z
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.
Applied to files:
tests/network/bgp/conftest.py
📚 Learning: 2025-08-14T10:28:22.958Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 1785
File: tests/virt/cluster/common_templates/utils.py:65-66
Timestamp: 2025-08-14T10:28:22.958Z
Learning: The restart_qemu_guest_agent_service function in tests/virt/cluster/common_templates/utils.py is only called for RHEL7 VMs, with OS version checks handled by the calling test code, not within the function itself. Guest agent functionality is verified by subsequent tests in the test class.
Applied to files:
utilities/unittests/test_ssp.py
🧬 Code graph analysis (23)
tests/network/upgrade/conftest.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/network/localnet/liblocalnet.py (1)
tests/network/libs/label_selector.py (1)
LabelSelector(5-6)
tests/infrastructure/sap/test_sap_hana_vm.py (2)
tests/conftest.py (3)
admin_client(305-309)namespace(664-677)sriov_namespace(1025-1026)utilities/ssp.py (1)
create_custom_template_from_url(153-164)
tests/network/l2_bridge/conftest.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/install_upgrade_operators/must_gather/conftest.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (2)
network_nad(533-582)network_device(973-1014)
tests/virt/node/migration_and_maintenance/test_dedicated_live_migration_network.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)tests/virt/node/migration_and_maintenance/utils.py (1)
MACVLANNetworkAttachmentDefinition(20-45)
tests/network/l2_bridge/utils.py (1)
tests/network/user_defined_network/test_user_defined_network.py (1)
client(86-93)
utilities/ssp.py (2)
tests/conftest.py (1)
namespace(664-677)tests/network/user_defined_network/test_user_defined_network.py (1)
client(86-93)
tests/network/bond/test_l2_bridge_over_bond.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (1)
network_nad(533-582)
tests/network/nmstate/conftest.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/conftest.py (2)
utilities/network.py (5)
EthernetNetworkConfigurationPolicy(585-643)create_sriov_node_policy(1077-1099)MacPool(653-696)network_device(973-1014)network_nad(533-582)utilities/infra.py (1)
get_node_selector_dict(1149-1150)
tests/network/libs/cluster_user_defined_network.py (2)
tests/network/libs/label_selector.py (1)
LabelSelector(5-6)tests/network/libs/apimachinery.py (1)
dict_normalization_for_dataclass(4-6)
tests/network/localnet/conftest.py (4)
tests/conftest.py (1)
admin_client(305-309)tests/network/libs/nodenetworkconfigurationpolicy.py (2)
NodeNetworkConfigurationPolicy(106-218)wait_for_status_success(157-179)tests/network/libs/cluster_user_defined_network.py (2)
wait_for_status_success(109-114)ClusterUserDefinedNetwork(69-114)tests/network/localnet/liblocalnet.py (1)
localnet_cudn(135-183)
tests/network/dry_run/test_dry_run_kubemacpool.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (2)
network_device(973-1014)network_nad(533-582)
tests/network/jumbo_frame/conftest.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (2)
network_nad(533-582)network_device(973-1014)
tests/network/conftest.py (3)
tests/conftest.py (1)
admin_client(305-309)tests/network/user_defined_network/test_user_defined_network.py (1)
client(86-93)tests/network/libs/cluster_user_defined_network.py (1)
Network(59-66)
tests/network/l2_bridge/test_ovs_bridge.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (1)
network_nad(533-582)
utilities/network.py (1)
tests/network/user_defined_network/test_user_defined_network.py (1)
client(86-93)
tests/network/kubemacpool/conftest.py (1)
tests/conftest.py (3)
admin_client(305-309)mac_pool(1093-1098)kmp_enabled_ns(1502-1505)
tests/network/general/test_bridge_marker.py (2)
utilities/network.py (2)
network_nad(533-582)network_device(973-1014)utilities/infra.py (1)
get_node_selector_dict(1149-1150)
tests/network/bgp/conftest.py (3)
tests/conftest.py (1)
admin_client(305-309)tests/network/libs/nodenetworkconfigurationpolicy.py (1)
NodeNetworkConfigurationPolicy(106-218)tests/network/libs/cluster_user_defined_network.py (1)
ClusterUserDefinedNetwork(69-114)
tests/network/migration/test_migration.py (2)
tests/conftest.py (2)
admin_client(305-309)namespace(664-677)utilities/network.py (1)
network_nad(533-582)
utilities/unittests/test_ssp.py (1)
utilities/ssp.py (1)
create_custom_template_from_url(153-164)
🪛 Ruff (0.14.7)
tests/conftest.py
1079-1079: Unused function argument: workers_utility_pods
(ARG001)
tests/network/localnet/conftest.py
111-111: Unused function argument: namespace_localnet_1
(ARG001)
tests/network/bgp/conftest.py
141-141: Unused function argument: namespace_cudn
(ARG001)
tests/network/kubemacpool/test_kubemacpool.py
131-131: Unused function argument: kmp_down
(ARG001)
tests/network/migration/test_migration.py
101-101: Unused function argument: bridge_worker_2
(ARG001)
tests/network/service_mesh/conftest.py
223-223: Unused function argument: httpbin_service_account_service_mesh
(ARG001)
utilities/unittests/test_ssp.py
355-355: Probable insecure usage of temporary file or directory: "/tmp"
(S108)
362-362: Probable insecure usage of temporary file or directory: "/tmp/custom-template"
(S108)
365-365: Probable insecure usage of temporary file or directory: "/tmp/custom-template"
(S108)
⏰ 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). (7)
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.