From 54964c838ea1acc6ca59e0bc7e3ac6800db26ed1 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Mon, 14 Jun 2021 20:33:32 +0530 Subject: [PATCH 1/3] orchestra/run: move Raw and quote to a separate class Class Raw is needed in teuthology.exceptions and importing it from teuthology.orchestra.run is not possible since it would lead to a circular dependecy. Move these classes to a new module, run_helper.py, to avoid this and import through run_helper.py instead. Signed-off-by: Rishabh Dave --- teuthology/orchestra/run.py | 73 +------------------------- teuthology/orchestra/run_helper.py | 82 ++++++++++++++++++++++++++++++ teuthology/packaging.py | 2 +- teuthology/test/task/test_pcp.py | 2 +- 4 files changed, 85 insertions(+), 74 deletions(-) create mode 100644 teuthology/orchestra/run_helper.py diff --git a/teuthology/orchestra/run.py b/teuthology/orchestra/run.py index f31dfd0d7f..3f2be458b5 100644 --- a/teuthology/orchestra/run.py +++ b/teuthology/orchestra/run.py @@ -9,10 +9,10 @@ import gevent import gevent.event import socket -import pipes import logging import shutil +from teuthology.orchestra.run_helper import quote, KludgeFile, PIPE from teuthology.contextutil import safe_while from teuthology.exceptions import (CommandCrashedError, CommandFailedError, ConnectionLostError) @@ -222,43 +222,6 @@ def __repr__(self): ) -class Raw(object): - - """ - Raw objects are passed to remote objects and are not processed locally. - """ - def __init__(self, value): - self.value = value - - def __repr__(self): - return '{cls}({value!r})'.format( - cls=self.__class__.__name__, - value=self.value, - ) - - def __eq__(self, value): - return self.value == value - - -def quote(args): - """ - Internal quote wrapper. - """ - def _quote(args): - """ - Handle quoted string, testing for raw charaters. - """ - for a in args: - if isinstance(a, Raw): - yield a.value - else: - yield pipes.quote(a) - if isinstance(args, list): - return ' '.join(_quote(args)) - else: - return args - - def copy_to_log(f, logger, loglevel=logging.INFO, capture=None, quiet=False): """ Copy line by line from file in f to the log from logger @@ -349,40 +312,6 @@ def wrapper(): return r -class Sentinel(object): - - """ - Sentinel -- used to define PIPE file-like object. - """ - def __init__(self, name): - self.name = name - - def __str__(self): - return self.name - -PIPE = Sentinel('PIPE') - - -class KludgeFile(object): - - """ - Wrap Paramiko's ChannelFile in a way that lets ``f.close()`` - actually cause an EOF for the remote command. - """ - def __init__(self, wrapped): - self._wrapped = wrapped - - def __getattr__(self, name): - return getattr(self._wrapped, name) - - def close(self): - """ - Close and shutdown. - """ - self._wrapped.close() - self._wrapped.channel.shutdown_write() - - def run( client, args, stdin=None, stdout=None, stderr=None, diff --git a/teuthology/orchestra/run_helper.py b/teuthology/orchestra/run_helper.py new file mode 100644 index 0000000000..33a9991f77 --- /dev/null +++ b/teuthology/orchestra/run_helper.py @@ -0,0 +1,82 @@ +""" +Contains classes that "help" RemoteProcess and rest of the code in run.py + +This module was started as it was needed that Raw and quote be separated from +teuthology.orchestra.run so that these can be imported in +teuthology.exceptions without causing circular dependency. +""" + +from pipes import quote as pipes_quote + + +class Raw(object): + """ + Raw objects are passed to remote objects and are not processed locally. + """ + def __init__(self, value): + self.value = value + + def __repr__(self): + return '{cls}({value!r})'.format( + cls=self.__class__.__name__, + value=self.value, + ) + + def __eq__(self, value): + return self.value == value + + def __str__(self, value): + return str(value) + + +def quote(args): + """ + Internal quote wrapper. + """ + def _quote(args): + """ + Handle quoted string, testing for raw charaters. + """ + for a in args: + if isinstance(a, Raw): + yield a.value + else: + yield pipes_quote(a) + + if isinstance(args, list): + return ' '.join(_quote(args)) + else: + return args + + +class Sentinel(object): + """ + Sentinel -- used to define PIPE file-like object. + """ + def __init__(self, name): + self.name = name + + def __str__(self): + return self.name + + +PIPE = Sentinel('PIPE') + + +class KludgeFile(object): + """ + Wrap Paramiko's ChannelFile in a way that lets ``f.close()`` + actually cause an EOF for the remote command. + """ + def __init__(self, wrapped): + self._wrapped = wrapped + + def __getattr__(self, name): + return getattr(self._wrapped, name) + + def close(self): + """ + Close and shutdown. + """ + self._wrapped.close() + self._wrapped.channel.shutdown_write() diff --git a/teuthology/packaging.py b/teuthology/packaging.py index 37b1147159..fd394accf2 100644 --- a/teuthology/packaging.py +++ b/teuthology/packaging.py @@ -19,7 +19,7 @@ NoRemoteError) from teuthology.misc import sudo_write_file from teuthology.orchestra.opsys import OS, DEFAULT_OS_VERSION -from teuthology.orchestra.run import Raw +from teuthology.orchestra.run_helper import Raw log = logging.getLogger(__name__) diff --git a/teuthology/test/task/test_pcp.py b/teuthology/test/task/test_pcp.py index c70e544533..6293beadbb 100644 --- a/teuthology/test/task/test_pcp.py +++ b/teuthology/test/task/test_pcp.py @@ -9,7 +9,7 @@ from teuthology.config import config, FakeNamespace from teuthology.orchestra.cluster import Cluster from teuthology.orchestra.remote import Remote -from teuthology.orchestra.run import Raw +from teuthology.orchestra.run_helper import Raw from teuthology.task.pcp import (PCPDataSource, PCPArchive, PCPGrapher, GrafanaGrapher, GraphiteGrapher, PCP) From af41a83b76849f714e6cc256483418bde7d5de69 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Wed, 9 Jun 2021 14:07:28 +0530 Subject: [PATCH 2/3] exceptions: change error message for CommandFailedError For the convenience of user, print the command that led to CommandFailedError in error message as a str than as a list or a tuple. This makes it more readable and enables the user to copy and use the command without any hassles. This commit also uses the opportunity to rename the variable "command" to "cmd" and "self.command" to "self.cmd" in CommandFailedError for convenience and so that it's easy to keep statements under 80 characters. Signed-off-by: Rishabh Dave --- teuthology/exceptions.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/teuthology/exceptions.py b/teuthology/exceptions.py index a33cec0415..40be08dc0d 100644 --- a/teuthology/exceptions.py +++ b/teuthology/exceptions.py @@ -1,3 +1,8 @@ +from re import sub + +from teuthology.orchestra.run_helper import Raw + + class BranchNotFoundError(ValueError): def __init__(self, branch, repo=None): self.branch = branch @@ -50,13 +55,22 @@ class CommandFailedError(Exception): """ Exception thrown on command failure """ - def __init__(self, command, exitstatus, node=None, label=None): - self.command = command + def __init__(self, cmd, exitstatus, node=None, label=None): + self.cmd = cmd self.exitstatus = exitstatus self.node = node self.label = label def __str__(self): + if isinstance(self.cmd, (list, tuple)): + cmd = [str(Raw) if isinstance(x, Raw) else x + for x in self.cmd] + cmd = ' '.join(self.cmd) + elif isinstance(self.cmd, str): + cmd = sub(r'Raw\([\'"](.*)[\'"]\)', r'\1', self.cmd) + else: + raise RuntimeError('variable "self.cmd" is not str, list or tuple') + prefix = "Command failed" if self.label: prefix += " ({label})".format(label=self.label) @@ -64,7 +78,7 @@ def __str__(self): prefix += " on {node}".format(node=self.node) return "{prefix} with status {status}: {cmd!r}".format( status=self.exitstatus, - cmd=self.command, + cmd=cmd, prefix=prefix, ) @@ -74,7 +88,7 @@ def fingerprint(self): Used by sentry instead of grouping by backtrace. """ return [ - self.label or self.command, + self.label or self.cmd, 'exit status {}'.format(self.exitstatus), '{{ type }}', ] From c013aa4e223ee52b3fb7eb1c096223a036240c26 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Wed, 16 Jun 2021 09:28:15 +0530 Subject: [PATCH 3/3] orchestra/run.py: remove spawn_asyncresult() Since the function is dead code, there are no calls to it in teuthology or ceph repository, remove it. Signed-off-by: Rishabh Dave --- teuthology/orchestra/run.py | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/teuthology/orchestra/run.py b/teuthology/orchestra/run.py index 3f2be458b5..399927177f 100644 --- a/teuthology/orchestra/run.py +++ b/teuthology/orchestra/run.py @@ -7,7 +7,6 @@ from paramiko import ChannelFile import gevent -import gevent.event import socket import logging import shutil @@ -285,32 +284,6 @@ def copy_file_to(src, logger, stream=None, quiet=False): """ copy_to_log(src, logger, capture=stream, quiet=quiet) -def spawn_asyncresult(fn, *args, **kwargs): - """ - Spawn a Greenlet and pass it's results to an AsyncResult. - - This function is useful to shuffle data from a Greenlet to - AsyncResult, which then again is useful because any Greenlets that - raise exceptions will cause tracebacks to be shown on stderr by - gevent, even when ``.link_exception`` has been called. Using an - AsyncResult avoids this. - """ - r = gevent.event.AsyncResult() - - def wrapper(): - """ - Internal wrapper. - """ - try: - value = fn(*args, **kwargs) - except Exception as e: - r.set_exception(e) - else: - r.set(value) - gevent.spawn(wrapper) - - return r - def run( client, args,