Merge ~wesley-wiedenmeier/cloud-init:error-output into cloud-init:master

[Wesley Wiedenmeier]

Improve formatting for util.ProcessExecutionError and properly display multi line error messages. Add unittests for util.ProcessExecutionError.

old error formatting:
http://paste.ubuntu.com/23232468/

new error formatting:
http://paste.ubuntu.com/23232463/

[Scott Moser (smoser)]

Please rebase.

Ryan, will your log analyzer handle this change?

[Ryan Harper (raharper)]

On Wed, Oct 19, 2016 at 9:09 AM, Scott Moser <email address hidden> wrote:

> Please rebase.
>
> Ryan, will your log analyzer handle this change?
>

Yes, the analyzer is primarily interested in the event 'start' and 'finish'
messages; the log exceptions
aren't utilized for event tracking.

I'll confirm.

>
> --
> https://code.launchpad.net/~wesley-wiedenmeier/cloud-init/
> +git/cloud-init/+merge/306731
> You are requested to review the proposed merge of
> ~wesley-wiedenmeier/cloud-init:error-output into cloud-init:master.
>

[Wesley Wiedenmeier (wesley-wiedenmeier)]

I'll get this rebased later today, thanks.

78ab9b6... by Wesley Wiedenmeier on 2016-10-19

Merge branch 'error-output' of git+ssh://git.launchpad.net/~wesley-wiedenmeier/cloud-init into error-output

[Wesley Wiedenmeier (wesley-wiedenmeier)]

This should be able to merge cleanly now

a0bc2ea... by Wesley Wiedenmeier on 2016-10-25

Better handling of string types in util.ProcessExecutionError

 - ProcessExecutionError._indent_text returns same type as input
 - before formatting message, decode bytes objects, if any

691e083... by Wesley Wiedenmeier on 2016-10-25

Better handling of string types in util.ProcessExecutionError

 - ProcessExecutionError._indent_text returns same type as input
 - before formatting message, decode bytes objects, if any

06eb297... by Wesley Wiedenmeier on 2016-10-25

Merge branch 'error-output' of git+ssh://git.launchpad.net/~wesley-wiedenmeier/cloud-init into error-output

[Scott Moser (smoser)]

this needs rebasing.
currently fails due to the now gone 'message' on the ProcessExecutionError in python3.

[Scott Moser (smoser)]

review: Needs Fixing

d7f2a49... by Wesley Wiedenmeier on 2016-10-26

Remove duplicate created by rebase

aef16f1... by Wesley Wiedenmeier on 2016-10-26

Merge branch 'error-output' of git+ssh://git.launchpad.net/~wesley-wiedenmeier/cloud-init into error-output

5481ff8... by Wesley Wiedenmeier on 2016-10-26

Fix whitespace damage

[Wesley Wiedenmeier (wesley-wiedenmeier)]

I rebased again and fixed up the unittests. I think the rebase was correct this time.

diff --git a/cloudinit/util.py b/cloudinit/util.py
index 4b3fd0c..9c1a6fb 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 881509a..b671838 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