use log_check_call everywhere

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):

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)

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):

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

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)