From 4c930a0ac7aa9e6e0c6439b68044a1392c708ae3 Mon Sep 17 00:00:00 2001 From: Alfredo Deza Date: Tue, 18 Feb 2014 09:47:09 -0500 Subject: [PATCH] Revert "Capture stderr and include in CommandFailedError" This reverts commit d0a434923100dd16548e5f76eaffa5462c4b91d4. --- teuthology/orchestra/run.py | 39 ++++++++++++--------------- teuthology/orchestra/test/test_run.py | 24 ++--------------- 2 files changed, 19 insertions(+), 44 deletions(-) diff --git a/teuthology/orchestra/run.py b/teuthology/orchestra/run.py index 7c17966be8309..0364cf8bcdba0 100644 --- 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 cde0f2936d1e4..96cee4417a3ff 100644 --- 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) -- 2.39.5