From cc9d7247b62d291a209746859c785408146e0445 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Mon, 12 Nov 2018 17:04:38 +0000 Subject: [PATCH] Avoid printing b'' across logged output Converts stdout input from bytes to unicode to avoid printing lines as b'' on python3. Address runtime warning related to deprecation of iter(). Stop using bugsize=1 as this is likely to break unicode encodings. Usually an implementation using context managers would look much better but due to several bugs related to only partial support of context managers in py27 we cannot use them here. Tested with py27 and py35-py37. See: http://logs.openstack.org/37/616937/9/check/tripleo-ci-fedora-28-standalone/4a015f3/logs/undercloud/home/zuul/standalone_deploy.log.txt.gz#_2018-11-12_16_15_51 Change-Id: I00eaa508a6413c05c99beaca49e70c635a9116cc Partial-Bug: #1787912 --- tripleoclient/tests/test_utils.py | 10 +++++----- tripleoclient/utils.py | 18 +++++++++++++----- tripleoclient/v1/undercloud_preflight.py | 11 +++++++---- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index a00c70b6a..ed54952ff 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -190,7 +190,7 @@ class TestRunCommandAndLog(TestCase): self.mock_popen.assert_called_once_with(self.cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=False, bufsize=1, + shell=False, cwd=None, env=None) self.assertEqual(retcode, 0) self.mock_logger.warning.assert_has_calls(self.log_calls, @@ -208,7 +208,7 @@ class TestRunCommandAndLog(TestCase): retcode = utils.run_command_and_log(self.mock_logger, self.e_cmd) mock_popen.assert_called_once_with(self.e_cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=False, bufsize=1, cwd=None, + shell=False, cwd=None, env=None) self.assertEqual(retcode, 1) @@ -221,7 +221,7 @@ class TestRunCommandAndLog(TestCase): self.mock_popen.assert_called_once_with(self.cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=False, bufsize=1, + shell=False, cwd=None, env=test_env) self.assertEqual(retcode, 0) self.mock_logger.warning.assert_has_calls(self.log_calls, @@ -234,7 +234,7 @@ class TestRunCommandAndLog(TestCase): self.mock_popen.assert_called_once_with(self.cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=False, bufsize=1, + shell=False, cwd=test_cwd, env=None) self.assertEqual(retcode, 0) self.mock_logger.warning.assert_has_calls(self.log_calls, @@ -246,7 +246,7 @@ class TestRunCommandAndLog(TestCase): self.mock_popen.assert_called_once_with(self.cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=False, bufsize=1, + shell=False, cwd=None, env=None) self.assertEqual(run, self.mock_process) self.mock_logger.warning.assert_not_called() diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 204d430a5..912e71992 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1144,12 +1144,20 @@ def run_command_and_log(log, cmd, cwd=None, env=None, retcode_only=True): """ proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=False, - bufsize=1, cwd=cwd, env=env) - + cwd=cwd, env=env) if retcode_only: - for line in iter(proc.stdout.readline, b''): - # TODO(aschultz): this should probably goto a log file - log.warning(line.rstrip()) + # TODO(aschultz): this should probably goto a log file + while True: + try: + line = proc.stdout.readline() + except StopIteration: + break + if line != b'': + if isinstance(line, bytes): + line = line.decode('utf-8') + log.warning(line.rstrip()) + else: + break proc.stdout.close() return proc.wait() else: diff --git a/tripleoclient/v1/undercloud_preflight.py b/tripleoclient/v1/undercloud_preflight.py index 351aa46fe..8905ef155 100644 --- a/tripleoclient/v1/undercloud_preflight.py +++ b/tripleoclient/v1/undercloud_preflight.py @@ -57,9 +57,12 @@ def _run_command(args, env=None, name=None): if name is None: name = args[0] try: - return subprocess.check_output(args, - stderr=subprocess.STDOUT, - env=env).decode('utf-8') + output = subprocess.check_output(args, + stderr=subprocess.STDOUT, + env=env) + if isinstance(output, bytes): + output = output.decode('utf-8') + return output except subprocess.CalledProcessError as e: message = '%s failed: %s' % (name, e.output) LOG.error(message) @@ -155,7 +158,7 @@ def _check_hostname(): message = _('Configured hostname is not fully qualified.') LOG.error(message) raise RuntimeError(message) - sed_cmd = ('sed -i "s/127.0.0.1\(\s*\)/127.0.0.1\\1%s %s /" ' + sed_cmd = (r'sed -i "s/127.0.0.1\(\s*\)/127.0.0.1\\1%s %s /" ' '/etc/hosts' % (detected_static_hostname, short_hostname)) args = ['sudo', '/bin/bash', '-c', sed_cmd]