<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,sans-serif"> </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 15, 2023 at 8:11 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The standard Python tool for generating API documentation, Sphinx, imports modules one-by-one when generating the documentation. This requires code changes: * properly guarding argument parsing in the if __name__ == '__main__' block, * the logger used by DTS runner underwent the same treatment so that it doesn't create log files outside of a DTS run, * however, DTS uses the arguments to construct an object holding global variables. The defaults for the global variables needed to be moved from argument parsing elsewhere, * importing the remote_session module from framework resulted in circular imports because of one module trying to import another module. This is fixed by reorganizing the code, * some code reorganization was done because the resulting structure makes more sense, improving documentation clarity. The are some other changes which are documentation related: * added missing type annotation so they appear in the generated docs, * reordered arguments in some methods, * removed superfluous arguments and attributes, * change private functions/methods/attributes to private and vice-versa. The above all appear in the generated documentation and the with them, the documentation is improved. Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech> --- dts/framework/config/__init__.py | 10 ++- dts/framework/dts.py | 33 +++++-- dts/framework/exception.py | 54 +++++------- dts/framework/remote_session/__init__.py | 41 ++++----- .../interactive_remote_session.py | 0 .../{remote => }/interactive_shell.py | 0 .../{remote => }/python_shell.py | 0 .../remote_session/remote/__init__.py | 27 ------ .../{remote => }/remote_session.py | 0 .../{remote => }/ssh_session.py | 12 +-- .../{remote => }/testpmd_shell.py | 0 dts/framework/settings.py | 87 +++++++++++-------- dts/framework/test_result.py | 4 +- dts/framework/test_suite.py | 7 +- dts/framework/testbed_model/__init__.py | 12 +-- dts/framework/testbed_model/{hw => }/cpu.py | 13 +++ dts/framework/testbed_model/hw/__init__.py | 27 ------ .../linux_session.py | 6 +- dts/framework/testbed_model/node.py | 25 ++++-- .../os_session.py | 22 ++--- dts/framework/testbed_model/{hw => }/port.py | 0 .../posix_session.py | 4 +- dts/framework/testbed_model/sut_node.py | 8 +- dts/framework/testbed_model/tg_node.py | 30 +------ .../traffic_generator/__init__.py | 24 +++++ .../capturing_traffic_generator.py | 6 +- .../{ => traffic_generator}/scapy.py | 23 ++--- .../traffic_generator.py | 16 +++- .../testbed_model/{hw => }/virtual_device.py | 0 dts/framework/utils.py | 46 +++------- dts/main.py | 9 +- 31 files changed, 258 insertions(+), 288 deletions(-) rename dts/framework/remote_session/{remote => }/interactive_remote_session.py (100%) rename dts/framework/remote_session/{remote => }/interactive_shell.py (100%) rename dts/framework/remote_session/{remote => }/python_shell.py (100%) delete mode 100644 dts/framework/remote_session/remote/__init__.py rename dts/framework/remote_session/{remote => }/remote_session.py (100%) rename dts/framework/remote_session/{remote => }/ssh_session.py (91%) rename dts/framework/remote_session/{remote => }/testpmd_shell.py (100%) rename dts/framework/testbed_model/{hw => }/cpu.py (95%) delete mode 100644 dts/framework/testbed_model/hw/__init__.py rename dts/framework/{remote_session => testbed_model}/linux_session.py (97%) rename dts/framework/{remote_session => testbed_model}/os_session.py (95%) rename dts/framework/testbed_model/{hw => }/port.py (100%) rename dts/framework/{remote_session => testbed_model}/posix_session.py (98%) create mode 100644 dts/framework/testbed_model/traffic_generator/__init__.py rename dts/framework/testbed_model/{ => traffic_generator}/capturing_traffic_generator.py (96%) rename dts/framework/testbed_model/{ => traffic_generator}/scapy.py (95%) rename dts/framework/testbed_model/{ => traffic_generator}/traffic_generator.py (80%) rename dts/framework/testbed_model/{hw => }/virtual_device.py (100%) diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py index cb7e00ba34..2044c82611 100644 --- a/dts/framework/config/__init__.py +++ b/dts/framework/config/__init__.py @@ -17,6 +17,7 @@ import warlock # type: ignore[import] import yaml +from framework.exception import ConfigurationError from framework.settings import SETTINGS from framework.utils import StrEnum @@ -89,7 +90,7 @@ class TrafficGeneratorConfig: traffic_generator_type: TrafficGeneratorType @staticmethod - def from_dict(d: dict): + def from_dict(d: dict) -> "ScapyTrafficGeneratorConfig": # This looks useless now, but is designed to allow expansion to traffic # generators that require more configuration later. match TrafficGeneratorType(d["type"]): @@ -97,6 +98,10 @@ def from_dict(d: dict): return ScapyTrafficGeneratorConfig( traffic_generator_type=TrafficGeneratorType.SCAPY ) + case _: + raise ConfigurationError( + f'Unknown traffic generator type "{d["type"]}".' + ) @dataclass(slots=True, frozen=True) @@ -324,6 +329,3 @@ def load_config() -> Configuration: config: dict[str, Any] = warlock.model_factory(schema, name="_Config")(config_data) config_obj: Configuration = Configuration.from_dict(dict(config)) return config_obj - - -CONFIGURATION = load_config() diff --git a/dts/framework/dts.py b/dts/framework/dts.py index f773f0c38d..4c7fb0c40a 100644 --- a/dts/framework/dts.py +++ b/dts/framework/dts.py @@ -6,19 +6,19 @@ import sys from .config import ( - CONFIGURATION, BuildTargetConfiguration, ExecutionConfiguration, TestSuiteConfig, + load_config, ) from .exception import BlockingTestSuiteError from .logger import DTSLOG, getLogger from .test_result import BuildTargetResult, DTSResult, ExecutionResult, Result from .test_suite import get_test_suites from .testbed_model import SutNode, TGNode -from .utils import check_dts_python_version -dts_logger: DTSLOG = getLogger("DTSRunner") +# dummy defaults to satisfy linters +dts_logger: DTSLOG = None # type: ignore[assignment] result: DTSResult = DTSResult(dts_logger) @@ -30,14 +30,18 @@ def run_all() -> None: global dts_logger global result + # create a regular DTS logger and create a new result with it + dts_logger = getLogger("DTSRunner") + result = DTSResult(dts_logger) + # check the python version of the server that run dts - check_dts_python_version() + _check_dts_python_version() sut_nodes: dict[str, SutNode] = {} tg_nodes: dict[str, TGNode] = {} try: # for all Execution sections - for execution in CONFIGURATION.executions: + for execution in load_config().executions: sut_node = sut_nodes.get(<a href="http://execution.system_under_test_node.name" rel="noreferrer" target="_blank">execution.system_under_test_node.name</a>) tg_node = tg_nodes.get(<a href="http://execution.traffic_generator_node.name" rel="noreferrer" target="_blank">execution.traffic_generator_node.name</a>) @@ -82,6 +86,25 @@ def run_all() -> None: _exit_dts() +def _check_dts_python_version() -> None: + def RED(text: str) -> str: + return f"\u001B[31;1m{str(text)}\u001B[0m" + + if sys.version_info.major < 3 or ( + sys.version_info.major == 3 and sys.version_info.minor < 10 + ): + print( + RED( + ( + "WARNING: DTS execution node's python version is lower than" + "python 3.10, is deprecated and will not work in future releases." + ) + ), + file=sys.stderr, + ) + print(RED("Please use Python >= 3.10 instead"), file=sys.stderr) + + def _run_execution( sut_node: SutNode, tg_node: TGNode, diff --git a/dts/framework/exception.py b/dts/framework/exception.py index 001a5a5496..7489c03570 100644 --- a/dts/framework/exception.py +++ b/dts/framework/exception.py @@ -42,19 +42,14 @@ class SSHTimeoutError(DTSError): Command execution timeout. """ - command: str - output: str severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR + _command: str - def __init__(self, command: str, output: str): - self.command = command - self.output = output + def __init__(self, command: str): + self._command = command def __str__(self) -> str: - return f"TIMEOUT on {self.command}" - - def get_output(self) -> str: - return self.output + return f"TIMEOUT on {self._command}" class SSHConnectionError(DTSError): @@ -62,18 +57,18 @@ class SSHConnectionError(DTSError): SSH connection error. """ - host: str - errors: list[str] severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR + _host: str + _errors: list[str] def __init__(self, host: str, errors: list[str] | None = None): - self.host = host - self.errors = [] if errors is None else errors + self._host = host + self._errors = [] if errors is None else errors def __str__(self) -> str: - message = f"Error trying to connect with {self.host}." - if self.errors: - message += f" Errors encountered while retrying: {', '.join(self.errors)}" + message = f"Error trying to connect with {self._host}." + if self._errors: + message += f" Errors encountered while retrying: {', '.join(self._errors)}" return message @@ -84,14 +79,14 @@ class SSHSessionDeadError(DTSError): It can no longer be used. """ - host: str severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR + _host: str def __init__(self, host: str): - self.host = host + self._host = host def __str__(self) -> str: - return f"SSH session with {self.host} has died" + return f"SSH session with {self._host} has died" class ConfigurationError(DTSError): @@ -107,18 +102,18 @@ class RemoteCommandExecutionError(DTSError): Raised when a command executed on a Node returns a non-zero exit status. """ - command: str - command_return_code: int severity: ClassVar[ErrorSeverity] = ErrorSeverity.REMOTE_CMD_EXEC_ERR + command: str + _command_return_code: int def __init__(self, command: str, command_return_code: int): self.command = command - self.command_return_code = command_return_code + self._command_return_code = command_return_code def __str__(self) -> str: return ( f"Command {self.command} returned a non-zero exit code: " - f"{self.command_return_code}" + f"{self._command_return_code}" ) @@ -143,22 +138,15 @@ class TestCaseVerifyError(DTSError): Used in test cases to verify the expected behavior. """ - value: str severity: ClassVar[ErrorSeverity] = ErrorSeverity.TESTCASE_VERIFY_ERR - def __init__(self, value: str): - self.value = value - - def __str__(self) -> str: - return repr(self.value) - </blockquote><div> </div><div><div style="font-family:arial,sans-serif" class="gmail_default">Does this change mean we are no longer providing descriptions for what failing the verification means? I guess there isn't really harm in removing that functionality, but I'm not sure I see the value in removing the extra information either. </div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> class BlockingTestSuiteError(DTSError): - suite_name: str severity: ClassVar[ErrorSeverity] = ErrorSeverity.BLOCKING_TESTSUITE_ERR + _suite_name: str def __init__(self, suite_name: str) -> None: - self.suite_name = suite_name + self._suite_name = suite_name def __str__(self) -> str: - return f"Blocking suite {self.suite_name} failed." + return f"Blocking suite {self._suite_name} failed." diff --git a/dts/framework/remote_session/__init__.py b/dts/framework/remote_session/__init__.py index 00b6d1f03a..5e7ddb2b05 100644 --- a/dts/framework/remote_session/__init__.py +++ b/dts/framework/remote_session/__init__.py @@ -12,29 +12,24 @@ # pylama:ignore=W0611 -from framework.config import OS, NodeConfiguration -from framework.exception import ConfigurationError +from framework.config import NodeConfiguration from framework.logger import DTSLOG -from .linux_session import LinuxSession -from .os_session import InteractiveShellType, OSSession -from .remote import ( - CommandResult, - InteractiveRemoteSession, - InteractiveShell, - PythonShell, - RemoteSession, - SSHSession, - TestPmdDevice, - TestPmdShell, -) - - -def create_session( +from .interactive_remote_session import InteractiveRemoteSession +from .interactive_shell import InteractiveShell +from .python_shell import PythonShell +from .remote_session import CommandResult, RemoteSession +from .ssh_session import SSHSession +from .testpmd_shell import TestPmdShell + + +def create_remote_session( node_config: NodeConfiguration, name: str, logger: DTSLOG -) -> OSSession: - match node_config.os: - case OS.linux: - return LinuxSession(node_config, name, logger) - case _: - raise ConfigurationError(f"Unsupported OS {node_config.os}") +) -> RemoteSession: + return SSHSession(node_config, name, logger) + + +def create_interactive_session( + node_config: NodeConfiguration, logger: DTSLOG +) -> InteractiveRemoteSession: + return InteractiveRemoteSession(node_config, logger) diff --git a/dts/framework/remote_session/remote/interactive_remote_session.py b/dts/framework/remote_session/interactive_remote_session.py similarity index 100% rename from dts/framework/remote_session/remote/interactive_remote_session.py rename to dts/framework/remote_session/interactive_remote_session.py diff --git a/dts/framework/remote_session/remote/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py similarity index 100% rename from dts/framework/remote_session/remote/interactive_shell.py rename to dts/framework/remote_session/interactive_shell.py diff --git a/dts/framework/remote_session/remote/python_shell.py b/dts/framework/remote_session/python_shell.py similarity index 100% rename from dts/framework/remote_session/remote/python_shell.py rename to dts/framework/remote_session/python_shell.py diff --git a/dts/framework/remote_session/remote/__init__.py b/dts/framework/remote_session/remote/__init__.py deleted file mode 100644 index 06403691a5..0000000000 --- a/dts/framework/remote_session/remote/__init__.py +++ /dev/null @@ -1,27 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause -# Copyright(c) 2023 PANTHEON.tech s.r.o. -# Copyright(c) 2023 University of New Hampshire - -# pylama:ignore=W0611 - -from framework.config import NodeConfiguration -from framework.logger import DTSLOG - -from .interactive_remote_session import InteractiveRemoteSession -from .interactive_shell import InteractiveShell -from .python_shell import PythonShell -from .remote_session import CommandResult, RemoteSession -from .ssh_session import SSHSession -from .testpmd_shell import TestPmdDevice, TestPmdShell - - -def create_remote_session( - node_config: NodeConfiguration, name: str, logger: DTSLOG -) -> RemoteSession: - return SSHSession(node_config, name, logger) - - -def create_interactive_session( - node_config: NodeConfiguration, logger: DTSLOG -) -> InteractiveRemoteSession: - return InteractiveRemoteSession(node_config, logger) diff --git a/dts/framework/remote_session/remote/remote_session.py b/dts/framework/remote_session/remote_session.py similarity index 100% rename from dts/framework/remote_session/remote/remote_session.py rename to dts/framework/remote_session/remote_session.py diff --git a/dts/framework/remote_session/remote/ssh_session.py b/dts/framework/remote_session/ssh_session.py similarity index 91% rename from dts/framework/remote_session/remote/ssh_session.py rename to dts/framework/remote_session/ssh_session.py index 8d127f1601..cee11d14d6 100644 --- a/dts/framework/remote_session/remote/ssh_session.py +++ b/dts/framework/remote_session/ssh_session.py @@ -18,9 +18,7 @@ SSHException, ) -from framework.config import NodeConfiguration from framework.exception import SSHConnectionError, SSHSessionDeadError, SSHTimeoutError -from framework.logger import DTSLOG from .remote_session import CommandResult, RemoteSession @@ -45,14 +43,6 @@ class SSHSession(RemoteSession): session: Connection - def __init__( - self, - node_config: NodeConfiguration, - session_name: str, - logger: DTSLOG, - ): - super(SSHSession, self).__init__(node_config, session_name, logger) - def _connect(self) -> None: errors = [] retry_attempts = 10 @@ -117,7 +107,7 @@ def _send_command( except CommandTimedOut as e: self._logger.exception(e) - raise SSHTimeoutError(command, e.result.stderr) from e + raise SSHTimeoutError(command) from e return CommandResult( <a href="http://self.name" rel="noreferrer" target="_blank">self.name</a>, command, output.stdout, output.stderr, output.return_code diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py similarity index 100% rename from dts/framework/remote_session/remote/testpmd_shell.py rename to dts/framework/remote_session/testpmd_shell.py diff --git a/dts/framework/settings.py b/dts/framework/settings.py index cfa39d011b..7f5841d073 100644 --- a/dts/framework/settings.py +++ b/dts/framework/settings.py @@ -6,7 +6,7 @@ import argparse import os from collections.abc import Callable, Iterable, Sequence -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from typing import Any, TypeVar @@ -22,8 +22,8 @@ def __init__( option_strings: Sequence[str], dest: str, nargs: str | int | None = None, - const: str | None = None, - default: str = None, + const: bool | None = None, + default: Any = None, type: Callable[[str], _T | argparse.FileType | None] = None, choices: Iterable[_T] | None = None, required: bool = False, @@ -32,6 +32,12 @@ def __init__( ) -> None: env_var_value = os.environ.get(env_var) default = env_var_value or default + if const is not None: + nargs = 0 + default = const if env_var_value else default + type = None + choices = None + metavar = None super(_EnvironmentArgument, self).__init__( option_strings, dest, @@ -52,22 +58,28 @@ def __call__( values: Any, option_string: str = None, ) -> None: - setattr(namespace, self.dest, values) + if self.const is not None: + setattr(namespace, self.dest, self.const) + else: + setattr(namespace, self.dest, values) return _EnvironmentArgument -@dataclass(slots=True, frozen=True) -class _Settings: - config_file_path: str - output_dir: str - timeout: float - verbose: bool - skip_setup: bool - dpdk_tarball_path: Path - compile_timeout: float - test_cases: list - re_run: int +@dataclass(slots=True) +class Settings: + config_file_path: Path = Path(__file__).parent.parent.joinpath("conf.yaml") + output_dir: str = "output" + timeout: float = 15 + verbose: bool = False + skip_setup: bool = False + dpdk_tarball_path: Path | str = "dpdk.tar.xz" + compile_timeout: float = 1200 + test_cases: list[str] = field(default_factory=list) + re_run: int = 0 + + +SETTINGS: Settings = Settings() def _get_parser() -> argparse.ArgumentParser: @@ -81,7 +93,8 @@ def _get_parser() -> argparse.ArgumentParser: parser.add_argument( "--config-file", action=_env_arg("DTS_CFG_FILE"), - default="conf.yaml", + default=SETTINGS.config_file_path, + type=Path, help="[DTS_CFG_FILE] configuration file that describes the test cases, SUTs " "and targets.", ) @@ -90,7 +103,7 @@ def _get_parser() -> argparse.ArgumentParser: "--output-dir", "--output", action=_env_arg("DTS_OUTPUT_DIR"), - default="output", + default=SETTINGS.output_dir, help="[DTS_OUTPUT_DIR] Output directory where dts logs and results are saved.", ) @@ -98,7 +111,7 @@ def _get_parser() -> argparse.ArgumentParser: "-t", "--timeout", action=_env_arg("DTS_TIMEOUT"), - default=15, + default=SETTINGS.timeout, type=float, help="[DTS_TIMEOUT] The default timeout for all DTS operations except for " "compiling DPDK.", @@ -108,8 +121,9 @@ def _get_parser() -> argparse.ArgumentParser: "-v", "--verbose", action=_env_arg("DTS_VERBOSE"), - default="N", - help="[DTS_VERBOSE] Set to 'Y' to enable verbose output, logging all messages " + default=SETTINGS.verbose, + const=True, + help="[DTS_VERBOSE] Specify to enable verbose output, logging all messages " "to the console.", ) @@ -117,8 +131,8 @@ def _get_parser() -> argparse.ArgumentParser: "-s", "--skip-setup", action=_env_arg("DTS_SKIP_SETUP"), - default="N", - help="[DTS_SKIP_SETUP] Set to 'Y' to skip all setup steps on SUT and TG nodes.", + const=True, + help="[DTS_SKIP_SETUP] Specify to skip all setup steps on SUT and TG nodes.", ) parser.add_argument( @@ -126,7 +140,7 @@ def _get_parser() -> argparse.ArgumentParser: "--snapshot", "--git-ref", action=_env_arg("DTS_DPDK_TARBALL"), - default="dpdk.tar.xz", + default=SETTINGS.dpdk_tarball_path, type=Path, help="[DTS_DPDK_TARBALL] Path to DPDK source code tarball or a git commit ID, " "tag ID or tree ID to test. To test local changes, first commit them, " @@ -136,7 +150,7 @@ def _get_parser() -> argparse.ArgumentParser: parser.add_argument( "--compile-timeout", action=_env_arg("DTS_COMPILE_TIMEOUT"), - default=1200, + default=SETTINGS.compile_timeout, type=float, help="[DTS_COMPILE_TIMEOUT] The timeout for compiling DPDK.", ) @@ -153,7 +167,7 @@ def _get_parser() -> argparse.ArgumentParser: "--re-run", "--re_run", action=_env_arg("DTS_RERUN"), - default=0, + default=SETTINGS.re_run, type=int, help="[DTS_RERUN] Re-run each test case the specified amount of times " "if a test failure occurs", @@ -162,23 +176,22 @@ def _get_parser() -> argparse.ArgumentParser: return parser -def _get_settings() -> _Settings: +def get_settings() -> Settings: parsed_args = _get_parser().parse_args() - return _Settings( + return Settings( config_file_path=parsed_args.config_file, output_dir=parsed_args.output_dir, timeout=parsed_args.timeout, - verbose=(parsed_args.verbose == "Y"), - skip_setup=(parsed_args.skip_setup == "Y"), + verbose=parsed_args.verbose, + skip_setup=parsed_args.skip_setup, dpdk_tarball_path=Path( - DPDKGitTarball(parsed_args.tarball, parsed_args.output_dir) - ) - if not os.path.exists(parsed_args.tarball) - else Path(parsed_args.tarball), + Path(DPDKGitTarball(parsed_args.tarball, parsed_args.output_dir)) + if not os.path.exists(parsed_args.tarball) + else Path(parsed_args.tarball) + ), compile_timeout=parsed_args.compile_timeout, - test_cases=parsed_args.test_cases.split(",") if parsed_args.test_cases else [], + test_cases=( + parsed_args.test_cases.split(",") if parsed_args.test_cases else [] + ), re_run=parsed_args.re_run, ) - - -SETTINGS: _Settings = _get_settings() diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py index f0fbe80f6f..603e18872c 100644 --- a/dts/framework/test_result.py +++ b/dts/framework/test_result.py @@ -254,7 +254,7 @@ def add_build_target( self._inner_results.append(build_target_result) return build_target_result - def add_sut_info(self, sut_info: NodeInfo): + def add_sut_info(self, sut_info: NodeInfo) -> None: self.sut_os_name = sut_info.os_name self.sut_os_version = sut_info.os_version self.sut_kernel_version = sut_info.kernel_version @@ -297,7 +297,7 @@ def add_execution(self, sut_node: NodeConfiguration) -> ExecutionResult: self._inner_results.append(execution_result) return execution_result - def add_error(self, error) -> None: + def add_error(self, error: Exception) -> None: self._errors.append(error) def process(self) -> None: diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py index 3b890c0451..d53553bf34 100644 --- a/dts/framework/test_suite.py +++ b/dts/framework/test_suite.py @@ -11,7 +11,7 @@ import re from ipaddress import IPv4Interface, IPv6Interface, ip_interface from types import MethodType -from typing import Union +from typing import Any, Union from scapy.layers.inet import IP # type: ignore[import] from scapy.layers.l2 import Ether # type: ignore[import] @@ -26,8 +26,7 @@ from .logger import DTSLOG, getLogger from .settings import SETTINGS from .test_result import BuildTargetResult, Result, TestCaseResult, TestSuiteResult -from .testbed_model import SutNode, TGNode -from .testbed_model.hw.port import Port, PortLink +from .testbed_model import Port, PortLink, SutNode, TGNode from .utils import get_packet_summaries @@ -453,7 +452,7 @@ def _execute_test_case( def get_test_suites(testsuite_module_path: str) -> list[type[TestSuite]]: - def is_test_suite(object) -> bool: + def is_test_suite(object: Any) -> bool: try: if issubclass(object, TestSuite) and object is not TestSuite: return True diff --git a/dts/framework/testbed_model/__init__.py b/dts/framework/testbed_model/__init__.py index 5cbb859e47..8ced05653b 100644 --- a/dts/framework/testbed_model/__init__.py +++ b/dts/framework/testbed_model/__init__.py @@ -9,15 +9,9 @@ # pylama:ignore=W0611 -from .hw import ( - LogicalCore, - LogicalCoreCount, - LogicalCoreCountFilter, - LogicalCoreList, - LogicalCoreListFilter, - VirtualDevice, - lcore_filter, -) +from .cpu import LogicalCoreCount, LogicalCoreCountFilter, LogicalCoreList from .node import Node +from .port import Port, PortLink from .sut_node import SutNode from .tg_node import TGNode +from .virtual_device import VirtualDevice diff --git a/dts/framework/testbed_model/hw/cpu.py b/dts/framework/testbed_model/cpu.py similarity index 95% rename from dts/framework/testbed_model/hw/cpu.py rename to dts/framework/testbed_model/cpu.py index d1918a12dc..8fe785dfe4 100644 --- a/dts/framework/testbed_model/hw/cpu.py +++ b/dts/framework/testbed_model/cpu.py @@ -272,3 +272,16 @@ def filter(self) -> list[LogicalCore]: ) return filtered_lcores + + +def lcore_filter( + core_list: list[LogicalCore], + filter_specifier: LogicalCoreCount | LogicalCoreList, + ascending: bool, +) -> LogicalCoreFilter: + if isinstance(filter_specifier, LogicalCoreList): + return LogicalCoreListFilter(core_list, filter_specifier, ascending) + elif isinstance(filter_specifier, LogicalCoreCount): + return LogicalCoreCountFilter(core_list, filter_specifier, ascending) + else: + raise ValueError(f"Unsupported filter r{filter_specifier}") diff --git a/dts/framework/testbed_model/hw/__init__.py b/dts/framework/testbed_model/hw/__init__.py deleted file mode 100644 index 88ccac0b0e..0000000000 --- a/dts/framework/testbed_model/hw/__init__.py +++ /dev/null @@ -1,27 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause -# Copyright(c) 2023 PANTHEON.tech s.r.o. - -# pylama:ignore=W0611 - -from .cpu import ( - LogicalCore, - LogicalCoreCount, - LogicalCoreCountFilter, - LogicalCoreFilter, - LogicalCoreList, - LogicalCoreListFilter, -) -from .virtual_device import VirtualDevice - - -def lcore_filter( - core_list: list[LogicalCore], - filter_specifier: LogicalCoreCount | LogicalCoreList, - ascending: bool, -) -> LogicalCoreFilter: - if isinstance(filter_specifier, LogicalCoreList): - return LogicalCoreListFilter(core_list, filter_specifier, ascending) - elif isinstance(filter_specifier, LogicalCoreCount): - return LogicalCoreCountFilter(core_list, filter_specifier, ascending) - else: - raise ValueError(f"Unsupported filter r{filter_specifier}") diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/testbed_model/linux_session.py similarity index 97% rename from dts/framework/remote_session/linux_session.py rename to dts/framework/testbed_model/linux_session.py index a3f1a6bf3b..f472bb8f0f 100644 --- a/dts/framework/remote_session/linux_session.py +++ b/dts/framework/testbed_model/linux_session.py @@ -9,10 +9,10 @@ from typing_extensions import NotRequired from framework.exception import RemoteCommandExecutionError -from framework.testbed_model import LogicalCore -from framework.testbed_model.hw.port import Port from framework.utils import expand_range +from .cpu import LogicalCore +from .port import Port from .posix_session import PosixSession @@ -64,7 +64,7 @@ def get_remote_cpus(self, use_first_core: bool) -> list[LogicalCore]: lcores.append(LogicalCore(lcore, core, socket, node)) return lcores - def get_dpdk_file_prefix(self, dpdk_prefix) -> str: + def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: return dpdk_prefix def setup_hugepages(self, hugepage_amount: int, force_first_numa: bool) -> None: diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py index fc01e0bf8e..fa5b143cdd 100644 --- a/dts/framework/testbed_model/node.py +++ b/dts/framework/testbed_model/node.py @@ -12,23 +12,26 @@ from typing import Any, Callable, Type, Union from framework.config import ( + OS, BuildTargetConfiguration, ExecutionConfiguration, NodeConfiguration, ) +from framework.exception import ConfigurationError from framework.logger import DTSLOG, getLogger -from framework.remote_session import InteractiveShellType, OSSession, create_session from framework.settings import SETTINGS -from .hw import ( +from .cpu import ( LogicalCore, LogicalCoreCount, LogicalCoreList, LogicalCoreListFilter, - VirtualDevice, lcore_filter, ) -from .hw.port import Port +from .linux_session import LinuxSession +from .os_session import InteractiveShellType, OSSession +from .port import Port +from .virtual_device import VirtualDevice class Node(ABC): @@ -172,9 +175,9 @@ def create_interactive_shell( return self.main_session.create_interactive_shell( shell_cls, - app_args, timeout, privileged, + app_args, ) def filter_lcores( @@ -205,7 +208,7 @@ def _get_remote_cpus(self) -> None: self._<a href="http://logger.info" rel="noreferrer" target="_blank">logger.info</a>("Getting CPU information.") self.lcores = self.main_session.get_remote_cpus(self.config.use_first_core) - def _setup_hugepages(self): + def _setup_hugepages(self) -> None: """ Setup hugepages on the Node. Different architectures can supply different amounts of memory for hugepages and numa-based hugepage allocation may need @@ -249,3 +252,13 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]: return lambda *args: None else: return func + + +def create_session( + node_config: NodeConfiguration, name: str, logger: DTSLOG +) -> OSSession: + match node_config.os: + case OS.linux: + return LinuxSession(node_config, name, logger) + case _: + raise ConfigurationError(f"Unsupported OS {node_config.os}") diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/testbed_model/os_session.py similarity index 95% rename from dts/framework/remote_session/os_session.py rename to dts/framework/testbed_model/os_session.py index 8a709eac1c..76e595a518 100644 --- a/dts/framework/remote_session/os_session.py +++ b/dts/framework/testbed_model/os_session.py @@ -10,19 +10,19 @@ from framework.config import Architecture, NodeConfiguration, NodeInfo from framework.logger import DTSLOG -from framework.remote_session.remote import InteractiveShell -from framework.settings import SETTINGS -from framework.testbed_model import LogicalCore -from framework.testbed_model.hw.port import Port -from framework.utils import MesonArgs - -from .remote import ( +from framework.remote_session import ( CommandResult, InteractiveRemoteSession, + InteractiveShell, RemoteSession, create_interactive_session, create_remote_session, ) +from framework.settings import SETTINGS +from framework.utils import MesonArgs + +from .cpu import LogicalCore +from .port import Port InteractiveShellType = TypeVar("InteractiveShellType", bound=InteractiveShell) @@ -85,9 +85,9 @@ def send_command( def create_interactive_shell( self, shell_cls: Type[InteractiveShellType], - eal_parameters: str, timeout: float, privileged: bool, + app_args: str, ) -> InteractiveShellType: """ See "create_interactive_shell" in SutNode @@ -96,7 +96,7 @@ def create_interactive_shell( self.interactive_session.session, self._logger, self._get_privileged_command if privileged else None, - eal_parameters, + app_args, timeout, ) @@ -113,7 +113,7 @@ def _get_privileged_command(command: str) -> str: """ @abstractmethod - def guess_dpdk_remote_dir(self, remote_dir) -> PurePath: + def guess_dpdk_remote_dir(self, remote_dir: str | PurePath) -> PurePath: """ Try to find DPDK remote dir in remote_dir. """ @@ -227,7 +227,7 @@ def kill_cleanup_dpdk_apps(self, dpdk_prefix_list: Iterable[str]) -> None: """ @abstractmethod - def get_dpdk_file_prefix(self, dpdk_prefix) -> str: + def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: """ Get the DPDK file prefix that will be used when running DPDK apps. """ diff --git a/dts/framework/testbed_model/hw/port.py b/dts/framework/testbed_model/port.py similarity index 100% rename from dts/framework/testbed_model/hw/port.py rename to dts/framework/testbed_model/port.py diff --git a/dts/framework/remote_session/posix_session.py b/dts/framework/testbed_model/posix_session.py similarity index 98% rename from dts/framework/remote_session/posix_session.py rename to dts/framework/testbed_model/posix_session.py index 5da0516e05..1d1d5b1b26 100644 --- a/dts/framework/remote_session/posix_session.py +++ b/dts/framework/testbed_model/posix_session.py @@ -32,7 +32,7 @@ def combine_short_options(**opts: bool) -> str: return ret_opts - def guess_dpdk_remote_dir(self, remote_dir) -> PurePosixPath: + def guess_dpdk_remote_dir(self, remote_dir: str | PurePath) -> PurePosixPath: remote_guess = self.join_remote_path(remote_dir, "dpdk-*") result = self.send_command(f"ls -d {remote_guess} | tail -1") return PurePosixPath(result.stdout) @@ -219,7 +219,7 @@ def _remove_dpdk_runtime_dirs( for dpdk_runtime_dir in dpdk_runtime_dirs: self.remove_remote_dir(dpdk_runtime_dir) - def get_dpdk_file_prefix(self, dpdk_prefix) -> str: + def get_dpdk_file_prefix(self, dpdk_prefix: str) -> str: return "" def get_compiler_version(self, compiler_name: str) -> str: diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py index 4161d3a4d5..17deea06e2 100644 --- a/dts/framework/testbed_model/sut_node.py +++ b/dts/framework/testbed_model/sut_node.py @@ -15,12 +15,14 @@ NodeInfo, SutNodeConfiguration, ) -from framework.remote_session import CommandResult, InteractiveShellType, OSSession +from framework.remote_session import CommandResult from framework.settings import SETTINGS from framework.utils import MesonArgs -from .hw import LogicalCoreCount, LogicalCoreList, VirtualDevice +from .cpu import LogicalCoreCount, LogicalCoreList from .node import Node +from .os_session import InteractiveShellType, OSSession +from .virtual_device import VirtualDevice class EalParameters(object): @@ -307,7 +309,7 @@ def create_eal_parameters( prefix: str = "dpdk", append_prefix_timestamp: bool = True, no_pci: bool = False, - vdevs: list[VirtualDevice] = None, + vdevs: list[VirtualDevice] | None = None, other_eal_param: str = "", ) -> "EalParameters": """ diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py index 27025cfa31..166eb8430e 100644 --- a/dts/framework/testbed_model/tg_node.py +++ b/dts/framework/testbed_model/tg_node.py @@ -16,16 +16,11 @@ from scapy.packet import Packet # type: ignore[import] -from framework.config import ( - ScapyTrafficGeneratorConfig, - TGNodeConfiguration, - TrafficGeneratorType, -) -from framework.exception import ConfigurationError - -from .capturing_traffic_generator import CapturingTrafficGenerator -from .hw.port import Port +from framework.config import TGNodeConfiguration + from .node import Node +from .port import Port +from .traffic_generator import CapturingTrafficGenerator, create_traffic_generator class TGNode(Node): @@ -80,20 +75,3 @@ def close(self) -> None: """Free all resources used by the node""" self.traffic_generator.close() super(TGNode, self).close() - - -def create_traffic_generator( - tg_node: TGNode, traffic_generator_config: ScapyTrafficGeneratorConfig -) -> CapturingTrafficGenerator: - """A factory function for creating traffic generator object from user config.""" - - from .scapy import ScapyTrafficGenerator - - match traffic_generator_config.traffic_generator_type: - case TrafficGeneratorType.SCAPY: - return ScapyTrafficGenerator(tg_node, traffic_generator_config) - case _: - raise ConfigurationError( - "Unknown traffic generator: " - f"{traffic_generator_config.traffic_generator_type}" - ) diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py b/dts/framework/testbed_model/traffic_generator/__init__.py new file mode 100644 index 0000000000..11bfa1ee0f --- /dev/null +++ b/dts/framework/testbed_model/traffic_generator/__init__.py @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2023 PANTHEON.tech s.r.o. + +from framework.config import ScapyTrafficGeneratorConfig, TrafficGeneratorType +from framework.exception import ConfigurationError +from framework.testbed_model.node import Node + +from .capturing_traffic_generator import CapturingTrafficGenerator +from .scapy import ScapyTrafficGenerator + + +def create_traffic_generator( + tg_node: Node, traffic_generator_config: ScapyTrafficGeneratorConfig +) -> CapturingTrafficGenerator: + """A factory function for creating traffic generator object from user config.""" + + match traffic_generator_config.traffic_generator_type: + case TrafficGeneratorType.SCAPY: + return ScapyTrafficGenerator(tg_node, traffic_generator_config) + case _: + raise ConfigurationError( + "Unknown traffic generator: " + f"{traffic_generator_config.traffic_generator_type}" + ) diff --git a/dts/framework/testbed_model/capturing_traffic_generator.py b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py similarity index 96% rename from dts/framework/testbed_model/capturing_traffic_generator.py rename to dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py index ab98987f8e..e521211ef0 100644 --- a/dts/framework/testbed_model/capturing_traffic_generator.py +++ b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py @@ -16,9 +16,9 @@ from scapy.packet import Packet # type: ignore[import] from framework.settings import SETTINGS +from framework.testbed_model.port import Port from framework.utils import get_packet_summaries -from .hw.port import Port from .traffic_generator import TrafficGenerator @@ -130,7 +130,9 @@ def _send_packets_and_capture( for the specified duration. It must be able to handle no received packets. """ - def _write_capture_from_packets(self, capture_name: str, packets: list[Packet]): + def _write_capture_from_packets( + self, capture_name: str, packets: list[Packet] + ) -> None: file_name = f"{SETTINGS.output_dir}/{capture_name}.pcap" self._logger.debug(f"Writing packets to {file_name}.") scapy.utils.wrpcap(file_name, packets) diff --git a/dts/framework/testbed_model/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py similarity index 95% rename from dts/framework/testbed_model/scapy.py rename to dts/framework/testbed_model/traffic_generator/scapy.py index af0d4dbb25..51864b6e6b 100644 --- a/dts/framework/testbed_model/scapy.py +++ b/dts/framework/testbed_model/traffic_generator/scapy.py @@ -24,16 +24,15 @@ from scapy.packet import Packet # type: ignore[import] from framework.config import OS, ScapyTrafficGeneratorConfig -from framework.logger import DTSLOG, getLogger from framework.remote_session import PythonShell from framework.settings import SETTINGS +from framework.testbed_model.node import Node +from framework.testbed_model.port import Port from .capturing_traffic_generator import ( CapturingTrafficGenerator, _get_default_capture_name, ) -from .hw.port import Port -from .tg_node import TGNode """ ========= BEGIN RPC FUNCTIONS ========= @@ -146,7 +145,7 @@ def quit(self) -> None: self._BaseServer__shutdown_request = True return None - def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary): + def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None: """Add a function to the server. This is meant to be executed remotely. @@ -191,15 +190,9 @@ class ScapyTrafficGenerator(CapturingTrafficGenerator): session: PythonShell rpc_server_proxy: xmlrpc.client.ServerProxy _config: ScapyTrafficGeneratorConfig - _tg_node: TGNode - _logger: DTSLOG - - def __init__(self, tg_node: TGNode, config: ScapyTrafficGeneratorConfig): - self._config = config - self._tg_node = tg_node - self._logger = getLogger( - f"{self._<a href="http://tg_node.name" rel="noreferrer" target="_blank">tg_node.name</a>} {self._config.traffic_generator_type}" - ) + + def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig): + super().__init__(tg_node, config) assert ( self._tg_node.config.os == OS.linux @@ -235,7 +228,7 @@ def __init__(self, tg_node: TGNode, config: ScapyTrafficGeneratorConfig): function_bytes = marshal.dumps(function.__code__) self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes) - def _start_xmlrpc_server_in_remote_python(self, listen_port: int): + def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None: # load the source of the function src = inspect.getsource(QuittableXMLRPCServer) # Lines with only whitespace break the repl if in the middle of a function @@ -280,7 +273,7 @@ def _send_packets_and_capture( scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets] return scapy_packets - def close(self): + def close(self) -> None: try: self.rpc_server_proxy.quit() except ConnectionRefusedError: diff --git a/dts/framework/testbed_model/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py similarity index 80% rename from dts/framework/testbed_model/traffic_generator.py rename to dts/framework/testbed_model/traffic_generator/traffic_generator.py index 28c35d3ce4..ea7c3963da 100644 --- a/dts/framework/testbed_model/traffic_generator.py +++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py @@ -12,11 +12,12 @@ from scapy.packet import Packet # type: ignore[import] -from framework.logger import DTSLOG +from framework.config import TrafficGeneratorConfig +from framework.logger import DTSLOG, getLogger +from framework.testbed_model.node import Node +from framework.testbed_model.port import Port from framework.utils import get_packet_summaries -from .hw.port import Port - class TrafficGenerator(ABC): """The base traffic generator. @@ -24,8 +25,17 @@ class TrafficGenerator(ABC): Defines the few basic methods that each traffic generator must implement. """ + _config: TrafficGeneratorConfig + _tg_node: Node </blockquote><div> </div><div><div style="font-family:arial,sans-serif" class="gmail_default">Is there a benefit to changing this to be a node instead of a TGNode? Wouldn't we want the capabilities of the TGNode to be accessible in the TrafficGenerator class? </div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> _logger: DTSLOG + def __init__(self, tg_node: Node, config: TrafficGeneratorConfig): + self._config = config + self._tg_node = tg_node + self._logger = getLogger( + f"{self._<a href="http://tg_node.name" rel="noreferrer" target="_blank">tg_node.name</a>} {self._config.traffic_generator_type}" + ) + def send_packet(self, packet: Packet, port: Port) -> None: """Send a packet and block until it is fully sent. diff --git a/dts/framework/testbed_model/hw/virtual_device.py b/dts/framework/testbed_model/virtual_device.py similarity index 100% rename from dts/framework/testbed_model/hw/virtual_device.py rename to dts/framework/testbed_model/virtual_device.py diff --git a/dts/framework/utils.py b/dts/framework/utils.py index d27c2c5b5f..f0c916471c 100644 --- a/dts/framework/utils.py +++ b/dts/framework/utils.py @@ -7,7 +7,6 @@ import json import os import subprocess -import sys from enum import Enum from pathlib import Path from subprocess import SubprocessError @@ -16,35 +15,7 @@ from .exception import ConfigurationError - -class StrEnum(Enum): - @staticmethod - def _generate_next_value_( - name: str, start: int, count: int, last_values: object - ) -> str: - return name - - def __str__(self) -> str: - return <a href="http://self.name" rel="noreferrer" target="_blank">self.name</a> - - -REGEX_FOR_PCI_ADDRESS = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/" - - -def check_dts_python_version() -> None: - if sys.version_info.major < 3 or ( - sys.version_info.major == 3 and sys.version_info.minor < 10 - ): - print( - RED( - ( - "WARNING: DTS execution node's python version is lower than" - "python 3.10, is deprecated and will not work in future releases." - ) - ), - file=sys.stderr, - ) - print(RED("Please use Python >= 3.10 instead"), file=sys.stderr) +REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/" def expand_range(range_str: str) -> list[int]: @@ -67,7 +38,7 @@ def expand_range(range_str: str) -> list[int]: return expanded_range -def get_packet_summaries(packets: list[Packet]): +def get_packet_summaries(packets: list[Packet]) -> str: if len(packets) == 1: packet_summaries = packets[0].summary() else: @@ -77,8 +48,15 @@ def get_packet_summaries(packets: list[Packet]): return f"Packet contents: \n{packet_summaries}" -def RED(text: str) -> str: - return f"\u001B[31;1m{str(text)}\u001B[0m" +class StrEnum(Enum): + @staticmethod + def _generate_next_value_( + name: str, start: int, count: int, last_values: object + ) -> str: + return name + + def __str__(self) -> str: + return <a href="http://self.name" rel="noreferrer" target="_blank">self.name</a> class MesonArgs(object): @@ -225,5 +203,5 @@ def _delete_tarball(self) -> None: if self._tarball_path and os.path.exists(self._tarball_path): os.remove(self._tarball_path) - def __fspath__(self): + def __fspath__(self) -> str: return str(self._tarball_path) diff --git a/dts/main.py b/dts/main.py index 43311fa847..5d4714b0c3 100755 --- a/dts/main.py +++ b/dts/main.py @@ -10,10 +10,17 @@ import logging -from framework import dts +from framework import settings def main() -> None: + """Set DTS settings, then run DTS. + + The DTS settings are taken from the command line arguments and the environment variables. + """ + settings.SETTINGS = settings.get_settings() + from framework import dts + dts.run_all() -- 2.34.1 </blockquote></div></div>