[3/4] dts: Self-Discovering Architecture Change

Message ID 20240613201831.9748-9-npratte@iol.unh.edu (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dts: Remove Excess Attributes From User Config |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Nicholas Pratte June 13, 2024, 8:18 p.m. UTC
The 'arch' attribute in the conf.yaml is unnecessary, as this can be
readily discovered within the constructor of any given node. Since OS is
determined within user configuration, finding system arch can be done
both reliably and easily within the framework.

For Linux/Posix systems, the 'uname' command is used to determine system
architecture. I believe that this is posix-standard and utilizes a
standardized output.

Bugzilla ID: 1360
Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>

---
 doc/guides/tools/dts.rst                     |  2 --
 dts/conf.yaml                                |  2 --
 dts/framework/config/__init__.py             |  4 ----
 dts/framework/config/conf_yaml_schema.json   | 12 ------------
 dts/framework/config/types.py                |  2 --
 dts/framework/testbed_model/node.py          |  3 +++
 dts/framework/testbed_model/os_session.py    |  8 ++++++++
 dts/framework/testbed_model/posix_session.py |  6 ++++++
 8 files changed, 17 insertions(+), 22 deletions(-)
  

Comments

Jeremy Spewock June 14, 2024, 6:09 p.m. UTC | #1
Looks good to me, just looks like a testing command got left behind.
Otherwise though:

Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>

On Thu, Jun 13, 2024 at 4:22 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> The 'arch' attribute in the conf.yaml is unnecessary, as this can be
> readily discovered within the constructor of any given node. Since OS is
> determined within user configuration, finding system arch can be done
> both reliably and easily within the framework.
>
> For Linux/Posix systems, the 'uname' command is used to determine system
> architecture. I believe that this is posix-standard and utilizes a
> standardized output.
>
> Bugzilla ID: 1360
> Signed-off-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> ---
<snip>
> diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
> index d279bb8b53..91afca61ea 100644
> --- a/dts/framework/testbed_model/posix_session.py
> +++ b/dts/framework/testbed_model/posix_session.py
> @@ -295,3 +295,9 @@ def get_node_info(self) -> NodeInfo:
>          ).stdout.split("\n")
>          kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
>          return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
> +
> +    def get_arch_info(self) -> str:
> +        """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
> +        # return str(self.send_command('arch')).stdout

Right here is the testing I was referencing.


> +
> +        return str(self.send_command("uname -m").stdout.removesuffix("\n"))
> --
> 2.44.0
>
  

Patch

diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index fbb5c6f17b..0453a15a73 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -542,8 +542,6 @@  involved in the testing. These can be defined with the following mappings:
    |                       |                                                                                       |
    |                       | **NB**: Use only as last resort. SSH keys are **strongly** preferred.                 |
    +-----------------------+---------------------------------------------------------------------------------------+
-   | ``arch``              | The architecture of this node. See `ARCH`_ for supported values.                      |
-   +-----------------------+---------------------------------------------------------------------------------------+
    | ``os``                | The operating system of this node. See `OS`_ for supported values.                    |
    +-----------------------+---------------------------------------------------------------------------------------+
    | ``lcores``            | | (*optional*, defaults to 1 if not used) *string* – Comma-separated list of logical  |
diff --git a/dts/conf.yaml b/dts/conf.yaml
index c20afb9621..b9f5704ca5 100644
--- a/dts/conf.yaml
+++ b/dts/conf.yaml
@@ -27,7 +27,6 @@  nodes:
   - name: "SUT 1"
     hostname: sut1.change.me.localhost
     user: dtsuser
-    arch: x86_64
     os: linux
     lcores: "" # use all available logical cores (Skips first core)
     memory_channels: 4 # tells DPDK to use 4 memory channels
@@ -52,7 +51,6 @@  nodes:
   - 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
diff --git a/dts/framework/config/__init__.py b/dts/framework/config/__init__.py
index 6bc290a56a..07b85a6afb 100644
--- a/dts/framework/config/__init__.py
+++ b/dts/framework/config/__init__.py
@@ -207,7 +207,6 @@  class NodeConfiguration:
             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.
@@ -219,7 +218,6 @@  class NodeConfiguration:
     hostname: str
     user: str
     password: str | None
-    arch: Architecture
     os: OS
     lcores: str
     use_first_core: bool
@@ -256,7 +254,6 @@  def from_dict(
                 hostname=d["hostname"],
                 user=d["user"],
                 password=d.get("password"),
-                arch=Architecture(d["arch"]),
                 os=OS(d["os"]),
                 lcores=lcores,
                 use_first_core=use_first_core,
@@ -270,7 +267,6 @@  def from_dict(
                 hostname=d["hostname"],
                 user=d["user"],
                 password=d.get("password"),
-                arch=Architecture(d["arch"]),
                 os=OS(d["os"]),
                 lcores=lcores,
                 use_first_core=use_first_core,
diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
index 7c8429abbc..49db384967 100644
--- a/dts/framework/config/conf_yaml_schema.json
+++ b/dts/framework/config/conf_yaml_schema.json
@@ -6,14 +6,6 @@ 
       "type": "string",
       "description": "A unique identifier for a node"
     },
-    "ARCH": {
-      "type": "string",
-      "enum": [
-        "x86_64",
-        "arm64",
-        "ppc64le"
-      ]
-    },
     "OS": {
       "type": "string",
       "enum": [
@@ -155,9 +147,6 @@ 
             "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"
           },
@@ -233,7 +222,6 @@ 
           "name",
           "hostname",
           "user",
-          "arch",
           "os"
         ]
       },
diff --git a/dts/framework/config/types.py b/dts/framework/config/types.py
index fccea61608..c841ab2d7c 100644
--- a/dts/framework/config/types.py
+++ b/dts/framework/config/types.py
@@ -56,8 +56,6 @@  class NodeConfigDict(TypedDict):
     #:
     password: str
     #:
-    arch: str
-    #:
     os: str
     #:
     lcores: str
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 470cd18e30..ee4577cf35 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -18,6 +18,7 @@ 
 
 from framework.config import (
     OS,
+    Architecture,
     BuildTargetConfiguration,
     ExecutionConfiguration,
     NodeConfiguration,
@@ -61,6 +62,7 @@  class Node(ABC):
     main_session: OSSession
     config: NodeConfiguration
     name: str
+    arch: Architecture
     lcores: list[LogicalCore]
     ports: list[Port]
     _logger: DTSLogger
@@ -84,6 +86,7 @@  def __init__(self, node_config: NodeConfiguration):
         self.name = node_config.name
         self._logger = get_dts_logger(self.name)
         self.main_session = create_session(self.config, self.name, self._logger)
+        self.arch = Architecture(self.main_session.get_arch_info())
 
         self._logger.info(f"Connected to node: {self.name}")
 
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index d5bf7e0401..e082102b00 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -375,6 +375,14 @@  def get_node_info(self) -> NodeInfo:
             Node information.
         """
 
+    @abstractmethod
+    def get_arch_info(self) -> str:
+        """Discover CPU architecture of the remote host.
+
+        Returns:
+            Remote host CPU architecture.
+        """
+
     @abstractmethod
     def update_ports(self, ports: list[Port]) -> None:
         """Get additional information about ports from the operating system and update them.
diff --git a/dts/framework/testbed_model/posix_session.py b/dts/framework/testbed_model/posix_session.py
index d279bb8b53..91afca61ea 100644
--- a/dts/framework/testbed_model/posix_session.py
+++ b/dts/framework/testbed_model/posix_session.py
@@ -295,3 +295,9 @@  def get_node_info(self) -> NodeInfo:
         ).stdout.split("\n")
         kernel_version = self.send_command("uname -r", SETTINGS.timeout).stdout
         return NodeInfo(os_release_info[0].strip(), os_release_info[1].strip(), kernel_version)
+
+    def get_arch_info(self) -> str:
+        """Overrides :meth'~.os_session.OSSession.get_arch_info'."""
+        # return str(self.send_command('arch')).stdout
+
+        return str(self.send_command("uname -m").stdout.removesuffix("\n"))