-
Notifications
You must be signed in to change notification settings - Fork 71
[WIP]Add vnic_info metric test for NAD reference change #5090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
b7e75dd
a383cdf
fcc50e7
0be6484
a7fb8e7
b31abcc
108e9d6
30dbdcd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import ipaddress | ||
| import logging | ||
| from collections.abc import Generator | ||
|
OhadRevah marked this conversation as resolved.
Outdated
OhadRevah marked this conversation as resolved.
Outdated
OhadRevah marked this conversation as resolved.
Outdated
|
||
| from functools import cache | ||
|
|
||
| from pytest_testconfig import py_config | ||
|
|
@@ -29,3 +30,23 @@ def ipv6_supported_cluster() -> bool: | |
|
|
||
| def _cluster_ip_family_supported(ip_family: int) -> bool: | ||
| return any(ipaddress.ip_network(ip).version == ip_family for ip in py_config.get("cluster_service_network")) | ||
|
|
||
|
|
||
| def cluster_vlan_iterator() -> Generator[int]: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Simply put, from my perspective
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is clearer than the current implementation Now we can use this fixture like this |
||
| """Yield VLAN IDs from the cluster config one at a time. | ||
|
|
||
| The underlying VLAN list is read from py_config once and cached. Each call | ||
| returns a fresh iterator so every fixture invocation starts from the beginning. | ||
| Raises ValueError when all VLANs have been consumed. | ||
| """ | ||
| vlans = _cluster_vlans() | ||
| yield from vlans | ||
| raise ValueError(f"vlans list is exhausted. Current list size is {len(vlans)} and all vlans are in use.") | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
|
|
||
|
|
||
| @cache | ||
| def _cluster_vlans() -> list[int]: | ||
| vlans = py_config["vlans"] | ||
| if not isinstance(vlans, list): | ||
| vlans = vlans.split(",") | ||
| return [int(v) for v in vlans] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ | |
| from ocp_resources.resource import Resource, ResourceEditor | ||
| from timeout_sampler import retry | ||
|
|
||
| from tests.network.libs.apimachinery import dict_normalization_for_dataclass | ||
| from libs.net.apimachinery import dict_normalization_for_dataclass | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test Execution Plan Run smoke tests: False Run gating tests: False Affected tests to run New test (primary deliverable of this PR):
Import-path-only refactors (
Real tests (cluster required)
Happy path (new observability test end-to-end): Regression (existing NAD-ref-change tests still collect after fixture relocation): |
||
| WAIT_FOR_STATUS_TIMEOUT_SEC = 120 | ||
| WAIT_FOR_STATUS_INTERVAL_SEC = 5 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import logging | ||
|
OhadRevah marked this conversation as resolved.
OhadRevah marked this conversation as resolved.
|
||
| import shlex | ||
| from collections.abc import Generator | ||
|
|
||
| import bitmath | ||
| import pytest | ||
|
|
@@ -15,7 +16,13 @@ | |
| from pytest_testconfig import py_config | ||
| from timeout_sampler import TimeoutExpiredError, TimeoutSampler | ||
|
|
||
| from libs.net import nodenetworkconfigurationpolicy as libnncp | ||
| from libs.net.netattachdef import CNIPluginBridgeConfig, NetConfig, NetworkAttachmentDefinition | ||
| from libs.vm.factory import base_vmspec, fedora_vm | ||
| from libs.vm.spec import Devices, Interface, Multus, Network | ||
| from tests.observability.metrics.constants import ( | ||
| BINDING_NAME, | ||
| BINDING_TYPE, | ||
| GUEST_LOAD_TIME_PERIODS, | ||
| KUBEVIRT_CONSOLE_ACTIVE_CONNECTIONS_BY_VMI, | ||
| KUBEVIRT_VM_CREATED_BY_POD_TOTAL, | ||
|
|
@@ -27,6 +34,7 @@ | |
| ) | ||
| from tests.observability.metrics.utils import ( | ||
| SINGLE_VM, | ||
| binding_name_and_type_from_vm_or_vmi, | ||
| create_windows11_wsl2_vm, | ||
| disk_file_system_info, | ||
| enable_swap_fedora_vm, | ||
|
|
@@ -37,7 +45,7 @@ | |
| vnic_info_from_vm_or_vmi, | ||
| ) | ||
| from tests.observability.utils import validate_metrics_value | ||
| from tests.utils import create_vms, start_stress_on_vm | ||
| from tests.utils import create_vms, start_stress_on_vm, update_nad_references | ||
| from utilities import console | ||
| from utilities.constants import ( | ||
| IPV4_STR, | ||
|
|
@@ -47,6 +55,7 @@ | |
| KUBEVIRT_VMI_MEMORY_SWAP_OUT_TRAFFIC_BYTES, | ||
| KUBEVIRT_VMI_MEMORY_UNUSED_BYTES, | ||
| KUBEVIRT_VMI_MEMORY_USABLE_BYTES, | ||
| LINUX_BRIDGE, | ||
| MIGRATION_POLICY_VM_LABEL, | ||
| ONE_CPU_CORE, | ||
| ONE_CPU_THREAD, | ||
|
|
@@ -63,6 +72,7 @@ | |
| TWO_CPU_THREADS, | ||
| U1_MEDIUM_STR, | ||
| VIRT_TEMPLATE_VALIDATOR, | ||
| WORKER_NODE_LABEL_KEY, | ||
| Images, | ||
| ) | ||
| from utilities.hco import ResourceEditorValidateHCOReconcile, enabled_aaq_in_hco | ||
|
|
@@ -661,3 +671,124 @@ def expected_cpu_affinity_metric_value(admin_client, vm_with_cpu_spec): | |
|
|
||
| # return multiplication for multi-CPU VMs | ||
| return str(cpu_count_from_vm_node * cpu_count_from_vm) | ||
|
|
||
|
|
||
| _NAD_SWAP_BRIDGE_NAME = "brnadswap" | ||
| _NAD_SWAP_SECONDARY_IFACE = "secondary" | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def nad_swap_bridge_nncp( | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| nmstate_dependent_placeholder, | ||
| admin_client, | ||
| hosts_common_available_ports, | ||
| ) -> Generator[libnncp.NodeNetworkConfigurationPolicy]: | ||
| with libnncp.NodeNetworkConfigurationPolicy( | ||
| client=admin_client, | ||
| name="nad-swap-vnic-info-nncp", | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| desired_state=libnncp.DesiredState( | ||
| interfaces=[ | ||
| libnncp.Interface( | ||
| name=_NAD_SWAP_BRIDGE_NAME, | ||
| type=LINUX_BRIDGE, | ||
| state=libnncp.Resource.Interface.State.UP, | ||
| bridge=libnncp.Bridge( | ||
| port=[libnncp.Port(name=hosts_common_available_ports[-1])], | ||
| options=libnncp.BridgeOptions(libnncp.STP(enabled=False)), | ||
| ), | ||
| ) | ||
| ] | ||
| ), | ||
| node_selector={WORKER_NODE_LABEL_KEY: ""}, | ||
| ) as nncp: | ||
| nncp.wait_for_status_success() | ||
| yield nncp | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def nad_a_for_vnic_info( | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| admin_client, | ||
| namespace, | ||
| nad_swap_bridge_nncp, | ||
| next_vlan_index_number, | ||
| ) -> Generator[NetworkAttachmentDefinition]: | ||
| with NetworkAttachmentDefinition( | ||
| name=f"{_NAD_SWAP_BRIDGE_NAME}-nad-a", | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| namespace=namespace.name, | ||
| config=NetConfig( | ||
| name=f"{_NAD_SWAP_BRIDGE_NAME}-nad-a", | ||
| plugins=[CNIPluginBridgeConfig(bridge=_NAD_SWAP_BRIDGE_NAME, vlan=next(next_vlan_index_number))], | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| ), | ||
| client=admin_client, | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| ) as nad: | ||
| yield nad | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def nad_b_for_vnic_info( | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| admin_client, | ||
| namespace, | ||
| nad_swap_bridge_nncp, | ||
| next_vlan_index_number, | ||
| ): | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| with NetworkAttachmentDefinition( | ||
| name=f"{_NAD_SWAP_BRIDGE_NAME}-nad-b", | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| namespace=namespace.name, | ||
| config=NetConfig( | ||
| name=f"{_NAD_SWAP_BRIDGE_NAME}-nad-b", | ||
| plugins=[CNIPluginBridgeConfig(bridge=_NAD_SWAP_BRIDGE_NAME, vlan=next(next_vlan_index_number))], | ||
| ), | ||
| client=admin_client, | ||
|
OhadRevah marked this conversation as resolved.
Outdated
|
||
| ) as nad: | ||
| yield nad | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def vm_for_nad_swap_test( | ||
| unprivileged_client, | ||
| namespace, | ||
| nad_a_for_vnic_info, | ||
| ): | ||
|
OhadRevah marked this conversation as resolved.
|
||
| vm_name = "vm-nad-swap-vnic-info" | ||
| spec = base_vmspec() | ||
|
OhadRevah marked this conversation as resolved.
|
||
| spec.template.spec.domain.devices = Devices( | ||
| interfaces=[ | ||
| Interface(name="default", masquerade={}), | ||
| Interface(name=_NAD_SWAP_SECONDARY_IFACE, bridge={}), | ||
| ] | ||
| ) | ||
| spec.template.spec.networks = [ | ||
| Network(name="default", pod={}), | ||
| Network(name=_NAD_SWAP_SECONDARY_IFACE, multus=Multus(networkName=nad_a_for_vnic_info.name)), | ||
| ] | ||
| with fedora_vm(namespace=namespace.name, name=vm_name, client=unprivileged_client, spec=spec) as vm: | ||
| vm.start(wait=True) | ||
| yield vm | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def post_nad_swap_vm( | ||
| vm_for_nad_swap_test, | ||
| nad_b_for_vnic_info, | ||
| ): | ||
| update_nad_references( | ||
| vm=vm_for_nad_swap_test, | ||
| nad_name_by_net={_NAD_SWAP_SECONDARY_IFACE: nad_b_for_vnic_info.name}, | ||
| ) | ||
| yield vm_for_nad_swap_test | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") | ||
| def expected_vnic_info_after_swap( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this is a fixture and not just helper? This is just a helper function in reality. Called directly in the test after the swap. WDYT?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is exactly what happen here in the test, this fixture depends on post_nad_swap_vm and nad_b_for_vnic_info, so it clearer in the code, then I just use the dict for checking the values, I don't think we need an helper for it. |
||
| post_nad_swap_vm, | ||
| nad_b_for_vnic_info, | ||
| ): | ||
| vm_interfaces = post_nad_swap_vm.instance.spec.template.spec.domain.devices.interfaces | ||
| secondary_interface = next(iface for iface in vm_interfaces if iface["name"] == _NAD_SWAP_SECONDARY_IFACE) | ||
| binding_info = binding_name_and_type_from_vm_or_vmi(vm_interface=secondary_interface) | ||
| return { | ||
| "vnic_name": _NAD_SWAP_SECONDARY_IFACE, | ||
| BINDING_NAME: binding_info[BINDING_NAME], | ||
| BINDING_TYPE: binding_info[BINDING_TYPE], | ||
| "network": nad_b_for_vnic_info.name, | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.