net, traffic_generator: improve naming + retry decorator using (RedHatQE#2389)

Client\Server looks quite common and such naming does not reflect
the real intention. The usage traffic_generator.Client does not
solve the problem as well. It's crucial to explicitly point out that
we work with Tcp traffic and the classes implement TCP client and
TCP server in reality. And this clarity is significant for tests also.

The commit also includes replacing TimeoutSampler with retry decorator,
that leads a little refactoring for classes to align the logic.
Retry-enabled `_ensure_is_running()` method is added to `__enter__()`.
PodTcpClient already uses a similar implementation. We need to make
sure once when entering into the context manager that the client\server
process is running, then it is enough to use the atomic method
`is_running()` without retry to determine the status of the process.

Signed-off-by: Sergei Volkov <sevolkov@redhat.com>

Author: servolkov

libs/net/traffic_generator.py

@@ -4,7 +4,7 @@
 
 from ocp_resources.pod import Pod
 from ocp_utilities.exceptions import CommandExecFailed
-from timeout_sampler import TimeoutExpiredError, TimeoutSampler, retry
+from timeout_sampler import retry
 
 from libs.vm.vm import BaseVirtualMachine
 
@@ -15,7 +15,7 @@
 LOGGER = logging.getLogger(__name__)
 
 
-class BaseClient(ABC):
+class BaseTcpClient(ABC):
     """Base abstract class for network traffic generator client."""
 
     def __init__(self, server_ip: str, server_port: int):
@@ -24,7 +24,7 @@ def __init__(self, server_ip: str, server_port: int):
         self._cmd = f"{_IPERF_BIN} --client {self._server_ip} --time 0 --port {self.server_port} --connect-timeout 300"
 
     @abstractmethod
-    def __enter__(self) -> "BaseClient":
+    def __enter__(self) -> "BaseTcpClient":
         pass
 
     @abstractmethod
@@ -36,7 +36,7 @@ def is_running(self) -> bool:
         pass
 
 
-class Server:
+class TcpServer:
     """
     Represents a server running on a virtual machine for testing network performance.
     Implemented with iperf3
@@ -55,11 +55,13 @@ def __init__(
         self._port = port
         self._cmd = f"{_IPERF_BIN} --server --port {self._port} --one-off"
 
-    def __enter__(self) -> "Server":
+    def __enter__(self) -> "TcpServer":
         self._vm.console(
             commands=[f"{self._cmd} &"],
             timeout=_DEFAULT_CMD_TIMEOUT_SEC,
         )
+        self._ensure_is_running()
+
         return self
 
     def __exit__(self, exc_type: BaseException, exc_value: BaseException, traceback: object) -> None:
@@ -72,10 +74,13 @@ def vm(self) -> BaseVirtualMachine:
     def is_running(self) -> bool:
         return _is_process_running(vm=self._vm, cmd=self._cmd)
 
+    @retry(wait_timeout=30, sleep=2, exceptions_dict={})
+    def _ensure_is_running(self) -> bool:
+        return self.is_running()
+
 
-class Client(BaseClient):
-    """
-    Represents a client that connects to a server to test network performance.
+class VMTcpClient(BaseTcpClient):
+    """Represents a TCP client that connects to a server to test network performance.
     Implemented with iperf3
 
     Args:
@@ -93,11 +98,13 @@ def __init__(
         super().__init__(server_ip=server_ip, server_port=server_port)
         self._vm = vm
 
-    def __enter__(self) -> "Client":
+    def __enter__(self) -> "VMTcpClient":
         self._vm.console(
             commands=[f"{self._cmd} &"],
             timeout=_DEFAULT_CMD_TIMEOUT_SEC,
         )
+        self._ensure_is_running()
+
         return self
 
     def __exit__(self, exc_type: BaseException, exc_value: BaseException, traceback: object) -> None:
@@ -110,6 +117,10 @@ def vm(self) -> BaseVirtualMachine:
     def is_running(self) -> bool:
         return _is_process_running(vm=self._vm, cmd=self._cmd)
 
+    @retry(wait_timeout=30, sleep=2, exceptions_dict={})
+    def _ensure_is_running(self) -> bool:
+        return self.is_running()
+
 
 def _stop_process(vm: BaseVirtualMachine, cmd: str) -> None:
     try:
@@ -118,25 +129,18 @@ def _stop_process(vm: BaseVirtualMachine, cmd: str) -> None:
         LOGGER.warning(str(e))
 
 
-def _is_process_running(  # type: ignore[return]
-    vm: BaseVirtualMachine, cmd: str
-) -> bool:
+def _is_process_running(vm: BaseVirtualMachine, cmd: str) -> bool:
     try:
-        for sample in TimeoutSampler(
-            wait_timeout=60,
-            sleep=5,
-            func=vm.console,
+        vm.console(
             commands=[f"pgrep -fx '{cmd}'"],
             timeout=_DEFAULT_CMD_TIMEOUT_SEC,
-        ):
-            if sample:
-                return True
-    except TimeoutExpiredError as e:
-        LOGGER.warning(f"Process is not running on VM {vm.name}. Error: {str(e.last_exp)}")
+        )
+        return True
+    except CommandExecFailed:
         return False
 
 
-class PodClient(BaseClient):
+class PodTcpClient(BaseTcpClient):
     """Represents a TCP client that connects to a server to test network performance.
 
     Expects pod to have iperf3 container.
@@ -155,7 +159,7 @@ def __init__(self, pod: Pod, server_ip: str, server_port: int, bind_interface: s
         self._container = _IPERF_BIN
         self._cmd += f" --bind {bind_interface}" if bind_interface else ""
 
-    def __enter__(self) -> "PodClient":
+    def __enter__(self) -> "PodTcpClient":
         # run the command in the background using nohup to ensure it keeps running after the exec session ends
         self._pod.execute(
             command=["sh", "-c", f"nohup {self._cmd} >/tmp/{_IPERF_BIN}.log 2>&1 &"], container=self._container
@@ -178,5 +182,5 @@ def _ensure_is_running(self) -> bool:
         return self.is_running()
 
 
-def is_tcp_connection(server: Server, client: Client | PodClient) -> bool:
+def is_tcp_connection(server: TcpServer, client: BaseTcpClient) -> bool:
     return server.is_running() and client.is_running()

tests/network/bgp/conftest.py

@@ -13,8 +13,8 @@
 from ocp_resources.pod import Pod
 
 from libs.net import netattachdef as libnad
-from libs.net.traffic_generator import PodClient as TcpClient
-from libs.net.traffic_generator import Server as TcpServer
+from libs.net.traffic_generator import PodTcpClient as TcpClient
+from libs.net.traffic_generator import TcpServer
 from libs.net.udn import create_udn_namespace
 from libs.net.vmspec import IP_ADDRESS, lookup_iface_status, lookup_primary_network
 from libs.vm.vm import BaseVirtualMachine

tests/network/localnet/conftest.py

@@ -6,7 +6,8 @@
 from ocp_resources.node import Node
 
 import tests.network.libs.nodenetworkconfigurationpolicy as libnncp
-from libs.net.traffic_generator import Client, Server
+from libs.net.traffic_generator import TcpServer
+from libs.net.traffic_generator import VMTcpClient as TcpClient
 from libs.net.vmspec import lookup_iface_status
 from libs.vm.vm import BaseVirtualMachine
 from tests.network.libs import cluster_user_defined_network as libcudn
@@ -143,14 +144,14 @@ def localnet_running_vms(
 
 
 @pytest.fixture()
-def localnet_server(localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine]) -> Generator[Server]:
+def localnet_server(localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine]) -> Generator[TcpServer]:
     with create_traffic_server(vm=localnet_running_vms[0]) as server:
         assert server.is_running()
         yield server
 
 
 @pytest.fixture()
-def localnet_client(localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine]) -> Generator[Client]:
+def localnet_client(localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine]) -> Generator[TcpClient]:
     with create_traffic_client(
         server_vm=localnet_running_vms[0],
         client_vm=localnet_running_vms[1],
@@ -295,7 +296,7 @@ def ovs_bridge_localnet_running_vms(
 @pytest.fixture()
 def localnet_ovs_bridge_server(
     ovs_bridge_localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine],
-) -> Generator[Server]:
+) -> Generator[TcpServer]:
     with create_traffic_server(vm=ovs_bridge_localnet_running_vms[0]) as server:
         assert server.is_running()
         yield server
