Thanks, Yoan, I'll make these changes in v8.
On Tue, Nov 21, 2023 at 4:08 PM Yoan Picchi <yoan.pic...@foss.arm.com> wrote: > > On 11/15/23 13:09, Juraj Linkeš wrote: > > Format according to the Google format and PEP257, with slight > > deviations. > > > > Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech> > > --- > > dts/framework/config/__init__.py | 371 ++++++++++++++++++++++++++----- > > dts/framework/config/types.py | 132 +++++++++++ > > 2 files changed, 446 insertions(+), 57 deletions(-) > > create mode 100644 dts/framework/config/types.py > > > > diff --git a/dts/framework/config/__init__.py > > b/dts/framework/config/__init__.py > > index 2044c82611..0aa149a53d 100644 > > --- a/dts/framework/config/__init__.py > > +++ b/dts/framework/config/__init__.py > > @@ -3,8 +3,34 @@ > > # Copyright(c) 2022-2023 University of New Hampshire > > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > > > -""" > > -Yaml config parsing methods > > +"""Testbed configuration and test suite specification. > > + > > +This package offers classes that hold real-time information about the > > testbed, hold test run > > +configuration describing the tested testbed and a loader function, > > :func:`load_config`, which loads > > +the YAML test run configuration file > > +and validates it according to :download:`the schema > > <conf_yaml_schema.json>`. > > + > > +The YAML test run configuration file is parsed into a dictionary, parts of > > which are used throughout > > +this package. The allowed keys and types inside this dictionary are > > defined in > > +the :doc:`types <framework.config.types>` module. > > + > > +The test run configuration has two main sections: > > + > > + * The :class:`ExecutionConfiguration` which defines what tests are > > going to be run > > + and how DPDK will be built. It also references the testbed where > > these tests and DPDK > > + are going to be run, > > + * The nodes of the testbed are defined in the other section, > > + a :class:`list` of :class:`NodeConfiguration` objects. > > + > > +The real-time information about testbed is supposed to be gathered at > > runtime. > > + > > +The classes defined in this package make heavy use of :mod:`dataclasses`. > > +All of them use slots and are frozen: > > + > > + * Slots enables some optimizations, by pre-allocating space for the > > defined > > + attributes in the underlying data structure, > > + * Frozen makes the object immutable. This enables further > > optimizations, > > + and makes it thread safe should we every want to move in that > > direction. > > every -> ever ? > > > """ > > > > import json > > @@ -12,11 +38,20 @@ > > import pathlib > > from dataclasses import dataclass > > from enum import auto, unique > > -from typing import Any, TypedDict, Union > > +from typing import Union > > > > import warlock # type: ignore[import] > > import yaml > > > > +from framework.config.types import ( > > + BuildTargetConfigDict, > > + ConfigurationDict, > > + ExecutionConfigDict, > > + NodeConfigDict, > > + PortConfigDict, > > + TestSuiteConfigDict, > > + TrafficGeneratorConfigDict, > > +) > > from framework.exception import ConfigurationError > > from framework.settings import SETTINGS > > from framework.utils import StrEnum > > @@ -24,55 +59,97 @@ > > > > @unique > > class Architecture(StrEnum): > > + r"""The supported architectures of > > :class:`~framework.testbed_model.node.Node`\s.""" > > + > > + #: > > i686 = auto() > > + #: > > x86_64 = auto() > > + #: > > x86_32 = auto() > > + #: > > arm64 = auto() > > + #: > > ppc64le = auto() > > > > > > @unique > > class OS(StrEnum): > > + r"""The supported operating systems of > > :class:`~framework.testbed_model.node.Node`\s.""" > > + > > + #: > > linux = auto() > > + #: > > freebsd = auto() > > + #: > > windows = auto() > > > > > > @unique > > class CPUType(StrEnum): > > + r"""The supported CPUs of > > :class:`~framework.testbed_model.node.Node`\s.""" > > + > > + #: > > native = auto() > > + #: > > armv8a = auto() > > + #: > > dpaa2 = auto() > > + #: > > thunderx = auto() > > + #: > > xgene1 = auto() > > > > > > @unique > > class Compiler(StrEnum): > > + r"""The supported compilers of > > :class:`~framework.testbed_model.node.Node`\s.""" > > + > > + #: > > gcc = auto() > > + #: > > clang = auto() > > + #: > > icc = auto() > > + #: > > msvc = auto() > > > > > > @unique > > class TrafficGeneratorType(StrEnum): > > + """The supported traffic generators.""" > > + > > + #: > > SCAPY = auto() > > > > > > -# Slots enables some optimizations, by pre-allocating space for the defined > > -# attributes in the underlying data structure. > > -# > > -# Frozen makes the object immutable. This enables further optimizations, > > -# and makes it thread safe should we every want to move in that direction. > > @dataclass(slots=True, frozen=True) > > class HugepageConfiguration: > > + r"""The hugepage configuration of > > :class:`~framework.testbed_model.node.Node`\s. > > + > > + Attributes: > > + amount: The number of hugepages. > > + force_first_numa: If :data:`True`, the hugepages will be > > configured on the first NUMA node. > > + """ > > + > > amount: int > > force_first_numa: bool > > > > > > @dataclass(slots=True, frozen=True) > > class PortConfig: > > + r"""The port configuration of > > :class:`~framework.testbed_model.node.Node`\s. > > + > > + Attributes: > > + node: The :class:`~framework.testbed_model.node.Node` where this > > port exists. > > + pci: The PCI address of the port. > > + os_driver_for_dpdk: The operating system driver name for use with > > DPDK. > > + os_driver: The operating system driver name when the operating > > system controls the port. > > + peer_node: The :class:`~framework.testbed_model.node.Node` of the > > port > > + connected to this port. > > + peer_pci: The PCI address of the port connected to this port. > > + """ > > + > > node: str > > pci: str > > os_driver_for_dpdk: str > > @@ -81,18 +158,44 @@ class PortConfig: > > peer_pci: str > > > > @staticmethod > > - def from_dict(node: str, d: dict) -> "PortConfig": > > + def from_dict(node: str, d: PortConfigDict) -> "PortConfig": > > + """A convenience method that creates the object from fewer inputs. > > + > > + Args: > > + node: The node where this port exists. > > + d: The configuration dictionary. > > + > > + Returns: > > + The port configuration instance. > > + """ > > return PortConfig(node=node, **d) > > > > > > @dataclass(slots=True, frozen=True) > > class TrafficGeneratorConfig: > > + """The configuration of traffic generators. > > + > > + The class will be expanded when more configuration is needed. > > + > > + Attributes: > > + traffic_generator_type: The type of the traffic generator. > > + """ > > + > > traffic_generator_type: TrafficGeneratorType > > > > @staticmethod > > - def from_dict(d: dict) -> "ScapyTrafficGeneratorConfig": > > - # This looks useless now, but is designed to allow expansion to > > traffic > > - # generators that require more configuration later. > > + def from_dict(d: TrafficGeneratorConfigDict) -> > > "ScapyTrafficGeneratorConfig": > > + """A convenience method that produces traffic generator config of > > the proper type. > > + > > + Args: > > + d: The configuration dictionary. > > + > > + Returns: > > + The traffic generator configuration instance. > > + > > + Raises: > > + ConfigurationError: An unknown traffic generator type was > > encountered. > > + """ > > match TrafficGeneratorType(d["type"]): > > case TrafficGeneratorType.SCAPY: > > return ScapyTrafficGeneratorConfig( > > @@ -106,11 +209,31 @@ def from_dict(d: dict) -> > > "ScapyTrafficGeneratorConfig": > > > > @dataclass(slots=True, frozen=True) > > class ScapyTrafficGeneratorConfig(TrafficGeneratorConfig): > > + """Scapy traffic generator specific configuration.""" > > + > > pass > > > > > > @dataclass(slots=True, frozen=True) > > class NodeConfiguration: > > + r"""The configuration of :class:`~framework.testbed_model.node.Node`\s. > > + > > + Attributes: > > + name: The name of the :class:`~framework.testbed_model.node.Node`. > > + hostname: The hostname of the > > :class:`~framework.testbed_model.node.Node`. > > + Can be an IP or a domain name. > > + user: The name of the user used to connect to > > + the :class:`~framework.testbed_model.node.Node`. > > + password: The password of the user. The use of passwords is > > heavily discouraged. > > + Please use keys instead. > > + arch: The architecture of the > > :class:`~framework.testbed_model.node.Node`. > > + os: The operating system of the > > :class:`~framework.testbed_model.node.Node`. > > + lcores: A comma delimited list of logical cores to use when > > running DPDK. > > + use_first_core: If :data:`True`, the first logical core won't be > > used. > > + hugepages: An optional hugepage configuration. > > + ports: The ports that can be used in testing. > > + """ > > + > > name: str > > hostname: str > > user: str > > @@ -123,57 +246,91 @@ class NodeConfiguration: > > ports: list[PortConfig] > > > > @staticmethod > > - def from_dict(d: dict) -> Union["SutNodeConfiguration", > > "TGNodeConfiguration"]: > > - hugepage_config = d.get("hugepages") > > - if hugepage_config: > > - if "force_first_numa" not in hugepage_config: > > - hugepage_config["force_first_numa"] = False > > - hugepage_config = HugepageConfiguration(**hugepage_config) > > - > > - common_config = { > > - "name": d["name"], > > - "hostname": d["hostname"], > > - "user": d["user"], > > - "password": d.get("password"), > > - "arch": Architecture(d["arch"]), > > - "os": OS(d["os"]), > > - "lcores": d.get("lcores", "1"), > > - "use_first_core": d.get("use_first_core", False), > > - "hugepages": hugepage_config, > > - "ports": [PortConfig.from_dict(d["name"], port) for port in > > d["ports"]], > > - } > > - > > + def from_dict( > > + d: NodeConfigDict, > > + ) -> Union["SutNodeConfiguration", "TGNodeConfiguration"]: > > + """A convenience method that processes the inputs before creating > > a specialized instance. > > + > > + Args: > > + d: The configuration dictionary. > > + > > + Returns: > > + Either an SUT or TG configuration instance. > > + """ > > + hugepage_config = None > > + if "hugepages" in d: > > + hugepage_config_dict = d["hugepages"] > > + if "force_first_numa" not in hugepage_config_dict: > > + hugepage_config_dict["force_first_numa"] = False > > + hugepage_config = HugepageConfiguration(**hugepage_config_dict) > > + > > + # The calls here contain duplicated code which is here because > > Mypy doesn't > > + # properly support dictionary unpacking with TypedDicts > > if "traffic_generator" in d: > > return TGNodeConfiguration( > > + name=d["name"], > > + hostname=d["hostname"], > > + user=d["user"], > > + password=d.get("password"), > > + arch=Architecture(d["arch"]), > > + os=OS(d["os"]), > > + lcores=d.get("lcores", "1"), > > + use_first_core=d.get("use_first_core", False), > > + hugepages=hugepage_config, > > + ports=[PortConfig.from_dict(d["name"], port) for port in > > d["ports"]], > > traffic_generator=TrafficGeneratorConfig.from_dict( > > d["traffic_generator"] > > ), > > - **common_config, > > ) > > else: > > return SutNodeConfiguration( > > - memory_channels=d.get("memory_channels", 1), > > **common_config > > + name=d["name"], > > + hostname=d["hostname"], > > + user=d["user"], > > + password=d.get("password"), > > + arch=Architecture(d["arch"]), > > + os=OS(d["os"]), > > + lcores=d.get("lcores", "1"), > > + use_first_core=d.get("use_first_core", False), > > + hugepages=hugepage_config, > > + ports=[PortConfig.from_dict(d["name"], port) for port in > > d["ports"]], > > + memory_channels=d.get("memory_channels", 1), > > ) > > > > > > @dataclass(slots=True, frozen=True) > > class SutNodeConfiguration(NodeConfiguration): > > + """:class:`~framework.testbed_model.sut_node.SutNode` specific > > configuration. > > + > > + Attributes: > > + memory_channels: The number of memory channels to use when running > > DPDK. > > + """ > > + > > memory_channels: int > > > > > > @dataclass(slots=True, frozen=True) > > class TGNodeConfiguration(NodeConfiguration): > > + """:class:`~framework.testbed_model.tg_node.TGNode` specific > > configuration. > > + > > + Attributes: > > + traffic_generator: The configuration of the traffic generator > > present on the TG node. > > + """ > > + > > traffic_generator: ScapyTrafficGeneratorConfig > > > > > > @dataclass(slots=True, frozen=True) > > class NodeInfo: > > - """Class to hold important versions within the node. > > - > > - This class, unlike the NodeConfiguration class, cannot be generated at > > the start. > > - This is because we need to initialize a connection with the node > > before we can > > - collect the information needed in this class. Therefore, it cannot be > > a part of > > - the configuration class above. > > + """Supplemental node information. > > + > > + Attributes: > > + os_name: The name of the running operating system of > > + the :class:`~framework.testbed_model.node.Node`. > > + os_version: The version of the running operating system of > > + the :class:`~framework.testbed_model.node.Node`. > > + kernel_version: The kernel version of the running operating system > > of > > + the :class:`~framework.testbed_model.node.Node`. > > """ > > > > os_name: str > > @@ -183,6 +340,20 @@ class NodeInfo: > > > > @dataclass(slots=True, frozen=True) > > class BuildTargetConfiguration: > > + """DPDK build configuration. > > + > > + The configuration used for building DPDK. > > + > > + Attributes: > > + arch: The target architecture to build for. > > + os: The target os to build for. > > + cpu: The target CPU to build for. > > + compiler: The compiler executable to use. > > + compiler_wrapper: This string will be put in front of the compiler > > when > > + executing the build. Useful for adding wrapper commands, such > > as ``ccache``. > > + name: The name of the compiler. > > + """ > > + > > arch: Architecture > > os: OS > > cpu: CPUType > > @@ -191,7 +362,18 @@ class BuildTargetConfiguration: > > name: str > > > > @staticmethod > > - def from_dict(d: dict) -> "BuildTargetConfiguration": > > + def from_dict(d: BuildTargetConfigDict) -> "BuildTargetConfiguration": > > + r"""A convenience method that processes the inputs before creating > > an instance. > > + > > + `arch`, `os`, `cpu` and `compiler` are converted to > > :class:`Enum`\s and > > + `name` is constructed from `arch`, `os`, `cpu` and `compiler`. > > + > > + Args: > > + d: The configuration dictionary. > > + > > + Returns: > > + The build target configuration instance. > > + """ > > return BuildTargetConfiguration( > > arch=Architecture(d["arch"]), > > os=OS(d["os"]), > > @@ -204,23 +386,29 @@ def from_dict(d: dict) -> "BuildTargetConfiguration": > > > > @dataclass(slots=True, frozen=True) > > class BuildTargetInfo: > > - """Class to hold important versions within the build target. > > + """Various versions and other information about a build target. > > > > - This is very similar to the NodeInfo class, it just instead holds > > information > > - for the build target. > > + Attributes: > > + dpdk_version: The DPDK version that was built. > > + compiler_version: The version of the compiler used to build DPDK. > > """ > > > > dpdk_version: str > > compiler_version: str > > > > > > -class TestSuiteConfigDict(TypedDict): > > - suite: str > > - cases: list[str] > > - > > - > > @dataclass(slots=True, frozen=True) > > class TestSuiteConfig: > > + """Test suite configuration. > > + > > + Information about a single test suite to be executed. > > + > > + Attributes: > > + test_suite: The name of the test suite module without the starting > > ``TestSuite_``. > > + test_cases: The names of test cases from this test suite to > > execute. > > + If empty, all test cases will be executed. > > + """ > > + > > test_suite: str > > test_cases: list[str] > > > > @@ -228,6 +416,14 @@ class TestSuiteConfig: > > def from_dict( > > entry: str | TestSuiteConfigDict, > > ) -> "TestSuiteConfig": > > + """Create an instance from two different types. > > + > > + Args: > > + entry: Either a suite name or a dictionary containing the > > config. > > + > > + Returns: > > + The test suite configuration instance. > > + """ > > if isinstance(entry, str): > > return TestSuiteConfig(test_suite=entry, test_cases=[]) > > elif isinstance(entry, dict): > > @@ -238,19 +434,49 @@ def from_dict( > > > > @dataclass(slots=True, frozen=True) > > class ExecutionConfiguration: > > + """The configuration of an execution. > > + > > + The configuration contains testbed information, what tests to execute > > + and with what DPDK build. > > + > > + Attributes: > > + build_targets: A list of DPDK builds to test. > > + perf: Whether to run performance tests. > > + func: Whether to run functional tests. > > + skip_smoke_tests: Whether to skip smoke tests. > > + test_suites: The names of test suites and/or test cases to execute. > > + system_under_test_node: The SUT node to use in this execution. > > + traffic_generator_node: The TG node to use in this execution. > > + vdevs: The names of virtual devices to test. > > + """ > > + > > build_targets: list[BuildTargetConfiguration] > > perf: bool > > func: bool > > + skip_smoke_tests: bool > > test_suites: list[TestSuiteConfig] > > system_under_test_node: SutNodeConfiguration > > traffic_generator_node: TGNodeConfiguration > > vdevs: list[str] > > - skip_smoke_tests: bool > > > > @staticmethod > > def from_dict( > > - d: dict, node_map: dict[str, Union[SutNodeConfiguration | > > TGNodeConfiguration]] > > + d: ExecutionConfigDict, > > + node_map: dict[str, Union[SutNodeConfiguration | > > TGNodeConfiguration]], > > ) -> "ExecutionConfiguration": > > + """A convenience method that processes the inputs before creating > > an instance. > > + > > + The build target and the test suite config is transformed into > > their respective objects. > > is -> are > > > + SUT and TG configuration are taken from `node_map`. The other > > (:class:`bool`) attributes are > > configuration*s* > > > + just stored. > > + > > + Args: > > + d: The configuration dictionary. > > + node_map: A dictionary mapping node names to their config > > objects. > > + > > + Returns: > > + The execution configuration instance. > > + """ > > build_targets: list[BuildTargetConfiguration] = list( > > map(BuildTargetConfiguration.from_dict, d["build_targets"]) > > ) > > @@ -291,10 +517,31 @@ def from_dict( > > > > @dataclass(slots=True, frozen=True) > > class Configuration: > > + """DTS testbed and test configuration. > > + > > + The node configuration is not stored in this object. Rather, all used > > node configurations > > + are stored inside the execution configuration where the nodes are > > actually used. > > + > > + Attributes: > > + executions: Execution configurations. > > + """ > > + > > executions: list[ExecutionConfiguration] > > > > @staticmethod > > - def from_dict(d: dict) -> "Configuration": > > + def from_dict(d: ConfigurationDict) -> "Configuration": > > + """A convenience method that processes the inputs before creating > > an instance. > > + > > + Build target and test suite config is transformed into their > > respective objects. > > is -> are > > > + SUT and TG configuration are taken from `node_map`. The other > > (:class:`bool`) attributes are > > configuration*s* > > > + just stored. > > + > > + Args: > > + d: The configuration dictionary. > > + > > + Returns: > > + The whole configuration instance. > > + """ > > nodes: list[Union[SutNodeConfiguration | TGNodeConfiguration]] = > > list( > > map(NodeConfiguration.from_dict, d["nodes"]) > > ) > > @@ -313,9 +560,17 @@ def from_dict(d: dict) -> "Configuration": > > > > > > def load_config() -> Configuration: > > - """ > > - Loads the configuration file and the configuration file schema, > > - validates the configuration file, and creates a configuration object. > > + """Load DTS test run configuration from a file. > > + > > + Load the YAML test run configuration file > > + and :download:`the configuration file schema <conf_yaml_schema.json>`, > > + validate the test run configuration file, and create a test run > > configuration object. > > + > > + The YAML test run configuration file is specified in the > > :option:`--config-file` command line > > + argument or the :envvar:`DTS_CFG_FILE` environment variable. > > + > > + Returns: > > + The parsed test run configuration. > > """ > > with open(SETTINGS.config_file_path, "r") as f: > > config_data = yaml.safe_load(f) > > @@ -326,6 +581,8 @@ def load_config() -> Configuration: > > > > with open(schema_path, "r") as f: > > schema = json.load(f) > > - config: dict[str, Any] = warlock.model_factory(schema, > > name="_Config")(config_data) > > - config_obj: Configuration = Configuration.from_dict(dict(config)) > > + config = warlock.model_factory(schema, name="_Config")(config_data) > > + config_obj: Configuration = Configuration.from_dict( > > + dict(config) # type: ignore[arg-type] > > + ) > > return config_obj > > diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py > > new file mode 100644 > > index 0000000000..1927910d88 > > --- /dev/null > > +++ b/dts/framework/config/types.py > > @@ -0,0 +1,132 @@ > > +# SPDX-License-Identifier: BSD-3-Clause > > +# Copyright(c) 2023 PANTHEON.tech s.r.o. > > + > > +"""Configuration dictionary contents specification. > > + > > +These type definitions serve as documentation of the configuration > > dictionary contents. > > + > > +The definitions use the built-in :class:`~typing.TypedDict` construct. > > +""" > > + > > +from typing import TypedDict > > + > > + > > +class PortConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + pci: str > > + #: > > + os_driver_for_dpdk: str > > + #: > > + os_driver: str > > + #: > > + peer_node: str > > + #: > > + peer_pci: str > > + > > + > > +class TrafficGeneratorConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + type: str > > + > > + > > +class HugepageConfigurationDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + amount: int > > + #: > > + force_first_numa: bool > > + > > + > > +class NodeConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + hugepages: HugepageConfigurationDict > > + #: > > + name: str > > + #: > > + hostname: str > > + #: > > + user: str > > + #: > > + password: str > > + #: > > + arch: str > > + #: > > + os: str > > + #: > > + lcores: str > > + #: > > + use_first_core: bool > > + #: > > + ports: list[PortConfigDict] > > + #: > > + memory_channels: int > > + #: > > + traffic_generator: TrafficGeneratorConfigDict > > + > > + > > +class BuildTargetConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + arch: str > > + #: > > + os: str > > + #: > > + cpu: str > > + #: > > + compiler: str > > + #: > > + compiler_wrapper: str > > + > > + > > +class TestSuiteConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + suite: str > > + #: > > + cases: list[str] > > + > > + > > +class ExecutionSUTConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + node_name: str > > + #: > > + vdevs: list[str] > > + > > + > > +class ExecutionConfigDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + build_targets: list[BuildTargetConfigDict] > > + #: > > + perf: bool > > + #: > > + func: bool > > + #: > > + skip_smoke_tests: bool > > + #: > > + test_suites: TestSuiteConfigDict > > + #: > > + system_under_test_node: ExecutionSUTConfigDict > > + #: > > + traffic_generator_node: str > > + > > + > > +class ConfigurationDict(TypedDict): > > + """Allowed keys and values.""" > > + > > + #: > > + nodes: list[NodeConfigDict] > > + #: > > + executions: list[ExecutionConfigDict] >