[v4,6/9] dts: add config parser module
Checks
Commit Message
From: Owen Hilyard <ohilyard@iol.unh.edu>
The configuration is split into two parts, one defining the parameters
of the test run and the other defining the topology to be used.
The format of the configuration is YAML. It is validated according to a
json schema which also servers as detailed documentation of the various
configuration fields. This means that the complete set of allowed values
are tied to the schema as a source of truth. This enables making changes
to parts of DTS that interface with config files without a high risk of
breaking someone's configuration.
This configuration system uses immutable objects to represent the
configuration, making IDE/LSP autocomplete work properly.
There are two ways to specify the configuration file path, an
environment variable or a command line argument, applied in that order.
Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
dts/conf.yaml | 7 ++
dts/framework/config/__init__.py | 99 ++++++++++++++++++++++
dts/framework/config/conf_yaml_schema.json | 73 ++++++++++++++++
dts/framework/exception.py | 14 +++
dts/framework/settings.py | 65 ++++++++++++++
5 files changed, 258 insertions(+)
create mode 100644 dts/conf.yaml
create mode 100644 dts/framework/config/__init__.py
create mode 100644 dts/framework/config/conf_yaml_schema.json
create mode 100644 dts/framework/settings.py
Comments
> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Friday, July 29, 2022 6:56 PM
> To: thomas@monjalon.net; david.marchand@redhat.com; Randles, Ronan
> <ronan.randles@intel.com>; Honnappa.Nagarahalli@arm.com;
> ohilyard@iol.unh.edu; Tu, Lijuan <lijuan.tu@intel.com>
> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> Subject: [PATCH v4 6/9] dts: add config parser module
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> The configuration is split into two parts, one defining the parameters of the test
> run and the other defining the topology to be used.
>
> The format of the configuration is YAML. It is validated according to a json
> schema which also servers as detailed documentation of the various
> configuration fields. This means that the complete set of allowed values are tied
> to the schema as a source of truth. This enables making changes to parts of DTS
> that interface with config files without a high risk of breaking someone's
> configuration.
>
> This configuration system uses immutable objects to represent the configuration,
> making IDE/LSP autocomplete work properly.
>
> There are two ways to specify the configuration file path, an environment
> variable or a command line argument, applied in that order.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
> dts/conf.yaml | 7 ++
> dts/framework/config/__init__.py | 99 ++++++++++++++++++++++
> dts/framework/config/conf_yaml_schema.json | 73 ++++++++++++++++
> dts/framework/exception.py | 14 +++
> dts/framework/settings.py | 65 ++++++++++++++
> 5 files changed, 258 insertions(+)
> create mode 100644 dts/conf.yaml
> create mode 100644 dts/framework/config/__init__.py create mode 100644
> dts/framework/config/conf_yaml_schema.json
> create mode 100644 dts/framework/settings.py
>
Reviewed-by: Lijuan Tu <lijuan.tu@intel.com>
Thanks,
Lijuan
On Fri, Jul 29, 2022 at 10:55:47AM +0000, Juraj Linkeš wrote:
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> The configuration is split into two parts, one defining the parameters
> of the test run and the other defining the topology to be used.
>
> The format of the configuration is YAML. It is validated according to a
> json schema which also servers as detailed documentation of the various
s/servers/serves/
> configuration fields. This means that the complete set of allowed values
> are tied to the schema as a source of truth. This enables making changes
> to parts of DTS that interface with config files without a high risk of
> breaking someone's configuration.
>
> This configuration system uses immutable objects to represent the
> configuration, making IDE/LSP autocomplete work properly.
>
> There are two ways to specify the configuration file path, an
> environment variable or a command line argument, applied in that order.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
> dts/conf.yaml | 7 ++
> dts/framework/config/__init__.py | 99 ++++++++++++++++++++++
> dts/framework/config/conf_yaml_schema.json | 73 ++++++++++++++++
> dts/framework/exception.py | 14 +++
> dts/framework/settings.py | 65 ++++++++++++++
> 5 files changed, 258 insertions(+)
> create mode 100644 dts/conf.yaml
> create mode 100644 dts/framework/config/__init__.py
> create mode 100644 dts/framework/config/conf_yaml_schema.json
> create mode 100644 dts/framework/settings.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> new file mode 100644
> index 0000000000..cb12ea3d0f
> --- /dev/null
> +++ b/dts/conf.yaml
> @@ -0,0 +1,7 @@
> +executions:
> + - system_under_test: "SUT 1"
> +nodes:
> + - name: "SUT 1"
> + hostname: "SUT IP address or hostname"
> + user: root
> + password: "Leave blank to use SSH keys"
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> new file mode 100644
> index 0000000000..a0fdffcd77
> --- /dev/null
> +++ b/dts/framework/config/__init__.py
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2010-2021 Intel Corporation
> +# Copyright(c) 2022 University of New Hampshire
> +#
> +
> +"""
> +Generic port and topology nodes configuration file load function
> +"""
> +import json
> +import os.path
> +import pathlib
> +from dataclasses import dataclass
> +from typing import Any, Optional
> +
> +import warlock
> +import yaml
> +
> +from framework.settings import SETTINGS
> +
> +
> +# 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 NodeConfiguration:
> + name: str
> + hostname: str
> + user: str
> + password: Optional[str]
> +
> + @staticmethod
> + def from_dict(d: dict) -> "NodeConfiguration":
> + return NodeConfiguration(
> + name=d["name"],
> + hostname=d["hostname"],
> + user=d["user"],
> + password=d.get("password"),
> + )
> +
Out of curiosity, what is the reason for having a static "from_dict" method
rather than just a regular constructor function that takes a dict as
parameter?
> +
> +@dataclass(slots=True, frozen=True)
> +class ExecutionConfiguration:
> + system_under_test: NodeConfiguration
> +
Minor comment: seems strange having only a single member variable in this
class, effectively duplicating the class above.
> + @staticmethod
> + def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
from reading the code it appears that node_map is a dict of
NodeConfiguration objects, right? Might be worth adding that to the
definition for clarity, and also the specific type of the dict "d" (if it
has one)
> + sut_name = d["system_under_test"]
> + assert sut_name in node_map, f"Unknown SUT {sut_name} in execution {d}"
> +
> + return ExecutionConfiguration(
> + system_under_test=node_map[sut_name],
> + )
> +
> +
> +@dataclass(slots=True, frozen=True)
> +class Configuration:
> + executions: list[ExecutionConfiguration]
> +
> + @staticmethod
> + def from_dict(d: dict) -> "Configuration":
> + nodes: list[NodeConfiguration] = list(
> + map(NodeConfiguration.from_dict, d["nodes"])
So "d" is a dict of dicts?
> + )
> + assert len(nodes) > 0, "There must be a node to test"
> +
> + node_map = {node.name: node for node in nodes}
> + assert len(nodes) == len(node_map), "Duplicate node names are not allowed"
> +
> + executions: list[ExecutionConfiguration] = list(
> + map(
> + ExecutionConfiguration.from_dict, d["executions"], [node_map for _ in d]
> + )
> + )
> +
> + return Configuration(executions=executions)
> +
> +
> +def load_config() -> Configuration:
> + """
> + Loads the configuration file and the configuration file schema,
> + validates the configuration file, and creates a configuration object.
> + """
> + with open(SETTINGS.config_file_path, "r") as f:
> + config_data = yaml.safe_load(f)
> +
> + schema_path = os.path.join(
> + pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"
> + )
> +
> + 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))
> + return config_obj
> +
> +
> +CONFIGURATION = load_config()
<snip>
<snip>
> > +# 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 NodeConfiguration:
> > + name: str
> > + hostname: str
> > + user: str
> > + password: Optional[str]
> > +
> > + @staticmethod
> > + def from_dict(d: dict) -> "NodeConfiguration":
> > + return NodeConfiguration(
> > + name=d["name"],
> > + hostname=d["hostname"],
> > + user=d["user"],
> > + password=d.get("password"),
> > + )
> > +
> Out of curiosity, what is the reason for having a static "from_dict" method
> rather than just a regular constructor function that takes a dict as
> parameter?
>
@dataclass(...) is a class annotation that transforms the thing it
annotates into a dataclass. This means it creates the constructor for you
based on the property type annotations. If you create your own constructor,
you need a constructor that can either take a single dictionary or all of
the parameters like a normal constructor. Making it a static method also
means that each class can manage how it should be constructed from a
dictionary. Some of the other classes will transform lists or perform other
assertions. It also makes it easier to have specialized types. For
instance, a NICConfiguration class would have to handle all of the possible
device arguments that could be passed to any PMD driver if things were
passed as parameters.
> > +
> > +@dataclass(slots=True, frozen=True)
> > +class ExecutionConfiguration:
> > + system_under_test: NodeConfiguration
> > +
> Minor comment: seems strange having only a single member variable in this
> class, effectively duplicating the class above.
>
More is intended to go here. For instance, what tests to run, configuration
for virtual machines, the traffic generator node.
<snip>
> > + @staticmethod
> > + def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
> from reading the code it appears that node_map is a dict of
> NodeConfiguration objects, right? Might be worth adding that to the
> definition for clarity, and also the specific type of the dict "d" (if it
> has one)
> > + sut_name = d["system_under_test"]
> > + assert sut_name in node_map, f"Unknown SUT {sut_name} in
> execution {d}"
> > +
> > + return ExecutionConfiguration(
> > + system_under_test=node_map[sut_name],
> > + )
> > +
> > +
> > +@dataclass(slots=True, frozen=True)
> > +class Configuration:
> > + executions: list[ExecutionConfiguration]
> > +
> > + @staticmethod
> > + def from_dict(d: dict) -> "Configuration":
> > + nodes: list[NodeConfiguration] = list(
> > + map(NodeConfiguration.from_dict, d["nodes"])
> So "d" is a dict of dicts?
>
d is a dictionary which matches the json schema for the class. In the case
of the Configuration class, it is a dictionary matching the entire json
schema.
> > + )
> > + assert len(nodes) > 0, "There must be a node to test"
> > +
> > + node_map = {node.name: node for node in nodes}
> > + assert len(nodes) == len(node_map), "Duplicate node names are
> not allowed"
> > +
> > + executions: list[ExecutionConfiguration] = list(
> > + map(
> > + ExecutionConfiguration.from_dict, d["executions"],
> [node_map for _ in d]
> > + )
> > + )
> > +
> > + return Configuration(executions=executions)
> > +
> > +
> > +def load_config() -> Configuration:
> > + """
> > + Loads the configuration file and the configuration file schema,
> > + validates the configuration file, and creates a configuration
> object.
> > + """
> > + with open(SETTINGS.config_file_path, "r") as f:
> > + config_data = yaml.safe_load(f)
> > +
> > + schema_path = os.path.join(
> > + pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"
> > + )
> > +
> > + 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))
> > + return config_obj
> > +
> > +
> > +CONFIGURATION = load_config()
> <snip>
On Tue, Sep 13, 2022 at 01:47:10PM -0400, Owen Hilyard wrote:
> <snip>
>
> > +# 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 NodeConfiguration:
> > + name: str
> > + hostname: str
> > + user: str
> > + password: Optional[str]
> > +
> > + @staticmethod
> > + def from_dict(d: dict) -> "NodeConfiguration":
> > + return NodeConfiguration(
> > + name=d["name"],
> > + hostname=d["hostname"],
> > + user=d["user"],
> > + password=d.get("password"),
> > + )
> > +
> Out of curiosity, what is the reason for having a static "from_dict"
> method
> rather than just a regular constructor function that takes a dict as
> parameter?
>
> @dataclass(...) is a class annotation that transforms the thing it
> annotates into a dataclass. This means it creates the constructor for
> you based on the property type annotations. If you create your own
> constructor, you need a constructor that can either take a single
> dictionary or all of the parameters like a normal constructor. Making
> it a static method also means that each class can manage how it should
> be constructed from a dictionary. Some of the other classes will
> transform lists or perform other assertions. It also makes it easier to
> have specialized types. For instance, a NICConfiguration class would
> have to handle all of the possible device arguments that could be
> passed to any PMD driver if things were passed as parameters.
>
> > +
> > +@dataclass(slots=True, frozen=True)
> > +class ExecutionConfiguration:
> > + system_under_test: NodeConfiguration
> > +
> Minor comment: seems strange having only a single member variable in
> this
> class, effectively duplicating the class above.
>
> More is intended to go here. For instance, what tests to run,
> configuration for virtual machines, the traffic generator node.
>
> <snip>
>
Thanks for all the explanations.
new file mode 100644
@@ -0,0 +1,7 @@
+executions:
+ - system_under_test: "SUT 1"
+nodes:
+ - name: "SUT 1"
+ hostname: "SUT IP address or hostname"
+ user: root
+ password: "Leave blank to use SSH keys"
new file mode 100644
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2010-2021 Intel Corporation
+# Copyright(c) 2022 University of New Hampshire
+#
+
+"""
+Generic port and topology nodes configuration file load function
+"""
+import json
+import os.path
+import pathlib
+from dataclasses import dataclass
+from typing import Any, Optional
+
+import warlock
+import yaml
+
+from framework.settings import SETTINGS
+
+
+# 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 NodeConfiguration:
+ name: str
+ hostname: str
+ user: str
+ password: Optional[str]
+
+ @staticmethod
+ def from_dict(d: dict) -> "NodeConfiguration":
+ return NodeConfiguration(
+ name=d["name"],
+ hostname=d["hostname"],
+ user=d["user"],
+ password=d.get("password"),
+ )
+
+
+@dataclass(slots=True, frozen=True)
+class ExecutionConfiguration:
+ system_under_test: NodeConfiguration
+
+ @staticmethod
+ def from_dict(d: dict, node_map: dict) -> "ExecutionConfiguration":
+ sut_name = d["system_under_test"]
+ assert sut_name in node_map, f"Unknown SUT {sut_name} in execution {d}"
+
+ return ExecutionConfiguration(
+ system_under_test=node_map[sut_name],
+ )
+
+
+@dataclass(slots=True, frozen=True)
+class Configuration:
+ executions: list[ExecutionConfiguration]
+
+ @staticmethod
+ def from_dict(d: dict) -> "Configuration":
+ nodes: list[NodeConfiguration] = list(
+ map(NodeConfiguration.from_dict, d["nodes"])
+ )
+ assert len(nodes) > 0, "There must be a node to test"
+
+ node_map = {node.name: node for node in nodes}
+ assert len(nodes) == len(node_map), "Duplicate node names are not allowed"
+
+ executions: list[ExecutionConfiguration] = list(
+ map(
+ ExecutionConfiguration.from_dict, d["executions"], [node_map for _ in d]
+ )
+ )
+
+ return Configuration(executions=executions)
+
+
+def load_config() -> Configuration:
+ """
+ Loads the configuration file and the configuration file schema,
+ validates the configuration file, and creates a configuration object.
+ """
+ with open(SETTINGS.config_file_path, "r") as f:
+ config_data = yaml.safe_load(f)
+
+ schema_path = os.path.join(
+ pathlib.Path(__file__).parent.resolve(), "conf_yaml_schema.json"
+ )
+
+ 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))
+ return config_obj
+
+
+CONFIGURATION = load_config()
new file mode 100644
@@ -0,0 +1,73 @@
+{
+ "$schema": "https://json-schema.org/draft-07/schema",
+ "title": "DPDK DTS Config Schema",
+ "definitions": {
+ "node_name": {
+ "type": "string",
+ "description": "A unique identifier for a node"
+ },
+ "node_role": {
+ "type": "string",
+ "description": "The role a node plays in DTS",
+ "enum": [
+ "system_under_test",
+ "traffic_generator"
+ ]
+ }
+ },
+ "type": "object",
+ "properties": {
+ "nodes": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "properties": {
+ "name": {
+ "type": "string",
+ "description": "A unique identifier for this node"
+ },
+ "hostname": {
+ "type": "string",
+ "description": "A hostname from which the node running DTS can access this node. This can also be an IP address."
+ },
+ "user": {
+ "type": "string",
+ "description": "The user to access this node with."
+ },
+ "password": {
+ "type": "string",
+ "description": "The password to use on this node. SSH keys are preferred."
+ }
+ },
+ "additionalProperties": false,
+ "required": [
+ "name",
+ "hostname",
+ "user"
+ ]
+ },
+ "minimum": 1
+ },
+ "executions": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "properties": {
+ "system_under_test": {
+ "$ref": "#/definitions/node_name"
+ }
+ },
+ "additionalProperties": false,
+ "required": [
+ "system_under_test"
+ ]
+ },
+ "minimum": 1
+ }
+ },
+ "required": [
+ "executions",
+ "nodes"
+ ],
+ "additionalProperties": false
+}
@@ -55,3 +55,17 @@ def __init__(self, host: str):
def __str__(self) -> str:
return f"SSH session with {self.host} has died"
+
+
+class ConfigParseException(Exception):
+ """
+ Configuration file parse failure exception.
+ """
+
+ config: str
+
+ def __init__(self, conf_file: str):
+ self.config = conf_file
+
+ def __str__(self) -> str:
+ return f"Failed to parse config file [{self.config}]"
new file mode 100644
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2010-2021 Intel Corporation
+# Copyright(c) 2022 PANTHEON.tech s.r.o.
+# Copyright(c) 2022 University of New Hampshire
+#
+
+import argparse
+import os
+from dataclasses import dataclass
+from typing import Any
+
+
+class _EnvironmentArgument(argparse.Action):
+ def __init__(
+ self, env_var: str, required: bool = True, default: Any = None, **kwargs
+ ):
+ env_var_value = os.environ.get(env_var)
+ default = env_var_value or default
+ super(_EnvironmentArgument, self).__init__(
+ default=default, required=default is None and required, **kwargs
+ )
+
+ def __call__(
+ self,
+ parser: argparse.ArgumentParser,
+ namespace: argparse.Namespace,
+ values: Any,
+ option_string: str = None,
+ ) -> None:
+ setattr(namespace, self.dest, values)
+
+
+def _env_arg(envvar: str) -> Any:
+ def wrapper(**kwargs) -> _EnvironmentArgument:
+ return _EnvironmentArgument(envvar, **kwargs)
+
+ return wrapper
+
+
+@dataclass(slots=True, frozen=True)
+class _Settings:
+ config_file_path: str
+
+
+def _get_parser() -> argparse.ArgumentParser:
+ parser = argparse.ArgumentParser(description="DPDK test framework.")
+
+ parser.add_argument(
+ "--config-file",
+ action=_env_arg("DTS_CFG_FILE"),
+ default="conf.yaml",
+ help="[DTS_CFG_FILE] configuration file that describes the test cases, SUTs and targets",
+ )
+
+ return parser
+
+
+def _get_settings() -> _Settings:
+ args = _get_parser().parse_args()
+ return _Settings(
+ config_file_path=args.config_file,
+ )
+
+
+SETTINGS: _Settings = _get_settings()