Revert "Capture stderr and include in CommandFailedError"

This reverts commit d0a434923100dd16548e5f76eaffa5462c4b91d4.

diff --git a/teuthology/orchestra/run.py b/teuthology/orchestra/run.py
index 7c17966be8309b9ef64ced503c3b6a2069d64993..0364cf8bcdba0495f469190b65515cb17344eb01 100644 (file)
--- a/teuthology/orchestra/run.py
+++ b/teuthology/orchestra/run.py
@@ -99,31 +99,29 @@ def copy_and_close(src, fdst):
         shutil.copyfileobj(src, fdst)
     fdst.close()
 
-
-def copy_file_to(f, destinations, host):
-    for dst in destinations:
-        if hasattr(dst, 'log'):
-            # looks like a Logger to me; not using isinstance to make life
-            # easier for unit tests
-            copy_to_log(f, dst, host)
-        else:
-            shutil.copyfileobj(f, dst)
+def copy_file_to(f, dst, host):
+    if hasattr(dst, 'log'):
+        # looks like a Logger to me; not using isinstance to make life
+        # easier for unit tests
+        handler = copy_to_log
+        return handler(f, dst, host)
+    else:
+        handler = shutil.copyfileobj
+        return handler(f, dst)
 
 
 class CommandFailedError(Exception):
-    def __init__(self, command, exitstatus, node, stderr):
+    def __init__(self, command, exitstatus, node=None):
         self.command = command
         self.exitstatus = exitstatus
         self.node = node
-        self.stderr = stderr
 
     def __str__(self):
-        return "Command failed on {node} with status {status}: {command!r}\nstderr output: {stderr}".format(
+        return "Command failed on {node} with status {status}: {command!r}".format(
             node=self.node,
             status=self.exitstatus,
             command=self.command,
-            stderr=self.stderr
-        )
+            )
 
 
 class CommandCrashedError(Exception):
@@ -232,14 +230,11 @@ def run(
     if name is None:
         name = host
 
-    err_buffer = StringIO()
-
     g_err = None
     if stderr is not PIPE:
-        err_outputs = [err_buffer, logger.getChild('err')]
-        if stderr is not None:
-            err_outputs.append(stderr)
-        g_err = gevent.spawn(copy_file_to, r.stderr, err_outputs, name)
+        if stderr is None:
+            stderr = logger.getChild('err')
+        g_err = gevent.spawn(copy_file_to, r.stderr, stderr, name)
         r.stderr = stderr
     else:
         assert not wait, "Using PIPE for stderr without wait=False would deadlock."
@@ -248,7 +243,7 @@ def run(
     if stdout is not PIPE:
         if stdout is None:
             stdout = logger.getChild('out')
-        g_out = gevent.spawn(copy_file_to, r.stdout, [stdout], name)
+        g_out = gevent.spawn(copy_file_to, r.stdout, stdout, name)
         r.stdout = stdout
     else:
         assert not wait, "Using PIPE for stdout without wait=False would deadlock."
@@ -275,7 +270,7 @@ def run(
                 # signal; sadly SSH does not tell us which signal
                 raise CommandCrashedError(command=r.command)
             if status != 0:
-                raise CommandFailedError(command=r.command, exitstatus=status, node=name, stderr=err_buffer.getvalue())
+                raise CommandFailedError(command=r.command, exitstatus=status, node=name)
         return status
 
     if wait:

diff --git a/teuthology/orchestra/test/test_run.py b/teuthology/orchestra/test/test_run.py
index cde0f2936d1e4fa40f907d2a1e2c8fed155d25b3..96cee4417a3fff70f05b41cead54866a30db0024 100644 (file)
--- a/teuthology/orchestra/test/test_run.py
+++ b/teuthology/orchestra/test/test_run.py
@@ -18,8 +18,6 @@ class TestRun(object):
         transport.expects('getpeername').with_args().returns(('HOST', 22))
         cmd = ssh.expects('exec_command')
         cmd.with_args("foo 'bar baz'")
-
-        # The I/O channels of the SSH child process
         in_ = fudge.Fake('ChannelFile(stdin)')
         out = fudge.Fake('ChannelFile(stdout)')
         err = fudge.Fake('ChannelFile(stderr)')
@@ -28,13 +26,8 @@ class TestRun(object):
         in_chan = fudge.Fake('channel')
         in_chan.expects('shutdown_write').with_args()
         in_.has_attr(channel=in_chan)
-
-        # The values we expect to see from the SSH child process
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         out.expects('xreadlines').with_args().returns(['foo', 'bar'])
         err.expects('xreadlines').with_args().returns(['bad'])
-
-        # The outputs we will expect to see child output forwarded to
         logger = fudge.Fake('logger')
         log_err = fudge.Fake('log_err')
         logger.expects('getChild').with_args('err').returns(log_err)
@@ -43,7 +36,6 @@ class TestRun(object):
         logger.expects('getChild').with_args('out').returns(log_out)
         log_out.expects('log').with_args(logging.INFO, '[HOST]: foo')
         log_out.expects('log').with_args(logging.INFO, '[HOST]: bar')
-
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
         channel.expects('recv_exit_status').with_args().returns(0)
@@ -74,7 +66,6 @@ class TestRun(object):
         out.expects('read').with_args().returns('foo\nb')
         out.expects('read').with_args().returns('ar\n')
         out.expects('read').with_args().returns('')
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         err.expects('xreadlines').with_args().returns(['bad'])
         logger = fudge.Fake('logger')
         log_err = fudge.Fake('log_err')
@@ -108,7 +99,6 @@ class TestRun(object):
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -122,8 +112,7 @@ class TestRun(object):
             )
         assert e.command == 'foo'
         assert e.exitstatus == 42
-        assert str(e) == """Command failed on HOST with status 42: 'foo'
-stderr output: bad"""
+        assert str(e) == "Command failed on HOST with status 42: 'foo'"
 
     @fudge.with_fakes
     def test_run_status_bad_nocheck(self):
@@ -139,7 +128,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -167,7 +155,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -196,7 +183,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -221,7 +207,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -254,7 +239,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -281,7 +265,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -299,8 +282,7 @@ stderr output: bad"""
             r.exitstatus.get,
             )
         assert e.exitstatus == 42
-        assert str(e) == """Command failed on HOST with status 42: 'foo'
-stderr output: bad"""
+        assert str(e) == "Command failed on HOST with status 42: 'foo'"
 
     @fudge.with_fakes
     def test_run_stdin_pipe(self):
@@ -316,7 +298,6 @@ stderr output: bad"""
         cmd.returns((in_, out, err))
         out.expects('xreadlines').with_args().returns([])
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)
@@ -351,7 +332,6 @@ stderr output: bad"""
         out.expects('read').with_args().returns('two')
         out.expects('read').with_args().returns('')
         err.expects('xreadlines').with_args().returns([])
-        err.expects('read').with_args().returns("bad").expects('read').with_args().returns(None)
         logger = fudge.Fake('logger').is_a_stub()
         channel = fudge.Fake('channel')
         out.has_attr(channel=channel)