Re: [PATCH v8 1/1] dts: add smoke tests

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

A few more things I noticed while running DTS.

On Mon, Jul 17, 2023 at 9:37 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Adds a new test suite for running smoke tests that verify general
> configuration aspects of the system under test. If any of these tests
> fail, the DTS execution terminates as part of a "fail-fast" model.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/conf.yaml                                 |  17 +-
>  dts/framework/config/__init__.py              |  79 ++++++--
>  dts/framework/config/conf_yaml_schema.json    | 142 ++++++++++++++-
>  dts/framework/dts.py                          |  84 ++++++---
>  dts/framework/exception.py                    |  12 ++
>  dts/framework/remote_session/__init__.py      |  13 +-
>  dts/framework/remote_session/os_session.py    |  48 ++++-
>  dts/framework/remote_session/posix_session.py |  29 ++-
>  .../remote_session/remote/__init__.py         |  10 ++
>  .../remote/interactive_remote_session.py      |  82 +++++++++
>  .../remote/interactive_shell.py               |  98 ++++++++++
>  .../remote_session/remote/testpmd_shell.py    |  46 +++++
>  dts/framework/test_result.py                  |  24 ++-
>  dts/framework/test_suite.py                   |  10 +-
>  dts/framework/testbed_model/node.py           |  43 ++++-
>  dts/framework/testbed_model/sut_node.py       | 169 +++++++++++++-----
>  dts/framework/utils.py                        |   3 +
>  dts/tests/TestSuite_smoke_tests.py            | 114 ++++++++++++
>  18 files changed, 931 insertions(+), 92 deletions(-)
>  create mode 100644 dts/framework/remote_session/remote/interactive_remote_session.py
>  create mode 100644 dts/framework/remote_session/remote/interactive_shell.py
>  create mode 100644 dts/framework/remote_session/remote/testpmd_shell.py
>  create mode 100644 dts/tests/TestSuite_smoke_tests.py
>
> diff --git a/dts/conf.yaml b/dts/conf.yaml
> index 129801d87c..3a5d87cb49 100644
> --- a/dts/conf.yaml
> +++ b/dts/conf.yaml
> @@ -10,9 +10,13 @@ executions:
>          compiler_wrapper: ccache
>      perf: false
>      func: true
> +    skip_smoke_tests: false # optional flag that allow you to skip smoke tests

Typo: allows

>
>      test_suites:
>        - hello_world
> -    system_under_test: "SUT 1"
> +    system_under_test:
> +      node_name: "SUT 1"
> +      vdevs: # optional; if removed, vdevs won't be used in the execution
> +        - "crypto_openssl"
>  nodes:
>    - name: "SUT 1"
>      hostname: sut1.change.me.localhost

<snip>

> diff --git a/dts/framework/config/conf_yaml_schema.json b/dts/framework/config/conf_yaml_schema.json
> index ca2d4a1ef2..09fcbaf498 100644
> --- a/dts/framework/config/conf_yaml_schema.json
> +++ b/dts/framework/config/conf_yaml_schema.json
> @@ -6,6 +6,76 @@
>        "type": "string",
>        "description": "A unique identifier for a node"
>      },
> +    "NIC": {
> +      "type": "string",
> +      "enum": [
> +        "ALL",
> +        "ConnectX3_MT4103",
> +        "ConnectX4_LX_MT4117",
> +        "ConnectX4_MT4115",
> +        "ConnectX5_MT4119",
> +        "ConnectX5_MT4121",
> +        "I40E_10G-10G_BASE_T_BC",
> +        "I40E_10G-10G_BASE_T_X722",
> +        "I40E_10G-SFP_X722",
> +        "I40E_10G-SFP_XL710",
> +        "I40E_10G-X722_A0",
> +        "I40E_1G-1G_BASE_T_X722",
> +        "I40E_25G-25G_SFP28",
> +        "I40E_40G-QSFP_A",
> +        "I40E_40G-QSFP_B",
> +        "IAVF-ADAPTIVE_VF",
> +        "IAVF-VF",
> +        "IAVF_10G-X722_VF",
> +        "ICE_100G-E810C_QSFP",
> +        "ICE_25G-E810C_SFP",
> +        "ICE_25G-E810_XXV_SFP",
> +        "IGB-I350_VF",
> +        "IGB_1G-82540EM",
> +        "IGB_1G-82545EM_COPPER",
> +        "IGB_1G-82571EB_COPPER",
> +        "IGB_1G-82574L",
> +        "IGB_1G-82576",
> +        "IGB_1G-82576_QUAD_COPPER",
> +        "IGB_1G-82576_QUAD_COPPER_ET2",
> +        "IGB_1G-82580_COPPER",
> +        "IGB_1G-I210_COPPER",
> +        "IGB_1G-I350_COPPER",
> +        "IGB_1G-I354_SGMII",
> +        "IGB_1G-PCH_LPTLP_I218_LM",
> +        "IGB_1G-PCH_LPTLP_I218_V",
> +        "IGB_1G-PCH_LPT_I217_LM",
> +        "IGB_1G-PCH_LPT_I217_V",
> +        "IGB_2.5G-I354_BACKPLANE_2_5GBPS",
> +        "IGC-I225_LM",
> +        "IGC-I226_LM",
> +        "IXGBE_10G-82599_SFP",
> +        "IXGBE_10G-82599_SFP_SF_QP",
> +        "IXGBE_10G-82599_T3_LOM",
> +        "IXGBE_10G-82599_VF",
> +        "IXGBE_10G-X540T",
> +        "IXGBE_10G-X540_VF",
> +        "IXGBE_10G-X550EM_A_SFP",
> +        "IXGBE_10G-X550EM_X_10G_T",
> +        "IXGBE_10G-X550EM_X_SFP",
> +        "IXGBE_10G-X550EM_X_VF",
> +        "IXGBE_10G-X550T",
> +        "IXGBE_10G-X550_VF",
> +        "brcm_57414",
> +        "brcm_P2100G",
> +        "cavium_0011",
> +        "cavium_a034",
> +        "cavium_a063",
> +        "cavium_a064",
> +        "fastlinq_ql41000",
> +        "fastlinq_ql41000_vf",
> +        "fastlinq_ql45000",
> +        "fastlinq_ql45000_vf",
> +        "hi1822",
> +        "virtio"
> +      ]
> +    },
> +
>      "ARCH": {
>        "type": "string",
>        "enum": [
> @@ -94,6 +164,19 @@
>          "amount"
>        ]
>      },
> +    "pci_address": {
> +      "type": "string",
> +      "pattern": "^[\\da-fA-F]{4}:[\\da-fA-F]{2}:[\\da-fA-F]{2}.\\d:?\\w*$"
> +    },
> +    "port_peer_address": {
> +      "description": "Peer is a TRex port, and IXIA port or a PCI address",
> +      "oneOf": [
> +        {
> +          "description": "PCI peer port",
> +          "$ref": "#/definitions/pci_address"
> +        }
> +      ]
> +    },
>      "test_suite": {
>        "type": "string",
>        "enum": [
> @@ -165,6 +248,44 @@
>            },
>            "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 an 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.",

Typo: extra an in are an inconsistencies

> +              "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"
> +              ]
> +            },
> +            "minimum": 1
>            }
>          },
>          "additionalProperties": false,

