Skip to content
File renamed without changes.
21 changes: 21 additions & 0 deletions libs/net/cluster.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ipaddress
import logging
from collections.abc import Generator
Comment thread
OhadRevah marked this conversation as resolved.
Outdated
Comment thread
OhadRevah marked this conversation as resolved.
Outdated
Comment thread
OhadRevah marked this conversation as resolved.
Outdated
Comment thread
OhadRevah marked this conversation as resolved.
Outdated
from functools import cache

from pytest_testconfig import py_config
Expand Down Expand Up @@ -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]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cluster_vlan_iterator is a one-liner yield from wrapper whose only addition is a ValueError after exhaustion which breaks the iterator protocol. StopIteration is raised automatically when a generator returns; the traceback already points to the exact next() call site, so the custom message adds nothing. The fixture could just do return iter(cluster_vlans()) and get correct behavior with no wrapper function needed.

Simply put, from my perspective _cluster_vlans() should be renamed to cluster_vlans() or just vlan_ids() (because it resides under cluster module already) and then the fixture should be (without next_ prefix: callers do next(next_...) — redundant and confusing):

@pytest.fixture(scope="module")
def cluster_vlan_ids():
    return iter(vlan_ids())

@azhivovk

@azhivovk azhivovk Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clearer than the current implementation
Done, thanks

Now we can use this fixture like this next(cluster_vlan_ids)

