Merge lp:~chris.macnaughton/mojo/py3003 into lp:~ost-maintainers/mojo/py3

[Chris MacNaughton]

[lp:~chris.macnaughton/mojo/py3003]

470. By Chris MacNaughton on 2018-02-05

cleanup error output

[Alexandre Gomes (alejdg)]

+1

[Colin Watson (cjwatson)]

Any reason not to use universal_newlines=True on subprocess.check_output calls that are supposed to emit text instead? As long as unicode output can be tolerated in the Python 2 case, this is generally a simpler way to do this.

Unmerged revisions

470. By Chris MacNaughton on 2018-02-05

cleanup error output

469. By Chris MacNaughton on 2018-02-05

add more decode calls

=== modified file 'mojo/cli.py'
--- mojo/cli.py	2018-01-22 06:58:28 +0000
+++ mojo/cli.py	2018-02-05 16:24:42 +0000
@@ -645,7 +645,7 @@
         logging.error(e)
         sys.exit(1)
     except mojo.phase.ScriptPhaseException as e:
-        logging.error(e.output)
+        logging.error(e)
         sys.exit(1)
     except mojo.exceptions.ConfigNotFoundException as e:
         logging.error(e.message)

=== modified file 'mojo/contain.py'
--- mojo/contain.py	2018-01-18 10:23:15 +0000
+++ mojo/contain.py	2018-02-05 16:24:42 +0000
@@ -11,6 +11,7 @@
 import os
 import platform
 import pwd
+import six
 import shlex
 import shutil
 import subprocess
@@ -568,11 +569,17 @@
         if state:
             cmd = ('sudo', 'lxc-info', '-s', '-n', self.container_name)
             logging.debug("Running command: {}".format(" ".join(cmd)))
-            containers = yaml.safe_load(subprocess.check_output(cmd))
+            output = subprocess.check_output(cmd)
+            if six.PY3:
+                output = output.decode('utf-8')
+            containers = yaml.safe_load(output)
             # lxc-info shows 'state' (precise) or 'State' (newer)
             return state in containers.get('state', containers.get('State', {}))
         cmd = ('sudo', 'lxc-ls', '-1')
-        containers = subprocess.check_output(cmd).split()
+        output = subprocess.check_output(cmd)
+        if six.PY3:
+            output = output.decode('utf-8')
+        containers = output.split()
         return self.container_name in containers
 
     @ensure_defined
@@ -633,7 +640,10 @@
                 logging.debug("Running command: {}".format(" ".join(cmd)))
 
                 # lxc-info -i will return a row for each IP, plus a newline
-                found_ips = subprocess.check_output(cmd).split('\n')
+                output = subprocess.check_output(cmd)
+                if six.PY3:
+                    output = output.decode('utf-8')
+                found_ips = output.split('\n')
                 if len(found_ips) > 1:
                     networking = True
                 else:
@@ -645,7 +655,10 @@
                 if attempts >= max_attempts:
                     logging.error("No networking after {} attempts, giving up".format(attempts))
                     cmd = ('sudo', 'lxc-info', '-n', self.container_name)
-                    lxc_info_output = subprocess.check_output(cmd).split('\n')
+                    output = subprocess.check_output(cmd)
+                    if six.PY3:
+                        output = output.decode('utf-8')
+                    lxc_info_output = output.split('\n')
                     logging.error("lxc-info output: {}".format(lxc_info_output))
                     return False
             return True
@@ -694,6 +707,8 @@
                      "".format(self.container_name, ' '.join(command)))
         try:
             output = subprocess.check_output(cmd, stderr=stderr)
+            if six.PY3:
+                output = output.decode('utf-8')
             if output.strip():
                 logging.info(output)
         finally:
@@ -791,9 +806,12 @@
             return False
 
     def create_user(self, user):
+        output = subprocess.check_output(
+            ['getent', 'passwd', user])
+        if six.PY3:
+            output = output.decode('utf-8')
         (username, shadow, uid,
-         gid, name, home, shell) = subprocess.check_output(
-            ['getent', 'passwd', user]).split(':')
+         gid, name, home, shell) = output.split(':')
         # if the uid exists update the name to match the caller
         try:
             command = "useradd --home {} --create-home --uid {} {}" \
@@ -843,6 +861,8 @@
                      "project. Running command without a container: {}"
                      "".format(command))
         output = subprocess.check_output(command, stderr=subprocess.STDOUT)
+        if six.PY3:
+            output = output.decode('utf-8')
         if output.strip():
             logging.info(output)
 

