* [PATCH v1] dts: Testbed And Node Configuration Split
@ 2024-06-10 19:34 Nicholas Pratte
2024-06-10 19:37 ` Nicholas Pratte
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Nicholas Pratte @ 2024-06-10 19:34 UTC (permalink / raw)
To: luca.vizzarro, juraj.linkes, probb, jspewock, bruce.richardson,
yoan.picchi, paul.szczepanek, Honnappa.Nagarahalli
Cc: dev, Nicholas Pratte
This implementation splits the execution and node configuration
components of the conf.yaml into two separate config files. A
new command line argument is added, allowing the user to specify
both a node configuration file and an execution configuration
file. Be default, these config files are now named node_conf.yaml and
execution_conf.yaml, respectively.
To assert these changes, the schema calls for one of these objects,
nodes or executions, in each file, but neither config file can have
both.
To avoid excess refactoring, both config files are merged together
early on in the load_config process.
Bugzilla ID: 1344
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/execution_conf.yaml | 26 ++
dts/framework/config/__init__.py | 26 +-
dts/framework/config/conf_yaml_schema.json | 340 ++++++++++-----------
dts/framework/runner.py | 4 +-
dts/framework/settings.py | 31 +-
dts/node_conf.yaml | 56 ++++
dts/testbed_conf.yaml | 26 ++
7 files changed, 322 insertions(+), 187 deletions(-)
create mode 100644 dts/execution_conf.yaml
create mode 100644 dts/node_conf.yaml
create mode 100644 dts/testbed_conf.yaml
diff --git a/dts/execution_conf.yaml b/dts/execution_conf.yaml
new file mode 100644
index 0000000000..af2180eac2
--- /dev/null
+++ b/dts/execution_conf.yaml
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+executions:
+ # define one execution environment
+ - build_targets:
+ - arch: x86_64
+ os: linux
+ cpu: native
+ # the combination of the following two makes CC="ccache gcc"
+ compiler: gcc
+ compiler_wrapper: ccache
+ perf: false # disable performance testing
+ func: true # enable functional testing
+ skip_smoke_tests: false # optional
+ test_suites: # the following test suites will be run in their entirety
+ - hello_world
+ - os_udp
+ # The machine running the DPDK test executable
+ system_under_test_node:
+ node_name: "SUT 1"
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
+ # Traffic generator node to use for this execution environment
+ traffic_generator_node: "TG 1"
\ No newline at end of file
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 4cb5c74059..b56fec9919 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -570,29 +570,39 @@ def from_dict(d: ConfigurationDict) -> "Configuration":
return Configuration(executions=executions)
-def load_config(config_file_path: Path) -> Configuration:
+def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
"""Load DTS test run configuration from a file.
- Load the YAML test run configuration file
+ Load both the YAML testbed and execution configuration files
and :download:`the configuration file schema <conf_yaml_schema.json>`,
- validate the test run configuration file, and create a test run configuration object.
+ validate both configuration files to 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.
Args:
- config_file_path: The path to the YAML test run configuration file.
+ node_config_file_path: The path to the testbed configuration YAML file.
+ exec_config_file_path: The path to the execution configuration YAML file.
Returns:
The parsed test run configuration.
"""
- with open(config_file_path, "r") as f:
- config_data = yaml.safe_load(f)
+ with open(node_config_file_path, "r") as f:
+ node_config_data = yaml.safe_load(f)
+ with open(exec_config_file_path, "r") as f:
+ execution_config_data = yaml.safe_load(f)
schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
with open(schema_path, "r") as f:
schema = json.load(f)
- config = warlock.model_factory(schema, name="_Config")(config_data)
- config_obj: Configuration = Configuration.from_dict(dict(config)) # type: ignore[arg-type]
+ config = {
+ **dict(warlock.model_factory(schema, name="_Config")(node_config_data)),
+ **dict(warlock.model_factory(schema, name="_Config")(execution_config_data)),
+ }
+ if "nodes" not in config or "executions" not in config:
+ raise ConfigurationError(
+ f"{'node' if 'nodes' not in config else 'execution'} data not configured."
+ )
+ config_obj: Configuration = Configuration.from_dict(config) # type: ignore[arg-type]
return config_obj
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 4731f4511d..7edfe72f00 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -211,192 +211,192 @@
"additionalProperties": false
}
},
- "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. Use only as a last resort. SSH keys are STRONGLY preferred."
- },
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
- "os": {
- "$ref": "#/definitions/OS"
- },
- "lcores": {
- "type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
- "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
- },
- "use_first_core": {
- "type": "boolean",
- "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
- },
- "memory_channels": {
- "type": "integer",
- "description": "How many memory channels to use. Optional, defaults to 1."
- },
- "hugepages": {
- "$ref": "#/definitions/hugepages"
- },
- "ports": {
- "type": "array",
- "items": {
- "type": "object",
- "description": "Each port should be described on both sides of the connection. This makes configuration slightly more verbose but greatly simplifies implementation. If there are inconsistencies, then DTS will not run until that issue is fixed. An example inconsistency would be port 1, node 1 says it is connected to port 1, node 2, but port 1, node 2 says it is connected to port 2, node 1.",
- "properties": {
- "pci": {
- "$ref": "#/definitions/pci_address",
- "description": "The local PCI address of the port"
- },
- "os_driver_for_dpdk": {
- "type": "string",
- "description": "The driver that the kernel should bind this device to for DPDK to use it. (ex: vfio-pci)"
- },
- "os_driver": {
- "type": "string",
- "description": "The driver normally used by this port (ex: i40e)"
- },
- "peer_node": {
- "type": "string",
- "description": "The name of the node the peer port is on"
+ "oneOf": [
+ {
+ "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. Use only as a last resort. SSH keys are STRONGLY preferred."
+ },
+ "arch": {
+ "$ref": "#/definitions/ARCH"
+ },
+ "os": {
+ "$ref": "#/definitions/OS"
+ },
+ "lcores": {
+ "type": "string",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
+ "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
+ },
+ "use_first_core": {
+ "type": "boolean",
+ "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
+ },
+ "memory_channels": {
+ "type": "integer",
+ "description": "How many memory channels to use. Optional, defaults to 1."
+ },
+ "hugepages": {
+ "$ref": "#/definitions/hugepages"
+ },
+ "ports": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "description": "Each port should be described on both sides of the connection. This makes configuration slightly more verbose but greatly simplifies implementation. If there are inconsistencies, then DTS will not run until that issue is fixed. An example inconsistency would be port 1, node 1 says it is connected to port 1, node 2, but port 1, node 2 says it is connected to port 2, node 1.",
+ "properties": {
+ "pci": {
+ "$ref": "#/definitions/pci_address",
+ "description": "The local PCI address of the port"
+ },
+ "os_driver_for_dpdk": {
+ "type": "string",
+ "description": "The driver that the kernel should bind this device to for DPDK to use it. (ex: vfio-pci)"
+ },
+ "os_driver": {
+ "type": "string",
+ "description": "The driver normally used by this port (ex: i40e)"
+ },
+ "peer_node": {
+ "type": "string",
+ "description": "The name of the node the peer port is on"
+ },
+ "peer_pci": {
+ "$ref": "#/definitions/pci_address",
+ "description": "The PCI address of the peer port"
+ }
+ },
+ "additionalProperties": false,
+ "required": [
+ "pci",
+ "os_driver_for_dpdk",
+ "os_driver",
+ "peer_node",
+ "peer_pci"
+ ]
},
- "peer_pci": {
- "$ref": "#/definitions/pci_address",
- "description": "The PCI address of the peer port"
- }
+ "minimum": 1
},
- "additionalProperties": false,
- "required": [
- "pci",
- "os_driver_for_dpdk",
- "os_driver",
- "peer_node",
- "peer_pci"
- ]
- },
- "minimum": 1
- },
- "traffic_generator": {
- "oneOf": [
- {
- "type": "object",
- "description": "Scapy traffic generator. Used for functional testing.",
- "properties": {
- "type": {
- "type": "string",
- "enum": [
- "SCAPY"
- ]
+ "traffic_generator": {
+ "oneOf": [
+ {
+ "type": "object",
+ "description": "Scapy traffic generator. Used for functional testing.",
+ "properties": {
+ "type": {
+ "type": "string",
+ "enum": [
+ "SCAPY"
+ ]
+ }
+ }
}
- }
+ ]
}
- ]
- }
- },
- "additionalProperties": false,
- "required": [
- "name",
- "hostname",
- "user",
- "arch",
- "os"
- ]
- },
- "minimum": 1
- },
- "executions": {
- "type": "array",
- "items": {
- "type": "object",
- "properties": {
- "build_targets": {
- "type": "array",
- "items": {
- "$ref": "#/definitions/build_target"
},
- "minimum": 1
- },
- "perf": {
- "type": "boolean",
- "description": "Enable performance testing."
- },
- "func": {
- "type": "boolean",
- "description": "Enable functional testing."
- },
- "test_suites": {
- "type": "array",
- "items": {
- "oneOf": [
- {
- "$ref": "#/definitions/test_suite"
- },
- {
- "$ref": "#/definitions/test_target"
- }
- ]
- }
- },
- "skip_smoke_tests": {
- "description": "Optional field that allows you to skip smoke testing",
- "type": "boolean"
+ "additionalProperties": false,
+ "required": [
+ "name",
+ "hostname",
+ "user",
+ "arch",
+ "os"
+ ]
},
- "system_under_test_node": {
- "type":"object",
+ "minimum": 1
+ },
+ "executions": {
+ "type": "array",
+ "items": {
+ "type": "object",
"properties": {
- "node_name": {
- "$ref": "#/definitions/node_name"
+ "build_targets": {
+ "type": "array",
+ "items": {
+ "$ref": "#/definitions/build_target"
+ },
+ "minimum": 1
+ },
+ "perf": {
+ "type": "boolean",
+ "description": "Enable performance testing."
+ },
+ "func": {
+ "type": "boolean",
+ "description": "Enable functional testing."
},
- "vdevs": {
- "description": "Optional list of names of vdevs to be used in execution",
+ "test_suites": {
"type": "array",
"items": {
- "type": "string"
+ "oneOf": [
+ {
+ "$ref": "#/definitions/test_suite"
+ },
+ {
+ "$ref": "#/definitions/test_target"
+ }
+ ]
}
+ },
+ "skip_smoke_tests": {
+ "description": "Optional field that allows you to skip smoke testing",
+ "type": "boolean"
+ },
+ "system_under_test_node": {
+ "type":"object",
+ "properties": {
+ "node_name": {
+ "$ref": "#/definitions/node_name"
+ },
+ "vdevs": {
+ "description": "Optional list of names of vdevs to be used in execution",
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ }
+ },
+ "required": [
+ "node_name"
+ ]
+ },
+ "traffic_generator_node": {
+ "$ref": "#/definitions/node_name"
}
},
+ "additionalProperties": false,
"required": [
- "node_name"
+ "build_targets",
+ "perf",
+ "func",
+ "test_suites",
+ "system_under_test_node",
+ "traffic_generator_node"
]
},
- "traffic_generator_node": {
- "$ref": "#/definitions/node_name"
- }
- },
- "additionalProperties": false,
- "required": [
- "build_targets",
- "perf",
- "func",
- "test_suites",
- "system_under_test_node",
- "traffic_generator_node"
- ]
+ "minimum": 1
+ }
},
- "minimum": 1
+ "additionalProperties": false
}
- },
- "required": [
- "executions",
- "nodes"
- ],
- "additionalProperties": false
+ ]
}
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index db8e3ba96b..373f2215a6 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -83,7 +83,9 @@ class DTSRunner:
def __init__(self):
"""Initialize the instance with configuration, logger, result and string constants."""
- self._configuration = load_config(SETTINGS.config_file_path)
+ self._configuration = load_config(
+ SETTINGS.node_config_file_path, SETTINGS.exec_config_file_path
+ )
self._logger = get_dts_logger()
if not os.path.exists(SETTINGS.output_dir):
os.makedirs(SETTINGS.output_dir)
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index 688e8679a7..ca78a3e72e 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -13,10 +13,15 @@
The command line arguments along with the supported environment variables are:
-.. option:: --config-file
-.. envvar:: DTS_CFG_FILE
+.. option:: --node-config-file
+.. envvar:: DTS_NODE_CFG_FILE
- The path to the YAML test run configuration file.
+ The path to the YAML testbed configuration file.
+
+.. option:: --exec-config-file
+.. envvar:: DTS_EXEC_CFG_FILE
+
+ The path to the YAML execution configuration file.
.. option:: --output-dir, --output
.. envvar:: DTS_OUTPUT_DIR
@@ -88,7 +93,9 @@ class Settings:
"""
#:
- config_file_path: Path = Path(__file__).parent.parent.joinpath("conf.yaml")
+ node_config_file_path: Path = Path(__file__).parent.parent.joinpath("node_conf.yaml")
+ #:
+ exec_config_file_path: Path = Path(__file__).parent.parent.joinpath("execution_conf.yaml")
#:
output_dir: str = "output"
#:
@@ -143,13 +150,20 @@ def env_arg(env_var: str, default: Any) -> Any:
)
parser.add_argument(
- "--config-file",
- default=env_arg("DTS_CFG_FILE", SETTINGS.config_file_path),
+ "--node-config-file",
+ default=env_arg("DTS_NODE_CFG_FILE", SETTINGS.node_config_file_path),
type=Path,
- help="[DTS_CFG_FILE] The configuration file that describes the test cases, "
+ help="[DTS_NODE_CFG_FILE] The configuration file that describes the testbed devices, "
"SUTs and targets.",
)
+ parser.add_argument(
+ "--exec-config-file",
+ default=env_arg("DTS_EXEC_CFG_FILE", SETTINGS.exec_config_file_path),
+ type=Path,
+ help="[DTS_EXEC_CFG_FILE] The configuration file that describes the test cases.",
+ )
+
parser.add_argument(
"--output-dir",
"--output",
@@ -263,7 +277,8 @@ def get_settings() -> Settings:
"""
parsed_args = _get_parser().parse_args()
return Settings(
- config_file_path=parsed_args.config_file,
+ node_config_file_path=parsed_args.node_config_file,
+ exec_config_file_path=parsed_args.exec_config_file,
output_dir=parsed_args.output_dir,
timeout=parsed_args.timeout,
verbose=parsed_args.verbose,
diff --git a/dts/node_conf.yaml b/dts/node_conf.yaml
new file mode 100644
index 0000000000..c7e64a00e0
--- /dev/null
+++ b/dts/node_conf.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+nodes:
+ # Define a system under test node, having two network ports physically
+ # connected to the corresponding ports in TG 1 (the peer node)
+ - name: "SUT 1"
+ hostname: sut1.change.me.localhost
+ user: dtsuser
+ arch: x86_64
+ os: linux
+ lcores: "" # use all the available logical cores
+ use_first_core: false # tells DPDK to use any physical core
+ memory_channels: 4 # tells DPDK to use 4 memory channels
+ hugepages: # optional; if removed, will use system hugepage configuration
+ amount: 256
+ force_first_numa: false
+ ports:
+ # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
+ - pci: "0000:00:08.0"
+ os_driver_for_dpdk: vfio-pci # OS driver that DPDK will use
+ os_driver: i40e # OS driver to bind when the tests are not running
+ peer_node: "TG 1"
+ peer_pci: "0000:00:08.0"
+ # sets up the physical link between "SUT 1"@000:00:08.1 and "TG 1"@0000:00:08.1
+ - pci: "0000:00:08.1"
+ os_driver_for_dpdk: vfio-pci
+ os_driver: i40e
+ peer_node: "TG 1"
+ peer_pci: "0000:00:08.1"
+ # Define a Scapy traffic generator node, having two network ports
+ # physically connected to the corresponding ports in SUT 1 (the peer node).
+ - name: "TG 1"
+ hostname: tg1.change.me.localhost
+ user: dtsuser
+ arch: x86_64
+ os: linux
+ ports:
+ # sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
+ - pci: "0000:00:08.0"
+ os_driver_for_dpdk: rdma
+ os_driver: rdma
+ peer_node: "SUT 1"
+ peer_pci: "0000:00:08.0"
+ # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
+ - pci: "0000:00:08.1"
+ os_driver_for_dpdk: rdma
+ os_driver: rdma
+ peer_node: "SUT 1"
+ peer_pci: "0000:00:08.1"
+ hugepages: # optional; if removed, will use system hugepage configuration
+ amount: 256
+ force_first_numa: false
+ traffic_generator:
+ type: SCAPY
diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml
new file mode 100644
index 0000000000..af2180eac2
--- /dev/null
+++ b/dts/testbed_conf.yaml
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+executions:
+ # define one execution environment
+ - build_targets:
+ - arch: x86_64
+ os: linux
+ cpu: native
+ # the combination of the following two makes CC="ccache gcc"
+ compiler: gcc
+ compiler_wrapper: ccache
+ perf: false # disable performance testing
+ func: true # enable functional testing
+ skip_smoke_tests: false # optional
+ test_suites: # the following test suites will be run in their entirety
+ - hello_world
+ - os_udp
+ # The machine running the DPDK test executable
+ system_under_test_node:
+ node_name: "SUT 1"
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
+ # Traffic generator node to use for this execution environment
+ traffic_generator_node: "TG 1"
\ No newline at end of file
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] dts: Testbed And Node Configuration Split
2024-06-10 19:34 [PATCH v1] dts: Testbed And Node Configuration Split Nicholas Pratte
@ 2024-06-10 19:37 ` Nicholas Pratte
2024-06-14 18:32 ` Jeremy Spewock
2024-06-14 18:27 ` Jeremy Spewock
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
2 siblings, 1 reply; 10+ messages in thread
From: Nicholas Pratte @ 2024-06-10 19:37 UTC (permalink / raw)
To: luca.vizzarro, juraj.linkes, probb, jspewock, bruce.richardson,
yoan.picchi, paul.szczepanek, Honnappa.Nagarahalli
Cc: dev
<snip>
> -def load_config(config_file_path: Path) -> Configuration:
> +def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
> """Load DTS test run configuration from a file.
>
> - Load the YAML test run configuration file
> + Load both the YAML testbed and execution configuration files
> and :download:`the configuration file schema <conf_yaml_schema.json>`,
> - validate the test run configuration file, and create a test run configuration object.
> + validate both configuration files to 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.
>
> Args:
> - config_file_path: The path to the YAML test run configuration file.
> + node_config_file_path: The path to the testbed configuration YAML file.
> + exec_config_file_path: The path to the execution configuration YAML file.
>
> Returns:
> The parsed test run configuration.
> """
> - with open(config_file_path, "r") as f:
> - config_data = yaml.safe_load(f)
> + with open(node_config_file_path, "r") as f:
> + node_config_data = yaml.safe_load(f)
> + with open(exec_config_file_path, "r") as f:
> + execution_config_data = yaml.safe_load(f)
>
> schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
>
> with open(schema_path, "r") as f:
> schema = json.load(f)
> - config = warlock.model_factory(schema, name="_Config")(config_data)
> - config_obj: Configuration = Configuration.from_dict(dict(config)) # type: ignore[arg-type]
> + config = {
> + **dict(warlock.model_factory(schema, name="_Config")(node_config_data)),
> + **dict(warlock.model_factory(schema, name="_Config")(execution_config_data)),
> + }
> + if "nodes" not in config or "executions" not in config:
> + raise ConfigurationError(
> + f"{'node' if 'nodes' not in config else 'execution'} data not configured."
> + )
> + config_obj: Configuration = Configuration.from_dict(config) # type: ignore[arg-type]
> return config_obj
There is an alternative approach to this that I currently have
implemented on a separate local branch. Essentially, both configs are
merged together before being validated by the schema, effectively
removing the need to change the schema at all as well as the above
assertion that both nodes and executions are in the two config files.
However, using this alternative method might mean that there is no added
control that prevents users from making funky errors when creating
both config files. I could also just be overthinking or missing
something.
<snip>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] dts: Testbed And Node Configuration Split
2024-06-10 19:34 [PATCH v1] dts: Testbed And Node Configuration Split Nicholas Pratte
2024-06-10 19:37 ` Nicholas Pratte
@ 2024-06-14 18:27 ` Jeremy Spewock
2024-09-10 9:00 ` Juraj Linkeš
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
2 siblings, 1 reply; 10+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:27 UTC (permalink / raw)
To: Nicholas Pratte
Cc: luca.vizzarro, juraj.linkes, probb, bruce.richardson,
yoan.picchi, paul.szczepanek, Honnappa.Nagarahalli, dev
I think this is definitely a step in the right direction in terms of
how we structure the config files. Something that I think could also
be a cool improvement for how we handle configs is just making a
`conf/` directory and then taking all of the configuration in each of
the yaml files present in the directory and combining them into one
config in the framework like we have now. That's definitely a larger
undertaking however, this split will save space if users want to
specify more than one host or use the same execution configuration
between different node configurations and I think that is an
improvement on its own.
On Mon, Jun 10, 2024 at 3:34 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> This implementation splits the execution and node configuration
> components of the conf.yaml into two separate config files. A
> new command line argument is added, allowing the user to specify
> both a node configuration file and an execution configuration
> file. Be default, these config files are now named node_conf.yaml and
> execution_conf.yaml, respectively.
>
> To assert these changes, the schema calls for one of these objects,
> nodes or executions, in each file, but neither config file can have
> both.
>
> To avoid excess refactoring, both config files are merged together
> early on in the load_config process.
>
> Bugzilla ID: 1344
>
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
<snip>
> diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml
> new file mode 100644
> index 0000000000..af2180eac2
> --- /dev/null
> +++ b/dts/testbed_conf.yaml
I don't think this file is used anywhere and it looks like another
name for the execution_conf.yaml so we likely don't need it.
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +executions:
> + # define one execution environment
> + - build_targets:
> + - arch: x86_64
> + os: linux
> + cpu: native
> + # the combination of the following two makes CC="ccache gcc"
> + compiler: gcc
> + compiler_wrapper: ccache
> + perf: false # disable performance testing
> + func: true # enable functional testing
> + skip_smoke_tests: false # optional
> + test_suites: # the following test suites will be run in their entirety
> + - hello_world
> + - os_udp
> + # The machine running the DPDK test executable
> + system_under_test_node:
> + node_name: "SUT 1"
> + vdevs: # optional; if removed, vdevs won't be used in the execution
> + - "crypto_openssl"
> + # Traffic generator node to use for this execution environment
> + traffic_generator_node: "TG 1"
> \ No newline at end of file
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] dts: Testbed And Node Configuration Split
2024-06-10 19:37 ` Nicholas Pratte
@ 2024-06-14 18:32 ` Jeremy Spewock
2024-09-10 8:57 ` Juraj Linkeš
0 siblings, 1 reply; 10+ messages in thread
From: Jeremy Spewock @ 2024-06-14 18:32 UTC (permalink / raw)
To: Nicholas Pratte
Cc: luca.vizzarro, juraj.linkes, probb, bruce.richardson,
yoan.picchi, paul.szczepanek, Honnappa.Nagarahalli, dev
On Mon, Jun 10, 2024 at 3:37 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> <snip>
>
> > -def load_config(config_file_path: Path) -> Configuration:
> > +def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
> > """Load DTS test run configuration from a file.
> >
> > - Load the YAML test run configuration file
> > + Load both the YAML testbed and execution configuration files
> > and :download:`the configuration file schema <conf_yaml_schema.json>`,
> > - validate the test run configuration file, and create a test run configuration object.
> > + validate both configuration files to 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.
> >
> > Args:
> > - config_file_path: The path to the YAML test run configuration file.
> > + node_config_file_path: The path to the testbed configuration YAML file.
> > + exec_config_file_path: The path to the execution configuration YAML file.
> >
> > Returns:
> > The parsed test run configuration.
> > """
> > - with open(config_file_path, "r") as f:
> > - config_data = yaml.safe_load(f)
> > + with open(node_config_file_path, "r") as f:
> > + node_config_data = yaml.safe_load(f)
> > + with open(exec_config_file_path, "r") as f:
> > + execution_config_data = yaml.safe_load(f)
> >
> > schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
> >
> > with open(schema_path, "r") as f:
> > schema = json.load(f)
> > - config = warlock.model_factory(schema, name="_Config")(config_data)
> > - config_obj: Configuration = Configuration.from_dict(dict(config)) # type: ignore[arg-type]
> > + config = {
> > + **dict(warlock.model_factory(schema, name="_Config")(node_config_data)),
> > + **dict(warlock.model_factory(schema, name="_Config")(execution_config_data)),
> > + }
> > + if "nodes" not in config or "executions" not in config:
> > + raise ConfigurationError(
> > + f"{'node' if 'nodes' not in config else 'execution'} data not configured."
> > + )
> > + config_obj: Configuration = Configuration.from_dict(config) # type: ignore[arg-type]
> > return config_obj
>
> There is an alternative approach to this that I currently have
> implemented on a separate local branch. Essentially, both configs are
> merged together before being validated by the schema, effectively
> removing the need to change the schema at all as well as the above
> assertion that both nodes and executions are in the two config files.
> However, using this alternative method might mean that there is no added
> control that prevents users from making funky errors when creating
> both config files. I could also just be overthinking or missing
> something.
That's an interesting idea and on one hand it makes the schema a
little less confusing because it wouldn't need to say "oneOf" when
both of the attributes are actually required for DTS to run, but on
the other it removes the ability for developers to know if they made a
mistake without actually running DTS. Making it a runtime error
wouldn't be the worst thing since it would at least be very early in
the run that the error is shown, but it loses some of its purpose I
feel if your IDE doesn't also show you some of the errors before
actually using the config. I could go either way on it personally.
>
> <snip>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] dts: Testbed And Node Configuration Split
2024-06-10 19:34 [PATCH v1] dts: Testbed And Node Configuration Split Nicholas Pratte
2024-06-10 19:37 ` Nicholas Pratte
2024-06-14 18:27 ` Jeremy Spewock
@ 2024-07-03 14:57 ` Nicholas Pratte
2024-07-16 14:11 ` Jeremy Spewock
` (2 more replies)
2 siblings, 3 replies; 10+ messages in thread
From: Nicholas Pratte @ 2024-07-03 14:57 UTC (permalink / raw)
To: juraj.linkes, dmarx, luca.vizzarro, Honnappa.Nagarahalli,
paul.szczepanek, probb, jspewock, yoan.picchi
Cc: dev, Nicholas Pratte
This implementation splits the execution and node configuration
components of the conf.yaml into two separate config files. A
new command line argument is added, allowing the user to specify
both a node configuration file and an execution configuration
file. Be default, these config files are now named node_conf.yaml and
execution_conf.yaml, respectively.
To assert these changes, the schema calls for one of these objects,
nodes or executions, in each file, but neither config file can have
both.
To avoid excess refactoring, both config files are merged together
early on in the load_config process.
Bugzilla ID: 1344
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/execution_conf.yaml | 26 ++
dts/framework/config/__init__.py | 26 +-
dts/framework/config/conf_yaml_schema.json | 340 ++++++++++-----------
dts/framework/runner.py | 4 +-
dts/framework/settings.py | 36 ++-
dts/node_conf.yaml | 56 ++++
dts/testbed_conf.yaml | 26 ++
7 files changed, 325 insertions(+), 189 deletions(-)
create mode 100644 dts/execution_conf.yaml
create mode 100644 dts/node_conf.yaml
create mode 100644 dts/testbed_conf.yaml
diff --git a/dts/execution_conf.yaml b/dts/execution_conf.yaml
new file mode 100644
index 0000000000..af2180eac2
--- /dev/null
+++ b/dts/execution_conf.yaml
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+executions:
+ # define one execution environment
+ - build_targets:
+ - arch: x86_64
+ os: linux
+ cpu: native
+ # the combination of the following two makes CC="ccache gcc"
+ compiler: gcc
+ compiler_wrapper: ccache
+ perf: false # disable performance testing
+ func: true # enable functional testing
+ skip_smoke_tests: false # optional
+ test_suites: # the following test suites will be run in their entirety
+ - hello_world
+ - os_udp
+ # The machine running the DPDK test executable
+ system_under_test_node:
+ node_name: "SUT 1"
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
+ # Traffic generator node to use for this execution environment
+ traffic_generator_node: "TG 1"
\ No newline at end of file
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index df60a5030e..2322f3d996 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -573,29 +573,39 @@ def from_dict(cls, d: ConfigurationDict) -> Self:
return cls(test_runs=test_runs)
-def load_config(config_file_path: Path) -> Configuration:
+def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
"""Load DTS test run configuration from a file.
- Load the YAML test run configuration file
+ Load both the YAML testbed and execution configuration files
and :download:`the configuration file schema <conf_yaml_schema.json>`,
- validate the test run configuration file, and create a test run configuration object.
+ validate both configuration files to 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.
Args:
- config_file_path: The path to the YAML test run configuration file.
+ node_config_file_path: The path to the testbed configuration YAML file.
+ exec_config_file_path: The path to the execution configuration YAML file.
Returns:
The parsed test run configuration.
"""
- with open(config_file_path, "r") as f:
- config_data = yaml.safe_load(f)
+ with open(node_config_file_path, "r") as f:
+ node_config_data = yaml.safe_load(f)
+ with open(exec_config_file_path, "r") as f:
+ execution_config_data = yaml.safe_load(f)
schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
with open(schema_path, "r") as f:
schema = json.load(f)
- config = warlock.model_factory(schema, name="_Config")(config_data)
- config_obj: Configuration = Configuration.from_dict(dict(config)) # type: ignore[arg-type]
+ config = {
+ **dict(warlock.model_factory(schema, name="_Config")(node_config_data)),
+ **dict(warlock.model_factory(schema, name="_Config")(execution_config_data)),
+ }
+ if "nodes" not in config or "executions" not in config:
+ raise ConfigurationError(
+ f"{'node' if 'nodes' not in config else 'execution'} data not configured."
+ )
+ config_obj: Configuration = Configuration.from_dict(config) # type: ignore[arg-type]
return config_obj
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index f02a310bb5..1962ed05dd 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -211,192 +211,192 @@
"additionalProperties": false
}
},
- "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. Use only as a last resort. SSH keys are STRONGLY preferred."
- },
- "arch": {
- "$ref": "#/definitions/ARCH"
- },
- "os": {
- "$ref": "#/definitions/OS"
- },
- "lcores": {
- "type": "string",
- "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
- "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
- },
- "use_first_core": {
- "type": "boolean",
- "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
- },
- "memory_channels": {
- "type": "integer",
- "description": "How many memory channels to use. Optional, defaults to 1."
- },
- "hugepages_2mb": {
- "$ref": "#/definitions/hugepages_2mb"
- },
- "ports": {
- "type": "array",
- "items": {
- "type": "object",
- "description": "Each port should be described on both sides of the connection. This makes configuration slightly more verbose but greatly simplifies implementation. If there are inconsistencies, then DTS will not run until that issue is fixed. An example inconsistency would be port 1, node 1 says it is connected to port 1, node 2, but port 1, node 2 says it is connected to port 2, node 1.",
- "properties": {
- "pci": {
- "$ref": "#/definitions/pci_address",
- "description": "The local PCI address of the port"
- },
- "os_driver_for_dpdk": {
- "type": "string",
- "description": "The driver that the kernel should bind this device to for DPDK to use it. (ex: vfio-pci)"
- },
- "os_driver": {
- "type": "string",
- "description": "The driver normally used by this port (ex: i40e)"
- },
- "peer_node": {
- "type": "string",
- "description": "The name of the node the peer port is on"
+ "oneOf": [
+ {
+ "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. Use only as a last resort. SSH keys are STRONGLY preferred."
+ },
+ "arch": {
+ "$ref": "#/definitions/ARCH"
+ },
+ "os": {
+ "$ref": "#/definitions/OS"
+ },
+ "lcores": {
+ "type": "string",
+ "pattern": "^(([0-9]+|([0-9]+-[0-9]+))(,([0-9]+|([0-9]+-[0-9]+)))*)?$",
+ "description": "Optional comma-separated list of logical cores to use, e.g.: 1,2,3,4,5,18-22. Defaults to 1. An empty string means use all lcores."
+ },
+ "use_first_core": {
+ "type": "boolean",
+ "description": "Indicate whether DPDK should use the first physical core. It won't be used by default."
+ },
+ "memory_channels": {
+ "type": "integer",
+ "description": "How many memory channels to use. Optional, defaults to 1."
+ },
+ "hugepages_2mb": {
+ "$ref": "#/definitions/hugepages_2mb"
+ },
+ "ports": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "description": "Each port should be described on both sides of the connection. This makes configuration slightly more verbose but greatly simplifies implementation. If there are inconsistencies, then DTS will not run until that issue is fixed. An example inconsistency would be port 1, node 1 says it is connected to port 1, node 2, but port 1, node 2 says it is connected to port 2, node 1.",
+ "properties": {
+ "pci": {
+ "$ref": "#/definitions/pci_address",
+ "description": "The local PCI address of the port"
+ },
+ "os_driver_for_dpdk": {
+ "type": "string",
+ "description": "The driver that the kernel should bind this device to for DPDK to use it. (ex: vfio-pci)"
+ },
+ "os_driver": {
+ "type": "string",
+ "description": "The driver normally used by this port (ex: i40e)"
+ },
+ "peer_node": {
+ "type": "string",
+ "description": "The name of the node the peer port is on"
+ },
+ "peer_pci": {
+ "$ref": "#/definitions/pci_address",
+ "description": "The PCI address of the peer port"
+ }
+ },
+ "additionalProperties": false,
+ "required": [
+ "pci",
+ "os_driver_for_dpdk",
+ "os_driver",
+ "peer_node",
+ "peer_pci"
+ ]
},
- "peer_pci": {
- "$ref": "#/definitions/pci_address",
- "description": "The PCI address of the peer port"
- }
+ "minimum": 1
},
- "additionalProperties": false,
- "required": [
- "pci",
- "os_driver_for_dpdk",
- "os_driver",
- "peer_node",
- "peer_pci"
- ]
- },
- "minimum": 1
- },
- "traffic_generator": {
- "oneOf": [
- {
- "type": "object",
- "description": "Scapy traffic generator. Used for functional testing.",
- "properties": {
- "type": {
- "type": "string",
- "enum": [
- "SCAPY"
- ]
+ "traffic_generator": {
+ "oneOf": [
+ {
+ "type": "object",
+ "description": "Scapy traffic generator. Used for functional testing.",
+ "properties": {
+ "type": {
+ "type": "string",
+ "enum": [
+ "SCAPY"
+ ]
+ }
+ }
}
- }
+ ]
}
- ]
- }
- },
- "additionalProperties": false,
- "required": [
- "name",
- "hostname",
- "user",
- "arch",
- "os"
- ]
- },
- "minimum": 1
- },
- "test_runs": {
- "type": "array",
- "items": {
- "type": "object",
- "properties": {
- "build_targets": {
- "type": "array",
- "items": {
- "$ref": "#/definitions/build_target"
},
- "minimum": 1
- },
- "perf": {
- "type": "boolean",
- "description": "Enable performance testing."
- },
- "func": {
- "type": "boolean",
- "description": "Enable functional testing."
- },
- "test_suites": {
- "type": "array",
- "items": {
- "oneOf": [
- {
- "$ref": "#/definitions/test_suite"
- },
- {
- "$ref": "#/definitions/test_target"
- }
- ]
- }
- },
- "skip_smoke_tests": {
- "description": "Optional field that allows you to skip smoke testing",
- "type": "boolean"
+ "additionalProperties": false,
+ "required": [
+ "name",
+ "hostname",
+ "user",
+ "arch",
+ "os"
+ ]
},
- "system_under_test_node": {
- "type":"object",
+ "minimum": 1
+ },
+ "executions": {
+ "type": "array",
+ "items": {
+ "type": "object",
"properties": {
- "node_name": {
- "$ref": "#/definitions/node_name"
+ "build_targets": {
+ "type": "array",
+ "items": {
+ "$ref": "#/definitions/build_target"
+ },
+ "minimum": 1
+ },
+ "perf": {
+ "type": "boolean",
+ "description": "Enable performance testing."
+ },
+ "func": {
+ "type": "boolean",
+ "description": "Enable functional testing."
},
- "vdevs": {
- "description": "Optional list of names of vdevs to be used in the test run",
+ "test_suites": {
"type": "array",
"items": {
- "type": "string"
+ "oneOf": [
+ {
+ "$ref": "#/definitions/test_suite"
+ },
+ {
+ "$ref": "#/definitions/test_target"
+ }
+ ]
}
+ },
+ "skip_smoke_tests": {
+ "description": "Optional field that allows you to skip smoke testing",
+ "type": "boolean"
+ },
+ "system_under_test_node": {
+ "type":"object",
+ "properties": {
+ "node_name": {
+ "$ref": "#/definitions/node_name"
+ },
+ "vdevs": {
+ "description": "Optional list of names of vdevs to be used in execution",
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ }
+ },
+ "required": [
+ "node_name"
+ ]
+ },
+ "traffic_generator_node": {
+ "$ref": "#/definitions/node_name"
}
},
+ "additionalProperties": false,
"required": [
- "node_name"
+ "build_targets",
+ "perf",
+ "func",
+ "test_suites",
+ "system_under_test_node",
+ "traffic_generator_node"
]
},
- "traffic_generator_node": {
- "$ref": "#/definitions/node_name"
- }
- },
- "additionalProperties": false,
- "required": [
- "build_targets",
- "perf",
- "func",
- "test_suites",
- "system_under_test_node",
- "traffic_generator_node"
- ]
+ "minimum": 1
+ }
},
- "minimum": 1
+ "additionalProperties": false
}
- },
- "required": [
- "test_runs",
- "nodes"
- ],
- "additionalProperties": false
+ ]
}
diff --git a/dts/framework/runner.py b/dts/framework/runner.py
index 6b6f6a05f5..ac54189025 100644
--- a/dts/framework/runner.py
+++ b/dts/framework/runner.py
@@ -85,7 +85,9 @@ class DTSRunner:
def __init__(self):
"""Initialize the instance with configuration, logger, result and string constants."""
- self._configuration = load_config(SETTINGS.config_file_path)
+ self._configuration = load_config(
+ SETTINGS.node_config_file_path, SETTINGS.exec_config_file_path
+ )
self._logger = get_dts_logger()
if not os.path.exists(SETTINGS.output_dir):
os.makedirs(SETTINGS.output_dir)
diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index f95876113f..48f46d5a46 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -14,10 +14,15 @@
The command line arguments along with the supported environment variables are:
-.. option:: --config-file
-.. envvar:: DTS_CFG_FILE
+.. option:: --node-config-file
+.. envvar:: DTS_NODE_CFG_FILE
- The path to the YAML test run configuration file.
+ The path to the YAML testbed configuration file.
+
+.. option:: --exec-config-file
+.. envvar:: DTS_EXEC_CFG_FILE
+
+ The path to the YAML execution configuration file.
.. option:: --output-dir, --output
.. envvar:: DTS_OUTPUT_DIR
@@ -98,7 +103,9 @@ class Settings:
"""
#:
- config_file_path: Path = Path(__file__).parent.parent.joinpath("conf.yaml")
+ node_config_file_path: Path = Path(__file__).parent.parent.joinpath("node_conf.yaml")
+ #:
+ exec_config_file_path: Path = Path(__file__).parent.parent.joinpath("execution_conf.yaml")
#:
output_dir: str = "output"
#:
@@ -267,14 +274,23 @@ def _get_parser() -> _DTSArgumentParser:
)
action = parser.add_argument(
- "--config-file",
- default=SETTINGS.config_file_path,
+ "--node-config-file",
+ default=SETTINGS.node_config_file_path,
type=Path,
- help="The configuration file that describes the test cases, SUTs and targets.",
- metavar="FILE_PATH",
- dest="config_file_path",
+ help="[DTS_NODE_CFG_FILE] The configuration file that describes the testbed devices.",
+ metavar="NODE_FILE_PATH",
+ dest="node_config_file_path",
+ )
+ _add_env_var_to_action(action, "NODE_CFG_FILE")
+ action = parser.add_argument(
+ "--exec-config-file",
+ default=SETTINGS.exec_config_file_path,
+ type=Path,
+ help="[DTS_EXEC_CFG_FILE] The configuration file that describes the test cases.",
+ metavar="EXEC_FILE_PATH",
+ dest="exec_config_file_path",
)
- _add_env_var_to_action(action, "CFG_FILE")
+ _add_env_var_to_action(action, "EXEC_CFG_FILE")
action = parser.add_argument(
"--output-dir",
diff --git a/dts/node_conf.yaml b/dts/node_conf.yaml
new file mode 100644
index 0000000000..c7e64a00e0
--- /dev/null
+++ b/dts/node_conf.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+nodes:
+ # Define a system under test node, having two network ports physically
+ # connected to the corresponding ports in TG 1 (the peer node)
+ - name: "SUT 1"
+ hostname: sut1.change.me.localhost
+ user: dtsuser
+ arch: x86_64
+ os: linux
+ lcores: "" # use all the available logical cores
+ use_first_core: false # tells DPDK to use any physical core
+ memory_channels: 4 # tells DPDK to use 4 memory channels
+ hugepages: # optional; if removed, will use system hugepage configuration
+ amount: 256
+ force_first_numa: false
+ ports:
+ # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
+ - pci: "0000:00:08.0"
+ os_driver_for_dpdk: vfio-pci # OS driver that DPDK will use
+ os_driver: i40e # OS driver to bind when the tests are not running
+ peer_node: "TG 1"
+ peer_pci: "0000:00:08.0"
+ # sets up the physical link between "SUT 1"@000:00:08.1 and "TG 1"@0000:00:08.1
+ - pci: "0000:00:08.1"
+ os_driver_for_dpdk: vfio-pci
+ os_driver: i40e
+ peer_node: "TG 1"
+ peer_pci: "0000:00:08.1"
+ # Define a Scapy traffic generator node, having two network ports
+ # physically connected to the corresponding ports in SUT 1 (the peer node).
+ - name: "TG 1"
+ hostname: tg1.change.me.localhost
+ user: dtsuser
+ arch: x86_64
+ os: linux
+ ports:
+ # sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
+ - pci: "0000:00:08.0"
+ os_driver_for_dpdk: rdma
+ os_driver: rdma
+ peer_node: "SUT 1"
+ peer_pci: "0000:00:08.0"
+ # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
+ - pci: "0000:00:08.1"
+ os_driver_for_dpdk: rdma
+ os_driver: rdma
+ peer_node: "SUT 1"
+ peer_pci: "0000:00:08.1"
+ hugepages: # optional; if removed, will use system hugepage configuration
+ amount: 256
+ force_first_numa: false
+ traffic_generator:
+ type: SCAPY
diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml
new file mode 100644
index 0000000000..af2180eac2
--- /dev/null
+++ b/dts/testbed_conf.yaml
@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright 2022-2023 The DPDK contributors
+# Copyright 2023 Arm Limited
+
+executions:
+ # define one execution environment
+ - build_targets:
+ - arch: x86_64
+ os: linux
+ cpu: native
+ # the combination of the following two makes CC="ccache gcc"
+ compiler: gcc
+ compiler_wrapper: ccache
+ perf: false # disable performance testing
+ func: true # enable functional testing
+ skip_smoke_tests: false # optional
+ test_suites: # the following test suites will be run in their entirety
+ - hello_world
+ - os_udp
+ # The machine running the DPDK test executable
+ system_under_test_node:
+ node_name: "SUT 1"
+ vdevs: # optional; if removed, vdevs won't be used in the execution
+ - "crypto_openssl"
+ # Traffic generator node to use for this execution environment
+ traffic_generator_node: "TG 1"
\ No newline at end of file
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dts: Testbed And Node Configuration Split
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
@ 2024-07-16 14:11 ` Jeremy Spewock
2024-09-10 9:19 ` Juraj Linkeš
2024-11-18 17:25 ` Luca Vizzarro
2 siblings, 0 replies; 10+ messages in thread
From: Jeremy Spewock @ 2024-07-16 14:11 UTC (permalink / raw)
To: Nicholas Pratte
Cc: juraj.linkes, dmarx, luca.vizzarro, Honnappa.Nagarahalli,
paul.szczepanek, probb, yoan.picchi, dev
On Wed, Jul 3, 2024 at 10:58 AM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> This implementation splits the execution and node configuration
> components of the conf.yaml into two separate config files. A
> new command line argument is added, allowing the user to specify
> both a node configuration file and an execution configuration
> file. Be default, these config files are now named node_conf.yaml and
> execution_conf.yaml, respectively.
>
> To assert these changes, the schema calls for one of these objects,
> nodes or executions, in each file, but neither config file can have
> both.
>
> To avoid excess refactoring, both config files are merged together
> early on in the load_config process.
>
> Bugzilla ID: 1344
>
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
> ---
> dts/execution_conf.yaml | 26 ++
> dts/framework/config/__init__.py | 26 +-
> dts/framework/config/conf_yaml_schema.json | 340 ++++++++++-----------
> dts/framework/runner.py | 4 +-
> dts/framework/settings.py | 36 ++-
> dts/node_conf.yaml | 56 ++++
> dts/testbed_conf.yaml | 26 ++
> 7 files changed, 325 insertions(+), 189 deletions(-)
> create mode 100644 dts/execution_conf.yaml
> create mode 100644 dts/node_conf.yaml
> create mode 100644 dts/testbed_conf.yaml
>
> diff --git a/dts/execution_conf.yaml b/dts/execution_conf.yaml
> new file mode 100644
> index 0000000000..af2180eac2
> --- /dev/null
> +++ b/dts/execution_conf.yaml
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +executions:
We probably don't want to rename this back to executions as Juraj's
patch recently got merged into main that calls these test_runs, and I
think people found that more clear. With that being said, we'll
probably have to rename this file too.
> + # define one execution environment
> + - build_targets:
> + - arch: x86_64
> + os: linux
> + cpu: native
> + # the combination of the following two makes CC="ccache gcc"
> + compiler: gcc
> + compiler_wrapper: ccache
> + perf: false # disable performance testing
> + func: true # enable functional testing
> + skip_smoke_tests: false # optional
> + test_suites: # the following test suites will be run in their entirety
> + - hello_world
> + - os_udp
> + # The machine running the DPDK test executable
> + system_under_test_node:
> + node_name: "SUT 1"
> + vdevs: # optional; if removed, vdevs won't be used in the execution
> + - "crypto_openssl"
> + # Traffic generator node to use for this execution environment
> + traffic_generator_node: "TG 1"
> \ No newline at end of file
<snip>
> +++ b/dts/node_conf.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +nodes:
> + # Define a system under test node, having two network ports physically
> + # connected to the corresponding ports in TG 1 (the peer node)
> + - name: "SUT 1"
> + hostname: sut1.change.me.localhost
> + user: dtsuser
> + arch: x86_64
> + os: linux
> + lcores: "" # use all the available logical cores
> + use_first_core: false # tells DPDK to use any physical core
> + memory_channels: 4 # tells DPDK to use 4 memory channels
> + hugepages: # optional; if removed, will use system hugepage configuration
> + amount: 256
> + force_first_numa: false
> + ports:
> + # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
> + - pci: "0000:00:08.0"
> + os_driver_for_dpdk: vfio-pci # OS driver that DPDK will use
> + os_driver: i40e # OS driver to bind when the tests are not running
I think the point of this comment having all the whitespace is to line
up the starts of these two comments but it seems a little off in this
series. I might even be more of a fan of removing it and just making
it one space, but if we want to keep it the way it is on main it's
probably better to give this a little more whitespace.
> + peer_node: "TG 1"
> + peer_pci: "0000:00:08.0"
> + # sets up the physical link between "SUT 1"@000:00:08.1 and "TG 1"@0000:00:08.1
> + - pci: "0000:00:08.1"
> + os_driver_for_dpdk: vfio-pci
> + os_driver: i40e
> + peer_node: "TG 1"
> + peer_pci: "0000:00:08.1"
> + # Define a Scapy traffic generator node, having two network ports
> + # physically connected to the corresponding ports in SUT 1 (the peer node).
> + - name: "TG 1"
> + hostname: tg1.change.me.localhost
> + user: dtsuser
> + arch: x86_64
> + os: linux
> + ports:
> + # sets up the physical link between "TG 1"@000:00:08.0 and "SUT 1"@0000:00:08.0
> + - pci: "0000:00:08.0"
> + os_driver_for_dpdk: rdma
> + os_driver: rdma
> + peer_node: "SUT 1"
> + peer_pci: "0000:00:08.0"
> + # sets up the physical link between "SUT 1"@000:00:08.0 and "TG 1"@0000:00:08.0
> + - pci: "0000:00:08.1"
> + os_driver_for_dpdk: rdma
> + os_driver: rdma
> + peer_node: "SUT 1"
> + peer_pci: "0000:00:08.1"
> + hugepages: # optional; if removed, will use system hugepage configuration
> + amount: 256
> + force_first_numa: false
> + traffic_generator:
> + type: SCAPY
> diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml
I'm still not sure what keeping this file is for, it seems like it
isn't used elsewhere in the framework.
> new file mode 100644
> index 0000000000..af2180eac2
> --- /dev/null
> +++ b/dts/testbed_conf.yaml
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright 2022-2023 The DPDK contributors
> +# Copyright 2023 Arm Limited
> +
> +executions:
> + # define one execution environment
> + - build_targets:
> + - arch: x86_64
> + os: linux
> + cpu: native
> + # the combination of the following two makes CC="ccache gcc"
> + compiler: gcc
> + compiler_wrapper: ccache
> + perf: false # disable performance testing
> + func: true # enable functional testing
> + skip_smoke_tests: false # optional
> + test_suites: # the following test suites will be run in their entirety
> + - hello_world
> + - os_udp
> + # The machine running the DPDK test executable
> + system_under_test_node:
> + node_name: "SUT 1"
> + vdevs: # optional; if removed, vdevs won't be used in the execution
> + - "crypto_openssl"
> + # Traffic generator node to use for this execution environment
> + traffic_generator_node: "TG 1"
> \ No newline at end of file
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] dts: Testbed And Node Configuration Split
2024-06-14 18:32 ` Jeremy Spewock
@ 2024-09-10 8:57 ` Juraj Linkeš
0 siblings, 0 replies; 10+ messages in thread
From: Juraj Linkeš @ 2024-09-10 8:57 UTC (permalink / raw)
To: Jeremy Spewock, Nicholas Pratte
Cc: luca.vizzarro, probb, bruce.richardson, yoan.picchi,
paul.szczepanek, Honnappa.Nagarahalli, dev
On 14. 6. 2024 20:32, Jeremy Spewock wrote:
> On Mon, Jun 10, 2024 at 3:37 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>>
>> <snip>
>>
>>> -def load_config(config_file_path: Path) -> Configuration:
>>> +def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
>>> """Load DTS test run configuration from a file.
>>>
>>> - Load the YAML test run configuration file
>>> + Load both the YAML testbed and execution configuration files
>>> and :download:`the configuration file schema <conf_yaml_schema.json>`,
>>> - validate the test run configuration file, and create a test run configuration object.
>>> + validate both configuration files to 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.
>>>
>>> Args:
>>> - config_file_path: The path to the YAML test run configuration file.
>>> + node_config_file_path: The path to the testbed configuration YAML file.
>>> + exec_config_file_path: The path to the execution configuration YAML file.
>>>
>>> Returns:
>>> The parsed test run configuration.
>>> """
>>> - with open(config_file_path, "r") as f:
>>> - config_data = yaml.safe_load(f)
>>> + with open(node_config_file_path, "r") as f:
>>> + node_config_data = yaml.safe_load(f)
>>> + with open(exec_config_file_path, "r") as f:
>>> + execution_config_data = yaml.safe_load(f)
>>>
>>> schema_path = os.path.join(Path(__file__).parent.resolve(), "conf_yaml_schema.json")
>>>
>>> with open(schema_path, "r") as f:
>>> schema = json.load(f)
>>> - config = warlock.model_factory(schema, name="_Config")(config_data)
>>> - config_obj: Configuration = Configuration.from_dict(dict(config)) # type: ignore[arg-type]
>>> + config = {
>>> + **dict(warlock.model_factory(schema, name="_Config")(node_config_data)),
>>> + **dict(warlock.model_factory(schema, name="_Config")(execution_config_data)),
>>> + }
>>> + if "nodes" not in config or "executions" not in config:
>>> + raise ConfigurationError(
>>> + f"{'node' if 'nodes' not in config else 'execution'} data not configured."
>>> + )
>>> + config_obj: Configuration = Configuration.from_dict(config) # type: ignore[arg-type]
>>> return config_obj
>>
>> There is an alternative approach to this that I currently have
>> implemented on a separate local branch. Essentially, both configs are
>> merged together before being validated by the schema, effectively
>> removing the need to change the schema at all as well as the above
>> assertion that both nodes and executions are in the two config files.
>> However, using this alternative method might mean that there is no added
>> control that prevents users from making funky errors when creating
>> both config files. I could also just be overthinking or missing
>> something.
>
> That's an interesting idea and on one hand it makes the schema a
> little less confusing because it wouldn't need to say "oneOf" when
> both of the attributes are actually required for DTS to run, but on
> the other it removes the ability for developers to know if they made a
> mistake without actually running DTS. Making it a runtime error
> wouldn't be the worst thing since it would at least be very early in
> the run that the error is shown, but it loses some of its purpose I
> feel if your IDE doesn't also show you some of the errors before
> actually using the config. I could go either way on it personally.
>
Since we're splitting the file, we can also split the schema. I like
splitting the schema since it'll be much clearer which files the schemas
describe (if we have everything in one schema, we have to carefully look
for which parts describe which file). It's also going to be easier to
maintain.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] dts: Testbed And Node Configuration Split
2024-06-14 18:27 ` Jeremy Spewock
@ 2024-09-10 9:00 ` Juraj Linkeš
0 siblings, 0 replies; 10+ messages in thread
From: Juraj Linkeš @ 2024-09-10 9:00 UTC (permalink / raw)
To: Jeremy Spewock, Nicholas Pratte
Cc: luca.vizzarro, probb, bruce.richardson, yoan.picchi,
paul.szczepanek, Honnappa.Nagarahalli, dev
On 14. 6. 2024 20:27, Jeremy Spewock wrote:
> I think this is definitely a step in the right direction in terms of
> how we structure the config files. Something that I think could also
> be a cool improvement for how we handle configs is just making a
> `conf/` directory and then taking all of the configuration in each of
> the yaml files present in the directory and combining them into one
> config in the framework like we have now. That's definitely a larger
> undertaking however, this split will save space if users want to
> specify more than one host or use the same execution configuration
> between different node configurations and I think that is an
> improvement on its own.
>
I like this suggestion. When we have multiple config files, it just
makes sense to group them.
> On Mon, Jun 10, 2024 at 3:34 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>>
>> This implementation splits the execution and node configuration
>> components of the conf.yaml into two separate config files. A
>> new command line argument is added, allowing the user to specify
>> both a node configuration file and an execution configuration
>> file. Be default, these config files are now named node_conf.yaml and
>> execution_conf.yaml, respectively.
>>
>> To assert these changes, the schema calls for one of these objects,
>> nodes or executions, in each file, but neither config file can have
>> both.
>>
>> To avoid excess refactoring, both config files are merged together
>> early on in the load_config process.
>>
>> Bugzilla ID: 1344
>>
>> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>> ---
> <snip>
>> diff --git a/dts/testbed_conf.yaml b/dts/testbed_conf.yaml
>> new file mode 100644
>> index 0000000000..af2180eac2
>> --- /dev/null
>> +++ b/dts/testbed_conf.yaml
>
> I don't think this file is used anywhere and it looks like another
> name for the execution_conf.yaml so we likely don't need it.
>
We actually don't want the execution_conf.yaml file (and use
testbed_conf.yaml), but the point stands - there are two files that are
the same.
>> @@ -0,0 +1,26 @@
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright 2022-2023 The DPDK contributors
>> +# Copyright 2023 Arm Limited
>> +
>> +executions:
>> + # define one execution environment
>> + - build_targets:
>> + - arch: x86_64
>> + os: linux
>> + cpu: native
>> + # the combination of the following two makes CC="ccache gcc"
>> + compiler: gcc
>> + compiler_wrapper: ccache
>> + perf: false # disable performance testing
>> + func: true # enable functional testing
>> + skip_smoke_tests: false # optional
>> + test_suites: # the following test suites will be run in their entirety
>> + - hello_world
>> + - os_udp
>> + # The machine running the DPDK test executable
>> + system_under_test_node:
>> + node_name: "SUT 1"
>> + vdevs: # optional; if removed, vdevs won't be used in the execution
>> + - "crypto_openssl"
>> + # Traffic generator node to use for this execution environment
>> + traffic_generator_node: "TG 1"
>> \ No newline at end of file
>> --
>> 2.44.0
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dts: Testbed And Node Configuration Split
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
2024-07-16 14:11 ` Jeremy Spewock
@ 2024-09-10 9:19 ` Juraj Linkeš
2024-11-18 17:25 ` Luca Vizzarro
2 siblings, 0 replies; 10+ messages in thread
From: Juraj Linkeš @ 2024-09-10 9:19 UTC (permalink / raw)
To: Nicholas Pratte, dmarx, luca.vizzarro, Honnappa.Nagarahalli,
paul.szczepanek, probb, jspewock, yoan.picchi
Cc: dev
> create mode 100644 dts/execution_conf.yaml
As Jeremy mentioned, this is a duplicate. I guess this patch needs to be
rebased as it mentions executions all over the place.
> create mode 100644 dts/node_conf.yaml
> create mode 100644 dts/testbed_conf.yaml
And this should be named test_run_conf.yaml or testrun_conf.yaml (it's
not just testbed conf, but also other things used in test runs). Or
maybe we can come up with an abbreviation or initialism for test run.
> diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
> index df60a5030e..2322f3d996 100644
> --- a/dts/framework/config/__init__.py
> +++ b/dts/framework/config/__init__.py
> @@ -573,29 +573,39 @@ def from_dict(cls, d: ConfigurationDict) -> Self:
> return cls(test_runs=test_runs)
>
>
> -def load_config(config_file_path: Path) -> Configuration:
> +def load_config(node_config_file_path: Path, exec_config_file_path: Path) -> Configuration:
> """Load DTS test run configuration from a file.
>
> - Load the YAML test run configuration file
> + Load both the YAML testbed and execution configuration files
> and :download:`the configuration file schema <conf_yaml_schema.json>`,
> - validate the test run configuration file, and create a test run configuration object.
> + validate both configuration files to create a test run configuration object.
Now that you're removed the last "and" the sentence feels like it's
missing something. If we add then to "then validate both" then it sounds
fine.
> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index f02a310bb5..1962ed05dd 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -211,192 +211,192 @@
> + "oneOf": [
Again, as mentioned in the other comment, splitting the schema would
make more sense. We'd probably have to make some doc modifications (in
guides/tools/dts.rtst)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] dts: Testbed And Node Configuration Split
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
2024-07-16 14:11 ` Jeremy Spewock
2024-09-10 9:19 ` Juraj Linkeš
@ 2024-11-18 17:25 ` Luca Vizzarro
2 siblings, 0 replies; 10+ messages in thread
From: Luca Vizzarro @ 2024-11-18 17:25 UTC (permalink / raw)
To: Nicholas Pratte, juraj.linkes, dmarx, Honnappa.Nagarahalli,
paul.szczepanek, probb, jspewock, yoan.picchi
Cc: dev
This is a good change, but I'd like to review it once you adapt it to
the Pydantic-based config, as it may look quite different.
While we are splitting things I suggest to create two new source files
under `config` for each sections:
conf/
__init__.py # where Configurations stays and brings everything together
test_run.py # to hold the `test_run` and all the related models
node.py # as above but for `node`
This will hopefully make it easier to read.
Best,
Luca
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-11-18 17:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-10 19:34 [PATCH v1] dts: Testbed And Node Configuration Split Nicholas Pratte
2024-06-10 19:37 ` Nicholas Pratte
2024-06-14 18:32 ` Jeremy Spewock
2024-09-10 8:57 ` Juraj Linkeš
2024-06-14 18:27 ` Jeremy Spewock
2024-09-10 9:00 ` Juraj Linkeš
2024-07-03 14:57 ` [PATCH v2] " Nicholas Pratte
2024-07-16 14:11 ` Jeremy Spewock
2024-09-10 9:19 ` Juraj Linkeš
2024-11-18 17:25 ` Luca Vizzarro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).