* [PATCH v2 1/3] dts: Improve output gathering in interactive shells
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
@ 2024-05-01 16:16 ` jspewock
2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 14:58 ` Juraj Linkeš
2024-05-01 16:16 ` [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server jspewock
` (6 subsequent siblings)
7 siblings, 2 replies; 57+ messages in thread
From: jspewock @ 2024-05-01 16:16 UTC (permalink / raw)
To: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
The current implementation of consuming output from interactive shells
relies on being able to find an expected prompt somewhere within the
output buffer after sending the command. This is useful in situations
where the prompt does not appear in the output itself, but in some
practical cases (such as the starting of an XML-RPC server for scapy)
the prompt exists in one of the commands sent to the shell and this can
cause the command to exit early and creates a race condition between the
server starting and the first command being sent to the server.
This patch addresses this problem by searching for a line that strictly
ends with the provided prompt, rather than one that simply contains it,
so that the detection that a command is finished is more consistent. It
also adds a catch to detect when a command times out before finding the
prompt or the underlying SSH session dies so that the exception can be
wrapped into a more explicit one and be more consistent with the
non-interactive shells.
Bugzilla ID: 1359
Fixes: 88489c0501af ("dts: add smoke tests")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/exception.py | 66 ++++++++++++-------
.../remote_session/interactive_shell.py | 46 +++++++++----
2 files changed, 77 insertions(+), 35 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index cce1e0231a..627190c781 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -48,26 +48,6 @@ class DTSError(Exception):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
-class SSHTimeoutError(DTSError):
- """The SSH execution of a command timed out."""
-
- #:
- severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
- _command: str
-
- def __init__(self, command: str):
- """Define the meaning of the first argument.
-
- Args:
- command: The executed command.
- """
- self._command = command
-
- def __str__(self) -> str:
- """Add some context to the string representation."""
- return f"{self._command} execution timed out."
-
-
class SSHConnectionError(DTSError):
"""An unsuccessful SSH connection."""
@@ -95,8 +75,42 @@ def __str__(self) -> str:
return message
-class SSHSessionDeadError(DTSError):
- """The SSH session is no longer alive."""
+class _SSHTimeoutError(DTSError):
+ """The execution of a command via SSH timed out.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
+
+ #:
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
+ _command: str
+
+ def __init__(self, command: str):
+ """Define the meaning of the first argument.
+
+ Args:
+ command: The executed command.
+ """
+ self._command = command
+
+ def __str__(self) -> str:
+ """Add some context to the string representation."""
+ return f"{self._command} execution timed out."
+
+
+class SSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on a non-interactive SSH session timed out."""
+
+
+class InteractiveSSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on an interactive SSH session timed out."""
+
+
+class _SSHSessionDeadError(DTSError):
+ """The SSH session is no longer alive.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
#:
severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
@@ -115,6 +129,14 @@ def __str__(self) -> str:
return f"SSH session with {self._host} has died."
+class SSHSessionDeadError(_SSHSessionDeadError):
+ """Non-interactive SSH session has died."""
+
+
+class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
+ """Interactive SSH session as died."""
+
+
class ConfigurationError(DTSError):
"""An invalid configuration."""
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 5cfe202e15..0b0ccdb545 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -18,11 +18,17 @@
from pathlib import PurePath
from typing import Callable, ClassVar
-from paramiko import Channel, SSHClient, channel # type: ignore[import]
+from paramiko import Channel, channel # type: ignore[import]
+from framework.exception import (
+ InteractiveSSHSessionDeadError,
+ InteractiveSSHTimeoutError,
+)
from framework.logger import DTSLogger
from framework.settings import SETTINGS
+from .interactive_remote_session import InteractiveRemoteSession
+
class InteractiveShell(ABC):
"""The base class for managing interactive shells.
@@ -34,7 +40,7 @@ class InteractiveShell(ABC):
session.
"""
- _interactive_session: SSHClient
+ _interactive_session: InteractiveRemoteSession
_stdin: channel.ChannelStdinFile
_stdout: channel.ChannelFile
_ssh_channel: Channel
@@ -60,7 +66,7 @@ class InteractiveShell(ABC):
def __init__(
self,
- interactive_session: SSHClient,
+ interactive_session: InteractiveRemoteSession,
logger: DTSLogger,
get_privileged_command: Callable[[str], str] | None,
app_args: str = "",
@@ -80,7 +86,7 @@ def __init__(
and no output is gathered within the timeout, an exception is thrown.
"""
self._interactive_session = interactive_session
- self._ssh_channel = self._interactive_session.invoke_shell()
+ self._ssh_channel = self._interactive_session.session.invoke_shell()
self._stdin = self._ssh_channel.makefile_stdin("w")
self._stdout = self._ssh_channel.makefile("r")
self._ssh_channel.settimeout(timeout)
@@ -124,20 +130,34 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
Returns:
All output in the buffer before expected string.
+
+ Raises:
+ InteractiveSSHSessionDeadError: The session died while executing the command.
+ InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
+ the output before the timeout.
"""
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}")
+ try:
+ self._stdin.write(f"{command}{self._command_extra_chars}\n")
+ self._stdin.flush()
+ for line in self._stdout:
+ out += line
+ if line.rstrip().endswith(prompt):
+ break
+ except TimeoutError as e:
+ self._logger.exception(e)
+ self._logger.debug(
+ f"Prompt ({prompt}) was not found in output from command before timeout."
+ )
+ raise InteractiveSSHTimeoutError(command) from e
+ except OSError as e:
+ self._logger.exception(e)
+ raise InteractiveSSHSessionDeadError(self._interactive_session.hostname) from e
+ finally:
+ self._logger.debug(f"Got output: {out}")
return out
def close(self) -> None:
--
2.44.0
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 1/3] dts: Improve output gathering in interactive shells
2024-05-01 16:16 ` [PATCH v2 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 14:58 ` Juraj Linkeš
1 sibling, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-09 9:57 UTC (permalink / raw)
To: jspewock, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v2 1/3] dts: Improve output gathering in interactive shells
2024-05-01 16:16 ` [PATCH v2 1/3] dts: Improve output gathering in interactive shells jspewock
2024-05-09 9:57 ` Luca Vizzarro
@ 2024-05-13 14:58 ` Juraj Linkeš
2024-05-15 19:13 ` Jeremy Spewock
1 sibling, 1 reply; 57+ messages in thread
From: Juraj Linkeš @ 2024-05-13 14:58 UTC (permalink / raw)
To: jspewock
Cc: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, paul.szczepanek,
probb, thomas, Honnappa.Nagarahalli, dev
Other than the one minor documentation nitpick,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
<snip>
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> @@ -124,20 +130,34 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
>
> Returns:
> All output in the buffer before expected string.
> +
> + Raises:
> + InteractiveSSHSessionDeadError: The session died while executing the command.
> + InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
> + the output before the timeout.
> """
> 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}")
> + try:
> + self._stdin.write(f"{command}{self._command_extra_chars}\n")
> + self._stdin.flush()
> + for line in self._stdout:
> + out += line
> + if line.rstrip().endswith(prompt):
> + break
We should document the (currently) hidden assumption of us needing to
use the extra command chars to force another prompt in the docstring.
> + except TimeoutError as e:
> + self._logger.exception(e)
> + self._logger.debug(
> + f"Prompt ({prompt}) was not found in output from command before timeout."
> + )
> + raise InteractiveSSHTimeoutError(command) from e
> + except OSError as e:
> + self._logger.exception(e)
> + raise InteractiveSSHSessionDeadError(self._interactive_session.hostname) from e
> + finally:
> + self._logger.debug(f"Got output: {out}")
> return out
>
> def close(self) -> None:
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 1/3] dts: Improve output gathering in interactive shells
2024-05-13 14:58 ` Juraj Linkeš
@ 2024-05-15 19:13 ` Jeremy Spewock
0 siblings, 0 replies; 57+ messages in thread
From: Jeremy Spewock @ 2024-05-15 19:13 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, paul.szczepanek,
probb, thomas, Honnappa.Nagarahalli, dev
On Mon, May 13, 2024 at 10:58 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
>
> Other than the one minor documentation nitpick,
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
> <snip>
> > diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
>
> > @@ -124,20 +130,34 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
<snip>
> > - 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}")
> > + try:
> > + self._stdin.write(f"{command}{self._command_extra_chars}\n")
> > + self._stdin.flush()
> > + for line in self._stdout:
> > + out += line
> > + if line.rstrip().endswith(prompt):
> > + break
>
> We should document the (currently) hidden assumption of us needing to
> use the extra command chars to force another prompt in the docstring.
>
This is a good point, it is mostly internal knowledge currently so it
would be good to explain this more. I'll add it to the comment
documenting what the class var is for.
<snip>
> > 2.44.0
> >
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
2024-05-01 16:16 ` [PATCH v2 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-05-01 16:16 ` jspewock
2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 14:58 ` Juraj Linkeš
2024-05-01 16:16 ` [PATCH v2 3/3] dts: Improve logging for interactive shells jspewock
` (5 subsequent siblings)
7 siblings, 2 replies; 57+ messages in thread
From: jspewock @ 2024-05-01 16:16 UTC (permalink / raw)
To: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
When this XML-RPC server implementation was added, the docstring had to
be shortened in order to reduce the chances of this race condition being
encountered. Now that this race condition issue is resolved, the full
docstring can be restored.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../testbed_model/traffic_generator/scapy.py | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index df3069d516..d0e0a7c64e 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -128,9 +128,53 @@ def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: s
class QuittableXMLRPCServer(SimpleXMLRPCServer):
- """Basic XML-RPC server.
+ r"""Basic XML-RPC server.
The server may be augmented by functions serializable by the :mod:`marshal` module.
+
+ Example:
+ ::
+
+ def hello_world():
+ # to be sent to the XML-RPC server
+ print("Hello World!")
+
+ # start the XML-RPC server on the remote node
+ # the example assumes you're already connect to a tg_node
+ # this is done by starting a Python shell on the remote node
+ from framework.remote_session import PythonShell
+ session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+
+ # then importing the modules needed to run the server
+ # and the modules for any functions later added to the server
+ session.send_command("import xmlrpc")
+ session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+
+ # sending the source code of this class to the Python shell
+ from xmlrpc.server import SimpleXMLRPCServer
+ src = inspect.getsource(QuittableXMLRPCServer)
+ src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
+ spacing = "\n" * 4
+ session.send_command(spacing + src + spacing)
+
+ # then starting the server with:
+ command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
+ session.send_command(command, "XMLRPC OK")
+
+ # now the server is running on the remote node and we can add functions to it
+ # first connect to the server from the execution node
+ import xmlrpc.client
+ server_url = f"http://{tg_node.config.hostname}:8000"
+ rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+
+ # get the function bytes to send
+ import marshal
+ function_bytes = marshal.dumps(hello_world.__code__)
+ rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+
+ # now we can execute the function on the server
+ xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
+ print(str(xmlrpc_binary_recv))
"""
def __init__(self, *args, **kwargs):
--
2.44.0
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server
2024-05-01 16:16 ` [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 14:58 ` Juraj Linkeš
1 sibling, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-09 9:57 UTC (permalink / raw)
To: jspewock, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server
2024-05-01 16:16 ` [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server jspewock
2024-05-09 9:57 ` Luca Vizzarro
@ 2024-05-13 14:58 ` Juraj Linkeš
1 sibling, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-05-13 14:58 UTC (permalink / raw)
To: jspewock
Cc: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, paul.szczepanek,
probb, thomas, Honnappa.Nagarahalli, dev
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
On Wed, May 1, 2024 at 6:17 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> When this XML-RPC server implementation was added, the docstring had to
> be shortened in order to reduce the chances of this race condition being
> encountered. Now that this race condition issue is resolved, the full
> docstring can be restored.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v2 3/3] dts: Improve logging for interactive shells
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
2024-05-01 16:16 ` [PATCH v2 1/3] dts: Improve output gathering in interactive shells jspewock
2024-05-01 16:16 ` [PATCH v2 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-05-01 16:16 ` jspewock
2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 15:02 ` Juraj Linkeš
2024-05-09 9:59 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging Luca Vizzarro
` (4 subsequent siblings)
7 siblings, 2 replies; 57+ messages in thread
From: jspewock @ 2024-05-01 16:16 UTC (permalink / raw)
To: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
---
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 +-
5 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 0b0ccdb545..eb9c9b6843 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
@@ -66,8 +66,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,
@@ -75,8 +75,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.
@@ -91,7 +92,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.44.0
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 3/3] dts: Improve logging for interactive shells
2024-05-01 16:16 ` [PATCH v2 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-05-09 9:57 ` Luca Vizzarro
2024-05-13 15:02 ` Juraj Linkeš
1 sibling, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-09 9:57 UTC (permalink / raw)
To: jspewock, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v2 3/3] dts: Improve logging for interactive shells
2024-05-01 16:16 ` [PATCH v2 3/3] dts: Improve logging for interactive shells jspewock
2024-05-09 9:57 ` Luca Vizzarro
@ 2024-05-13 15:02 ` Juraj Linkeš
2024-05-15 19:23 ` Jeremy Spewock
1 sibling, 1 reply; 57+ messages in thread
From: Juraj Linkeš @ 2024-05-13 15:02 UTC (permalink / raw)
To: jspewock
Cc: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, paul.szczepanek,
probb, thomas, Honnappa.Nagarahalli, dev
Good idea. And again, with the one minor point below,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> index 0b0ccdb545..eb9c9b6843 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -91,7 +92,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}")
We should make interactive_session._node_config public since we're
referencing it outside the class.
> self._timeout = timeout
> self._app_args = app_args
> self._start_application(get_privileged_command)
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 3/3] dts: Improve logging for interactive shells
2024-05-13 15:02 ` Juraj Linkeš
@ 2024-05-15 19:23 ` Jeremy Spewock
0 siblings, 0 replies; 57+ messages in thread
From: Jeremy Spewock @ 2024-05-15 19:23 UTC (permalink / raw)
To: Juraj Linkeš
Cc: Luca.Vizzarro, wathsala.vithanage, yoan.picchi, paul.szczepanek,
probb, thomas, Honnappa.Nagarahalli, dev
On Mon, May 13, 2024 at 11:03 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
>
> Good idea. And again, with the one minor point below,
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
> > diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> > index 0b0ccdb545..eb9c9b6843 100644
> > --- a/dts/framework/remote_session/interactive_shell.py
> > +++ b/dts/framework/remote_session/interactive_shell.py
>
> > @@ -91,7 +92,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}")
>
> We should make interactive_session._node_config public since we're
> referencing it outside the class.
good point, I'll make this change as well.
>
> > self._timeout = timeout
> > self._app_args = app_args
> > self._start_application(get_privileged_command)
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v2 0/3] Improve interactive shell output gathering and logging
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
` (2 preceding siblings ...)
2024-05-01 16:16 ` [PATCH v2 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-05-09 9:59 ` Luca Vizzarro
2024-05-20 15:08 ` Nicholas Pratte
2024-05-29 19:49 ` [PATCH v3 " jspewock
` (3 subsequent siblings)
7 siblings, 1 reply; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-09 9:59 UTC (permalink / raw)
To: jspewock, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli
Cc: dev
On 01/05/2024 17:16, jspewock@iol.unh.edu wrote:
> This version addresses comments from the last and adds an additional
> improvement to the logging output that comes from interactive shells.
> The new way of handling logging allows for not only specification of the
> host that the command is being sent to like before, but also the type of
> shell that you are addressing. This is beneficial for 2 reasons:
>
> 1. Less ambiguity on if a command is being sent to the host directly or
> into an interactive shell
> 2. Ability to name shells so that there is distinction in the logs
> between two different interactive shells of the same type that serve a
> different purpose (e.g. a python shell and a python shell that is
> serving and XML-RPC server for scapy interactions)
Hi Jeremy,
Thank you for your patches! Everything looks good to me.
Best,
Luca
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v2 0/3] Improve interactive shell output gathering and logging
2024-05-09 9:59 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging Luca Vizzarro
@ 2024-05-20 15:08 ` Nicholas Pratte
0 siblings, 0 replies; 57+ messages in thread
From: Nicholas Pratte @ 2024-05-20 15:08 UTC (permalink / raw)
To: Luca Vizzarro
Cc: jspewock, wathsala.vithanage, yoan.picchi, juraj.linkes,
paul.szczepanek, probb, thomas, Honnappa.Nagarahalli, dev
Acked-by: Nicholas Pratte <npratte@iol.unh.edu>
On Thu, May 9, 2024 at 5:59 AM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> On 01/05/2024 17:16, jspewock@iol.unh.edu wrote:
> > This version addresses comments from the last and adds an additional
> > improvement to the logging output that comes from interactive shells.
> > The new way of handling logging allows for not only specification of the
> > host that the command is being sent to like before, but also the type of
> > shell that you are addressing. This is beneficial for 2 reasons:
> >
> > 1. Less ambiguity on if a command is being sent to the host directly or
> > into an interactive shell
> > 2. Ability to name shells so that there is distinction in the logs
> > between two different interactive shells of the same type that serve a
> > different purpose (e.g. a python shell and a python shell that is
> > serving and XML-RPC server for scapy interactions)
>
> Hi Jeremy,
>
> Thank you for your patches! Everything looks good to me.
>
> Best,
> Luca
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v3 0/3] Improve interactive shell output gathering and logging
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
` (3 preceding siblings ...)
2024-05-09 9:59 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging Luca Vizzarro
@ 2024-05-29 19:49 ` jspewock
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
` (2 more replies)
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
` (2 subsequent siblings)
7 siblings, 3 replies; 57+ messages in thread
From: jspewock @ 2024-05-29 19:49 UTC (permalink / raw)
To: paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, juraj.linkes, Honnappa.Nagarahalli,
yoan.picchi
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
This version addresses the small comments that were on the previous
version such as documentation of variable name changes.
Jeremy Spewock (3):
dts: Improve output gathering in interactive shells
dts: Add missing docstring from XML-RPC server
dts: Improve logging for interactive shells
dts/framework/exception.py | 66 ++++++++++++-------
.../interactive_remote_session.py | 5 +-
.../remote_session/interactive_shell.py | 60 ++++++++++++-----
dts/framework/testbed_model/node.py | 7 ++
dts/framework/testbed_model/os_session.py | 6 +-
dts/framework/testbed_model/sut_node.py | 7 +-
.../testbed_model/traffic_generator/scapy.py | 48 +++++++++++++-
7 files changed, 152 insertions(+), 47 deletions(-)
--
2.45.1
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v3 1/3] dts: Improve output gathering in interactive shells
2024-05-29 19:49 ` [PATCH v3 " jspewock
@ 2024-05-29 19:49 ` jspewock
2024-05-31 16:49 ` Luca Vizzarro
` (3 more replies)
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
2 siblings, 4 replies; 57+ messages in thread
From: jspewock @ 2024-05-29 19:49 UTC (permalink / raw)
To: paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, juraj.linkes, Honnappa.Nagarahalli,
yoan.picchi
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
The current implementation of consuming output from interactive shells
relies on being able to find an expected prompt somewhere within the
output buffer after sending the command. This is useful in situations
where the prompt does not appear in the output itself, but in some
practical cases (such as the starting of an XML-RPC server for scapy)
the prompt exists in one of the commands sent to the shell and this can
cause the command to exit early and creates a race condition between the
server starting and the first command being sent to the server.
This patch addresses this problem by searching for a line that strictly
ends with the provided prompt, rather than one that simply contains it,
so that the detection that a command is finished is more consistent. It
also adds a catch to detect when a command times out before finding the
prompt or the underlying SSH session dies so that the exception can be
wrapped into a more explicit one and be more consistent with the
non-interactive shells.
Bugzilla ID: 1359
Fixes: 88489c0501af ("dts: add smoke tests")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/exception.py | 66 ++++++++++++-------
.../remote_session/interactive_shell.py | 51 ++++++++++----
2 files changed, 81 insertions(+), 36 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index cce1e0231a..627190c781 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -48,26 +48,6 @@ class DTSError(Exception):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
-class SSHTimeoutError(DTSError):
- """The SSH execution of a command timed out."""
-
- #:
- severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
- _command: str
-
- def __init__(self, command: str):
- """Define the meaning of the first argument.
-
- Args:
- command: The executed command.
- """
- self._command = command
-
- def __str__(self) -> str:
- """Add some context to the string representation."""
- return f"{self._command} execution timed out."
-
-
class SSHConnectionError(DTSError):
"""An unsuccessful SSH connection."""
@@ -95,8 +75,42 @@ def __str__(self) -> str:
return message
-class SSHSessionDeadError(DTSError):
- """The SSH session is no longer alive."""
+class _SSHTimeoutError(DTSError):
+ """The execution of a command via SSH timed out.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
+
+ #:
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
+ _command: str
+
+ def __init__(self, command: str):
+ """Define the meaning of the first argument.
+
+ Args:
+ command: The executed command.
+ """
+ self._command = command
+
+ def __str__(self) -> str:
+ """Add some context to the string representation."""
+ return f"{self._command} execution timed out."
+
+
+class SSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on a non-interactive SSH session timed out."""
+
+
+class InteractiveSSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on an interactive SSH session timed out."""
+
+
+class _SSHSessionDeadError(DTSError):
+ """The SSH session is no longer alive.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
#:
severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
@@ -115,6 +129,14 @@ def __str__(self) -> str:
return f"SSH session with {self._host} has died."
+class SSHSessionDeadError(_SSHSessionDeadError):
+ """Non-interactive SSH session has died."""
+
+
+class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
+ """Interactive SSH session as died."""
+
+
class ConfigurationError(DTSError):
"""An invalid configuration."""
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 5cfe202e15..148907f645 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -18,11 +18,17 @@
from pathlib import PurePath
from typing import Callable, ClassVar
-from paramiko import Channel, SSHClient, channel # type: ignore[import]
+from paramiko import Channel, channel # type: ignore[import]
+from framework.exception import (
+ InteractiveSSHSessionDeadError,
+ InteractiveSSHTimeoutError,
+)
from framework.logger import DTSLogger
from framework.settings import SETTINGS
+from .interactive_remote_session import InteractiveRemoteSession
+
class InteractiveShell(ABC):
"""The base class for managing interactive shells.
@@ -34,7 +40,7 @@ class InteractiveShell(ABC):
session.
"""
- _interactive_session: SSHClient
+ _interactive_session: InteractiveRemoteSession
_stdin: channel.ChannelStdinFile
_stdout: channel.ChannelFile
_ssh_channel: Channel
@@ -48,7 +54,10 @@ class InteractiveShell(ABC):
#: Extra characters to add to the end of every command
#: before sending them. This is often overridden by subclasses and is
- #: most commonly an additional newline character.
+ #: most commonly an additional newline character. This additional newline
+ #: character is used to force the line that is currently awaiting input
+ #: into the stdout buffer so that it can be consumed and checked against
+ #: the expected prompt.
_command_extra_chars: ClassVar[str] = ""
#: Path to the executable to start the interactive application.
@@ -60,7 +69,7 @@ class InteractiveShell(ABC):
def __init__(
self,
- interactive_session: SSHClient,
+ interactive_session: InteractiveRemoteSession,
logger: DTSLogger,
get_privileged_command: Callable[[str], str] | None,
app_args: str = "",
@@ -80,7 +89,7 @@ def __init__(
and no output is gathered within the timeout, an exception is thrown.
"""
self._interactive_session = interactive_session
- self._ssh_channel = self._interactive_session.invoke_shell()
+ self._ssh_channel = self._interactive_session.session.invoke_shell()
self._stdin = self._ssh_channel.makefile_stdin("w")
self._stdout = self._ssh_channel.makefile("r")
self._ssh_channel.settimeout(timeout)
@@ -124,20 +133,34 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
Returns:
All output in the buffer before expected string.
+
+ Raises:
+ InteractiveSSHSessionDeadError: The session died while executing the command.
+ InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
+ the output before the timeout.
"""
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}")
+ try:
+ self._stdin.write(f"{command}{self._command_extra_chars}\n")
+ self._stdin.flush()
+ for line in self._stdout:
+ out += line
+ if line.rstrip().endswith(prompt):
+ break
+ except TimeoutError as e:
+ self._logger.exception(e)
+ self._logger.debug(
+ f"Prompt ({prompt}) was not found in output from command before timeout."
+ )
+ raise InteractiveSSHTimeoutError(command) from e
+ except OSError as e:
+ self._logger.exception(e)
+ raise InteractiveSSHSessionDeadError(self._interactive_session.hostname) from e
+ finally:
+ self._logger.debug(f"Got output: {out}")
return out
def close(self) -> None:
--
2.45.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 1/3] dts: Improve output gathering in interactive shells
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-05-31 16:49 ` Luca Vizzarro
2024-06-07 13:37 ` Juraj Linkeš
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-31 16:49 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 1/3] dts: Improve output gathering in interactive shells
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
2024-05-31 16:49 ` Luca Vizzarro
@ 2024-06-07 13:37 ` Juraj Linkeš
2024-06-14 20:58 ` Nicholas Pratte
2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-07 13:37 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
On 29. 5. 2024 21:49, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> The current implementation of consuming output from interactive shells
> relies on being able to find an expected prompt somewhere within the
> output buffer after sending the command. This is useful in situations
> where the prompt does not appear in the output itself, but in some
> practical cases (such as the starting of an XML-RPC server for scapy)
> the prompt exists in one of the commands sent to the shell and this can
> cause the command to exit early and creates a race condition between the
> server starting and the first command being sent to the server.
>
> This patch addresses this problem by searching for a line that strictly
> ends with the provided prompt, rather than one that simply contains it,
> so that the detection that a command is finished is more consistent. It
> also adds a catch to detect when a command times out before finding the
> prompt or the underlying SSH session dies so that the exception can be
> wrapped into a more explicit one and be more consistent with the
> non-interactive shells.
>
> Bugzilla ID: 1359
> Fixes: 88489c0501af ("dts: add smoke tests")
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 1/3] dts: Improve output gathering in interactive shells
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
2024-05-31 16:49 ` Luca Vizzarro
2024-06-07 13:37 ` Juraj Linkeš
@ 2024-06-14 20:58 ` Nicholas Pratte
2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Nicholas Pratte @ 2024-06-14 20:58 UTC (permalink / raw)
To: jspewock
Cc: paul.szczepanek, wathsala.vithanage, probb, Luca.Vizzarro,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi, dev
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Wed, May 29, 2024 at 3:49 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> The current implementation of consuming output from interactive shells
> relies on being able to find an expected prompt somewhere within the
> output buffer after sending the command. This is useful in situations
> where the prompt does not appear in the output itself, but in some
> practical cases (such as the starting of an XML-RPC server for scapy)
> the prompt exists in one of the commands sent to the shell and this can
> cause the command to exit early and creates a race condition between the
> server starting and the first command being sent to the server.
>
> This patch addresses this problem by searching for a line that strictly
> ends with the provided prompt, rather than one that simply contains it,
> so that the detection that a command is finished is more consistent. It
> also adds a catch to detect when a command times out before finding the
> prompt or the underlying SSH session dies so that the exception can be
> wrapped into a more explicit one and be more consistent with the
> non-interactive shells.
>
> Bugzilla ID: 1359
> Fixes: 88489c0501af ("dts: add smoke tests")
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/exception.py | 66 ++++++++++++-------
> .../remote_session/interactive_shell.py | 51 ++++++++++----
> 2 files changed, 81 insertions(+), 36 deletions(-)
>
> diff --git a/dts/framework/exception.py b/dts/framework/exception.py
> index cce1e0231a..627190c781 100644
> --- a/dts/framework/exception.py
> +++ b/dts/framework/exception.py
> @@ -48,26 +48,6 @@ class DTSError(Exception):
> severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
>
>
> -class SSHTimeoutError(DTSError):
> - """The SSH execution of a command timed out."""
> -
> - #:
> - severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
> - _command: str
> -
> - def __init__(self, command: str):
> - """Define the meaning of the first argument.
> -
> - Args:
> - command: The executed command.
> - """
> - self._command = command
> -
> - def __str__(self) -> str:
> - """Add some context to the string representation."""
> - return f"{self._command} execution timed out."
> -
> -
> class SSHConnectionError(DTSError):
> """An unsuccessful SSH connection."""
>
> @@ -95,8 +75,42 @@ def __str__(self) -> str:
> return message
>
>
> -class SSHSessionDeadError(DTSError):
> - """The SSH session is no longer alive."""
> +class _SSHTimeoutError(DTSError):
> + """The execution of a command via SSH timed out.
> +
> + This class is private and meant to be raised as its interactive and non-interactive variants.
> + """
> +
> + #:
> + severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
> + _command: str
> +
> + def __init__(self, command: str):
> + """Define the meaning of the first argument.
> +
> + Args:
> + command: The executed command.
> + """
> + self._command = command
> +
> + def __str__(self) -> str:
> + """Add some context to the string representation."""
> + return f"{self._command} execution timed out."
> +
> +
> +class SSHTimeoutError(_SSHTimeoutError):
> + """The execution of a command on a non-interactive SSH session timed out."""
> +
> +
> +class InteractiveSSHTimeoutError(_SSHTimeoutError):
> + """The execution of a command on an interactive SSH session timed out."""
> +
> +
> +class _SSHSessionDeadError(DTSError):
> + """The SSH session is no longer alive.
> +
> + This class is private and meant to be raised as its interactive and non-interactive variants.
> + """
>
> #:
> severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
> @@ -115,6 +129,14 @@ def __str__(self) -> str:
> return f"SSH session with {self._host} has died."
>
>
> +class SSHSessionDeadError(_SSHSessionDeadError):
> + """Non-interactive SSH session has died."""
> +
> +
> +class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
> + """Interactive SSH session as died."""
> +
> +
> class ConfigurationError(DTSError):
> """An invalid configuration."""
>
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> index 5cfe202e15..148907f645 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -18,11 +18,17 @@
> from pathlib import PurePath
> from typing import Callable, ClassVar
>
> -from paramiko import Channel, SSHClient, channel # type: ignore[import]
> +from paramiko import Channel, channel # type: ignore[import]
>
> +from framework.exception import (
> + InteractiveSSHSessionDeadError,
> + InteractiveSSHTimeoutError,
> +)
> from framework.logger import DTSLogger
> from framework.settings import SETTINGS
>
> +from .interactive_remote_session import InteractiveRemoteSession
> +
>
> class InteractiveShell(ABC):
> """The base class for managing interactive shells.
> @@ -34,7 +40,7 @@ class InteractiveShell(ABC):
> session.
> """
>
> - _interactive_session: SSHClient
> + _interactive_session: InteractiveRemoteSession
> _stdin: channel.ChannelStdinFile
> _stdout: channel.ChannelFile
> _ssh_channel: Channel
> @@ -48,7 +54,10 @@ class InteractiveShell(ABC):
>
> #: Extra characters to add to the end of every command
> #: before sending them. This is often overridden by subclasses and is
> - #: most commonly an additional newline character.
> + #: most commonly an additional newline character. This additional newline
> + #: character is used to force the line that is currently awaiting input
> + #: into the stdout buffer so that it can be consumed and checked against
> + #: the expected prompt.
> _command_extra_chars: ClassVar[str] = ""
>
> #: Path to the executable to start the interactive application.
> @@ -60,7 +69,7 @@ class InteractiveShell(ABC):
>
> def __init__(
> self,
> - interactive_session: SSHClient,
> + interactive_session: InteractiveRemoteSession,
> logger: DTSLogger,
> get_privileged_command: Callable[[str], str] | None,
> app_args: str = "",
> @@ -80,7 +89,7 @@ def __init__(
> and no output is gathered within the timeout, an exception is thrown.
> """
> self._interactive_session = interactive_session
> - self._ssh_channel = self._interactive_session.invoke_shell()
> + self._ssh_channel = self._interactive_session.session.invoke_shell()
> self._stdin = self._ssh_channel.makefile_stdin("w")
> self._stdout = self._ssh_channel.makefile("r")
> self._ssh_channel.settimeout(timeout)
> @@ -124,20 +133,34 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
>
> Returns:
> All output in the buffer before expected string.
> +
> + Raises:
> + InteractiveSSHSessionDeadError: The session died while executing the command.
> + InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
> + the output before the timeout.
> """
> 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}")
> + try:
> + self._stdin.write(f"{command}{self._command_extra_chars}\n")
> + self._stdin.flush()
> + for line in self._stdout:
> + out += line
> + if line.rstrip().endswith(prompt):
> + break
> + except TimeoutError as e:
> + self._logger.exception(e)
> + self._logger.debug(
> + f"Prompt ({prompt}) was not found in output from command before timeout."
> + )
> + raise InteractiveSSHTimeoutError(command) from e
> + except OSError as e:
> + self._logger.exception(e)
> + raise InteractiveSSHSessionDeadError(self._interactive_session.hostname) from e
> + finally:
> + self._logger.debug(f"Got output: {out}")
> return out
>
> def close(self) -> None:
> --
> 2.45.1
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 1/3] dts: Improve output gathering in interactive shells
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
` (2 preceding siblings ...)
2024-06-14 20:58 ` Nicholas Pratte
@ 2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-06-17 15:00 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-05-29 19:49 ` [PATCH v3 " jspewock
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-05-29 19:49 ` jspewock
2024-05-31 16:50 ` Luca Vizzarro
` (3 more replies)
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
2 siblings, 4 replies; 57+ messages in thread
From: jspewock @ 2024-05-29 19:49 UTC (permalink / raw)
To: paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, juraj.linkes, Honnappa.Nagarahalli,
yoan.picchi
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
When this XML-RPC server implementation was added, the docstring had to
be shortened in order to reduce the chances of this race condition being
encountered. Now that this race condition issue is resolved, the full
docstring can be restored.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../testbed_model/traffic_generator/scapy.py | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index df3069d516..d0e0a7c64e 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -128,9 +128,53 @@ def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: s
class QuittableXMLRPCServer(SimpleXMLRPCServer):
- """Basic XML-RPC server.
+ r"""Basic XML-RPC server.
The server may be augmented by functions serializable by the :mod:`marshal` module.
+
+ Example:
+ ::
+
+ def hello_world():
+ # to be sent to the XML-RPC server
+ print("Hello World!")
+
+ # start the XML-RPC server on the remote node
+ # the example assumes you're already connect to a tg_node
+ # this is done by starting a Python shell on the remote node
+ from framework.remote_session import PythonShell
+ session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+
+ # then importing the modules needed to run the server
+ # and the modules for any functions later added to the server
+ session.send_command("import xmlrpc")
+ session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+
+ # sending the source code of this class to the Python shell
+ from xmlrpc.server import SimpleXMLRPCServer
+ src = inspect.getsource(QuittableXMLRPCServer)
+ src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
+ spacing = "\n" * 4
+ session.send_command(spacing + src + spacing)
+
+ # then starting the server with:
+ command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
+ session.send_command(command, "XMLRPC OK")
+
+ # now the server is running on the remote node and we can add functions to it
+ # first connect to the server from the execution node
+ import xmlrpc.client
+ server_url = f"http://{tg_node.config.hostname}:8000"
+ rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+
+ # get the function bytes to send
+ import marshal
+ function_bytes = marshal.dumps(hello_world.__code__)
+ rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+
+ # now we can execute the function on the server
+ xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
+ print(str(xmlrpc_binary_recv))
"""
def __init__(self, *args, **kwargs):
--
2.45.1
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-05-31 16:50 ` Luca Vizzarro
2024-06-07 13:37 ` Juraj Linkeš
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-31 16:50 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
2024-05-31 16:50 ` Luca Vizzarro
@ 2024-06-07 13:37 ` Juraj Linkeš
2024-06-14 20:48 ` Nicholas Pratte
2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-07 13:37 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
On 29. 5. 2024 21:49, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> When this XML-RPC server implementation was added, the docstring had to
> be shortened in order to reduce the chances of this race condition being
> encountered. Now that this race condition issue is resolved, the full
> docstring can be restored.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
2024-05-31 16:50 ` Luca Vizzarro
2024-06-07 13:37 ` Juraj Linkeš
@ 2024-06-14 20:48 ` Nicholas Pratte
2024-06-17 15:06 ` Jeremy Spewock
2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 1 reply; 57+ messages in thread
From: Nicholas Pratte @ 2024-06-14 20:48 UTC (permalink / raw)
To: jspewock
Cc: paul.szczepanek, wathsala.vithanage, probb, Luca.Vizzarro,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi, dev
Just a small nitpick. Otherwise:
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
<snip>
> class QuittableXMLRPCServer(SimpleXMLRPCServer):
> - """Basic XML-RPC server.
> + r"""Basic XML-RPC server.
>
> The server may be augmented by functions serializable by the :mod:`marshal` module.
> +
> + Example:
> + ::
> +
> + def hello_world():
> + # to be sent to the XML-RPC server
> + print("Hello World!")
> +
> + # start the XML-RPC server on the remote node
> + # the example assumes you're already connect to a tg_node
This just a very small nitpick, but you wrote 'connect' when it should
be 'connected.'
> + # this is done by starting a Python shell on the remote node
> + from framework.remote_session import PythonShell
This comment is related to the one I made above, but maybe you could
move the above comment 'assumes you're already connected to a tg_node'
on this line instead of where it is right now. I had to rescan this a
couple times when reading. Again, this is just an extremely minuscule
nitpick, but I figured I'd bring it up, in any case.
> + session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
> +
> + # then importing the modules needed to run the server
> + # and the modules for any functions later added to the server
> + session.send_command("import xmlrpc")
> + session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
> +
> + # sending the source code of this class to the Python shell
> + from xmlrpc.server import SimpleXMLRPCServer
> + src = inspect.getsource(QuittableXMLRPCServer)
> + src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
> + spacing = "\n" * 4
> + session.send_command(spacing + src + spacing)
> +
> + # then starting the server with:
> + command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
> + session.send_command(command, "XMLRPC OK")
> +
> + # now the server is running on the remote node and we can add functions to it
> + # first connect to the server from the execution node
> + import xmlrpc.client
> + server_url = f"http://{tg_node.config.hostname}:8000"
> + rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
> +
> + # get the function bytes to send
> + import marshal
> + function_bytes = marshal.dumps(hello_world.__code__)
> + rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
> +
> + # now we can execute the function on the server
> + xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
> + print(str(xmlrpc_binary_recv))
> """
>
> def __init__(self, *args, **kwargs):
> --
> 2.45.1
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-06-14 20:48 ` Nicholas Pratte
@ 2024-06-17 15:06 ` Jeremy Spewock
0 siblings, 0 replies; 57+ messages in thread
From: Jeremy Spewock @ 2024-06-17 15:06 UTC (permalink / raw)
To: Nicholas Pratte
Cc: paul.szczepanek, wathsala.vithanage, probb, Luca.Vizzarro,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi, dev
On Fri, Jun 14, 2024 at 4:48 PM Nicholas Pratte <npratte@iol.unh.edu> wrote:
>
> Just a small nitpick. Otherwise:
>
> Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
>
> <snip>
> > class QuittableXMLRPCServer(SimpleXMLRPCServer):
> > - """Basic XML-RPC server.
> > + r"""Basic XML-RPC server.
> >
> > The server may be augmented by functions serializable by the :mod:`marshal` module.
> > +
> > + Example:
> > + ::
> > +
> > + def hello_world():
> > + # to be sent to the XML-RPC server
> > + print("Hello World!")
> > +
> > + # start the XML-RPC server on the remote node
> > + # the example assumes you're already connect to a tg_node
>
> This just a very small nitpick, but you wrote 'connect' when it should
> be 'connected.'
Good catch, I'll fix this.
>
> > + # this is done by starting a Python shell on the remote node
> > + from framework.remote_session import PythonShell
>
> This comment is related to the one I made above, but maybe you could
> move the above comment 'assumes you're already connected to a tg_node'
> on this line instead of where it is right now. I had to rescan this a
> couple times when reading. Again, this is just an extremely minuscule
> nitpick, but I figured I'd bring it up, in any case.
I agree when looking at this more in-depth that the order should be
swapped, this comment looks like a continuation of the first line, but
then there is another just kind of thrown in-between. good catch.
>
> > + session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
> > +
> > + # then importing the modules needed to run the server
> > + # and the modules for any functions later added to the server
> > + session.send_command("import xmlrpc")
> > + session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
> > +
> > + # sending the source code of this class to the Python shell
> > + from xmlrpc.server import SimpleXMLRPCServer
> > + src = inspect.getsource(QuittableXMLRPCServer)
> > + src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
> > + spacing = "\n" * 4
> > + session.send_command(spacing + src + spacing)
> > +
> > + # then starting the server with:
> > + command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
> > + session.send_command(command, "XMLRPC OK")
> > +
> > + # now the server is running on the remote node and we can add functions to it
> > + # first connect to the server from the execution node
> > + import xmlrpc.client
> > + server_url = f"http://{tg_node.config.hostname}:8000"
> > + rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
> > +
> > + # get the function bytes to send
> > + import marshal
> > + function_bytes = marshal.dumps(hello_world.__code__)
> > + rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
> > +
> > + # now we can execute the function on the server
> > + xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
> > + print(str(xmlrpc_binary_recv))
> > """
> >
> > def __init__(self, *args, **kwargs):
> > --
> > 2.45.1
> >
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
` (2 preceding siblings ...)
2024-06-14 20:48 ` Nicholas Pratte
@ 2024-06-17 15:00 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-06-17 15:00 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v3 3/3] dts: Improve logging for interactive shells
2024-05-29 19:49 ` [PATCH v3 " jspewock
2024-05-29 19:49 ` [PATCH v3 1/3] dts: Improve output gathering in interactive shells jspewock
2024-05-29 19:49 ` [PATCH v3 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-05-29 19:49 ` jspewock
2024-05-31 16:50 ` Luca Vizzarro
` (3 more replies)
2 siblings, 4 replies; 57+ messages in thread
From: jspewock @ 2024-05-29 19:49 UTC (permalink / raw)
To: paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, juraj.linkes, Honnappa.Nagarahalli,
yoan.picchi
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@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 <jspewock@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
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 3/3] dts: Improve logging for interactive shells
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-05-31 16:50 ` Luca Vizzarro
2024-06-07 13:38 ` Juraj Linkeš
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-05-31 16:50 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 3/3] dts: Improve logging for interactive shells
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
2024-05-31 16:50 ` Luca Vizzarro
@ 2024-06-07 13:38 ` Juraj Linkeš
2024-06-14 20:26 ` Nicholas Pratte
2024-06-17 15:01 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-07 13:38 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
Luca.Vizzarro, thomas, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
On 29. 5. 2024 21:49, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v3 3/3] dts: Improve logging for interactive shells
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
2024-05-31 16:50 ` Luca Vizzarro
2024-06-07 13:38 ` Juraj Linkeš
@ 2024-06-14 20:26 ` Nicholas Pratte
2024-06-17 15:01 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Nicholas Pratte @ 2024-06-14 20:26 UTC (permalink / raw)
To: jspewock
Cc: paul.szczepanek, wathsala.vithanage, probb, Luca.Vizzarro,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi, dev
Ran a couple test suites to observe the output. As expected,
everything works fine.
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Wed, May 29, 2024 at 3:49 PM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@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 <jspewock@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
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v3 3/3] dts: Improve logging for interactive shells
2024-05-29 19:49 ` [PATCH v3 3/3] dts: Improve logging for interactive shells jspewock
` (2 preceding siblings ...)
2024-06-14 20:26 ` Nicholas Pratte
@ 2024-06-17 15:01 ` Luca Vizzarro
3 siblings, 0 replies; 57+ messages in thread
From: Luca Vizzarro @ 2024-06-17 15:01 UTC (permalink / raw)
To: jspewock, paul.szczepanek, wathsala.vithanage, probb, npratte,
thomas, juraj.linkes, Honnappa.Nagarahalli, yoan.picchi
Cc: dev
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 0/3] Improve interactive shell output gathering and logging
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
` (4 preceding siblings ...)
2024-05-29 19:49 ` [PATCH v3 " jspewock
@ 2024-06-20 17:36 ` jspewock
2024-06-20 17:36 ` [PATCH v4 1/3] dts: Improve output gathering in interactive shells jspewock
` (3 more replies)
2024-07-24 14:08 ` [PATCH v5 " jspewock
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
7 siblings, 4 replies; 57+ messages in thread
From: jspewock @ 2024-06-20 17:36 UTC (permalink / raw)
To: yoan.picchi, juraj.linkes, thomas, Luca.Vizzarro,
wathsala.vithanage, Honnappa.Nagarahalli, paul.szczepanek,
npratte, probb
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
v4:
* rebase on top of rc1.
* address comments and fix typos in the added docstring example.
Jeremy Spewock (3):
dts: Improve output gathering in interactive shells
dts: Add missing docstring from XML-RPC server
dts: Improve logging for interactive shells
dts/framework/exception.py | 66 ++++++++++++-------
dts/framework/remote_session/dpdk_shell.py | 3 +-
.../remote_session/interactive_shell.py | 58 +++++++++++-----
dts/framework/remote_session/testpmd_shell.py | 2 +
.../testbed_model/traffic_generator/scapy.py | 50 +++++++++++++-
5 files changed, 139 insertions(+), 40 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v4 1/3] dts: Improve output gathering in interactive shells
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
@ 2024-06-20 17:36 ` jspewock
2024-06-21 9:10 ` Juraj Linkeš
2024-06-20 17:36 ` [PATCH v4 2/3] dts: Add missing docstring from XML-RPC server jspewock
` (2 subsequent siblings)
3 siblings, 1 reply; 57+ messages in thread
From: jspewock @ 2024-06-20 17:36 UTC (permalink / raw)
To: yoan.picchi, juraj.linkes, thomas, Luca.Vizzarro,
wathsala.vithanage, Honnappa.Nagarahalli, paul.szczepanek,
npratte, probb
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
The current implementation of consuming output from interactive shells
relies on being able to find an expected prompt somewhere within the
output buffer after sending the command. This is useful in situations
where the prompt does not appear in the output itself, but in some
practical cases (such as the starting of an XML-RPC server for scapy)
the prompt exists in one of the commands sent to the shell and this can
cause the command to exit early and creates a race condition between the
server starting and the first command being sent to the server.
This patch addresses this problem by searching for a line that strictly
ends with the provided prompt, rather than one that simply contains it,
so that the detection that a command is finished is more consistent. It
also adds a catch to detect when a command times out before finding the
prompt or the underlying SSH session dies so that the exception can be
wrapped into a more explicit one and be more consistent with the
non-interactive shells.
Bugzilla ID: 1359
Fixes: 88489c0501af ("dts: add smoke tests")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/exception.py | 66 ++++++++++++-------
.../remote_session/interactive_shell.py | 49 ++++++++++----
2 files changed, 80 insertions(+), 35 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index 74fd2af3b6..f45f789825 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -51,26 +51,6 @@ class DTSError(Exception):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
-class SSHTimeoutError(DTSError):
- """The SSH execution of a command timed out."""
-
- #:
- severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
- _command: str
-
- def __init__(self, command: str):
- """Define the meaning of the first argument.
-
- Args:
- command: The executed command.
- """
- self._command = command
-
- def __str__(self) -> str:
- """Add some context to the string representation."""
- return f"{self._command} execution timed out."
-
-
class SSHConnectionError(DTSError):
"""An unsuccessful SSH connection."""
@@ -98,8 +78,42 @@ def __str__(self) -> str:
return message
-class SSHSessionDeadError(DTSError):
- """The SSH session is no longer alive."""
+class _SSHTimeoutError(DTSError):
+ """The execution of a command via SSH timed out.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
+
+ #:
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
+ _command: str
+
+ def __init__(self, command: str):
+ """Define the meaning of the first argument.
+
+ Args:
+ command: The executed command.
+ """
+ self._command = command
+
+ def __str__(self) -> str:
+ """Add some context to the string representation."""
+ return f"{self._command} execution timed out."
+
+
+class SSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on a non-interactive SSH session timed out."""
+
+
+class InteractiveSSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on an interactive SSH session timed out."""
+
+
+class _SSHSessionDeadError(DTSError):
+ """The SSH session is no longer alive.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
#:
severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
@@ -118,6 +132,14 @@ def __str__(self) -> str:
return f"SSH session with {self._host} has died."
+class SSHSessionDeadError(_SSHSessionDeadError):
+ """Non-interactive SSH session has died."""
+
+
+class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
+ """Interactive SSH session as died."""
+
+
class ConfigurationError(DTSError):
"""An invalid configuration."""
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 254aa29f89..6aa5281d6a 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -21,6 +21,10 @@
from paramiko import Channel, channel # type: ignore[import-untyped]
+from framework.exception import (
+ InteractiveSSHSessionDeadError,
+ InteractiveSSHTimeoutError,
+)
from framework.logger import DTSLogger
from framework.params import Params
from framework.settings import SETTINGS
@@ -53,7 +57,10 @@ class InteractiveShell(ABC):
#: Extra characters to add to the end of every command
#: before sending them. This is often overridden by subclasses and is
- #: most commonly an additional newline character.
+ #: most commonly an additional newline character. This additional newline
+ #: character is used to force the line that is currently awaiting input
+ #: into the stdout buffer so that it can be consumed and checked against
+ #: the expected prompt.
_command_extra_chars: ClassVar[str] = ""
#: Path to the executable to start the interactive application.
@@ -134,23 +141,39 @@ def send_command(
Returns:
All output in the buffer before expected string.
+
+ Raises:
+ InteractiveSSHSessionDeadError: The session died while executing the command.
+ InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
+ the output before the timeout.
"""
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:
- if skip_first_line:
- skip_first_line = False
- continue
- if prompt in line and not line.rstrip().endswith(
- command.rstrip()
- ): # ignore line that sent command
- break
- out += line
- self._logger.debug(f"Got output: {out}")
+ try:
+ self._stdin.write(f"{command}{self._command_extra_chars}\n")
+ self._stdin.flush()
+ for line in self._stdout:
+ if skip_first_line:
+ skip_first_line = False
+ continue
+ if line.rstrip().endswith(prompt):
+ break
+ out += line
+ except TimeoutError as e:
+ self._logger.exception(e)
+ self._logger.debug(
+ f"Prompt ({prompt}) was not found in output from command before timeout."
+ )
+ raise InteractiveSSHTimeoutError(command) from e
+ except OSError as e:
+ self._logger.exception(e)
+ raise InteractiveSSHSessionDeadError(
+ self._node.main_session.interactive_session.hostname
+ ) from e
+ finally:
+ self._logger.debug(f"Got output: {out}")
return out
def close(self) -> None:
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 1/3] dts: Improve output gathering in interactive shells
2024-06-20 17:36 ` [PATCH v4 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-06-21 9:10 ` Juraj Linkeš
0 siblings, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-21 9:10 UTC (permalink / raw)
To: jspewock, yoan.picchi, thomas, Luca.Vizzarro, wathsala.vithanage,
Honnappa.Nagarahalli, paul.szczepanek, npratte, probb
Cc: dev
On 20. 6. 2024 19:36, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> The current implementation of consuming output from interactive shells
> relies on being able to find an expected prompt somewhere within the
> output buffer after sending the command. This is useful in situations
> where the prompt does not appear in the output itself, but in some
> practical cases (such as the starting of an XML-RPC server for scapy)
> the prompt exists in one of the commands sent to the shell and this can
> cause the command to exit early and creates a race condition between the
> server starting and the first command being sent to the server.
>
> This patch addresses this problem by searching for a line that strictly
> ends with the provided prompt, rather than one that simply contains it,
> so that the detection that a command is finished is more consistent. It
> also adds a catch to detect when a command times out before finding the
> prompt or the underlying SSH session dies so that the exception can be
> wrapped into a more explicit one and be more consistent with the
> non-interactive shells.
>
> Bugzilla ID: 1359
> Fixes: 88489c0501af ("dts: add smoke tests")
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 2/3] dts: Add missing docstring from XML-RPC server
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
2024-06-20 17:36 ` [PATCH v4 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-06-20 17:36 ` jspewock
2024-06-21 9:12 ` Juraj Linkeš
2024-06-20 17:36 ` [PATCH v4 3/3] dts: Improve logging for interactive shells jspewock
2024-07-23 22:17 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging Thomas Monjalon
3 siblings, 1 reply; 57+ messages in thread
From: jspewock @ 2024-06-20 17:36 UTC (permalink / raw)
To: yoan.picchi, juraj.linkes, thomas, Luca.Vizzarro,
wathsala.vithanage, Honnappa.Nagarahalli, paul.szczepanek,
npratte, probb
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
When this XML-RPC server implementation was added, the docstring had to
be shortened in order to reduce the chances of this race condition being
encountered. Now that this race condition issue is resolved, the full
docstring can be restored.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
.../testbed_model/traffic_generator/scapy.py | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index bf58ad1c5e..c3648134fc 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -128,9 +128,53 @@ def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: s
class QuittableXMLRPCServer(SimpleXMLRPCServer):
- """Basic XML-RPC server.
+ r"""Basic XML-RPC server.
The server may be augmented by functions serializable by the :mod:`marshal` module.
+
+ Example:
+ ::
+
+ def hello_world():
+ # to be sent to the XML-RPC server
+ print("Hello World!")
+
+ # start the XML-RPC server on the remote node
+ # this is done by starting a Python shell on the remote node
+ from framework.remote_session import PythonShell
+ # the example assumes you're already connected to a tg_node
+ session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+
+ # then importing the modules needed to run the server
+ # and the modules for any functions later added to the server
+ session.send_command("import xmlrpc")
+ session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+
+ # sending the source code of this class to the Python shell
+ from xmlrpc.server import SimpleXMLRPCServer
+ src = inspect.getsource(QuittableXMLRPCServer)
+ src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
+ spacing = "\n" * 4
+ session.send_command(spacing + src + spacing)
+
+ # then starting the server with:
+ command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
+ session.send_command(command, "XMLRPC OK")
+
+ # now the server is running on the remote node and we can add functions to it
+ # first connect to the server from the execution node
+ import xmlrpc.client
+ server_url = f"http://{tg_node.config.hostname}:8000"
+ rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+
+ # get the function bytes to send
+ import marshal
+ function_bytes = marshal.dumps(hello_world.__code__)
+ rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+
+ # now we can execute the function on the server
+ xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
+ print(str(xmlrpc_binary_recv))
"""
def __init__(self, *args, **kwargs):
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 2/3] dts: Add missing docstring from XML-RPC server
2024-06-20 17:36 ` [PATCH v4 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-06-21 9:12 ` Juraj Linkeš
0 siblings, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-21 9:12 UTC (permalink / raw)
To: jspewock, yoan.picchi, thomas, Luca.Vizzarro, wathsala.vithanage,
Honnappa.Nagarahalli, paul.szczepanek, npratte, probb
Cc: dev
On 20. 6. 2024 19:36, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> When this XML-RPC server implementation was added, the docstring had to
> be shortened in order to reduce the chances of this race condition being
> encountered. Now that this race condition issue is resolved, the full
> docstring can be restored.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 3/3] dts: Improve logging for interactive shells
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
2024-06-20 17:36 ` [PATCH v4 1/3] dts: Improve output gathering in interactive shells jspewock
2024-06-20 17:36 ` [PATCH v4 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-06-20 17:36 ` jspewock
2024-06-21 9:23 ` Juraj Linkeš
2024-06-21 15:31 ` Patrick Robb
2024-07-23 22:17 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging Thomas Monjalon
3 siblings, 2 replies; 57+ messages in thread
From: jspewock @ 2024-06-20 17:36 UTC (permalink / raw)
To: yoan.picchi, juraj.linkes, thomas, Luca.Vizzarro,
wathsala.vithanage, Honnappa.Nagarahalli, paul.szczepanek,
npratte, probb
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
---
dts/framework/remote_session/dpdk_shell.py | 3 ++-
dts/framework/remote_session/interactive_shell.py | 9 +++++++--
dts/framework/remote_session/testpmd_shell.py | 2 ++
dts/framework/testbed_model/traffic_generator/scapy.py | 4 +++-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/remote_session/dpdk_shell.py
index 296639f37d..9b4ff47334 100644
--- a/dts/framework/remote_session/dpdk_shell.py
+++ b/dts/framework/remote_session/dpdk_shell.py
@@ -81,6 +81,7 @@ def __init__(
append_prefix_timestamp: bool = True,
start_on_init: bool = True,
app_params: EalParams = EalParams(),
+ name: str | None = None,
) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell.__init__`.
@@ -95,7 +96,7 @@ def __init__(
append_prefix_timestamp,
)
- super().__init__(node, privileged, timeout, start_on_init, app_params)
+ super().__init__(node, privileged, timeout, start_on_init, app_params, name)
def _update_real_path(self, path: PurePath) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell._update_real_path`.
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 6aa5281d6a..c92fdbfcdf 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -25,7 +25,7 @@
InteractiveSSHSessionDeadError,
InteractiveSSHTimeoutError,
)
-from framework.logger import DTSLogger
+from framework.logger import DTSLogger, get_dts_logger
from framework.params import Params
from framework.settings import SETTINGS
from framework.testbed_model.node import Node
@@ -73,6 +73,7 @@ def __init__(
timeout: float = SETTINGS.timeout,
start_on_init: bool = True,
app_params: Params = Params(),
+ name: str | None = None,
) -> None:
"""Create an SSH channel during initialization.
@@ -84,9 +85,13 @@ def __init__(
and no output is gathered within the timeout, an exception is thrown.
start_on_init: Start interactive shell automatically after object initialisation.
app_params: The command line parameters to be passed to the application on startup.
+ 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.
"""
self._node = node
- self._logger = node._logger
+ if name is None:
+ name = type(self).__name__
+ self._logger = get_dts_logger(f"{node.name}.{name}")
self._app_params = app_params
self._privileged = privileged
self._timeout = timeout
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index ec22f72221..1f00556187 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -605,6 +605,7 @@ def __init__(
ascending_cores: bool = True,
append_prefix_timestamp: bool = True,
start_on_init: bool = True,
+ name: str | None = None,
**app_params: Unpack[TestPmdParamsDict],
) -> None:
"""Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`. Changes app_params to kwargs."""
@@ -617,6 +618,7 @@ def __init__(
append_prefix_timestamp,
start_on_init,
TestPmdParams(**app_params),
+ name,
)
def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index c3648134fc..ca0ea6aca3 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -261,7 +261,9 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
self._tg_node.config.os == OS.linux
), "Linux is the only supported OS for scapy traffic generation"
- self.session = PythonShell(self._tg_node, timeout=5, privileged=True)
+ self.session = PythonShell(
+ self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
+ )
# import libs in remote python console
for import_statement in SCAPY_RPC_SERVER_IMPORTS:
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 3/3] dts: Improve logging for interactive shells
2024-06-20 17:36 ` [PATCH v4 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-06-21 9:23 ` Juraj Linkeš
2024-06-21 15:31 ` Patrick Robb
1 sibling, 0 replies; 57+ messages in thread
From: Juraj Linkeš @ 2024-06-21 9:23 UTC (permalink / raw)
To: jspewock, yoan.picchi, thomas, Luca.Vizzarro, wathsala.vithanage,
Honnappa.Nagarahalli, paul.szczepanek, npratte, probb
Cc: dev
On 20. 6. 2024 19:36, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 3/3] dts: Improve logging for interactive shells
2024-06-20 17:36 ` [PATCH v4 3/3] dts: Improve logging for interactive shells jspewock
2024-06-21 9:23 ` Juraj Linkeš
@ 2024-06-21 15:31 ` Patrick Robb
1 sibling, 0 replies; 57+ messages in thread
From: Patrick Robb @ 2024-06-21 15:31 UTC (permalink / raw)
To: jspewock
Cc: yoan.picchi, juraj.linkes, thomas, Luca.Vizzarro,
wathsala.vithanage, Honnappa.Nagarahalli, paul.szczepanek,
npratte, dev
Recheck-request: iol-intel-Functional
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 0/3] Improve interactive shell output gathering and logging
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
` (2 preceding siblings ...)
2024-06-20 17:36 ` [PATCH v4 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-07-23 22:17 ` Thomas Monjalon
3 siblings, 0 replies; 57+ messages in thread
From: Thomas Monjalon @ 2024-07-23 22:17 UTC (permalink / raw)
To: Jeremy Spewock
Cc: yoan.picchi, juraj.linkes, Luca.Vizzarro, wathsala.vithanage,
Honnappa.Nagarahalli, paul.szczepanek, npratte, probb, dev
> Jeremy Spewock (3):
> dts: Improve output gathering in interactive shells
> dts: Add missing docstring from XML-RPC server
> dts: Improve logging for interactive shells
Please could you rebase in a v5?
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 0/3] Improve interactive shell output gathering and logging
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
` (5 preceding siblings ...)
2024-06-20 17:36 ` [PATCH v4 0/3] Improve interactive shell output gathering and logging jspewock
@ 2024-07-24 14:08 ` jspewock
2024-07-24 14:08 ` [PATCH v5 1/3] dts: Improve output gathering in interactive shells jspewock
` (2 more replies)
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
7 siblings, 3 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 14:08 UTC (permalink / raw)
To: Honnappa.Nagarahalli, npratte, Luca.Vizzarro, paul.szczepanek,
wathsala.vithanage, probb, thomas, yoan.picchi, juraj.linkes
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
v5:
* rebased on main
* pulled tags forward from previous versions since there has been no
change to the series outside of rebases since then.
Jeremy Spewock (3):
dts: Improve output gathering in interactive shells
dts: Add missing docstring from XML-RPC server
dts: Improve logging for interactive shells
dts/framework/exception.py | 66 ++++++++++++-------
dts/framework/remote_session/dpdk_shell.py | 3 +-
.../single_active_interactive_shell.py | 58 +++++++++++-----
dts/framework/remote_session/testpmd_shell.py | 2 +
.../testbed_model/traffic_generator/scapy.py | 50 +++++++++++++-
5 files changed, 138 insertions(+), 41 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v5 1/3] dts: Improve output gathering in interactive shells
2024-07-24 14:08 ` [PATCH v5 " jspewock
@ 2024-07-24 14:08 ` jspewock
2024-07-24 14:08 ` [PATCH v5 2/3] dts: Add missing docstring from XML-RPC server jspewock
2024-07-24 14:08 ` [PATCH v5 3/3] dts: Improve logging for interactive shells jspewock
2 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 14:08 UTC (permalink / raw)
To: Honnappa.Nagarahalli, npratte, Luca.Vizzarro, paul.szczepanek,
wathsala.vithanage, probb, thomas, yoan.picchi, juraj.linkes
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@iol.unh.edu>
The current implementation of consuming output from interactive shells
relies on being able to find an expected prompt somewhere within the
output buffer after sending the command. This is useful in situations
where the prompt does not appear in the output itself, but in some
practical cases (such as the starting of an XML-RPC server for scapy)
the prompt exists in one of the commands sent to the shell and this can
cause the command to exit early and creates a race condition between the
server starting and the first command being sent to the server.
This patch addresses this problem by searching for a line that strictly
ends with the provided prompt, rather than one that simply contains it,
so that the detection that a command is finished is more consistent. It
also adds a catch to detect when a command times out before finding the
prompt or the underlying SSH session dies so that the exception can be
wrapped into a more explicit one and be more consistent with the
non-interactive shells.
Bugzilla ID: 1359
Fixes: 88489c0501af ("dts: add smoke tests")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/exception.py | 66 ++++++++++++-------
.../single_active_interactive_shell.py | 49 ++++++++++----
2 files changed, 79 insertions(+), 36 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index 74fd2af3b6..f45f789825 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -51,26 +51,6 @@ class DTSError(Exception):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
-class SSHTimeoutError(DTSError):
- """The SSH execution of a command timed out."""
-
- #:
- severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
- _command: str
-
- def __init__(self, command: str):
- """Define the meaning of the first argument.
-
- Args:
- command: The executed command.
- """
- self._command = command
-
- def __str__(self) -> str:
- """Add some context to the string representation."""
- return f"{self._command} execution timed out."
-
-
class SSHConnectionError(DTSError):
"""An unsuccessful SSH connection."""
@@ -98,8 +78,42 @@ def __str__(self) -> str:
return message
-class SSHSessionDeadError(DTSError):
- """The SSH session is no longer alive."""
+class _SSHTimeoutError(DTSError):
+ """The execution of a command via SSH timed out.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
+
+ #:
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
+ _command: str
+
+ def __init__(self, command: str):
+ """Define the meaning of the first argument.
+
+ Args:
+ command: The executed command.
+ """
+ self._command = command
+
+ def __str__(self) -> str:
+ """Add some context to the string representation."""
+ return f"{self._command} execution timed out."
+
+
+class SSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on a non-interactive SSH session timed out."""
+
+
+class InteractiveSSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on an interactive SSH session timed out."""
+
+
+class _SSHSessionDeadError(DTSError):
+ """The SSH session is no longer alive.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
#:
severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
@@ -118,6 +132,14 @@ def __str__(self) -> str:
return f"SSH session with {self._host} has died."
+class SSHSessionDeadError(_SSHSessionDeadError):
+ """Non-interactive SSH session has died."""
+
+
+class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
+ """Interactive SSH session as died."""
+
+
class ConfigurationError(DTSError):
"""An invalid configuration."""
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 38094c0fe2..0e5a04885f 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -27,7 +27,11 @@
from paramiko import Channel, channel # type: ignore[import-untyped]
from typing_extensions import Self
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import (
+ InteractiveCommandExecutionError,
+ InteractiveSSHSessionDeadError,
+ InteractiveSSHTimeoutError,
+)
from framework.logger import DTSLogger
from framework.params import Params
from framework.settings import SETTINGS
@@ -71,7 +75,10 @@ class SingleActiveInteractiveShell(ABC):
#: Extra characters to add to the end of every command
#: before sending them. This is often overridden by subclasses and is
- #: most commonly an additional newline character.
+ #: most commonly an additional newline character. This additional newline
+ #: character is used to force the line that is currently awaiting input
+ #: into the stdout buffer so that it can be consumed and checked against
+ #: the expected prompt.
_command_extra_chars: ClassVar[str] = ""
#: Path to the executable to start the interactive application.
@@ -175,6 +182,9 @@ def send_command(
Raises:
InteractiveCommandExecutionError: If attempting to send a command to a shell that is
not currently running.
+ InteractiveSSHSessionDeadError: The session died while executing the command.
+ InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
+ the output before the timeout.
"""
if not self.is_alive:
raise InteractiveCommandExecutionError(
@@ -183,19 +193,30 @@ def send_command(
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:
- if skip_first_line:
- skip_first_line = False
- continue
- if prompt in line and not line.rstrip().endswith(
- command.rstrip()
- ): # ignore line that sent command
- break
- out += line
- self._logger.debug(f"Got output: {out}")
+ try:
+ self._stdin.write(f"{command}{self._command_extra_chars}\n")
+ self._stdin.flush()
+ for line in self._stdout:
+ if skip_first_line:
+ skip_first_line = False
+ continue
+ if line.rstrip().endswith(prompt):
+ break
+ out += line
+ except TimeoutError as e:
+ self._logger.exception(e)
+ self._logger.debug(
+ f"Prompt ({prompt}) was not found in output from command before timeout."
+ )
+ raise InteractiveSSHTimeoutError(command) from e
+ except OSError as e:
+ self._logger.exception(e)
+ raise InteractiveSSHSessionDeadError(
+ self._node.main_session.interactive_session.hostname
+ ) from e
+ finally:
+ self._logger.debug(f"Got output: {out}")
return out
def _close(self) -> None:
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v5 2/3] dts: Add missing docstring from XML-RPC server
2024-07-24 14:08 ` [PATCH v5 " jspewock
2024-07-24 14:08 ` [PATCH v5 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-07-24 14:08 ` jspewock
2024-07-24 14:08 ` [PATCH v5 3/3] dts: Improve logging for interactive shells jspewock
2 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 14:08 UTC (permalink / raw)
To: Honnappa.Nagarahalli, npratte, Luca.Vizzarro, paul.szczepanek,
wathsala.vithanage, probb, thomas, yoan.picchi, juraj.linkes
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@iol.unh.edu>
When this XML-RPC server implementation was added, the docstring had to
be shortened in order to reduce the chances of this race condition being
encountered. Now that this race condition issue is resolved, the full
docstring can be restored.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
---
.../testbed_model/traffic_generator/scapy.py | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 7f0cc2bc18..08e1f4ae7e 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -128,9 +128,53 @@ def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: s
class QuittableXMLRPCServer(SimpleXMLRPCServer):
- """Basic XML-RPC server.
+ r"""Basic XML-RPC server.
The server may be augmented by functions serializable by the :mod:`marshal` module.
+
+ Example:
+ ::
+
+ def hello_world():
+ # to be sent to the XML-RPC server
+ print("Hello World!")
+
+ # start the XML-RPC server on the remote node
+ # this is done by starting a Python shell on the remote node
+ from framework.remote_session import PythonShell
+ # the example assumes you're already connected to a tg_node
+ session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+
+ # then importing the modules needed to run the server
+ # and the modules for any functions later added to the server
+ session.send_command("import xmlrpc")
+ session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+
+ # sending the source code of this class to the Python shell
+ from xmlrpc.server import SimpleXMLRPCServer
+ src = inspect.getsource(QuittableXMLRPCServer)
+ src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
+ spacing = "\n" * 4
+ session.send_command(spacing + src + spacing)
+
+ # then starting the server with:
+ command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
+ session.send_command(command, "XMLRPC OK")
+
+ # now the server is running on the remote node and we can add functions to it
+ # first connect to the server from the execution node
+ import xmlrpc.client
+ server_url = f"http://{tg_node.config.hostname}:8000"
+ rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+
+ # get the function bytes to send
+ import marshal
+ function_bytes = marshal.dumps(hello_world.__code__)
+ rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+
+ # now we can execute the function on the server
+ xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
+ print(str(xmlrpc_binary_recv))
"""
def __init__(self, *args, **kwargs):
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v5 3/3] dts: Improve logging for interactive shells
2024-07-24 14:08 ` [PATCH v5 " jspewock
2024-07-24 14:08 ` [PATCH v5 1/3] dts: Improve output gathering in interactive shells jspewock
2024-07-24 14:08 ` [PATCH v5 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-07-24 14:08 ` jspewock
2 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 14:08 UTC (permalink / raw)
To: Honnappa.Nagarahalli, npratte, Luca.Vizzarro, paul.szczepanek,
wathsala.vithanage, probb, thomas, yoan.picchi, juraj.linkes
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
dts/framework/remote_session/dpdk_shell.py | 3 ++-
.../remote_session/single_active_interactive_shell.py | 9 +++++++--
dts/framework/remote_session/testpmd_shell.py | 2 ++
dts/framework/testbed_model/traffic_generator/scapy.py | 4 +++-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/remote_session/dpdk_shell.py
index 950c6ca670..c5f5c2d116 100644
--- a/dts/framework/remote_session/dpdk_shell.py
+++ b/dts/framework/remote_session/dpdk_shell.py
@@ -82,6 +82,7 @@ def __init__(
ascending_cores: bool = True,
append_prefix_timestamp: bool = True,
app_params: EalParams = EalParams(),
+ name: str | None = None,
) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell.__init__`.
@@ -96,7 +97,7 @@ def __init__(
append_prefix_timestamp,
)
- super().__init__(node, privileged, timeout, app_params)
+ super().__init__(node, privileged, timeout, app_params, name)
def _update_real_path(self, path: PurePath) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell._update_real_path`.
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 0e5a04885f..7014444d0c 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -32,7 +32,7 @@
InteractiveSSHSessionDeadError,
InteractiveSSHTimeoutError,
)
-from framework.logger import DTSLogger
+from framework.logger import DTSLogger, get_dts_logger
from framework.params import Params
from framework.settings import SETTINGS
from framework.testbed_model.node import Node
@@ -92,6 +92,7 @@ def __init__(
privileged: bool = False,
timeout: float = SETTINGS.timeout,
app_params: Params = Params(),
+ name: str | None = None,
) -> None:
"""Create an SSH channel during initialization.
@@ -102,9 +103,13 @@ def __init__(
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.
app_params: The command line parameters to be passed to the application on startup.
+ 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.
"""
self._node = node
- self._logger = node._logger
+ if name is None:
+ name = type(self).__name__
+ self._logger = get_dts_logger(f"{node.name}.{name}")
self._app_params = app_params
self._privileged = privileged
self._timeout = timeout
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index eda6eb320f..43e9f56517 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -604,6 +604,7 @@ def __init__(
lcore_filter_specifier: LogicalCoreCount | LogicalCoreList = LogicalCoreCount(),
ascending_cores: bool = True,
append_prefix_timestamp: bool = True,
+ name: str | None = None,
**app_params: Unpack[TestPmdParamsDict],
) -> None:
"""Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`. Changes app_params to kwargs."""
@@ -615,6 +616,7 @@ def __init__(
ascending_cores,
append_prefix_timestamp,
TestPmdParams(**app_params),
+ name,
)
def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 08e1f4ae7e..13fc1107aa 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -261,7 +261,9 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
self._tg_node.config.os == OS.linux
), "Linux is the only supported OS for scapy traffic generation"
- self.session = PythonShell(self._tg_node, timeout=5, privileged=True)
+ self.session = PythonShell(
+ self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
+ )
self.session.start_application()
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v6 0/3] Improve interactive shell output gathering and logging
2024-05-01 16:16 ` [PATCH v2 0/3] Improve interactive shell output gathering and logging jspewock
` (6 preceding siblings ...)
2024-07-24 14:08 ` [PATCH v5 " jspewock
@ 2024-07-24 18:39 ` jspewock
2024-07-24 18:39 ` [PATCH v6 1/3] dts: Improve output gathering in interactive shells jspewock
` (3 more replies)
7 siblings, 4 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 18:39 UTC (permalink / raw)
To: juraj.linkes, probb, yoan.picchi, wathsala.vithanage,
Honnappa.Nagarahalli, npratte, paul.szczepanek, thomas,
Luca.Vizzarro
Cc: dev, Jeremy Spewock
From: Jeremy Spewock <jspewock@iol.unh.edu>
v6:
* Fix error catch for retries. This series changed the error that
is thrown in the case of a timeout, but it was originally overlooked
that the context manager patch added a catch that is looking for the
old timeout error. This version fixes the patch by adjusting the
error that is expected in the context manager patch to match what
this series changes it to.
Jeremy Spewock (3):
dts: Improve output gathering in interactive shells
dts: Add missing docstring from XML-RPC server
dts: Improve logging for interactive shells
dts/framework/exception.py | 66 ++++++++++++-------
dts/framework/remote_session/dpdk_shell.py | 3 +-
.../single_active_interactive_shell.py | 60 ++++++++++++-----
dts/framework/remote_session/testpmd_shell.py | 2 +
.../testbed_model/traffic_generator/scapy.py | 50 +++++++++++++-
5 files changed, 139 insertions(+), 42 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v6 1/3] dts: Improve output gathering in interactive shells
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
@ 2024-07-24 18:39 ` jspewock
2024-07-24 18:39 ` [PATCH v6 2/3] dts: Add missing docstring from XML-RPC server jspewock
` (2 subsequent siblings)
3 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 18:39 UTC (permalink / raw)
To: juraj.linkes, probb, yoan.picchi, wathsala.vithanage,
Honnappa.Nagarahalli, npratte, paul.szczepanek, thomas,
Luca.Vizzarro
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@iol.unh.edu>
The current implementation of consuming output from interactive shells
relies on being able to find an expected prompt somewhere within the
output buffer after sending the command. This is useful in situations
where the prompt does not appear in the output itself, but in some
practical cases (such as the starting of an XML-RPC server for scapy)
the prompt exists in one of the commands sent to the shell and this can
cause the command to exit early and creates a race condition between the
server starting and the first command being sent to the server.
This patch addresses this problem by searching for a line that strictly
ends with the provided prompt, rather than one that simply contains it,
so that the detection that a command is finished is more consistent. It
also adds a catch to detect when a command times out before finding the
prompt or the underlying SSH session dies so that the exception can be
wrapped into a more explicit one and be more consistent with the
non-interactive shells.
Bugzilla ID: 1359
Fixes: 88489c0501af ("dts: add smoke tests")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
---
dts/framework/exception.py | 66 ++++++++++++-------
.../single_active_interactive_shell.py | 51 +++++++++-----
2 files changed, 80 insertions(+), 37 deletions(-)
diff --git a/dts/framework/exception.py b/dts/framework/exception.py
index 74fd2af3b6..f45f789825 100644
--- a/dts/framework/exception.py
+++ b/dts/framework/exception.py
@@ -51,26 +51,6 @@ class DTSError(Exception):
severity: ClassVar[ErrorSeverity] = ErrorSeverity.GENERIC_ERR
-class SSHTimeoutError(DTSError):
- """The SSH execution of a command timed out."""
-
- #:
- severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
- _command: str
-
- def __init__(self, command: str):
- """Define the meaning of the first argument.
-
- Args:
- command: The executed command.
- """
- self._command = command
-
- def __str__(self) -> str:
- """Add some context to the string representation."""
- return f"{self._command} execution timed out."
-
-
class SSHConnectionError(DTSError):
"""An unsuccessful SSH connection."""
@@ -98,8 +78,42 @@ def __str__(self) -> str:
return message
-class SSHSessionDeadError(DTSError):
- """The SSH session is no longer alive."""
+class _SSHTimeoutError(DTSError):
+ """The execution of a command via SSH timed out.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
+
+ #:
+ severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
+ _command: str
+
+ def __init__(self, command: str):
+ """Define the meaning of the first argument.
+
+ Args:
+ command: The executed command.
+ """
+ self._command = command
+
+ def __str__(self) -> str:
+ """Add some context to the string representation."""
+ return f"{self._command} execution timed out."
+
+
+class SSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on a non-interactive SSH session timed out."""
+
+
+class InteractiveSSHTimeoutError(_SSHTimeoutError):
+ """The execution of a command on an interactive SSH session timed out."""
+
+
+class _SSHSessionDeadError(DTSError):
+ """The SSH session is no longer alive.
+
+ This class is private and meant to be raised as its interactive and non-interactive variants.
+ """
#:
severity: ClassVar[ErrorSeverity] = ErrorSeverity.SSH_ERR
@@ -118,6 +132,14 @@ def __str__(self) -> str:
return f"SSH session with {self._host} has died."
+class SSHSessionDeadError(_SSHSessionDeadError):
+ """Non-interactive SSH session has died."""
+
+
+class InteractiveSSHSessionDeadError(_SSHSessionDeadError):
+ """Interactive SSH session as died."""
+
+
class ConfigurationError(DTSError):
"""An invalid configuration."""
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 38094c0fe2..38318aa764 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -27,7 +27,11 @@
from paramiko import Channel, channel # type: ignore[import-untyped]
from typing_extensions import Self
-from framework.exception import InteractiveCommandExecutionError
+from framework.exception import (
+ InteractiveCommandExecutionError,
+ InteractiveSSHSessionDeadError,
+ InteractiveSSHTimeoutError,
+)
from framework.logger import DTSLogger
from framework.params import Params
from framework.settings import SETTINGS
@@ -71,7 +75,10 @@ class SingleActiveInteractiveShell(ABC):
#: Extra characters to add to the end of every command
#: before sending them. This is often overridden by subclasses and is
- #: most commonly an additional newline character.
+ #: most commonly an additional newline character. This additional newline
+ #: character is used to force the line that is currently awaiting input
+ #: into the stdout buffer so that it can be consumed and checked against
+ #: the expected prompt.
_command_extra_chars: ClassVar[str] = ""
#: Path to the executable to start the interactive application.
@@ -138,7 +145,7 @@ def _start_application(self) -> None:
try:
self.send_command(start_command)
break
- except TimeoutError:
+ except InteractiveSSHTimeoutError:
self._logger.info(
f"Interactive shell failed to start (attempt {attempt+1} out of "
f"{self._init_attempts})"
@@ -175,6 +182,9 @@ def send_command(
Raises:
InteractiveCommandExecutionError: If attempting to send a command to a shell that is
not currently running.
+ InteractiveSSHSessionDeadError: The session died while executing the command.
+ InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
+ the output before the timeout.
"""
if not self.is_alive:
raise InteractiveCommandExecutionError(
@@ -183,19 +193,30 @@ def send_command(
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:
- if skip_first_line:
- skip_first_line = False
- continue
- if prompt in line and not line.rstrip().endswith(
- command.rstrip()
- ): # ignore line that sent command
- break
- out += line
- self._logger.debug(f"Got output: {out}")
+ try:
+ self._stdin.write(f"{command}{self._command_extra_chars}\n")
+ self._stdin.flush()
+ for line in self._stdout:
+ if skip_first_line:
+ skip_first_line = False
+ continue
+ if line.rstrip().endswith(prompt):
+ break
+ out += line
+ except TimeoutError as e:
+ self._logger.exception(e)
+ self._logger.debug(
+ f"Prompt ({prompt}) was not found in output from command before timeout."
+ )
+ raise InteractiveSSHTimeoutError(command) from e
+ except OSError as e:
+ self._logger.exception(e)
+ raise InteractiveSSHSessionDeadError(
+ self._node.main_session.interactive_session.hostname
+ ) from e
+ finally:
+ self._logger.debug(f"Got output: {out}")
return out
def _close(self) -> None:
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v6 2/3] dts: Add missing docstring from XML-RPC server
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
2024-07-24 18:39 ` [PATCH v6 1/3] dts: Improve output gathering in interactive shells jspewock
@ 2024-07-24 18:39 ` jspewock
2024-07-24 18:39 ` [PATCH v6 3/3] dts: Improve logging for interactive shells jspewock
2024-07-26 11:01 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging Juraj Linkeš
3 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 18:39 UTC (permalink / raw)
To: juraj.linkes, probb, yoan.picchi, wathsala.vithanage,
Honnappa.Nagarahalli, npratte, paul.szczepanek, thomas,
Luca.Vizzarro
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@iol.unh.edu>
When this XML-RPC server implementation was added, the docstring had to
be shortened in order to reduce the chances of this race condition being
encountered. Now that this race condition issue is resolved, the full
docstring can be restored.
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
---
.../testbed_model/traffic_generator/scapy.py | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 7f0cc2bc18..08e1f4ae7e 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -128,9 +128,53 @@ def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: s
class QuittableXMLRPCServer(SimpleXMLRPCServer):
- """Basic XML-RPC server.
+ r"""Basic XML-RPC server.
The server may be augmented by functions serializable by the :mod:`marshal` module.
+
+ Example:
+ ::
+
+ def hello_world():
+ # to be sent to the XML-RPC server
+ print("Hello World!")
+
+ # start the XML-RPC server on the remote node
+ # this is done by starting a Python shell on the remote node
+ from framework.remote_session import PythonShell
+ # the example assumes you're already connected to a tg_node
+ session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+
+ # then importing the modules needed to run the server
+ # and the modules for any functions later added to the server
+ session.send_command("import xmlrpc")
+ session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+
+ # sending the source code of this class to the Python shell
+ from xmlrpc.server import SimpleXMLRPCServer
+ src = inspect.getsource(QuittableXMLRPCServer)
+ src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
+ spacing = "\n" * 4
+ session.send_command(spacing + src + spacing)
+
+ # then starting the server with:
+ command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
+ session.send_command(command, "XMLRPC OK")
+
+ # now the server is running on the remote node and we can add functions to it
+ # first connect to the server from the execution node
+ import xmlrpc.client
+ server_url = f"http://{tg_node.config.hostname}:8000"
+ rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+
+ # get the function bytes to send
+ import marshal
+ function_bytes = marshal.dumps(hello_world.__code__)
+ rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+
+ # now we can execute the function on the server
+ xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
+ print(str(xmlrpc_binary_recv))
"""
def __init__(self, *args, **kwargs):
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* [PATCH v6 3/3] dts: Improve logging for interactive shells
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
2024-07-24 18:39 ` [PATCH v6 1/3] dts: Improve output gathering in interactive shells jspewock
2024-07-24 18:39 ` [PATCH v6 2/3] dts: Add missing docstring from XML-RPC server jspewock
@ 2024-07-24 18:39 ` jspewock
2024-07-26 11:01 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging Juraj Linkeš
3 siblings, 0 replies; 57+ messages in thread
From: jspewock @ 2024-07-24 18:39 UTC (permalink / raw)
To: juraj.linkes, probb, yoan.picchi, wathsala.vithanage,
Honnappa.Nagarahalli, npratte, paul.szczepanek, thomas,
Luca.Vizzarro
Cc: dev, Jeremy Spewock, Luca Vizzarro
From: Jeremy Spewock <jspewock@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 <jspewock@iol.unh.edu>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
dts/framework/remote_session/dpdk_shell.py | 3 ++-
.../remote_session/single_active_interactive_shell.py | 9 +++++++--
dts/framework/remote_session/testpmd_shell.py | 2 ++
dts/framework/testbed_model/traffic_generator/scapy.py | 4 +++-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/remote_session/dpdk_shell.py
index 950c6ca670..c5f5c2d116 100644
--- a/dts/framework/remote_session/dpdk_shell.py
+++ b/dts/framework/remote_session/dpdk_shell.py
@@ -82,6 +82,7 @@ def __init__(
ascending_cores: bool = True,
append_prefix_timestamp: bool = True,
app_params: EalParams = EalParams(),
+ name: str | None = None,
) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell.__init__`.
@@ -96,7 +97,7 @@ def __init__(
append_prefix_timestamp,
)
- super().__init__(node, privileged, timeout, app_params)
+ super().__init__(node, privileged, timeout, app_params, name)
def _update_real_path(self, path: PurePath) -> None:
"""Extends :meth:`~.interactive_shell.InteractiveShell._update_real_path`.
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 38318aa764..77a4dcefdf 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -32,7 +32,7 @@
InteractiveSSHSessionDeadError,
InteractiveSSHTimeoutError,
)
-from framework.logger import DTSLogger
+from framework.logger import DTSLogger, get_dts_logger
from framework.params import Params
from framework.settings import SETTINGS
from framework.testbed_model.node import Node
@@ -92,6 +92,7 @@ def __init__(
privileged: bool = False,
timeout: float = SETTINGS.timeout,
app_params: Params = Params(),
+ name: str | None = None,
) -> None:
"""Create an SSH channel during initialization.
@@ -102,9 +103,13 @@ def __init__(
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.
app_params: The command line parameters to be passed to the application on startup.
+ 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.
"""
self._node = node
- self._logger = node._logger
+ if name is None:
+ name = type(self).__name__
+ self._logger = get_dts_logger(f"{node.name}.{name}")
self._app_params = app_params
self._privileged = privileged
self._timeout = timeout
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index eda6eb320f..43e9f56517 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -604,6 +604,7 @@ def __init__(
lcore_filter_specifier: LogicalCoreCount | LogicalCoreList = LogicalCoreCount(),
ascending_cores: bool = True,
append_prefix_timestamp: bool = True,
+ name: str | None = None,
**app_params: Unpack[TestPmdParamsDict],
) -> None:
"""Overrides :meth:`~.dpdk_shell.DPDKShell.__init__`. Changes app_params to kwargs."""
@@ -615,6 +616,7 @@ def __init__(
ascending_cores,
append_prefix_timestamp,
TestPmdParams(**app_params),
+ name,
)
def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 08e1f4ae7e..13fc1107aa 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -261,7 +261,9 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
self._tg_node.config.os == OS.linux
), "Linux is the only supported OS for scapy traffic generation"
- self.session = PythonShell(self._tg_node, timeout=5, privileged=True)
+ self.session = PythonShell(
+ self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
+ )
self.session.start_application()
--
2.45.2
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v6 0/3] Improve interactive shell output gathering and logging
2024-07-24 18:39 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
` (2 preceding siblings ...)
2024-07-24 18:39 ` [PATCH v6 3/3] dts: Improve logging for interactive shells jspewock
@ 2024-07-26 11:01 ` Juraj Linkeš
2024-07-29 16:56 ` Thomas Monjalon
3 siblings, 1 reply; 57+ messages in thread
From: Juraj Linkeš @ 2024-07-26 11:01 UTC (permalink / raw)
To: jspewock, probb, yoan.picchi, wathsala.vithanage,
Honnappa.Nagarahalli, npratte, paul.szczepanek, thomas,
Luca.Vizzarro
Cc: dev
For series:
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
On 24. 7. 2024 20:39, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> v6:
> * Fix error catch for retries. This series changed the error that
> is thrown in the case of a timeout, but it was originally overlooked
> that the context manager patch added a catch that is looking for the
> old timeout error. This version fixes the patch by adjusting the
> error that is expected in the context manager patch to match what
> this series changes it to.
>
Here's the diff for anyone interested:
diff --git
a/dts/framework/remote_session/single_active_interactive_shell.py
b/dts/framework/remote_session/single_active_interactive_shell.py
index 7014444d0c..77a4dcefdf 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -150,7 +150,7 @@ def _start_application(self) -> None:
try:
self.send_command(start_command)
break
- except TimeoutError:
+ except InteractiveSSHTimeoutError:
self._logger.info(
f"Interactive shell failed to start (attempt
{attempt+1} out of "
f"{self._init_attempts})"
self.send_command raises InteractiveSSHTimeoutError (and not
TimeoutError) which is why we needed this change.
> Jeremy Spewock (3):
> dts: Improve output gathering in interactive shells
> dts: Add missing docstring from XML-RPC server
> dts: Improve logging for interactive shells
>
> dts/framework/exception.py | 66 ++++++++++++-------
> dts/framework/remote_session/dpdk_shell.py | 3 +-
> .../single_active_interactive_shell.py | 60 ++++++++++++-----
> dts/framework/remote_session/testpmd_shell.py | 2 +
> .../testbed_model/traffic_generator/scapy.py | 50 +++++++++++++-
> 5 files changed, 139 insertions(+), 42 deletions(-)
>
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v6 0/3] Improve interactive shell output gathering and logging
2024-07-26 11:01 ` [PATCH v6 0/3] Improve interactive shell output gathering and logging Juraj Linkeš
@ 2024-07-29 16:56 ` Thomas Monjalon
0 siblings, 0 replies; 57+ messages in thread
From: Thomas Monjalon @ 2024-07-29 16:56 UTC (permalink / raw)
To: jspewock
Cc: probb, yoan.picchi, wathsala.vithanage, Honnappa.Nagarahalli,
npratte, paul.szczepanek, Luca.Vizzarro, dev, Juraj Linkeš
26/07/2024 13:01, Juraj Linkeš:
> For series:
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
> On 24. 7. 2024 20:39, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > v6:
> > * Fix error catch for retries. This series changed the error that
> > is thrown in the case of a timeout, but it was originally overlooked
> > that the context manager patch added a catch that is looking for the
> > old timeout error. This version fixes the patch by adjusting the
> > error that is expected in the context manager patch to match what
> > this series changes it to.
> >
>
> Here's the diff for anyone interested:
> diff --git
> a/dts/framework/remote_session/single_active_interactive_shell.py
> b/dts/framework/remote_session/single_active_interactive_shell.py
> index 7014444d0c..77a4dcefdf 100644
> --- a/dts/framework/remote_session/single_active_interactive_shell.py
> +++ b/dts/framework/remote_session/single_active_interactive_shell.py
> @@ -150,7 +150,7 @@ def _start_application(self) -> None:
> try:
> self.send_command(start_command)
> break
> - except TimeoutError:
> + except InteractiveSSHTimeoutError:
> self._logger.info(
> f"Interactive shell failed to start (attempt
> {attempt+1} out of "
> f"{self._init_attempts})"
>
> self.send_command raises InteractiveSSHTimeoutError (and not
> TimeoutError) which is why we needed this change.
>
> > Jeremy Spewock (3):
> > dts: Improve output gathering in interactive shells
> > dts: Add missing docstring from XML-RPC server
> > dts: Improve logging for interactive shells
Applied, thanks.
^ permalink raw reply [flat|nested] 57+ messages in thread