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

Proposed by Chris MacNaughton
Status: Needs review
Proposed branch: lp:~chris.macnaughton/mojo/py3003
Merge into: lp:~ost-maintainers/mojo/py3
Diff against target: 217 lines (+56/-13)
5 files modified
mojo/cli.py (+1/-1)
mojo/contain.py (+26/-6)
mojo/juju/debuglogs.py (+7/-1)
mojo/juju/status.py (+9/-2)
mojo/phase.py (+13/-3)
To merge this branch: bzr merge lp:~chris.macnaughton/mojo/py3003
Reviewer Review Type Date Requested Status
OpenStack Charm Testing Maintainers Pending
Review via email: mp+337159@code.launchpad.net
To post a comment you must log in.
lp:~chris.macnaughton/mojo/py3003 updated
470. By Chris MacNaughton

cleanup error output

Revision history for this message
Alexandre Gomes (alejdg) wrote :

+1

Revision history for this message
Colin Watson (cjwatson) wrote :

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

cleanup error output

469. By Chris MacNaughton

add more decode calls

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
645 logging.error(e)645 logging.error(e)
646 sys.exit(1)646 sys.exit(1)
647 except mojo.phase.ScriptPhaseException as e:647 except mojo.phase.ScriptPhaseException as e:
648 logging.error(e.output)648 logging.error(e)
649 sys.exit(1)649 sys.exit(1)
650 except mojo.exceptions.ConfigNotFoundException as e:650 except mojo.exceptions.ConfigNotFoundException as e:
651 logging.error(e.message)651 logging.error(e.message)
652652
=== 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 @@
11import os11import os
12import platform12import platform
13import pwd13import pwd
14import six
14import shlex15import shlex
15import shutil16import shutil
16import subprocess17import subprocess
@@ -568,11 +569,17 @@
568 if state:569 if state:
569 cmd = ('sudo', 'lxc-info', '-s', '-n', self.container_name)570 cmd = ('sudo', 'lxc-info', '-s', '-n', self.container_name)
570 logging.debug("Running command: {}".format(" ".join(cmd)))571 logging.debug("Running command: {}".format(" ".join(cmd)))
571 containers = yaml.safe_load(subprocess.check_output(cmd))572 output = subprocess.check_output(cmd)
573 if six.PY3:
574 output = output.decode('utf-8')
575 containers = yaml.safe_load(output)
572 # lxc-info shows 'state' (precise) or 'State' (newer)576 # lxc-info shows 'state' (precise) or 'State' (newer)
573 return state in containers.get('state', containers.get('State', {}))577 return state in containers.get('state', containers.get('State', {}))
574 cmd = ('sudo', 'lxc-ls', '-1')578 cmd = ('sudo', 'lxc-ls', '-1')
575 containers = subprocess.check_output(cmd).split()579 output = subprocess.check_output(cmd)
580 if six.PY3:
581 output = output.decode('utf-8')
582 containers = output.split()
576 return self.container_name in containers583 return self.container_name in containers
577584
578 @ensure_defined585 @ensure_defined
@@ -633,7 +640,10 @@
633 logging.debug("Running command: {}".format(" ".join(cmd)))640 logging.debug("Running command: {}".format(" ".join(cmd)))
634641
635 # lxc-info -i will return a row for each IP, plus a newline642 # lxc-info -i will return a row for each IP, plus a newline
636 found_ips = subprocess.check_output(cmd).split('\n')643 output = subprocess.check_output(cmd)
644 if six.PY3:
645 output = output.decode('utf-8')
646 found_ips = output.split('\n')
637 if len(found_ips) > 1:647 if len(found_ips) > 1:
638 networking = True648 networking = True
639 else:649 else:
@@ -645,7 +655,10 @@
645 if attempts >= max_attempts:655 if attempts >= max_attempts:
646 logging.error("No networking after {} attempts, giving up".format(attempts))656 logging.error("No networking after {} attempts, giving up".format(attempts))
647 cmd = ('sudo', 'lxc-info', '-n', self.container_name)657 cmd = ('sudo', 'lxc-info', '-n', self.container_name)
648 lxc_info_output = subprocess.check_output(cmd).split('\n')658 output = subprocess.check_output(cmd)
659 if six.PY3:
660 output = output.decode('utf-8')
661 lxc_info_output = output.split('\n')
649 logging.error("lxc-info output: {}".format(lxc_info_output))662 logging.error("lxc-info output: {}".format(lxc_info_output))
650 return False663 return False
651 return True664 return True
@@ -694,6 +707,8 @@
694 "".format(self.container_name, ' '.join(command)))707 "".format(self.container_name, ' '.join(command)))
695 try:708 try:
696 output = subprocess.check_output(cmd, stderr=stderr)709 output = subprocess.check_output(cmd, stderr=stderr)
710 if six.PY3:
711 output = output.decode('utf-8')
697 if output.strip():712 if output.strip():
698 logging.info(output)713 logging.info(output)
699 finally:714 finally:
@@ -791,9 +806,12 @@
791 return False806 return False
792807
793 def create_user(self, user):808 def create_user(self, user):
809 output = subprocess.check_output(
810 ['getent', 'passwd', user])
811 if six.PY3:
812 output = output.decode('utf-8')
794 (username, shadow, uid,813 (username, shadow, uid,
795 gid, name, home, shell) = subprocess.check_output(814 gid, name, home, shell) = output.split(':')
796 ['getent', 'passwd', user]).split(':')
797 # if the uid exists update the name to match the caller815 # if the uid exists update the name to match the caller
798 try:816 try:
799 command = "useradd --home {} --create-home --uid {} {}" \817 command = "useradd --home {} --create-home --uid {} {}" \
@@ -843,6 +861,8 @@
843 "project. Running command without a container: {}"861 "project. Running command without a container: {}"
844 "".format(command))862 "".format(command))
845 output = subprocess.check_output(command, stderr=subprocess.STDOUT)863 output = subprocess.check_output(command, stderr=subprocess.STDOUT)
864 if six.PY3:
865 output = output.decode('utf-8')
846 if output.strip():866 if output.strip():
847 logging.info(output)867 logging.info(output)
848868
849869
=== 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 @@
1import ast1import ast
2import logging2import logging
3import subprocess3import subprocess
4import six
4import yaml5import yaml
56
6import mojo.juju7import mojo.juju
@@ -97,11 +98,16 @@
97 '[ -z "$PYTHON" ] && {{ echo ERROR: No python3 or python found. ; exit 1 ; }} ; '98 '[ -z "$PYTHON" ] && {{ echo ERROR: No python3 or python found. ; exit 1 ; }} ; '
98 '$PYTHON -c "import glob; print(glob.glob(\'{}\'))"'.format(path)]99 '$PYTHON -c "import glob; print(glob.glob(\'{}\'))"'.format(path)]
99 output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)100 output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
101 if six.PY3:
102 output = output.decode('utf-8')
100 return ast.literal_eval(output)103 return ast.literal_eval(output)
101104
102 def _run_remote_command(self, unit, command, sanitized_path_list):105 def _run_remote_command(self, unit, command, sanitized_path_list):
103 cmd = ['juju', 'run', '--unit', unit, "{} {}".format(command, " ".join(sanitized_path_list))]106 cmd = ['juju', 'run', '--unit', unit, "{} {}".format(command, " ".join(sanitized_path_list))]
104 return subprocess.check_output(cmd, stderr=subprocess.STDOUT)107 output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
108 if six.PY3:
109 output = output.decode('utf-8')
110 return output
105111
106 def _gather_logs(self):112 def _gather_logs(self):
107 output = []113 output = []
108114
=== 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 @@
4242
43 cmd.extend(command)43 cmd.extend(command)
44 try:44 try:
45 return subprocess.check_output(cmd)45 output = subprocess.check_output(cmd)
46 if six.PY3:
47 output = output.decode('utf-8')
48 return output
46 except subprocess.CalledProcessError as cpe:49 except subprocess.CalledProcessError as cpe:
47 if command_timeout is not None:50 if command_timeout is not None:
48 # magic numbers here as per timeout(1)51 # magic numbers here as per timeout(1)
@@ -474,7 +477,11 @@
474 """ Parses the output of 'juju controllers' returning the controller version for the current controller.477 """ Parses the output of 'juju controllers' returning the controller version for the current controller.
475 Returns: The controller version reported by Juju478 Returns: The controller version reported by Juju
476 """479 """
477 controllers = yaml.safe_load(subprocess.check_output(['juju', 'controllers', '--format', 'yaml']))480 output = subprocess.check_output(
481 ['juju', 'controllers', '--format', 'yaml'])
482 if six.PY3:
483 output = output.decode('utf-8')
484 controllers = yaml.safe_load(output)
478 current_controller = controllers['current-controller']485 current_controller = controllers['current-controller']
479 return controllers['controllers'][current_controller]['agent-version']486 return controllers['controllers'][current_controller]['agent-version']
480487
481488
=== 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 @@
302 config = workspace.spec.get_config(self.config_base, stage)302 config = workspace.spec.get_config(self.config_base, stage)
303 cmd = [mojovol, '--config', config]303 cmd = [mojovol, '--config', config]
304 output = subprocess.check_output(cmd)304 output = subprocess.check_output(cmd)
305 if six.PY3:
306 output = output.decode('utf-8')
305 logging.info(output)307 logging.info(output)
306308
307309
@@ -396,10 +398,14 @@
396 logging.error("Running exec-on-failure {}".format(exec_on_failure_script))398 logging.error("Running exec-on-failure {}".format(exec_on_failure_script))
397 command = env_vars + [exec_on_failure_script]399 command = env_vars + [exec_on_failure_script]
398 try:400 try:
399 logging.error(subprocess.check_output(command, stderr=subprocess.STDOUT))401 output = subprocess.check_output(
402 command, stderr=subprocess.STDOUT)
403 if six.PY3:
404 output = output.decode('utf-8')
405 logging.error(output)
400 except subprocess.CalledProcessError as e:406 except subprocess.CalledProcessError as e:
401 logging.error("There was an error running exec-on-failure")407 logging.error("There was an error running exec-on-failure")
402 logging.error(e.output)408 logging.error(e)
403 except ConfigNotFoundException:409 except ConfigNotFoundException:
404 logging.debug("No exec-on-failure config found, see https://mojo.canonical.com/readme.html#exec-on-failure "410 logging.debug("No exec-on-failure config found, see https://mojo.canonical.com/readme.html#exec-on-failure "
405 "for more details")411 "for more details")
@@ -718,6 +724,8 @@
718 subprocess.call(cmd)724 subprocess.call(cmd)
719 else:725 else:
720 diff = subprocess.check_output(cmd)726 diff = subprocess.check_output(cmd)
727 if six.PY3:
728 diff = diff.decode('utf-8')
721 diff_processor(diff)729 diff_processor(diff)
722730
723 def run(self, project, workspace, stage, auto_secrets=True):731 def run(self, project, workspace, stage, auto_secrets=True):
@@ -821,6 +829,8 @@
821 failed_unit.replace("/", "-"),)]829 failed_unit.replace("/", "-"),)]
822 with chdir(workspace.repo_dir):830 with chdir(workspace.repo_dir):
823 failure = subprocess.check_output(cmd)831 failure = subprocess.check_output(cmd)
832 if six.PY3:
833 failure = failure.decode('utf-8')
824 logging.error(failure)834 logging.error(failure)
825 # Show juju status835 # Show juju status
826 juju_status = mojo.juju.Status(environment=juju_env)836 juju_status = mojo.juju.Status(environment=juju_env)
@@ -965,7 +975,7 @@
965 auto_secrets=auto_secrets, gather_debug_logs=False)975 auto_secrets=auto_secrets, gather_debug_logs=False)
966 except ScriptPhaseException as e:976 except ScriptPhaseException as e:
967 if i < retry:977 if i < retry:
968 logging.warning(e.output)978 logging.warning(e)
969 logging.warning(979 logging.warning(
970 "Error: verify script exited with status {} "980 "Error: verify script exited with status {} "
971 "(attempt {} of {}); sleeping for {}s".format(981 "(attempt {} of {}); sleeping for {}s".format(

Subscribers

People subscribed via source and target branches