DPDK patches and discussions
 help / color / mirror / Atom feed
From: jspewock@iol.unh.edu
To: juraj.linkes@pantheon.tech, probb@iol.unh.edu,
	yoan.picchi@foss.arm.com, wathsala.vithanage@arm.com,
	Honnappa.Nagarahalli@arm.com, npratte@iol.unh.edu,
	paul.szczepanek@arm.com, thomas@monjalon.net,
	Luca.Vizzarro@arm.com
Cc: dev@dpdk.org, Jeremy Spewock <jspewock@iol.unh.edu>,
	Luca Vizzarro <luca.vizzarro@arm.com>
Subject: [PATCH v6 1/3] dts: Improve output gathering in interactive shells
Date: Wed, 24 Jul 2024 14:39:12 -0400	[thread overview]
Message-ID: <20240724183914.436188-2-jspewock@iol.unh.edu> (raw)
In-Reply-To: <20240724183914.436188-1-jspewock@iol.unh.edu>

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


  reply	other threads:[~2024-07-24 18:39 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-12 17:25 [PATCH v1 0/2] Improve interactive shell output gathering jspewock
2024-03-12 17:25 ` [PATCH v1 1/2] dts: Improve output gathering in interactive shells jspewock
2024-04-03  9:00   ` Juraj Linkeš
2024-04-08 16:20     ` Jeremy Spewock
2024-04-10 10:20       ` Juraj Linkeš
2024-03-12 17:25 ` [PATCH v1 2/2] dts: Add missing docstring from XML-RPC server jspewock
2024-04-24 13:42   ` Patrick Robb
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-09  9:57     ` Luca Vizzarro
2024-05-13 14:58     ` Juraj Linkeš
2024-05-15 19:13       ` Jeremy Spewock
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š
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
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
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-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
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
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
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-21  9:10       ` Juraj Linkeš
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š
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
2024-07-23 22:17     ` [PATCH v4 0/3] Improve interactive shell output gathering and logging Thomas Monjalon
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     ` [PATCH v5 3/3] dts: Improve logging for interactive shells jspewock
2024-07-24 18:39   ` [PATCH v6 0/3] Improve interactive shell output gathering and logging jspewock
2024-07-24 18:39     ` jspewock [this message]
2024-07-24 18:39     ` [PATCH v6 2/3] dts: Add missing docstring from XML-RPC server 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š
2024-07-29 16:56       ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240724183914.436188-2-jspewock@iol.unh.edu \
    --to=jspewock@iol.unh.edu \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=Luca.Vizzarro@arm.com \
    --cc=dev@dpdk.org \
    --cc=juraj.linkes@pantheon.tech \
    --cc=npratte@iol.unh.edu \
    --cc=paul.szczepanek@arm.com \
    --cc=probb@iol.unh.edu \
    --cc=thomas@monjalon.net \
    --cc=wathsala.vithanage@arm.com \
    --cc=yoan.picchi@foss.arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).