"""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.")
Comment thread
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
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

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/, and tests/infrastructure/. None of those files or their conftest hierarchy import cluster_vlan_iterator, next_vlan_index_number, wait_for_vmi_condition_status, wait_for_no_vmi_condition, or update_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_references callers (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_swap

Import-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.py promotes bridge_nncp (package-scope), bridge_nad_a, bridge_nad_b, vlan_index_number, and vlans_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 -s
Expected: VM starts attached to bridge_nad_a; NAD swap triggers live migration; kubevirt_vm_vnic_info / kubevirt_vmi_vnic_info labels update to reflect bridge_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_number resolves from tests/conftest.py (moved from tests/network/conftest.py).

WAIT_FOR_STATUS_TIMEOUT_SEC = 120
WAIT_FOR_STATUS_INTERVAL_SEC = 5
Expand Down
25 changes: 23 additions & 2 deletions libs/net/vmspec.py
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -299,3 +300,23 @@ 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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All defensive checks are redundant - you can use the helper as is
We create the NADs which we use and if there's a problem with resource creation we'll get an API error

"""Update secondary network 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.
"""
resource_version = vm.vmi.instance.metadata.resourceVersion
networks = deepcopy(vm.template_spec.networks) or []
for network in networks:
if network.name in nad_name_by_net and network.multus:
network.multus.networkName = nad_name_by_net[network.name]
vm.set_networks(networks=networks)
wait_for_vmi_condition_status(vm=vm, condition="MigrationRequired", resource_version=resource_version)
Comment thread
OhadRevah marked this conversation as resolved.
Outdated
wait_for_no_vmi_condition(vm=vm, condition="MigrationRequired")
7 changes: 6 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
from timeout_sampler import TimeoutSampler

import utilities.hco
from libs.net.cluster import ipv4_supported_cluster, ipv6_supported_cluster
from libs.net.cluster import cluster_vlan_iterator, ipv4_supported_cluster, ipv6_supported_cluster
from libs.net.ip import filter_link_local_addresses, random_ipv4_address, random_ipv6_address
from libs.net.vmspec import lookup_iface_status
from tests.utils import download_and_extract_tar
Expand Down Expand Up @@ -2734,3 +2734,8 @@ def hugepages_gib_values(workers):
for worker in workers
if (value := worker.instance.status.allocatable.get(NODE_HUGE_PAGES_1GI_KEY))
]


@pytest.fixture(scope="module")
def next_vlan_index_number():
return cluster_vlan_iterator()
2 changes: 1 addition & 1 deletion tests/network/bgp/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
from ocp_resources.node import Node

from libs.net import netattachdef as libnad
from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import random_ipv4_address
from libs.net.traffic_generator import PodTcpClient as TcpClient
from libs.net.traffic_generator import TcpServer
from libs.net.udn import UDN_BINDING_DEFAULT_PLUGIN_NAME, create_udn_namespace
from libs.net.vmspec import lookup_iface_status_ip, lookup_primary_network
from libs.vm.vm import BaseVirtualMachine
from tests.network.libs import cluster_user_defined_network as libcudn
from tests.network.libs import nodenetworkconfigurationpolicy as libnncp
from tests.network.libs.bgp import (
EXTERNAL_FRR_POD_LABEL,
NET_TOOLS_CONTAINER_NAME,
Expand Down
2 changes: 1 addition & 1 deletion tests/network/l2_bridge/bandwidth/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.namespace import Namespace

import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import random_ip_addresses_by_family
from libs.net.netattachdef import (
CNIPluginBandwidthConfig,
Expand Down
2 changes: 1 addition & 1 deletion tests/network/l2_bridge/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from kubernetes.dynamic import DynamicClient
from pyhelper_utils.shell import run_ssh_commands

import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import random_ipv4_address
from libs.net.netattachdef import CNIPluginBridgeConfig, NetConfig, NetworkAttachmentDefinition
from tests.network.l2_bridge.libl2bridge import DHCP_INTERFACE_NAME, bridge_attached_vm
Expand Down
2 changes: 1 addition & 1 deletion tests/network/l2_bridge/migration_stuntime/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.namespace import Namespace

import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
from libs.net import netattachdef as libnad
from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import random_ipv4_address, random_ipv6_address
from libs.net.vmspec import lookup_iface_status_ip
from libs.vm.affinity import new_pod_affinity
Expand Down
2 changes: 1 addition & 1 deletion tests/network/l2_bridge/nad_ref_change/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.namespace import Namespace

from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import filter_link_local_addresses, random_cidr_addresses_by_family
from libs.net.netattachdef import CNIPluginBridgeConfig, NetConfig, NetworkAttachmentDefinition
from libs.net.vmspec import lookup_iface_status, wait_for_ifaces_status
Expand All @@ -15,7 +16,6 @@
NET_SEED,
two_secondary_bridge_vm,
)
from tests.network.libs import nodenetworkconfigurationpolicy as libnncp
from tests.network.libs.connectivity import ARP_ISOLATION_SYSCTL_CMD, poll_tcp_connectivity


Expand Down
22 changes: 0 additions & 22 deletions tests/network/l2_bridge/nad_ref_change/lib_helpers.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
from copy import deepcopy
from typing import Final

from kubernetes.dynamic import DynamicClient

from libs.net.vmspec import wait_for_no_vmi_condition, wait_for_vmi_condition_status
from libs.vm.factory import base_vmspec, fedora_vm
from libs.vm.spec import (
CloudInitNoCloud,
Expand Down Expand Up @@ -75,26 +73,6 @@ def assert_no_connectivity(
)


def update_nad_references(vm: BaseVirtualMachine, nad_name_by_net: dict[str, str]) -> None:
"""Update secondary network 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.
"""
resource_version = vm.vmi.instance.metadata.resourceVersion
networks = deepcopy(vm.template_spec.networks) or []
for network in networks:
if network.name in nad_name_by_net and network.multus:
network.multus.networkName = nad_name_by_net[network.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")


def two_secondary_bridge_vm(
namespace: str,
name: str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,13 @@
import pytest

from libs.net.ip import filter_link_local_addresses
from libs.net.vmspec import lookup_iface_status
from libs.net.vmspec import lookup_iface_status, update_nad_references
from tests.network.l2_bridge.libl2bridge import LINUX_BRIDGE_IFACE_NAME_1, LINUX_BRIDGE_IFACE_NAME_2
from tests.network.l2_bridge.nad_ref_change.lib_helpers import (
GUEST_IFACE_1,
GUEST_IFACE_2,
assert_connectivity,
assert_no_connectivity,
update_nad_references,
)


Expand Down
2 changes: 1 addition & 1 deletion tests/network/libs/cloudinit.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

import yaml

from libs.net.apimachinery import dict_normalization_for_dataclass
from libs.net.cluster import ipv4_supported_cluster, ipv6_supported_cluster
from tests.network.libs.apimachinery import dict_normalization_for_dataclass

NETWORK_DATA: Final[str] = "networkData"

Expand Down
2 changes: 1 addition & 1 deletion tests/network/libs/cluster_user_defined_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.cluster_user_defined_network import ClusterUserDefinedNetwork as Cudn

from tests.network.libs.apimachinery import dict_normalization_for_dataclass
from libs.net.apimachinery import dict_normalization_for_dataclass
from tests.network.libs.label_selector import LabelSelector


Expand Down
2 changes: 1 addition & 1 deletion tests/network/localnet/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.namespace import Namespace

import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
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.traffic_generator import TcpServer, VMTcpClient, active_tcp_connections
Expand Down
2 changes: 1 addition & 1 deletion tests/network/localnet/ipam/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
from kubernetes.dynamic import DynamicClient
from ocp_resources.namespace import Namespace

import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
from libs.net import netattachdef as libnad
from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.ip import random_ipv4_address
from libs.vm.oper import run_vms
from libs.vm.spec import Interface, Multus, Network
Expand Down
2 changes: 1 addition & 1 deletion tests/network/localnet/liblocalnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@

from kubernetes.dynamic import DynamicClient

from libs.net import nodenetworkconfigurationpolicy as libnncp
from libs.net.cluster import ipv4_supported_cluster, ipv6_supported_cluster
from libs.vm.affinity import new_pod_anti_affinity
from libs.vm.factory import base_vmspec, fedora_vm
from libs.vm.spec import Affinity, CloudInitNoCloud, Devices, Interface, Metadata, Network
from libs.vm.vm import BaseVirtualMachine, add_volume_disk, cloudinitdisk_storage
from tests.network.libs import cloudinit
from tests.network.libs import cluster_user_defined_network as libcudn
from tests.network.libs import nodenetworkconfigurationpolicy as libnncp
from tests.network.libs.label_selector import LabelSelector
from utilities.constants import OVS_BRIDGE, WORKER_NODE_LABEL_KEY

Expand Down
Loading