Ran a couple test suites to observe the output. As expected,
everything works fine.
Tested-by: Nicholas Pratte <npra...@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npra...@iol.unh.edu>
On Wed, May 29, 2024 at 3:49 PM <jspew...@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspew...@iol.unh.edu>
>
> The messages being logged by interactive shells currently are using the
> same logger as the node they were created from. Because of this, when
> sending interactive commands, the logs make no distinction between when
> you are sending a command directly to the host and when you are using an
> interactive shell on the host. This change adds names to interactive
> shells so that they are able to use their own loggers with distinct
> names.
>
> Signed-off-by: Jeremy Spewock <jspew...@iol.unh.edu>
> ---
> .../remote_session/interactive_remote_session.py | 5 +++--
> dts/framework/remote_session/interactive_shell.py | 9 +++++----
> dts/framework/testbed_model/node.py | 7 +++++++
> dts/framework/testbed_model/os_session.py | 6 ++++--
> dts/framework/testbed_model/sut_node.py | 7 ++++++-
> dts/framework/testbed_model/traffic_generator/scapy.py | 2 +-
> 6 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/dts/framework/remote_session/interactive_remote_session.py
> b/dts/framework/remote_session/interactive_remote_session.py
> index c50790db79..4ddad4f428 100644
> --- a/dts/framework/remote_session/interactive_remote_session.py
> +++ b/dts/framework/remote_session/interactive_remote_session.py
> @@ -39,6 +39,7 @@ class InteractiveRemoteSession:
> password: Password of the user connecting to the host. This will
> default to an
> empty string if a password is not provided.
> session: The underlying paramiko connection.
> + node_config: The configuration of the node that this session is
> connected to.
>
> Raises:
> SSHConnectionError: There is an error creating the SSH connection.
> @@ -50,8 +51,8 @@ class InteractiveRemoteSession:
> username: str
> password: str
> session: SSHClient
> + node_config: NodeConfiguration
> _logger: DTSLogger
> - _node_config: NodeConfiguration
> _transport: Transport | None
>
> def __init__(self, node_config: NodeConfiguration, logger: DTSLogger) ->
> None:
> @@ -61,7 +62,7 @@ def __init__(self, node_config: NodeConfiguration, logger:
> DTSLogger) -> None:
> node_config: The test run configuration of the node to connect
> to.
> logger: The logger instance this session will use.
> """
> - self._node_config = node_config
> + self.node_config = node_config
> self._logger = logger
> self.hostname = node_config.hostname
> self.username = node_config.user
> diff --git a/dts/framework/remote_session/interactive_shell.py
> b/dts/framework/remote_session/interactive_shell.py
> index 148907f645..edc8f11443 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -24,7 +24,7 @@
> InteractiveSSHSessionDeadError,
> InteractiveSSHTimeoutError,
> )
> -from framework.logger import DTSLogger
> +from framework.logger import DTSLogger, get_dts_logger
> from framework.settings import SETTINGS
>
> from .interactive_remote_session import InteractiveRemoteSession
> @@ -69,8 +69,8 @@ class InteractiveShell(ABC):
>
> def __init__(
> self,
> + name: str,
> interactive_session: InteractiveRemoteSession,
> - logger: DTSLogger,
> get_privileged_command: Callable[[str], str] | None,
> app_args: str = "",
> timeout: float = SETTINGS.timeout,
> @@ -78,8 +78,9 @@ def __init__(
> """Create an SSH channel during initialization.
>
> Args:
> + name: Name for the interactive shell to use for logging. This
> name will be appended to
> + the name of the underlying node which it is running on.
> interactive_session: The SSH session dedicated to interactive
> shells.
> - logger: The logger instance this session will use.
> get_privileged_command: A method for modifying a command to
> allow it to use
> elevated privileges. If :data:`None`, the application will
> not be started
> with elevated privileges.
> @@ -94,7 +95,7 @@ def __init__(
> 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._logger =
> get_dts_logger(f"{interactive_session.node_config.name}.{name}")
> self._timeout = timeout
> self._app_args = app_args
> self._start_application(get_privileged_command)
> diff --git a/dts/framework/testbed_model/node.py
> b/dts/framework/testbed_model/node.py
> index 74061f6262..a5beeae7e7 100644
> --- a/dts/framework/testbed_model/node.py
> +++ b/dts/framework/testbed_model/node.py
> @@ -199,6 +199,7 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float = SETTINGS.timeout,
> privileged: bool = False,
> + name: str = "",
> app_args: str = "",
> ) -> InteractiveShellType:
> """Factory for interactive session handlers.
> @@ -210,6 +211,8 @@ def create_interactive_shell(
> timeout: Timeout for reading output from the SSH channel. If you
> are reading from
> the buffer and don't receive any data within the timeout it
> will throw an error.
> privileged: Whether to run the shell with administrative
> privileges.
> + name: The name to use for the interactive application in the
> logs. If not specified,
> + this will default to the name of `shell_cls`.
> app_args: The arguments to be passed to the application.
>
> Returns:
> @@ -218,10 +221,14 @@ def create_interactive_shell(
> if not shell_cls.dpdk_app:
> shell_cls.path =
> self.main_session.join_remote_path(shell_cls.path)
>
> + if name == "":
> + name = shell_cls.__name__
> +
> return self.main_session.create_interactive_shell(
> shell_cls,
> timeout,
> privileged,
> + name,
> app_args,
> )
>
> diff --git a/dts/framework/testbed_model/os_session.py
> b/dts/framework/testbed_model/os_session.py
> index d5bf7e0401..1f6d8257ed 100644
> --- a/dts/framework/testbed_model/os_session.py
> +++ b/dts/framework/testbed_model/os_session.py
> @@ -134,6 +134,7 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float,
> privileged: bool,
> + name: str,
> app_args: str,
> ) -> InteractiveShellType:
> """Factory for interactive session handlers.
> @@ -146,14 +147,15 @@ def create_interactive_shell(
> reading from the buffer and don't receive any data within
> the timeout
> it will throw an error.
> privileged: Whether to run the shell with administrative
> privileges.
> + name: Name for the shell to use in the logs.
> app_args: The arguments to be passed to the application.
>
> Returns:
> An instance of the desired interactive application shell.
> """
> return shell_cls(
> - self.interactive_session.session,
> - self._logger,
> + name,
> + self.interactive_session,
> self._get_privileged_command if privileged else None,
> app_args,
> timeout,
> diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> index 97aa26d419..0bddef6f44 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -442,6 +442,7 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float = SETTINGS.timeout,
> privileged: bool = False,
> + name: str = "",
> app_parameters: str = "",
> eal_parameters: EalParameters | None = None,
> ) -> InteractiveShellType:
> @@ -459,6 +460,8 @@ def create_interactive_shell(
> reading from the buffer and don't receive any data within
> the timeout
> it will throw an error.
> privileged: Whether to run the shell with administrative
> privileges.
> + name: The name to use for the interactive application in the
> logs. If not specified,
> + this will default to the name `shell_cls`.
> eal_parameters: List of EAL parameters to use to launch the app.
> If this
> isn't provided or an empty string is passed, it will default
> to calling
> :meth:`create_eal_parameters`.
> @@ -478,7 +481,9 @@ def create_interactive_shell(
> self.remote_dpdk_build_dir, shell_cls.path
> )
>
> - return super().create_interactive_shell(shell_cls, timeout,
> privileged, app_parameters)
> + return super().create_interactive_shell(
> + shell_cls, timeout, privileged, name, app_parameters
> + )
>
> def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> """Bind all ports on the SUT to a driver.
> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py
> b/dts/framework/testbed_model/traffic_generator/scapy.py
> index d0e0a7c64e..5676235119 100644
> --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> +++ b/dts/framework/testbed_model/traffic_generator/scapy.py
> @@ -262,7 +262,7 @@ def __init__(self, tg_node: Node, config:
> ScapyTrafficGeneratorConfig):
> ), "Linux is the only supported OS for scapy traffic generation"
>
> self.session = self._tg_node.create_interactive_shell(
> - PythonShell, timeout=5, privileged=True
> + PythonShell, timeout=5, privileged=True, name="ScapyXMLRPCServer"
> )
>
> # import libs in remote python console
> --
> 2.45.1
>