=== modified file 'mojo/juju/debuglogs.py'
--- mojo/juju/debuglogs.py	2017-01-11 02:34:42 +0000
+++ mojo/juju/debuglogs.py	2018-02-05 16:24:42 +0000
@@ -1,6 +1,7 @@
 import ast
 import logging
 import subprocess
+import six
 import yaml
 
 import mojo.juju
@@ -97,11 +98,16 @@
                '[ -z "$PYTHON" ] && {{ echo ERROR: No python3 or python found. ; exit 1 ; }} ; '
                '$PYTHON -c "import glob; print(glob.glob(\'{}\'))"'.format(path)]
         output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+        if six.PY3:
+            output = output.decode('utf-8')
         return ast.literal_eval(output)
 
     def _run_remote_command(self, unit, command, sanitized_path_list):
         cmd = ['juju', 'run', '--unit', unit, "{} {}".format(command, " ".join(sanitized_path_list))]
-        return subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+        output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+        if six.PY3:
+            output = output.decode('utf-8')
+        return output
 
     def _gather_logs(self):
         output = []

=== modified file 'mojo/juju/status.py'
--- mojo/juju/status.py	2018-01-25 09:10:29 +0000
+++ mojo/juju/status.py	2018-02-05 16:24:42 +0000
@@ -42,7 +42,10 @@
 
     cmd.extend(command)
     try:
-        return subprocess.check_output(cmd)
+        output = subprocess.check_output(cmd)
+        if six.PY3:
+            output = output.decode('utf-8')
+        return output
     except subprocess.CalledProcessError as cpe:
         if command_timeout is not None:
             # magic numbers here as per timeout(1)
@@ -474,7 +477,11 @@
         """ Parses the output of 'juju controllers' returning the controller version for the current controller.
         Returns: The controller version reported by Juju
         """
-        controllers = yaml.safe_load(subprocess.check_output(['juju', 'controllers', '--format', 'yaml']))
+        output = subprocess.check_output(
+            ['juju', 'controllers', '--format', 'yaml'])
+        if six.PY3:
+            output = output.decode('utf-8')
+        controllers = yaml.safe_load(output)
         current_controller = controllers['current-controller']
         return controllers['controllers'][current_controller]['agent-version']
 

=== modified file 'mojo/phase.py'
--- mojo/phase.py	2018-01-29 11:51:05 +0000
+++ mojo/phase.py	2018-02-05 16:24:42 +0000
@@ -302,6 +302,8 @@
         config = workspace.spec.get_config(self.config_base, stage)
         cmd = [mojovol, '--config', config]
         output = subprocess.check_output(cmd)
+        if six.PY3:
+            output = output.decode('utf-8')
         logging.info(output)
 
 
@@ -396,10 +398,14 @@
             logging.error("Running exec-on-failure {}".format(exec_on_failure_script))
             command = env_vars + [exec_on_failure_script]
             try:
-                logging.error(subprocess.check_output(command, stderr=subprocess.STDOUT))
+                output = subprocess.check_output(
+                    command, stderr=subprocess.STDOUT)
+                if six.PY3:
+                    output = output.decode('utf-8')
+                logging.error(output)
             except subprocess.CalledProcessError as e:
                 logging.error("There was an error running exec-on-failure")
-                logging.error(e.output)
+                logging.error(e)
         except ConfigNotFoundException:
             logging.debug("No exec-on-failure config found, see https://mojo.canonical.com/readme.html#exec-on-failure "
                           "for more details")
@@ -718,6 +724,8 @@
                 subprocess.call(cmd)
             else:
                 diff = subprocess.check_output(cmd)
+                if six.PY3:
+                    diff = diff.decode('utf-8')
                 diff_processor(diff)
 
     def run(self, project, workspace, stage, auto_secrets=True):
@@ -821,6 +829,8 @@
                            failed_unit.replace("/", "-"),)]
                 with chdir(workspace.repo_dir):
                     failure = subprocess.check_output(cmd)
+                    if six.PY3:
+                        failure = failure.decode('utf-8')
                     logging.error(failure)
             # Show juju status
             juju_status = mojo.juju.Status(environment=juju_env)
@@ -965,7 +975,7 @@
                      auto_secrets=auto_secrets, gather_debug_logs=False)
             except ScriptPhaseException as e:
                 if i < retry:
-                    logging.warning(e.output)
+                    logging.warning(e)
                     logging.warning(
                         "Error: verify script exited with status {} "
                         "(attempt {} of {}); sleeping for {}s".format(