diff --git a/common/tools.py b/common/tools.py index 24e25cf..9cc694c 100644 --- a/common/tools.py +++ b/common/tools.py @@ -1,8 +1,18 @@ -def log_command(command, logger): +def log_check_call(command, logger): + status = log_call(command, logger) + if status != 0: + from subprocess import CalledProcessError + msg = ('Command \'{command}\' returned non-zero exit status ' + '{status}'.format(command=command, status=status)) + raise CalledProcessError(msg) + + +def log_call(command, logger): import subprocess import select + process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) while True: reads = [process.stdout.fileno(), process.stderr.fileno()] @@ -17,8 +27,7 @@ def log_command(command, logger): if line != '': logger.error(line.strip()) if process.poll() is not None: - break - return process.returncode + return process.returncode def sed_i(file_path, pattern, subst): diff --git a/providers/ec2/tasks/bootstrap.py b/providers/ec2/tasks/bootstrap.py index 0d8e498..787576d 100644 --- a/providers/ec2/tasks/bootstrap.py +++ b/providers/ec2/tasks/bootstrap.py @@ -1,7 +1,6 @@ from base import Task from common import phases -from common.exceptions import TaskError -from common.tools import log_command +from common.tools import log_check_call import logging log = logging.getLogger(__name__) @@ -31,8 +30,7 @@ class MakeTarball(Task): info.tarball = os.path.join(info.manifest.bootstrapper['tarball_dir'], tarball_filename) command = executable + options + ['--make-tarball=' + info.tarball] + arguments - if log_command(command, log) != 0: - raise TaskError('Unable to create bootstrap tarball') + log_check_call(command, log) class Bootstrap(Task): @@ -46,5 +44,4 @@ class Bootstrap(Task): options.extend(['--unpack-tarball=' + info.tarball]) command = executable + options + arguments - if log_command(command, log) != 0: - raise TaskError('Unable to bootstrap') + log_check_call(command, log) diff --git a/providers/ec2/tasks/config.py b/providers/ec2/tasks/config.py index d48399a..17252ae 100644 --- a/providers/ec2/tasks/config.py +++ b/providers/ec2/tasks/config.py @@ -1,7 +1,6 @@ from base import Task from common import phases -from common.exceptions import TaskError -from common.tools import log_command +from common.tools import log_check_call import os.path import logging log = logging.getLogger(__name__) @@ -18,8 +17,9 @@ class GenerateLocale(Task): charmap=info.manifest.system['charmap']) search = '# ' + locale_str sed_i(locale_gen, search, locale_str) - if log_command(['chroot', info.root, 'dpkg-reconfigure', '--priority=critical', 'locales'], log) != 0: - raise TaskError('Failed to regenerate locales') + + command = ['chroot', info.root, 'dpkg-reconfigure', '--priority=critical', 'locales'] + log_check_call(command, log) class SetTimezone(Task): diff --git a/providers/ec2/tasks/filesystem.py b/providers/ec2/tasks/filesystem.py index f298e54..fe69171 100644 --- a/providers/ec2/tasks/filesystem.py +++ b/providers/ec2/tasks/filesystem.py @@ -1,10 +1,8 @@ from base import Task from common import phases from common.exceptions import TaskError -from common.tools import log_command +from common.tools import log_check_call from bootstrap import Bootstrap -import subprocess -import os import logging log = logging.getLogger(__name__) @@ -16,8 +14,7 @@ class FormatVolume(Task): def run(self, info): dev_path = info.bootstrap_device['path'] mkfs = '/sbin/mkfs.{fs}'.format(fs=info.manifest.volume['filesystem']) - with open(os.devnull, 'w') as dev_null: - subprocess.check_call([mkfs, dev_path], stdout=dev_null, stderr=dev_null) + log_check_call([mkfs, dev_path], log) class TuneVolumeFS(Task): @@ -28,8 +25,8 @@ class TuneVolumeFS(Task): def run(self, info): dev_path = info.bootstrap_device['path'] # Disable the time based filesystem check - with open(os.devnull, 'w') as dev_null: - subprocess.check_call(['/sbin/tune2fs', '-i', '0', dev_path], stdout=dev_null, stderr=dev_null) + command = ['/sbin/tune2fs', '-i', '0', dev_path] + log_check_call(command, log) class AddXFSProgs(Task): @@ -46,6 +43,7 @@ class CreateMountDir(Task): phase = phases.volume_mounting def run(self, info): + import os mount_dir = info.manifest.bootstrapper['mount_dir'] info.root = '{mount_dir}/{vol_id}'.format(mount_dir=mount_dir, vol_id=info.volume.id) # Works recursively, fails if last part exists, which is exaclty what we want. @@ -64,9 +62,8 @@ class MountVolume(Task): msg = 'Something is already mounted at {root}'.format(root=info.root) raise TaskError(msg) - dev_path = info.bootstrap_device['path'] - with open(os.devnull, 'w') as dev_null: - subprocess.check_call(['mount', dev_path, info.root], stdout=dev_null, stderr=dev_null) + command = ['mount', info.bootstrap_device['path'], info.root] + log_check_call(command, log) class MountSpecials(Task): @@ -75,14 +72,10 @@ class MountSpecials(Task): after = [Bootstrap] def run(self, info): - if log_command(['mount', '--bind', '/dev', '{root}/dev'.format(root=info.root)], log) != 0: - raise TaskError('Failed to bind /dev to {root}/dev'.format(root=info.root)) - if log_command(['chroot', info.root, 'mount', '-t', 'proc', 'none', '/proc'], log) != 0: - raise TaskError('Failed to mount /proc') - if log_command(['chroot', info.root, 'mount', '-t', 'sysfs', 'none', '/sys'], log) != 0: - raise TaskError('Failed to mount /sys') - if log_command(['chroot', info.root, 'mount', '-t', 'devpts', 'none', '/dev/pts'], log) != 0: - raise TaskError('Failed to mount /dev/pts') + log_check_call(['mount', '--bind', '/dev', '{root}/dev'.format(root=info.root)], log) + log_check_call(['chroot', info.root, 'mount', '-t', 'proc', 'none', '/proc'], log) + log_check_call(['chroot', info.root, 'mount', '-t', 'sysfs', 'none', '/sys'], log) + log_check_call(['chroot', info.root, 'mount', '-t', 'devpts', 'none', '/dev/pts'], log) class UnmountSpecials(Task): @@ -90,14 +83,10 @@ class UnmountSpecials(Task): phase = phases.volume_unmounting def run(self, info): - if log_command(['chroot', info.root, 'umount', '/dev/pts'], log) != 0: - raise TaskError('Failed to unmount /dev/pts') - if log_command(['chroot', info.root, 'umount', '/sys'], log) != 0: - raise TaskError('Failed to unmount /sys') - if log_command(['chroot', info.root, 'umount', '/proc'], log) != 0: - raise TaskError('Failed to unmount /proc') - if log_command(['umount', '{root}/dev'.format(root=info.root)], log) != 0: - raise TaskError('Failed to unmount /dev from {root}/dev'.format(root=info.root)) + log_check_call(['chroot', info.root, 'umount', '/dev/pts'], log) + log_check_call(['chroot', info.root, 'umount', '/sys'], log) + log_check_call(['chroot', info.root, 'umount', '/proc'], log) + log_check_call(['umount', '{root}/dev'.format(root=info.root)], log) class UnmountVolume(Task): @@ -106,8 +95,7 @@ class UnmountVolume(Task): after = [UnmountSpecials] def run(self, info): - with open(os.devnull, 'w') as dev_null: - subprocess.check_call(['umount', info.root], stdout=dev_null, stderr=dev_null) + log_check_call(['umount', info.root], log) class DeleteMountDir(Task): @@ -116,5 +104,6 @@ class DeleteMountDir(Task): after = [UnmountVolume] def run(self, info): + import os os.rmdir(info.root) del info.root diff --git a/providers/ec2/tasks/host.py b/providers/ec2/tasks/host.py index 84fadd0..b0b7828 100644 --- a/providers/ec2/tasks/host.py +++ b/providers/ec2/tasks/host.py @@ -1,7 +1,10 @@ from base import Task from common import phases from common.exceptions import TaskError +from common.tools import log_check_call import packages +import logging +log = logging.getLogger(__name__) class CheckPackages(Task): @@ -11,11 +14,9 @@ class CheckPackages(Task): def run(self, info): import subprocess - from os import devnull for package in info.host_packages: try: - with open(devnull, 'w') as dev_null: - subprocess.check_call(['/usr/bin/dpkg', '-s', package], stdout=dev_null, stderr=dev_null) + log_check_call(['/usr/bin/dpkg', '-s', package], log) except subprocess.CalledProcessError: msg = "The package ``{0}\'\' is not installed".format(package) raise TaskError(msg)