From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 977454545B; Fri, 14 Jun 2024 22:58:52 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6DD2640674; Fri, 14 Jun 2024 22:58:52 +0200 (CEST) Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by mails.dpdk.org (Postfix) with ESMTP id 6F11A4060B for ; Fri, 14 Jun 2024 22:58:50 +0200 (CEST) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2ebdd684ef6so3146491fa.1 for ; Fri, 14 Jun 2024 13:58:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1718398730; x=1719003530; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=eSsDFJomelCODXiXmlrC0Kx5Fx+RkBcDvsdOgtjGsAM=; b=PuFPDwbiUBDWlhz9D3dwxootNe3Q2RwdA5l3/LX8+34AHnBkoEsVJNmkfNJ5bm0M6X glMu7DrqwFX0KvV3Yi5LqOSEhW7aI5GAhhvVDZIKTl5FIfCSDB7XRuiway4YxBBDJzH/ qLeUgu6Op5cubG9uhGc7pt7HhwKVqJrmg158w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718398730; x=1719003530; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eSsDFJomelCODXiXmlrC0Kx5Fx+RkBcDvsdOgtjGsAM=; b=Bba6DRDeF0jScMTsIJ4iGhTTMSkhI41XMgJ9NBxYuh1bbBS8zpPH/wx4YwIekCySCo zoFDvclNgRUkM4yjdFgr6yAvHC9V+aq8O028g1hJnDWfge7QhFTO71xn51I5ufFVz4mI Ql3G36+MQsQ2yflEJg7s6m+Kvit1azSGvKrJiLeWBP6+XPzljBS6ApBYKnhmQKM5TJQ0 NTAeAndLVavMB0jrEiVqP+ZcTQeF+WmjqEOInrVZdN7bSGTYQP2WC7vjbjm7yFldjFKJ gIzNDQGX6x5CeJ8+YJdum2ZR00Tan5tObotNpnmAbW32BmdVdfPQdQCp2x8qgpqPEl/c z+Vg== X-Forwarded-Encrypted: i=1; AJvYcCWChK9jp6E6mPuurgGqawzQ7pF1pNtkzyuyfgfdhwPWmCjYWiK8QrNfvCWjT8ubb3Y8xH58vjCCwmTNVEE= X-Gm-Message-State: AOJu0YwG62Wt6CIQorWCe6gJfGVTAMfdFmj2XA4iFwMEuaRqFKpythmh xUdgsqrv8fau8lJnqEZztqx7U4+uxw2RXBeoufxtzDqayez+2b/ZjiwAVsRhrWK1lgVwGCjncsc qBB1ax297asmfMX2tC2FNBHbrxMahZL/9eAnuZA== X-Google-Smtp-Source: AGHT+IHRzmsONveAXcxqTGPZiWwSSSMbr57GhbiM9eyJD3S4+57wwHGSWRZ8nkGyI5lQYezxLw8/5dYSXERc4EN72/g= X-Received: by 2002:a05:651c:1a1e:b0:2ea:eac6:6872 with SMTP id 38308e7fff4ca-2ec0e3e2157mr29619191fa.0.1718398729734; Fri, 14 Jun 2024 13:58:49 -0700 (PDT) MIME-Version: 1.0 References: <20240501161623.26672-1-jspewock@iol.unh.edu> <20240529194910.26803-1-jspewock@iol.unh.edu> <20240529194910.26803-2-jspewock@iol.unh.edu> In-Reply-To: <20240529194910.26803-2-jspewock@iol.unh.edu> From: Nicholas Pratte Date: Fri, 14 Jun 2024 16:58:38 -0400 Message-ID: Subject: Re: [PATCH v3 1/3] dts: Improve output gathering in interactive shells To: jspewock@iol.unh.edu Cc: paul.szczepanek@arm.com, wathsala.vithanage@arm.com, probb@iol.unh.edu, Luca.Vizzarro@arm.com, thomas@monjalon.net, juraj.linkes@pantheon.tech, Honnappa.Nagarahalli@arm.com, yoan.picchi@foss.arm.com, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Reviewed-by: Nicholas Pratte On Wed, May 29, 2024 at 3:49=E2=80=AFPM wrote: > > From: Jeremy Spewock > > 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 > --- > 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] =3D ErrorSeverity.GENERIC_ERR > > > -class SSHTimeoutError(DTSError): > - """The SSH execution of a command timed out.""" > - > - #: > - severity: ClassVar[ErrorSeverity] =3D ErrorSeverity.SSH_ERR > - _command: str > - > - def __init__(self, command: str): > - """Define the meaning of the first argument. > - > - Args: > - command: The executed command. > - """ > - self._command =3D 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] =3D ErrorSeverity.SSH_ERR > + _command: str > + > + def __init__(self, command: str): > + """Define the meaning of the first argument. > + > + Args: > + command: The executed command. > + """ > + self._command =3D 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 ou= t.""" > + > + > +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] =3D 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/fram= ework/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 i= s > - #: most commonly an additional newline character. > + #: most commonly an additional newline character. This additional ne= wline > + #: character is used to force the line that is currently awaiting in= put > + #: into the stdout buffer so that it can be consumed and checked aga= inst > + #: the expected prompt. > _command_extra_chars: ClassVar[str] =3D "" > > #: 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 =3D "", > @@ -80,7 +89,7 @@ def __init__( > and no output is gathered within the timeout, an excepti= on is thrown. > """ > self._interactive_session =3D interactive_session > - self._ssh_channel =3D self._interactive_session.invoke_shell() > + self._ssh_channel =3D self._interactive_session.session.invoke_s= hell() > self._stdin =3D self._ssh_channel.makefile_stdin("w") > self._stdout =3D self._ssh_channel.makefile("r") > self._ssh_channel.settimeout(timeout) > @@ -124,20 +133,34 @@ def send_command(self, command: str, prompt: str | = None =3D None) -> str: > > Returns: > All output in the buffer before expected string. > + > + Raises: > + InteractiveSSHSessionDeadError: The session died while execu= ting the command. > + InteractiveSSHTimeoutError: If command was sent but prompt c= ould not be found in > + the output before the timeout. > """ > self._logger.info(f"Sending: '{command}'") > if prompt is None: > prompt =3D self._default_prompt > - self._stdin.write(f"{command}{self._command_extra_chars}\n") > - self._stdin.flush() > out: str =3D "" > - for line in self._stdout: > - out +=3D 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 +=3D 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_sessi= on.hostname) from e > + finally: > + self._logger.debug(f"Got output: {out}") > return out > > def close(self) -> None: > -- > 2.45.1 >