net, sriov: Use existing sriov policy#3072
Conversation
WalkthroughSR-IOV fixture/helpers for creating and polling node policies were removed. Tests now fetch existing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (53)📓 Common learnings📚 Learning: 2025-10-27T15:30:06.412ZApplied to files:
📚 Learning: 2025-08-28T12:30:40.692ZApplied to files:
📚 Learning: 2025-10-08T07:16:46.347ZApplied to files:
📚 Learning: 2025-12-17T12:33:03.550ZApplied to files:
📚 Learning: 2025-09-02T11:16:59.950ZApplied to files:
📚 Learning: 2025-06-18T09:21:34.315ZApplied to files:
📚 Learning: 2025-09-02T11:18:02.529ZApplied to files:
📚 Learning: 2025-12-16T15:09:49.597ZApplied to files:
📚 Learning: 2025-09-29T20:33:51.007ZApplied to files:
📚 Learning: 2025-12-16T20:11:03.645ZApplied to files:
📚 Learning: 2025-12-30T08:14:07.812ZApplied to files:
📚 Learning: 2025-08-20T23:57:48.380ZApplied to files:
📚 Learning: 2025-09-17T14:56:18.111ZApplied to files:
📚 Learning: 2025-05-28T09:56:15.556ZApplied to files:
📚 Learning: 2025-05-28T09:58:22.310ZApplied to files:
📚 Learning: 2025-05-28T09:58:09.482ZApplied to files:
📚 Learning: 2025-05-28T09:57:09.904ZApplied to files:
📚 Learning: 2025-09-12T14:14:28.329ZApplied to files:
📚 Learning: 2025-10-31T13:05:24.570ZApplied to files:
📚 Learning: 2025-12-07T13:52:56.070ZApplied to files:
📚 Learning: 2025-09-29T19:05:24.987ZApplied to files:
📚 Learning: 2025-11-08T07:36:57.616ZApplied to files:
📚 Learning: 2025-09-29T19:05:24.987ZApplied to files:
📚 Learning: 2025-12-16T10:28:54.212ZApplied to files:
📚 Learning: 2025-12-22T15:56:00.157ZApplied to files:
📚 Learning: 2025-12-15T12:33:06.686ZApplied to files:
📚 Learning: 2025-12-22T16:27:40.244ZApplied to files:
📚 Learning: 2025-09-08T21:34:28.924ZApplied to files:
📚 Learning: 2025-06-18T09:31:06.311ZApplied to files:
📚 Learning: 2025-11-20T16:27:01.693ZApplied to files:
📚 Learning: 2025-06-13T01:08:18.579ZApplied to files:
📚 Learning: 2026-01-04T19:37:57.919ZApplied to files:
📚 Learning: 2025-11-26T16:03:07.813ZApplied to files:
📚 Learning: 2025-09-15T06:49:53.478ZApplied to files:
📚 Learning: 2025-09-28T14:43:07.181ZApplied to files:
📚 Learning: 2025-06-18T09:19:05.769ZApplied to files:
📚 Learning: 2025-09-28T14:43:07.181ZApplied to files:
📚 Learning: 2025-08-09T01:52:26.683ZApplied to files:
📚 Learning: 2025-08-04T15:27:14.175ZApplied to files:
📚 Learning: 2025-09-10T23:16:25.845ZApplied to files:
📚 Learning: 2025-08-20T23:43:28.117ZApplied to files:
📚 Learning: 2025-09-12T08:10:48.874ZApplied to files:
📚 Learning: 2025-12-16T14:00:59.076ZApplied to files:
📚 Learning: 2026-01-05T10:33:50.728ZApplied to files:
📚 Learning: 2025-11-19T17:00:58.250ZApplied to files:
📚 Learning: 2025-05-28T10:50:56.122ZApplied to files:
📚 Learning: 2025-08-27T08:34:49.132ZApplied to files:
📚 Learning: 2025-11-19T08:13:30.263ZApplied to files:
📚 Learning: 2025-08-09T01:46:48.039ZApplied to files:
📚 Learning: 2025-06-13T00:47:10.948ZApplied to files:
📚 Learning: 2025-09-07T13:16:32.011ZApplied to files:
📚 Learning: 2025-09-09T11:51:37.860ZApplied to files:
🧬 Code graph analysis (3)tests/network/conftest.py (1)
tests/virt/node/high_performance_vm/test_numa.py (1)
tests/conftest.py (1)
⏰ 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)
🔇 Additional comments (6)
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3072 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 25 25
Lines 2166 2166
=======================================
Hits 2131 2131
Misses 35 35
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:
|
|
/retest build-container |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/network/conftest.py (1)
37-42: SR-IOV sanity check only validates config truthiness, not policy existence, and success logging is tied to a nearly-impossible conditionThe
_verify_sriovlogic at line 318:
if not py_config.get("sriov_policy_name", DEFAULT_SRIOV_POLICY_NAME):whereDEFAULT_SRIOV_POLICY_NAME = "sriov-network-policy"(a non-empty string) means this condition is almost never true. The failure message at lines 319-321 is effectively dead code.- The
else:block at line 322-328 (logging "Validated SRIOV operator…") is attached only to that nearly-impossible condition, so success logging will rarely or never execute.This creates two problems:
- You don't verify that a
SriovNetworkNodePolicywith the configured/default name actually exists in the cluster—only that a config value is set.- Success logging can only fire in the rare case where
py_config.get("sriov_policy_name", DEFAULT_SRIOV_POLICY_NAME)evaluates to falsy, which contradicts the intent of the sanity check.Refactor to actually query the cluster for the policy:
- Capture the error count before checks:
errors_before = len(failure_msgs)- Derive the policy name:
sriov_policy_name = py_config.get("sriov_policy_name", DEFAULT_SRIOV_POLICY_NAME)- Use
SriovNetworkNodePolicy.get(dyn_client=admin_client, namespace=sriov_namespace)to check for the policy's existence- Log success only if
len(failure_msgs) == errors_before(no new errors added)This aligns with how
SriovNetworkNodePolicy.get()is already used elsewhere in the codebase and ensures the sanity check validates actual cluster state, not just config values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/conftest.py(3 hunks)tests/network/conftest.py(2 hunks)utilities/network.py(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 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: 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: 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.
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: 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: 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: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-07T14:51:53.484Z
Learning: In the openshift-virtualization-tests repository, the team has decided to avoid using predefined time constants (like TIMEOUT_2MIN, TIMEOUT_5SEC) and prefers using explicit numeric values for timeout parameters.
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: 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: 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: 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.
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.
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.
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: 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.
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-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:
utilities/network.pytests/conftest.pytests/network/conftest.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/conftest.pytests/network/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-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.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/conftest.pytests/network/conftest.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/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/conftest.pytests/network/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/conftest.pytests/network/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-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/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.py
🧬 Code graph analysis (2)
tests/conftest.py (1)
tests/infrastructure/sap/test_sap_hana_vm.py (1)
sriov_network_node_policy(325-339)
tests/network/conftest.py (1)
tests/conftest.py (1)
sriov_workers(1031-1033)
🔇 Additional comments (1)
utilities/network.py (1)
47-55: Centralized default SR-IOV policy name looks goodDefining
DEFAULT_SRIOV_POLICY_NAME = "sriov-network-policy"here and reusing it from tests/conftest.py and tests/network/conftest.py removes duplicated literals and keeps tests aligned on a single default name. No issues from this change itself.
|
/verified Verified locally on bm10b-tlv2 with cnv-4.21: The test |
|
/retest build-container |
|
/retest all |
|
Change: Change dyn_client -> client and small edit in SR-IOV sanity logger info |
|
/verified Verified on bm10b-tlv2 with cnv-4.21: The test After running all SR-IOV related tests, ran all relevant network tests on the same cluster - all passed and cluster is healthy |
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: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 15894 🏁 Script executed: Length of output: 1321 🏁 Script executed: Length of output: 717 🏁 Script executed: Length of output: 442 🏁 Script executed: Length of output: 307 🏁 Script executed: Length of output: 224 🏁 Script executed: Length of output: 2025 🏁 Script executed: Length of output: 6255 🏁 Script executed: Length of output: 1781 🧠 Learnings used |
Problem: Creating an SR-IOV node network policy can take a long time because it may trigger a reboot of all nodes, which can take around 17 minutes per node. Instead of creating a new policy, the SR-IOV tests can use the policy that is already applied after cluster deployment. Using the existing policy saves both time and resources. Fix: - Remove helpers used in the process of policy creation - Refactor sriov_node_policy to use the existing policy Signed-off-by: Asia Khromov <azhivovk@redhat.com>
Signed-off-by: Asia Khromov <azhivovk@redhat.com>
To get the resource name of an existing SNNP in the cluster, it should be accessed via policy.instance.spec.resourceName Signed-off-by: Asia Khromov <azhivovk@redhat.com>
|
Change: Removed |
|
/verified The test tests/virt/node/high_performance_vm/test_numa.py::test_numa_with_sriov is not maintained and does not need verification |
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: |
Test Execution PlanRun smoke tests: False SR-IOV Tests (fixture dependency on
Alternative using markers:
Verification Notes:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 3303 🏁 Script executed: Length of output: 481 🧠 Learnings used |
|
Thanks! /approve |
|
/approve |
|
Successfully removed PR tag: quay.io/openshift-cnv/openshift-virtualization-tests:pr-3072. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
Problem:
Creating an SR-IOV node network policy can take a long time because it may trigger
a reboot of all nodes, which can take around 17 minutes per node.
Instead of creating a new policy, the SR-IOV tests can use the policy that is already
applied after cluster deployment.
Using the existing policy saves both time and resources.
Fix:
sriov_node_policyto use the existing policyjira-ticket: https://issues.redhat.com/browse/CNV-20564
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.