From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 070F843F5D;
	Wed,  1 May 2024 18:17:28 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id CA7BB402C7;
	Wed,  1 May 2024 18:17:27 +0200 (CEST)
Received: from mail-il1-f228.google.com (mail-il1-f228.google.com
 [209.85.166.228])
 by mails.dpdk.org (Postfix) with ESMTP id 143DF402C7
 for <dev@dpdk.org>; Wed,  1 May 2024 18:17:27 +0200 (CEST)
Received: by mail-il1-f228.google.com with SMTP id
 e9e14a558f8ab-36c6b4d7dc8so2213275ab.2
 for <dev@dpdk.org>; Wed, 01 May 2024 09:17:27 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=iol.unh.edu; s=unh-iol; t=1714580246; x=1715185046; darn=dpdk.org;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:from:to:cc:subject:date
 :message-id:reply-to;
 bh=EtlyOdDnX/Jty1BYDfb2e3EDOlPtLNciyHxYKgLd28E=;
 b=P7FKdWeSBwjpnhrMsc6ZouLhShZrYTuTbG/m6bK49T32m3R0VrPsmcn95YS2Dd/MjM
 abjIDpoqb8YFTx8kLfjknkUUZXHRkaJeXIttmxb/StnHJVyR+L5IlQgdXqGfugi9dUqE
 17hLwnK1kbeWbpo2MWESvAXcuNe+hOGEZbwQQ=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1714580246; x=1715185046;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=EtlyOdDnX/Jty1BYDfb2e3EDOlPtLNciyHxYKgLd28E=;
 b=Nf9VE+lRXoqTuNABbEXHVyN1VMJ/fyxtgzyC2xgQQglxXAeVojtZruMxaJ9bO3AgZi
 YiqzA/uzvTc7m/77H8p5DuYaL+BNO5qHUJxncMqaGNUrWzyKgsoJ2EwqBSVDuxAamnSg
 1ahMkk3648igBOhcRFeKkSFS61HOURY1SOBXYtnMhuCfyola13ux9zz2X8cubQqKEova
 M+g3tkSBOvGnFhGtohXmnFgAVvzJL8qajqlbfJSrWG9liCPB0mj+nCFeCnnamWv0GVro
 qZ+biaHdm5xkILOUI9cNTQFVTbh6chWmac63gwcuI1adUSJPmtNy5RBlPSHUQOjRb6GM
 diPQ==
X-Gm-Message-State: AOJu0Yw6QYf68QHBOPPsz+9X8bRBX6hgLlTtT+w0JCHR/IfljPNZ5NJm
 m+2YRfxfHm+WWzkzLcWt9wXWDMWBGffUPFiQLRDVdHZITbYxPb6Cqobp5CpM8O0TIIc0Klrq+YY
 uSkNulg2wPnKYM/Rp25PVOodpOck/5w+H
X-Google-Smtp-Source: AGHT+IEd0STBNdEhAXxUvOsTLXJWQ7PgJhTiiRzC7jvPqtAPBCOUCmBbch9G7whpKxCcyHwzdCT90ciaqX/J
X-Received: by 2002:a92:cdaf:0:b0:36c:3ad4:ef5a with SMTP id
 g15-20020a92cdaf000000b0036c3ad4ef5amr3726170ild.27.1714580246242; 
 Wed, 01 May 2024 09:17:26 -0700 (PDT)
Received: from postal.iol.unh.edu (postal.iol.unh.edu. [132.177.123.84])
 by smtp-relay.gmail.com with ESMTPS id
 r28-20020a638f5c000000b005dc4202b40bsm1863183pgn.6.2024.05.01.09.17.25
 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128);
 Wed, 01 May 2024 09:17:26 -0700 (PDT)
X-Relaying-Domain: iol.unh.edu
Received: from iol.unh.edu (unknown
 [IPv6:2606:4100:3880:1271:90f9:1b64:f6e6:867f])
 by postal.iol.unh.edu (Postfix) with ESMTP id 3CEBE6052514;
 Wed,  1 May 2024 12:17:25 -0400 (EDT)
From: jspewock@iol.unh.edu
To: Luca.Vizzarro@arm.com, wathsala.vithanage@arm.com,
 yoan.picchi@foss.arm.com, juraj.linkes@pantheon.tech,
 paul.szczepanek@arm.com, probb@iol.unh.edu, thomas@monjalon.net,
 Honnappa.Nagarahalli@arm.com
Cc: dev@dpdk.org,
	Jeremy Spewock <jspewock@iol.unh.edu>
Subject: [PATCH v2 1/3] dts: Improve output gathering in interactive shells
Date: Wed,  1 May 2024 12:16:21 -0400
Message-ID: <20240501161623.26672-2-jspewock@iol.unh.edu>
X-Mailer: git-send-email 2.44.0
In-Reply-To: <20240501161623.26672-1-jspewock@iol.unh.edu>
References: <20240312172558.11844-1-jspewock@iol.unh.edu>
 <20240501161623.26672-1-jspewock@iol.unh.edu>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

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