From 7e67d265288dee88018475fb72e63dcca14e494c Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Tue, 9 Dec 2014 13:04:26 +0000 Subject: Give less scary error messages when a containerised command fails This affects errors encountered at build time and at system-integration time. New errors look like this: ERROR: Command failed: baserock/system-integration/02-install-gerrit-gerrit-installation-binaries-misc-0000: Containerisation settings: {'mounts': (('dev/shm', 'tmpfs', 'none'), ('tmp', 'tmpfs', 'none')), 'mount_proc': True, 'root': '/var/tmp/staging/tmp1YQ2yN/minimal-system-x86_64-generic.inst'} Error output: + install -D /usr/share/gerrit/gerrit-2.9.war /home/gerrit2/gerrit/gerrit-2.9.war -o gerrit2 -g gerrit2 -m 644 install: can't change ownership of /home/gerrit2/gerrit/gerrit-2.9.war: Operation not permitted Previously the error message would have been this: Command failed: unshare --mount -- sh -ec. mount --make-rprivate / root="$1" shift while true; do case "$1" in --) shift break ;; *) mount_point="$1" mount_type="$2" mount_source="$3" shift 3 path="$root/$mount_point" mount -t "$mount_type" "$mount_source" "$path" ;; esac done exec "$@" - /var/tmp/staging/tmppeA1Iw/gerrit-x86_64.inst/ dev/shm tmpfs none tmp tmpfs none -- linux-user-chroot --chdir . --mount-proc proc /var/tmp/staging/tmppeA1Iw/gerrit-x86_64.inst/ baserock/system-integration/02-install-gerrit-gerrit-installation-binaries-misc-0000 + install -D /usr/share/gerrit/gerrit-2.9.war /home/gerrit2/gerrit/gerrit-2.9.war -o gerrit2 -g gerrit2 -m 644 install: can't change ownership of /home/gerrit2/gerrit/gerrit-2.9.war: Operation not permitted --- morphlib/app.py | 9 +++++++-- morphlib/builder2.py | 17 ++++++++++++----- morphlib/stagingarea.py | 41 ++++++++++++++++++++++++++--------------- morphlib/util.py | 16 ++++++++++++++++ 4 files changed, 61 insertions(+), 22 deletions(-) diff --git a/morphlib/app.py b/morphlib/app.py index 930e023d..b5bcb601 100644 --- a/morphlib/app.py +++ b/morphlib/app.py @@ -348,7 +348,7 @@ class Morph(cliapp.Application): self.output.write('%s %s\n' % (timestamp, text)) self.output.flush() - def runcmd(self, argv, *args, **kwargs): + def _prepare_for_runcmd(self, argv, args, kwargs): if 'env' not in kwargs: kwargs['env'] = dict(os.environ) @@ -377,9 +377,14 @@ class Morph(cliapp.Application): morphlib.util.log_environment_changes(self, kwargs['env'], prev) self.prev_env = kwargs['env'] - # run the command line + def runcmd(self, argv, *args, **kwargs): + self._prepare_for_runcmd(argv, args, kwargs) return cliapp.Application.runcmd(self, argv, *args, **kwargs) + def runcmd_unchecked(self, argv, *args, **kwargs): + self._prepare_for_runcmd(argv, args, kwargs) + return cliapp.Application.runcmd_unchecked(self, argv, *args, **kwargs) + def parse_args(self, args, configs_only=False): return self.settings.parse_args(args, configs_only=configs_only, diff --git a/morphlib/builder2.py b/morphlib/builder2.py index f71f21db..9e158ea1 100644 --- a/morphlib/builder2.py +++ b/morphlib/builder2.py @@ -665,11 +665,18 @@ class SystemBuilder(BuilderBase): # pragma: no cover ) try: for bin in sorted(os.listdir(sys_integration_dir)): - self.app.runcmd( - morphlib.util.containerised_cmdline( - [os.path.join(SYSTEM_INTEGRATION_PATH, bin)], - root=rootdir, mounts=to_mount, mount_proc=True), - env=env) + argv = [os.path.join(SYSTEM_INTEGRATION_PATH, bin)] + container_config = dict( + root=rootdir, mounts=to_mount, mount_proc=True) + cmdline = morphlib.util.containerised_cmdline( + argv, **container_config) + exit, out, err = self.app.runcmd_unchecked( + cmdline, env=env) + if exit != 0: + logging.debug('Command returned code %i', exit) + msg = error_message_for_containerised_commandline( + argv, err, container_config) + raise cliapp.AppException(msg) except BaseException, e: self.app.status( msg='Error while running system integration commands', diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py index b676d4db..7060382a 100644 --- a/morphlib/stagingarea.py +++ b/morphlib/stagingarea.py @@ -274,22 +274,33 @@ class StagingArea(object): else: binds = () + container_config=dict( + cwd=kwargs.pop('cwd', '/'), + root=chroot_dir, + mounts=self.to_mount, + binds=binds, + mount_proc=mount_proc, + writable_paths=do_not_mount_dirs) + cmdline = morphlib.util.containerised_cmdline( - argv, cwd=kwargs.pop('cwd', '/'), - root=chroot_dir, mounts=self.to_mount, - binds=binds, mount_proc=mount_proc, - writable_paths=do_not_mount_dirs) - try: - if kwargs.get('logfile') != None: - logfile = kwargs.pop('logfile') - teecmd = ['tee', '-a', logfile] - return self._app.runcmd(cmdline, teecmd, **kwargs) - else: - return self._app.runcmd(cmdline, **kwargs) - except cliapp.AppException as e: - raise cliapp.AppException('In staging area %s: running ' - 'command \'%s\' failed.' % - (self.dirname, ' '.join(argv))) + argv, **container_config) + + if kwargs.get('logfile') != None: + logfile = kwargs.pop('logfile') + teecmd = ['tee', '-a', logfile] + exit, out, err = self._app.runcmd_unchecked( + cmdline, teecmd, **kwargs) + else: + exit, out, err = self._app.runcmd_unchecked(cmdline, **kwargs) + + if exit == 0: + return out + else: + logging.debug('Command returned code %i', exit) + msg = morphlib.util.error_message_for_containerised_commandline( + argv, err, container_config) + raise cliapp.AppException( + 'In staging area %s: %s' % (self.dirname, msg)) def abort(self): # pragma: no cover '''Handle what to do with a staging area in the case of failure. diff --git a/morphlib/util.py b/morphlib/util.py index 6f735387..736af92e 100644 --- a/morphlib/util.py +++ b/morphlib/util.py @@ -15,11 +15,13 @@ import contextlib import itertools +import logging import os import re import subprocess import textwrap +import cliapp import fs.osfs import morphlib @@ -626,3 +628,17 @@ def containerised_cmdline(args, cwd='.', root='/', binds=(), cmdargs.append(root) cmdargs.extend(args) return unshared_cmdline(cmdargs, root=root, **kwargs) + + +def error_message_for_containerised_commandline( + argv, err, container_kwargs): # pragma: no cover + '''Return a semi-readable error message for a containerised command.''' + + # This function should do some formatting of the container_kwargs dict, + # rather than just dumping it in the error message, but that is better than + # nothing. + + return 'Command failed: %s:\n' \ + 'Containerisation settings: %s\n' \ + 'Error output:\n%s' \ + % (' '.join(argv), container_kwargs, err) -- cgit v1.2.1 From ca24c606a043dd9dd6a380248f9d4d6dff3db597 Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Tue, 9 Dec 2014 13:10:27 +0000 Subject: Remove duplicate 'import' statement --- morphlib/util.py | 1 - 1 file changed, 1 deletion(-) diff --git a/morphlib/util.py b/morphlib/util.py index 736af92e..9b284d6e 100644 --- a/morphlib/util.py +++ b/morphlib/util.py @@ -43,7 +43,6 @@ try: from multiprocessing import cpu_count except NotImplementedError: # pragma: no cover cpu_count = lambda: 1 -import os def indent(string, spaces=4): -- cgit v1.2.1 From 8557427c00aecf63bbd0090395acdaf659e77e0f Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Tue, 9 Dec 2014 13:13:05 +0000 Subject: Give a useful path to failed staging areas Morph tells the user that an error occurred in the staging area, then moves the staging area somewhere else. Giving the old path rather than the new path is pretty annoying. --- morphlib/stagingarea.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py index 7060382a..e7783890 100644 --- a/morphlib/stagingarea.py +++ b/morphlib/stagingarea.py @@ -300,7 +300,12 @@ class StagingArea(object): msg = morphlib.util.error_message_for_containerised_commandline( argv, err, container_config) raise cliapp.AppException( - 'In staging area %s: %s' % (self.dirname, msg)) + 'In staging area %s: %s' % (self._failed_location(), msg)) + + def _failed_location(self): # pragma: no cover + '''Path this staging area will be moved to if an error occurs.''' + return os.path.join(self._app.settings['tempdir'], 'failed', + os.path.basename(self.dirname)) def abort(self): # pragma: no cover '''Handle what to do with a staging area in the case of failure. @@ -309,9 +314,7 @@ class StagingArea(object): # TODO: when we add the option to throw away failed builds, # hook it up here - - dest_dir = os.path.join(self._app.settings['tempdir'], - 'failed', os.path.basename(self.dirname)) + dest_dir = self._failed_location() os.rename(self.dirname, dest_dir) self.dirname = dest_dir -- cgit v1.2.1 From 9449dbfe1bb1800dfb15de025b87e1846e25e74a Mon Sep 17 00:00:00 2001 From: Sam Thursfield Date: Tue, 9 Dec 2014 13:58:28 +0000 Subject: Tiny optimisation of app.runcmd code path --- morphlib/app.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/morphlib/app.py b/morphlib/app.py index b5bcb601..6964be56 100644 --- a/morphlib/app.py +++ b/morphlib/app.py @@ -358,16 +358,15 @@ class Morph(cliapp.Application): else: print_command = True - # convert the command line arguments into a string - commands = [argv] + list(args) - for command in commands: - if isinstance(command, list): - for i in xrange(0, len(command)): - command[i] = str(command[i]) - commands = [' '.join(command) for command in commands] - - # print the command line if print_command: + # Print the command line + commands = [argv] + list(args) + for command in commands: + if isinstance(command, list): + for i in xrange(0, len(command)): + command[i] = str(command[i]) + commands = [' '.join(command) for command in commands] + self.status(msg='# %(cmdline)s', cmdline=' | '.join(commands), chatty=True) -- cgit v1.2.1