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

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

Two more things and we're done.

On Tue, Jul 18, 2023 at 11:49 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/linux_session.py |   3 +-
>  dts/framework/remote_session/os_session.py    |  49 ++++-
>  dts/framework/remote_session/posix_session.py |  29 ++-
>  .../remote_session/remote/__init__.py         |  10 ++
>  .../remote/interactive_remote_session.py      |  82 +++++++++
>  .../remote/interactive_shell.py               | 132 ++++++++++++++
>  .../remote_session/remote/testpmd_shell.py    |  49 +++++
>  dts/framework/test_result.py                  |  21 ++-
>  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 ++++++++++++
>  19 files changed, 967 insertions(+), 94 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
>

<snip>

> diff --git a/dts/framework/remote_session/remote/interactive_remote_session.py b/dts/framework/remote_session/remote/interactive_remote_session.py
> new file mode 100644
> index 0000000000..2d94daf2a7
> --- /dev/null
> +++ b/dts/framework/remote_session/remote/interactive_remote_session.py

We forgot to add proper docstring to this module.

> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +import socket
> +import traceback
> +
> +from paramiko import AutoAddPolicy, SSHClient, Transport  # type: ignore
> +from paramiko.ssh_exception import (  # type: ignore
> +    AuthenticationException,
> +    BadHostKeyException,
> +    NoValidConnectionsError,
> +    SSHException,
> +)
> +
> +from framework.config import NodeConfiguration
> +from framework.exception import SSHConnectionError
> +from framework.logger import DTSLOG
> +
> +
> +class InteractiveRemoteSession:
> +    hostname: str
> +    ip: str
> +    port: int
> +    username: str
> +    password: str
> +    _logger: DTSLOG
> +    _node_config: NodeConfiguration
> +    session: SSHClient
> +    _transport: Transport | None
> +
> +    def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG) -> None:
> +        self._node_config = node_config
> +        self._logger = _logger
> +        self.hostname = node_config.hostname
> +        self.username = node_config.user
> +        self.password = node_config.password if node_config.password else ""
> +        port = "22"
> +        self.ip = node_config.hostname
> +        if ":" in node_config.hostname:
> +            self.ip, port = node_config.hostname.split(":")
> +        self.port = int(port)
> +        self._logger.info(
> +            f"Initializing interactive connection for {self.username}@{self.hostname}"
> +        )
> +        self._connect()
> +        self._logger.info(
> +            f"Interactive connection successful for {self.username}@{self.hostname}"
> +        )
> +
> +    def _connect(self) -> None:
> +        client = SSHClient()
> +        client.set_missing_host_key_policy(AutoAddPolicy)
> +        self.session = client
> +        retry_attempts = 10
> +        for retry_attempt in range(retry_attempts):
> +            try:
> +                client.connect(
> +                    self.ip,
> +                    username=self.username,
> +                    port=self.port,
> +                    password=self.password,
> +                    timeout=20 if self.port else 10,
> +                )
> +            except (TypeError, BadHostKeyException, AuthenticationException) as e:
> +                self._logger.exception(e)
> +                raise SSHConnectionError(self.hostname) from e
> +            except (NoValidConnectionsError, socket.error, SSHException) as e:
> +                self._logger.debug(traceback.format_exc())
> +                self._logger.warning(e)
> +                self._logger.info(
> +                    "Retrying interactive session connection: "
> +                    f"retry number {retry_attempt +1}"
> +                )
> +            else:
> +                break
> +        else:
> +            raise SSHConnectionError(self.hostname)
> +        # Interactive sessions are used on an "as needed" basis so we have
> +        # to set a keepalive
> +        self._transport = self.session.get_transport()
> +        if self._transport is not None:
> +            self._transport.set_keepalive(30)
> 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..0a1be4071f
> --- /dev/null
> +++ b/dts/framework/remote_session/remote/interactive_shell.py
> @@ -0,0 +1,132 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 University of New Hampshire
> +
> +"""Common functionality for interactive shell handling.
> +
> +This base class, InteractiveShell, is meant to be extended by other classes that
> +contain functionality specific to that shell type. These derived classes will often
> +modify things like the prompt to expect or the arguments to pass into the application,
> +but still utilize the same method for sending a command and collecting output. How
> +this output is handled however is often application specific. If an application needs
> +elevated privileges to start it is expected that the method for gaining those
> +privileges is provided when initializing the class.
> +"""
> +
> +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:
> +    """The base class for managing interactive shells.
> +
> +    This class shouldn't be instantiated directly, but instead be extended. It contains

I agree it shouldn't be instantiated, so let's make it an abstract
class (just like RemoteSession). It won't have any effect (there
aren't any abstract methods or properties), but at least it'll be
marked as abstract.
That reminds me I need to make Node abstract in my patch as well.

> +    methods for starting interactive shells as well as sending commands to these shells
> +    and collecting input until reaching a certain prompt. All interactive applications
> +    will use the same SSH connection, but each will create their own channel on that
> +    session.
> +
> +    Arguments:
> +        interactive_session: The SSH session dedicated to interactive shells.
> +        logger: Logger used for displaying information in the console.
> +        get_privileged_command: Method for modifying a command to allow it to use
> +            elevated privileges. If this is None, the application will not be started
> +            with elevated privileges.
> +        app_args: Command line arguments to be passed to the application on startup.
> +        timeout: Timeout used for the SSH channel that is dedicated to this interactive
> +            shell. This timeout is for collecting output, so if reading from the buffer
> +            and no output is gathered within the timeout, an exception is thrown.
> +
> +    Attributes
> +        _default_prompt: Prompt to expect at the end of output when sending a command.
> +            This is often overridden by derived classes.
> +        _command_extra_chars: Extra characters to add to the end of every command
> +            before sending them. This is often overridden by derived classes and is
> +            most commonly an additional newline character.
> +        path: Path to the executable to start the interactive application.
> +        dpdk_app: Whether this application is a DPDK app. If it is, the build
> +            directory for DPDK on the node will be prepended to the path to the
> +            executable.
> +    """
> +
> +    _interactive_session: SSHClient
> +    _stdin: channel.ChannelStdinFile
> +    _stdout: channel.ChannelFile
> +    _ssh_channel: Channel
> +    _logger: DTSLOG
> +    _timeout: float
> +    _app_args: str
> +    _default_prompt: str = ""
> +    _command_extra_chars: str = ""
> +    path: PurePath
> +    dpdk_app: bool = False
> +
> +    def __init__(
> +        self,
> +        interactive_session: SSHClient,
> +        logger: DTSLOG,
> +        get_privileged_command: Callable[[str], str] | None,
> +        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._app_args = app_args
> +        self._start_application(get_privileged_command)
> +
> +    def _start_application(
> +        self, get_privileged_command: Callable[[str], str] | None
> +    ) -> None:
> +        """Starts a new interactive application based on the path to the app.
> +
> +        This method is often overridden by subclasses as their process for
> +        starting may look different.
> +        """
> +        start_command = f"{self.path} {self._app_args}"
> +        if get_privileged_command is not None:
> +            start_command = get_privileged_command(start_command)
> +        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 workaround 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}'")
> +        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()