infra/tests, net, nncp: require explicit client for NNCP #2936
Conversation
WalkthroughThreads a DynamicClient ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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-3
requested review from
OhadRevah,
SamAlber,
SiboWang1997,
albarker-rh,
duyanyan,
jerry7z,
rnetser and
yossisegev
openshift-virtualization-qe-bot-2
requested review from
EdDev,
hmeir and
rlobillo
openshift-virtualization-qe-bot-2
requested review from
dalia-frank,
kbidarkar and
vsibirsk
|
/build-and-push-container |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Change: rebase on main, resolve a conflict in utilities/network.py |
![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
🧹 Nitpick comments (2)
tests/conftest.py (1)
1073-1088: SR-IOV node policy fixture correctly injects admin_client (minor Ruff hint)
sriov_node_policynow threadsadmin_clientthrough tocreate_sriov_node_policy(client=admin_client), which matches the new SR‑IOV/NNCP client contract. Theworkers_utility_podsparameter is intentionally unused to force that fixture to run first; if Ruff’s ARG001 warning is noisy, consider renaming it to_workers_utility_podsto document the intent and silence the linter.utilities/network.py (1)
61-121: NNCP helpers now consistently require and propagate client
BridgeNodeNetworkConfigurationPolicy, its Linux/OVS subclasses,VLANInterfaceNodeNetworkConfigurationPolicy,BondNodeNetworkConfigurationPolicy,EthernetNetworkConfigurationPolicy,network_device, andcreate_sriov_node_policyall now take an explicitclientand forward it down toNodeNetworkConfigurationPolicy/SriovNetworkNodePolicy, which cleanly removes reliance on implicit clients for NNCP creation. The wiring looks consistent across all call paths. If you later want to fully eliminate implicit clients, you could also threadclientinto the fewNodeNetworkStateconstructions here, but that’s strictly a follow‑up polish.Also applies to: 177-260, 338-375, 387-419, 585-627, 972-1015, 1077-1099
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
tests/conftest.py(4 hunks)tests/install_upgrade_operators/must_gather/conftest.py(1 hunks)tests/network/bgp/conftest.py(1 hunks)tests/network/bond/test_bond_modes.py(9 hunks)tests/network/bond/test_l2_bridge_over_bond.py(7 hunks)tests/network/connectivity/conftest.py(5 hunks)tests/network/dry_run/test_dry_run_kubemacpool.py(1 hunks)tests/network/general/test_bridge_marker.py(1 hunks)tests/network/general/test_cnv_tuning_regression.py(1 hunks)tests/network/jumbo_frame/conftest.py(1 hunks)tests/network/jumbo_frame/test_bond.py(7 hunks)tests/network/jumbo_frame/test_bridge.py(3 hunks)tests/network/kubemacpool/conftest.py(3 hunks)tests/network/l2_bridge/conftest.py(3 hunks)tests/network/l2_bridge/test_bridge_nic_hot_plug.py(2 hunks)tests/network/l2_bridge/utils.py(2 hunks)tests/network/libs/nodenetworkconfigurationpolicy.py(3 hunks)tests/network/localnet/conftest.py(3 hunks)tests/network/localnet/liblocalnet.py(3 hunks)tests/network/macspoof/conftest.py(1 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(1 hunks)tests/storage/cdi_import/conftest.py(1 hunks)tests/virt/cluster/common_templates/windows/test_windows_custom_options.py(1 hunks)utilities/network.py(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/install_upgrade_operators/must_gather/conftest.py
- tests/virt/cluster/common_templates/windows/test_windows_custom_options.py
- tests/network/nmstate/test_connectivity_after_nmstate_changes.py
- tests/network/dry_run/test_dry_run_kubemacpool.py
- tests/network/l2_bridge/utils.py
- tests/network/l2_bridge/test_bridge_nic_hot_plug.py
- tests/network/localnet/liblocalnet.py
- tests/network/bond/test_bond_modes.py
- tests/network/nmstate/conftest.py
- tests/network/bgp/conftest.py
- tests/network/libs/nodenetworkconfigurationpolicy.py
- tests/network/jumbo_frame/test_bond.py
- tests/network/macspoof/conftest.py
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
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.
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: 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.
📚 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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/test_bridge.pytests/network/l2_bridge/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/kubemacpool/conftest.pytests/storage/cdi_import/conftest.pytests/network/general/test_bridge_marker.pytests/network/localnet/conftest.pytests/network/migration/test_migration.pytests/network/bond/test_l2_bridge_over_bond.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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/test_bridge.pytests/network/l2_bridge/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/kubemacpool/conftest.pytests/storage/cdi_import/conftest.pytests/network/general/test_bridge_marker.pytests/network/migration/test_migration.pytests/network/bond/test_l2_bridge_over_bond.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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/kubemacpool/conftest.pytests/storage/cdi_import/conftest.pytests/network/general/test_bridge_marker.pytests/network/localnet/conftest.pytests/network/migration/test_migration.pytests/network/bond/test_l2_bridge_over_bond.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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/storage/cdi_import/conftest.pytests/network/general/test_bridge_marker.pytests/network/localnet/conftest.pytests/network/bond/test_l2_bridge_over_bond.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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/kubemacpool/conftest.pytests/storage/cdi_import/conftest.pytests/network/general/test_bridge_marker.pytests/network/localnet/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/connectivity/conftest.pytests/conftest.pytests/network/jumbo_frame/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/localnet/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/conftest.pytests/network/localnet/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/conftest.pytests/network/general/test_cnv_tuning_regression.pytests/network/localnet/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-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/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/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/general/test_bridge_marker.pytests/network/localnet/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/general/test_cnv_tuning_regression.pytests/network/localnet/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/localnet/conftest.py
🧬 Code graph analysis (10)
tests/conftest.py (1)
utilities/network.py (3)
EthernetNetworkConfigurationPolicy(585-643)create_sriov_node_policy(1077-1099)network_device(973-1014)
tests/network/l2_bridge/conftest.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/network/jumbo_frame/conftest.py (2)
tests/conftest.py (1)
admin_client(305-309)utilities/network.py (1)
network_device(973-1014)
tests/network/general/test_cnv_tuning_regression.py (3)
tests/conftest.py (2)
admin_client(305-309)worker_node1(1006-1008)utilities/network.py (1)
network_device(973-1014)utilities/infra.py (1)
get_node_selector_dict(1149-1150)
tests/network/kubemacpool/conftest.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/storage/cdi_import/conftest.py (2)
tests/conftest.py (1)
admin_client(305-309)utilities/network.py (1)
network_device(973-1014)
tests/network/general/test_bridge_marker.py (3)
tests/conftest.py (3)
admin_client(305-309)worker_node1(1006-1008)worker_node2(1012-1014)utilities/network.py (1)
network_device(973-1014)utilities/infra.py (1)
get_node_selector_dict(1149-1150)
utilities/network.py (1)
tests/network/user_defined_network/test_user_defined_network.py (1)
client(86-93)
tests/network/migration/test_migration.py (1)
tests/conftest.py (1)
admin_client(305-309)
tests/network/bond/test_l2_bridge_over_bond.py (1)
tests/conftest.py (1)
admin_client(305-309)
🪛 Ruff (0.14.7)
tests/conftest.py
1078-1078: Unused function argument: workers_utility_pods
(ARG001)
⏰ 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). (14)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- 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
- 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
🔇 Additional comments (14)
tests/network/l2_bridge/conftest.py (1)
55-69: Explicit admin_client wiring for bridge NNCPs looks correctBoth
l2_bridge_device_worker_1and_worker_2now correctly takeadmin_clientfirst and passclient=admin_clientintonetwork_device, aligning NNCP creation with cluster‑admin privileges. No further issues spotted here.Also applies to: 74-88
tests/network/localnet/conftest.py (1)
43-63: Localnet NNCP/CUDN fixtures correctly enforce admin_client and jumbo MTU
nncp_localnet, the secondary‑NIC NNCP fixtures, andcudn_localnet_ovs_bridge_jumbo_frameall now takeadmin_clientexplicitly and passclient=admin_client(andmtu=cluster_hardware_mtuwhere relevant), which is consistent with the new NNCP/client contract. Looks good.Also applies to: 361-385, 387-403
tests/network/connectivity/conftest.py (1)
37-52: Bridge NNCP connectivity fixtures now correctly use admin_clientAll four
nncp_*bridge*fixtures takeadmin_clientas the first parameter and passclient=admin_clientintonetwork_device, which matches the new requirement for cluster‑wide NNCP operations. No issues found.Also applies to: 55-70, 73-88, 91-106
tests/conftest.py (3)
963-983: IPv4-disable NNCP now correctly uses admin_client
worker_nodes_ipv4_false_secondary_nicsexplicitly takesadmin_clientand passesclient=admin_clientintoEthernetNetworkConfigurationPolicy, which is appropriate for this cluster‑wide NNCP. Logic remains unchanged otherwise.
1685-1699: upgrade_bridge_on_all_nodes correctly requires admin_clientThe
upgrade_bridge_on_all_nodesfixture now requiresadmin_clientand passesclient=admin_clientintonetwork_device, ensuring the upgrade bridge NNCP is always applied with cluster‑admin privileges. Change is consistent with the rest of the PR.
1701-1711: bridge_on_one_node fixture aligned with explicit admin_client pattern
bridge_on_one_nodenow takesadmin_clientfirst and passesclient=admin_clientintonetwork_device, making this marker/upgrade bridge NNCP also explicitly admin‑scoped. No further concerns.tests/network/bond/test_l2_bridge_over_bond.py (1)
41-58: Bond/bridge NNCP fixtures now properly use admin_clientThe bond and bridge‑on‑bond fixtures consistently take
admin_clientand passclient=admin_clientintoBondNodeNetworkConfigurationPolicyandnetwork_device, so BOND and bridge NNCPs are created with the correct cluster‑admin client. The pattern is coherent across worker 1/2; no issues seen.Also applies to: 62-82, 86-105, 108-126
tests/storage/cdi_import/conftest.py (1)
49-56: CDI bridge_on_node now correctly uses admin_client
bridge_on_nodetakesadmin_clientand passesclient=admin_clientintonetwork_device, so the underlying LINUX_BRIDGE NNCP is applied with cluster‑admin privileges as required by the updated API. Looks good.tests/network/general/test_cnv_tuning_regression.py (1)
24-32: linux_bridge_device fixture correctly threads admin_client into NNCPThe
linux_bridge_devicefixture now requiresadmin_clientand passesclient=admin_clientintonetwork_device, ensuring the tuning NNCP is created with the proper cluster‑admin client. No other behavior changes introduced.tests/network/kubemacpool/conftest.py (1)
21-35: LGTM! Explicit admin_client properly threaded through fixtures.Both
kubemacpool_bridge_device_worker_1andkubemacpool_bridge_device_worker_2fixtures now correctly acceptadmin_clientas the first parameter and pass it tonetwork_devicecalls. This ensures NNCP operations use admin privileges as required for cluster-wide resources.Also applies to: 39-53
tests/network/jumbo_frame/conftest.py (1)
124-133: LGTM! Admin client correctly propagated to network device.The
linux_bridge_interfacefixture now acceptsadmin_clientas its first parameter and properly passes it through to thenetwork_devicecall, ensuring admin-level operations for NNCP creation.tests/network/migration/test_migration.py (1)
66-79: LGTM! Migration test fixtures properly updated with admin client.Both
bridge_worker_1andbridge_worker_2fixtures correctly acceptadmin_clientas the first parameter and passclient=admin_clientto their respectivenetwork_devicecalls. This ensures the bridge network configurations for migration tests use admin privileges.Also applies to: 83-97
tests/network/general/test_bridge_marker.py (1)
107-114: LGTM! Bridge marker fixtures correctly updated.Both
bridge_device_on_all_nodesandnon_homogenous_bridgesfixtures properly acceptadmin_clientas the first parameter and pass it to allnetwork_devicecalls. The nested context managers innon_homogenous_bridgescorrectly thread the client through both bridge device creations.Also applies to: 118-133
tests/network/jumbo_frame/test_bridge.py (1)
36-53: LGTM! Jumbo frame bridge fixtures properly updated with admin client.Both
jumbo_frame_bridge_device_worker_1andjumbo_frame_bridge_device_worker_2fixtures correctly acceptadmin_clientas the first parameter and passclient=admin_clientto theirnetwork_devicecalls. This ensures jumbo frame bridge configurations use admin privileges for NNCP operations.Also applies to: 57-74
|
/verified Tested on bm02-ibm All network tests: tests/virt/cluster/common_templates/windows/test_windows_custom_options.py tests/install_upgrade_operators/must_gather/test_must_gather.py tests/storage/cdi_import/test_import_http.py
|
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:
|
|
Tip For best results, initiate chat on the files or code changes. @openshift-virtualization-qe-bot-3, I'll provide a comprehensive test execution plan for this PR. Based on the signature changes to NNCP-related fixtures and the admin_client parameter addition, here's the targeted testing approach: Test Execution PlanNetwork Tests (Primary Impact Area)
Infrastructure/Core Tests
Storage Tests
Virtualization Tests
Install/Upgrade Tests
Recommended Marker-Based Execution
Verification StatusNote: User @Anatw has already successfully verified this PR with comprehensive test execution on cluster bm02-ibm (CNV 4.21.0-ec.3):
The verification covers all primary impact areas. Any additional testing should focus on edge cases or environments not covered in the initial verification. 🧠 Learnings used |
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-2936. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
9 participants
Short description:
Make NNCP-related classes take a mandatory DynamicClient and update all NNCP-related fixtures to pass an admin_client explicitly (no implicit default clients).
NNCPs are cluster-wide resources, that affect cluster nodes. For this reason project admins cannot apply them and they must be applied by cluster admin (privileged clients).
What this PR does / why we need it:
As of its next releas, openshift-python-wrapper will enforce passing
clientwhen working with cluster resources.openshift-virtualization-tests must align with this change.
All calls in the code to openshift-python-wrapper resources should be updated to pass
clientarg.jira-ticket:
https://issues.redhat.com/browse/CNV-72392
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.