Improve formatting for ProcessExecutionError

This replaces long single lines in a log or console output
with multiple lines that are much easier to read.
It indents the stdout and stderr so logs are more easily
read also.

2 files changed, 105 insertions, 15 deletions

diff --git a/cloudinit/util.py b/cloudinit/util.py
index ed2d93e5..cc084719 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -235,15 +235,16 @@ class ProcessExecutionError(IOError):
                     'Command: %(cmd)s\n'
                     'Exit code: %(exit_code)s\n'
                     'Reason: %(reason)s\n'
-                    'Stdout: %(stdout)r\n'
-                    'Stderr: %(stderr)r')
+                    'Stdout: %(stdout)s\n'
+                    'Stderr: %(stderr)s')
+    empty_attr = '-'
 
     def __init__(self, stdout=None, stderr=None,
                  exit_code=None, cmd=None,
                  description=None, reason=None,
                  errno=None):
         if not cmd:
-            self.cmd = '-'
+            self.cmd = self.empty_attr
         else:
             self.cmd = cmd
 
@@ -253,36 +254,56 @@ class ProcessExecutionError(IOError):
             self.description = description
 
         if not isinstance(exit_code, six.integer_types):
-            self.exit_code = '-'
+            self.exit_code = self.empty_attr
         else:
             self.exit_code = exit_code
 
         if not stderr:
-            self.stderr = ''
+            self.stderr = self.empty_attr
         else:
-            self.stderr = stderr
+            self.stderr = self._indent_text(stderr)
 
         if not stdout:
-            self.stdout = ''
+            self.stdout = self.empty_attr
         else:
-            self.stdout = stdout
+            self.stdout = self._indent_text(stdout)
 
         if reason:
             self.reason = reason
         else:
-            self.reason = '-'
+            self.reason = self.empty_attr
 
         self.errno = errno
         message = self.MESSAGE_TMPL % {
-            'description': self.description,
-            'cmd': self.cmd,
-            'exit_code': self.exit_code,
-            'stdout': self.stdout,
-            'stderr': self.stderr,
-            'reason': self.reason,
+            'description': self._ensure_string(self.description),
+            'cmd': self._ensure_string(self.cmd),
+            'exit_code': self._ensure_string(self.exit_code),
+            'stdout': self._ensure_string(self.stdout),
+            'stderr': self._ensure_string(self.stderr),
+            'reason': self._ensure_string(self.reason),
         }
         IOError.__init__(self, message)
 
+    def _ensure_string(self, text):
+        """
+        if data is bytes object, decode
+        """
+        return text.decode() if isinstance(text, six.binary_type) else text
+
+    def _indent_text(self, text, indent_level=8):
+        """
+        indent text on all but the first line, allowing for easy to read output
+        """
+        cr = '\n'
+        indent = ' ' * indent_level
+        # if input is bytes, return bytes
+        if isinstance(text, six.binary_type):
+            cr = cr.encode()
+            indent = indent.encode()
+        # remove any newlines at end of text first to prevent unneeded blank
+        # line in output
+        return text.rstrip(cr).replace(cr, cr + indent)
+
 
 class SeLinuxGuard(object):
     def __init__(self, path, recursive=False):
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index f6a8ab75..bf9df561 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -611,4 +611,73 @@ class TestEncode(helpers.TestCase):
         text = util.decode_binary(blob)
         self.assertEqual(text, blob)
 
+
+class TestProcessExecutionError(helpers.TestCase):
+
+    template = ('{description}\n'
+                'Command: {cmd}\n'
+                'Exit code: {exit_code}\n'
+                'Reason: {reason}\n'
+                'Stdout: {stdout}\n'
+                'Stderr: {stderr}')
+    empty_attr = '-'
+    empty_description = 'Unexpected error while running command.'
+
+    def test_pexec_error_indent_text(self):
+        error = util.ProcessExecutionError()
+        msg = 'abc\ndef'
+        formatted = 'abc\n{}def'.format(' ' * 4)
+        self.assertEqual(error._indent_text(msg, indent_level=4), formatted)
+        self.assertEqual(error._indent_text(msg.encode(), indent_level=4),
+                         formatted.encode())
+        self.assertIsInstance(
+            error._indent_text(msg.encode()), type(msg.encode()))
+
+    def test_pexec_error_type(self):
+        self.assertIsInstance(util.ProcessExecutionError(), IOError)
+
+    def test_pexec_error_empty_msgs(self):
+        error = util.ProcessExecutionError()
+        self.assertTrue(all(attr == self.empty_attr for attr in
+                            (error.stderr, error.stdout, error.reason)))
+        self.assertEqual(error.description, self.empty_description)
+        self.assertEqual(str(error), self.template.format(
+            description=self.empty_description, exit_code=self.empty_attr,
+            reason=self.empty_attr, stdout=self.empty_attr,
+            stderr=self.empty_attr, cmd=self.empty_attr))
+
+    def test_pexec_error_single_line_msgs(self):
+        stdout_msg = 'out out'
+        stderr_msg = 'error error'
+        cmd = 'test command'
+        exit_code = 3
+        error = util.ProcessExecutionError(
+            stdout=stdout_msg, stderr=stderr_msg, exit_code=3, cmd=cmd)
+        self.assertEqual(str(error), self.template.format(
+            description=self.empty_description, stdout=stdout_msg,
+            stderr=stderr_msg, exit_code=str(exit_code),
+            reason=self.empty_attr, cmd=cmd))
+
+    def test_pexec_error_multi_line_msgs(self):
+        # make sure bytes is converted handled properly when formatting
+        stdout_msg = 'multi\nline\noutput message'.encode()
+        stderr_msg = 'multi\nline\nerror message\n\n\n'
+        error = util.ProcessExecutionError(
+            stdout=stdout_msg, stderr=stderr_msg)
+        self.assertEqual(
+            str(error),
+            '\n'.join((
+                '{description}',
+                'Command: {empty_attr}',
+                'Exit code: {empty_attr}',
+                'Reason: {empty_attr}',
+                'Stdout: multi',
+                '        line',
+                '        output message',
+                'Stderr: multi',
+                '        line',
+                '        error message',
+            )).format(description=self.empty_description,
+                      empty_attr=self.empty_attr))
+
 # vi: ts=4 expandtab