@@ -304,7 +305,7 @@ def localnet_ovs_bridge_server(
 @pytest.fixture()
 def localnet_ovs_bridge_client(
     ovs_bridge_localnet_running_vms: tuple[BaseVirtualMachine, BaseVirtualMachine],
-) -> Generator[Client]:
+) -> Generator[TcpClient]:
     with create_traffic_client(
         server_vm=ovs_bridge_localnet_running_vms[0],
         client_vm=ovs_bridge_localnet_running_vms[1],

tests/network/localnet/liblocalnet.py

@@ -5,7 +5,8 @@
 from kubernetes.client import ApiException
 from kubernetes.dynamic import DynamicClient
 
-from libs.net.traffic_generator import Client, Server
+from libs.net.traffic_generator import TcpServer
+from libs.net.traffic_generator import VMTcpClient as TcpClient
 from libs.net.vmspec import IP_ADDRESS, add_network_interface, add_volume_disk, lookup_iface_status
 from libs.vm.affinity import new_pod_anti_affinity
 from libs.vm.factory import base_vmspec, fedora_vm
@@ -38,14 +39,14 @@ def run_vms(vms: tuple[BaseVirtualMachine, ...]) -> tuple[BaseVirtualMachine, ..
     return vms
 
 
-def create_traffic_server(vm: BaseVirtualMachine) -> Server:
-    return Server(vm=vm, port=_IPERF_SERVER_PORT)
+def create_traffic_server(vm: BaseVirtualMachine) -> TcpServer:
+    return TcpServer(vm=vm, port=_IPERF_SERVER_PORT)
 
 
 def create_traffic_client(
     server_vm: BaseVirtualMachine, client_vm: BaseVirtualMachine, spec_logical_network: str
-) -> Client:
-    return Client(
+) -> TcpClient:
+    return TcpClient(
         vm=client_vm,
         server_ip=lookup_iface_status(vm=server_vm, iface_name=spec_logical_network)[IP_ADDRESS],
         server_port=_IPERF_SERVER_PORT,
@@ -150,9 +151,9 @@ def client_server_active_connection(
     server_vm: BaseVirtualMachine,
     spec_logical_network: str,
     port: int = _IPERF_SERVER_PORT,
-) -> Generator[tuple[Client, Server], None, None]:
-    with Server(vm=server_vm, port=port) as server:
-        with Client(
+) -> Generator[tuple[TcpClient, TcpServer], None, None]:
+    with TcpServer(vm=server_vm, port=port) as server:
+        with TcpClient(
             vm=client_vm,
             server_ip=lookup_iface_status(vm=server_vm, iface_name=spec_logical_network)[IP_ADDRESS],
             server_port=port,

tests/network/user_defined_network/test_user_defined_network.py

@@ -4,7 +4,8 @@
 from ocp_resources.user_defined_network import Layer2UserDefinedNetwork
 from ocp_resources.utils.constants import TIMEOUT_1MINUTE
 
-from libs.net.traffic_generator import Client, Server, is_tcp_connection
+from libs.net.traffic_generator import TcpServer, is_tcp_connection
+from libs.net.traffic_generator import VMTcpClient as TcpClient
 from libs.net.vmspec import lookup_iface_status, lookup_primary_network
 from libs.vm import affinity
 from tests.network.libs.vm_factory import udn_vm
@@ -74,14 +75,14 @@ def vmb_udn(udn_namespace, namespaced_layer2_user_defined_network, udn_affinity_
 
 @pytest.fixture(scope="class")
 def server(vmb_udn):
-    with Server(vm=vmb_udn, port=SERVER_PORT) as server:
+    with TcpServer(vm=vmb_udn, port=SERVER_PORT) as server:
         assert server.is_running()
         yield server
 
 
 @pytest.fixture(scope="class")
 def client(vma_udn, vmb_udn):
-    with Client(
+    with TcpClient(
         vm=vma_udn,
         server_ip=lookup_iface_status(vm=vmb_udn, iface_name=lookup_primary_network(vm=vmb_udn).name)[IP_ADDRESS],
         server_port=SERVER_PORT,