<snip>

> diff --git a/dts/framework/remote_session/remote/interactive_shell.py b/dts/framework/remote_session/remote/interactive_shell.py
> new file mode 100644
> index 0000000000..4d9c7638a5
> --- /dev/null
> +++ b/dts/framework/remote_session/remote/interactive_shell.py
> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +from pathlib import PurePath
> +from typing import Callable
> +
> +from paramiko import Channel, SSHClient, channel  # type: ignore
> +
> +from framework.logger import DTSLOG
> +from framework.settings import SETTINGS
> +
> +
> +class InteractiveShell:
> +
> +    _interactive_session: SSHClient
> +    _stdin: channel.ChannelStdinFile
> +    _stdout: channel.ChannelFile
> +    _ssh_channel: Channel
> +    _logger: DTSLOG
> +    _timeout: float
> +    _startup_command: str
> +    _app_args: str
> +    _default_prompt: str = ""
> +    _privileged: bool
> +    _get_privileged_command: Callable[[str], str]
> +    # Allows for app specific extra characters to be appended to commands
> +    _command_extra_chars: str = ""
> +    path: PurePath
> +    dpdk_app: bool = False
> +
> +    def __init__(
> +        self,
> +        interactive_session: SSHClient,
> +        logger: DTSLOG,
> +        startup_command: str,
> +        privileged: bool,
> +        _get_privileged_command: Callable[[str], str],
> +        app_args: str = "",
> +        timeout: float = SETTINGS.timeout,
> +    ) -> None:
> +        self._interactive_session = interactive_session
> +        self._ssh_channel = self._interactive_session.invoke_shell()
> +        self._stdin = self._ssh_channel.makefile_stdin("w")
> +        self._stdout = self._ssh_channel.makefile("r")
> +        self._ssh_channel.settimeout(timeout)
> +        self._ssh_channel.set_combine_stderr(True)  # combines stdout and stderr streams
> +        self._logger = logger
> +        self._timeout = timeout
> +        self._startup_command = startup_command
> +        self._app_args = app_args
> +        self._get_privileged_command = _get_privileged_command  # type: ignore
> +        self._privileged = privileged
> +        self._start_application()
> +
> +    def _start_application(self) -> None:
> +        """Starts a new interactive application based on _startup_command.
> +
> +        This method is often overridden by subclasses as their process for
> +        starting may look different.
> +        """
> +        start_command = f"{self._startup_command} {self._app_args}"
> +        if self._privileged:
> +            start_command = self._get_privileged_command(start_command)  # type: ignore
> +        self.send_command(start_command)
> +
> +    def send_command(self, command: str, prompt: str | None = None) -> str:
> +        """Send a command and get all output before the expected ending string.
> +
> +        Lines that expect input are not included in the stdout buffer so they cannot be
> +        used for expect. For example, if you were prompted to log into something
> +        with a username and password, you cannot expect "username:" because it won't
> +        yet be in the stdout buffer. A work around for this could be consuming an
> +        extra newline character to force the current prompt into the stdout buffer.
> +
> +        Returns:
> +            All output in the buffer before expected string
> +        """
> +        self._logger.info(f"Sending command {command.strip()}...")

Let's unite this log with with remote remote session:
self._logger.info(
f"Sending: '{command}'" + (f" with env vars: '{env}'" if env else "")
)

We don't have env vars, but the rest should be the same.


> +        if prompt is None:
> +            prompt = self._default_prompt
> +        self._stdin.write(f"{command}{self._command_extra_chars}\n")
> +        self._stdin.flush()
> +        out: str = ""
> +        for line in self._stdout:
> +            out += line
> +            if prompt in line and not line.rstrip().endswith(
> +                command.rstrip()
> +            ):  # ignore line that sent command
> +                break
> +        self._logger.debug(f"Got output: {out}")
> +        return out
> +
> +    def close(self) -> None:
> +        self._stdin.close()
> +        self._ssh_channel.close()
> +
> +    def __del__(self) -> None:
> +        self.close()