-
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 all 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 | ||
| from collections.abc import Callable | ||
| from copy import deepcopy | ||
| from typing import Any, Final | ||
|
|
||
| from kubernetes.dynamic.client import ResourceField | ||
|
|
@@ -206,7 +207,7 @@ def wait_for_ifaces_status( | |
|
|
||
|
|
||
| def wait_for_vmi_condition_status( | ||
| vm: BaseVirtualMachine, | ||
| vm: VirtualMachine, | ||
| condition: str, | ||
| status: str = ResourceConstants.Condition.Status.TRUE, | ||
| timeout: int = 300, | ||
|
|
@@ -249,7 +250,7 @@ def wait_for_vmi_condition_status( | |
|
|
||
|
|
||
| def wait_for_no_vmi_condition( | ||
| vm: BaseVirtualMachine, | ||
| vm: VirtualMachine, | ||
| condition: str, | ||
| timeout: int = 300, | ||
| resource_version: str | None = None, | ||
|
|
@@ -299,3 +300,35 @@ def _vmi_condition_not_set(existing_conditions: list[ResourceField], required_co | |
| for cond in existing_conditions | ||
| if cond.type == required_condition | ||
| ) | ||
|
|
||
|
|
||
| def update_nad_references(vm: BaseVirtualMachine, nad_name_by_net: dict[str, str]) -> None: | ||
|
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. All defensive checks are redundant - you can use the helper as is |
||
| """Update secondary NAD references and wait for the change to be fully applied. | ||
|
|
||
| Patches the VM spec atomically, then waits for the MigrationRequired condition to | ||
| appear (change detected) and disappear (migration completed). | ||
|
|
||
| Args: | ||
| vm: The virtual machine to update. | ||
| nad_name_by_net: Mapping of interface name to new NAD name. | ||
| """ | ||
| if not nad_name_by_net: | ||
| raise ValueError(f"NAD update mapping is empty for VM {vm.name}") | ||
| resource_version = vm.vmi.instance.metadata.resourceVersion | ||
| networks = vm.template_spec.networks | ||
| if not networks: | ||
| raise ValueError(f"VM {vm.name} has no template networks to update") | ||
| networks = deepcopy(networks) | ||
| updated_names = set() | ||
|
OhadRevah marked this conversation as resolved.
|
||
| for network in networks: | ||
| if network.name in nad_name_by_net: | ||
| if not network.multus: | ||
| raise ValueError(f"Network {network.name!r} on VM {vm.name} is not a Multus network") | ||
| network.multus.networkName = nad_name_by_net[network.name] | ||
| updated_names.add(network.name) | ||
| missing = set(nad_name_by_net) - updated_names | ||
| if missing: | ||
| raise ValueError(f"NAD update requested for unknown networks {sorted(missing)} on VM {vm.name}") | ||
| vm.set_networks(networks=networks) | ||
| wait_for_vmi_condition_status(vm=vm, condition="MigrationRequired", resource_version=resource_version) | ||
| wait_for_no_vmi_condition(vm=vm, condition="MigrationRequired") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import tempfile | ||
|
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 conftest is messy and carries many unrelated fixtures |
||
| from bisect import bisect_left | ||
| from collections import defaultdict | ||
| from collections.abc import Generator | ||
| from datetime import UTC, datetime | ||
| from signal import SIGINT, SIGTERM, getsignal, signal | ||
|
|
||
|
|
@@ -23,6 +24,7 @@ | |
| import requests | ||
| import yaml | ||
| from bs4 import BeautifulSoup | ||
| from kubernetes.dynamic import DynamicClient | ||
| from kubernetes.dynamic.exceptions import ResourceNotFoundError | ||
| from ocp_resources.application_aware_resource_quota import ApplicationAwareResourceQuota | ||
| from ocp_resources.catalog_source import CatalogSource | ||
|
|
@@ -69,10 +71,12 @@ | |
| from timeout_sampler import TimeoutSampler | ||
|
|
||
| import utilities.hco | ||
| from libs.net import nodenetworkconfigurationpolicy as libnncp | ||
| from libs.net.cluster import ipv4_supported_cluster, ipv6_supported_cluster | ||
| from libs.net.ip import filter_link_local_addresses, random_ipv4_address, random_ipv6_address | ||
| from libs.net.netattachdef import CNIPluginBridgeConfig, NetConfig, NetworkAttachmentDefinition | ||
| from libs.net.vmspec import lookup_iface_status | ||
| from tests.utils import download_and_extract_tar | ||
| from tests.utils import download_and_extract_tar, get_vlan_index_number | ||
| from utilities.artifactory import get_artifactory_header, get_http_image_url, get_test_artifact_server_url | ||
| from utilities.bitwarden import get_cnv_tests_secret_by_name | ||
| from utilities.cluster import cache_admin_client, get_oc_whoami_username | ||
|
|
@@ -2739,3 +2743,84 @@ def hugepages_gib_values(workers): | |
| for worker in workers | ||
| if (value := worker.instance.status.allocatable.get(NODE_HUGE_PAGES_1GI_KEY)) | ||
| ] | ||
|
|
||
|
|
||
| @pytest.fixture(scope="package") | ||
| def bridge_nncp( | ||
| nmstate_dependent_placeholder: None, | ||
| admin_client: DynamicClient, | ||
| hosts_common_available_ports: list[str], | ||
| ) -> Generator[libnncp.NodeNetworkConfigurationPolicy]: | ||
| if not hosts_common_available_ports: | ||
| raise ValueError("No common worker NICs available for bridge_nncp fixture") | ||
| with libnncp.NodeNetworkConfigurationPolicy( | ||
| client=admin_client, | ||
| name="l2-bridge-test-nncp", | ||
| desired_state=libnncp.DesiredState( | ||
| interfaces=[ | ||
| libnncp.Interface( | ||
| name="br1-test", | ||
| type=LINUX_BRIDGE, | ||
| state=libnncp.Resource.Interface.State.UP, | ||
| bridge=libnncp.Bridge( | ||
| port=[libnncp.Port(name=hosts_common_available_ports[-1])], | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| options=libnncp.BridgeOptions(libnncp.STP(enabled=False)), | ||
| ), | ||
| ) | ||
| ] | ||
| ), | ||
| node_selector={WORKER_NODE_LABEL_KEY: ""}, | ||
| ) as nncp_br: | ||
| nncp_br.wait_for_status_success() | ||
| yield nncp_br | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def bridge_nad_a( | ||
| admin_client: DynamicClient, | ||
| namespace: Namespace, | ||
| bridge_nncp: libnncp.NodeNetworkConfigurationPolicy, | ||
| vlan_index_number: Generator[int], | ||
| ) -> Generator[NetworkAttachmentDefinition]: | ||
| bridge = bridge_nncp.desired_state_spec.interfaces[0].name # type: ignore | ||
|
OhadRevah marked this conversation as resolved.
|
||
| with NetworkAttachmentDefinition( | ||
| name="nad-vlan-a", | ||
| namespace=namespace.name, | ||
| config=NetConfig( | ||
| name="nad-vlan-a", plugins=[CNIPluginBridgeConfig(bridge=bridge, vlan=next(vlan_index_number))] | ||
| ), | ||
| client=admin_client, | ||
| ) as nad: | ||
| yield nad | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def vlan_index_number(vlans_list): | ||
| return get_vlan_index_number(vlans_list=vlans_list) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def vlans_list(): | ||
| vlans = py_config["vlans"] | ||
| if not isinstance(vlans, list): | ||
| vlans = vlans.split(",") | ||
| return [int(_id) for _id in vlans] | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def bridge_nad_b( | ||
| admin_client: DynamicClient, | ||
| namespace: Namespace, | ||
| bridge_nncp: libnncp.NodeNetworkConfigurationPolicy, | ||
| vlan_index_number: Generator[int], | ||
| ) -> Generator[NetworkAttachmentDefinition]: | ||
| bridge = bridge_nncp.desired_state_spec.interfaces[0].name # type: ignore[union-attr, index] | ||
| with NetworkAttachmentDefinition( | ||
| name="nad-vlan-b", | ||
| namespace=namespace.name, | ||
| config=NetConfig( | ||
| name="nad-vlan-b", plugins=[CNIPluginBridgeConfig(bridge=bridge, vlan=next(vlan_index_number))] | ||
| ), | ||
| client=admin_client, | ||
| ) as nad: | ||
| yield nad | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test Execution Plan
Run smoke tests: False
Verified: smoke-marked files are in
tests/storage/,tests/virt/,tests/install_upgrade_operators/, andtests/infrastructure/. None of those files or their conftest hierarchy importcluster_vlan_iterator,next_vlan_index_number,wait_for_vmi_condition_status,wait_for_no_vmi_condition, orupdate_nad_references. No traceable dependency path.Run gating tests: False
Verified: gating-marked network test files (
tests/network/nmstate/,tests/network/connectivity/,tests/network/localnet/test_default_bridge.py, etc.) have zero imports of any modified symbol.update_nad_referencescallers (tests/network/l2_bridge/nad_ref_change/,tests/network/localnet/nad_ref_change/) carry no gating marker.Affected tests to run
New test (primary deliverable of this PR):
tests/observability/metrics/test_vms_metrics.py::TestVmVnicInfo::test_metric_kubevirt_vm_vnic_info_after_nad_swapImport-path-only refactors (
libnncp:tests.network.libs.*->libs.net.*) — collection verification only:pytest --collect-only tests/network/bgp/pytest --collect-only tests/network/l2_bridge/pytest --collect-only tests/network/localnet/Real tests (cluster required)
tests/conftest.pypromotesbridge_nncp(package-scope),bridge_nad_a,bridge_nad_b,vlan_index_number, andvlans_list(session-scope) from per-directory conftest files to the root conftest. These run at session/setup time and must be verified on a real cluster:Happy path (new observability test end-to-end):
pytest tests/observability/metrics/test_vms_metrics.py::TestVmVnicInfo::test_metric_kubevirt_vm_vnic_info_after_nad_swap -sExpected: VM starts attached to
bridge_nad_a; NAD swap triggers live migration;kubevirt_vm_vnic_info/kubevirt_vmi_vnic_infolabels update to reflectbridge_nad_b.Regression (existing NAD-ref-change tests still collect after fixture relocation):
pytest --collect-only tests/network/l2_bridge/nad_ref_change/ tests/network/localnet/nad_ref_change/Expected: session collects without import errors;
vlan_index_numberresolves fromtests/conftest.py(moved fromtests/network/conftest.py).