Introduce some awesome signal handling

We can now press Ctrl+C remotely  while any subprocess of the bootstrapping process is running,
previously SIGINTs weren't propagated to the bootstrapping process because there was a thread
in between it all. Now the bootstrapping process is in it's own process group.

bootstrapvz/base/main.py

@@ -83,7 +83,7 @@ def setup_loggers(opts):
 	root.addHandler(console_handler)
 
 
-def run(manifest, debug=False, pause_on_error=False, dry_run=False, check_continue=None):
+def run(manifest, debug=False, pause_on_error=False, dry_run=False):
 	"""Runs the bootstrapping process
 
 	:params Manifest manifest: The manifest to run the bootstrapping process for
@@ -106,7 +106,7 @@ def run(manifest, debug=False, pause_on_error=False, dry_run=False, check_contin
 	log = logging.getLogger(__name__)
 	try:
 		# Run all the tasks the tasklist has gathered
-		tasklist.run(info=bootstrap_info, dry_run=dry_run, check_continue=check_continue)
+		tasklist.run(info=bootstrap_info, dry_run=dry_run)
 		# We're done! :-)
 		log.info('Successfully completed bootstrapping')
 	except (Exception, KeyboardInterrupt) as e:

bootstrapvz/base/tasklist.py

@@ -15,7 +15,7 @@ class TaskList(object):
 		self.tasks = tasks
 		self.tasks_completed = []
 
-	def run(self, info, dry_run=False, check_continue=None):
+	def run(self, info, dry_run=False):
 		"""Converts the taskgraph into a list and runs all tasks in that list
 
 		:param dict info: The bootstrap information object
@@ -27,9 +27,6 @@ class TaskList(object):
 		log.debug('Tasklist:\n\t' + ('\n\t'.join(map(repr, task_list))))
 
 		for task in task_list:
-			# Check if we should abort the run (used for asynchronous run abortion through remote building)
-			if callable(check_continue) and not check_continue():
-				raise TaskListError('Run was aborted.')
 			# Tasks are not required to have a description
 			if hasattr(task, 'description'):
 				log.info(task.description)

bootstrapvz/remote/build_servers/callback.py

@@ -12,7 +12,6 @@ class CallbackServer(object):
 		                           nathost='localhost', natport=remote_port,
 		                           unixsocket=None)
 		self.daemon.register(self)
-		self.abort = False
 
 	def __enter__(self):
 		def serve():
@@ -36,10 +35,3 @@ class CallbackServer(object):
 		record.extra = getattr(record, 'extra', {})
 		record.extra['source'] = 'remote'
 		log.handle(record)
-
-	@Pyro4.expose
-	def get_abort_run(self):
-		return self.abort
-
-	def abort_run(self):
-		self.abort = True

bootstrapvz/remote/build_servers/local.py

@@ -1,8 +1,16 @@
 from build_server import BuildServer
+from contextlib import contextmanager
 
 
 class LocalBuildServer(BuildServer):
 
-	def run(self, manifest):
+	@contextmanager
+	def connect(self):
+		yield LocalConnection()
+
+
+class LocalConnection(object):
+
+	def run(self, *args, **kwargs):
 		from bootstrapvz.base.main import run
-		return run(manifest)
+		return run(*args, **kwargs)

bootstrapvz/remote/build_servers/remote.py

@@ -25,7 +25,7 @@ class RemoteBuildServer(BuildServer):
 				        'remote_port': forwards['remote_callback_port']}
 				with CallbackServer(**args) as callback_server:
 					connection.set_callback_server(callback_server)
-					yield (connection, callback_server)
+					yield connection
 
 	@contextmanager
 	def spawn_server(self):
@@ -96,10 +96,6 @@ class RemoteBuildServer(BuildServer):
 		                  '--'] + command
 		log_check_call(ssh_cmd)
 
-	def run(self, manifest):
-		from bootstrapvz.remote.main import run
-		return run(manifest, self)
-
 
 @contextmanager
 def connect_pyro(host, port):

bootstrapvz/remote/main.py

@@ -40,10 +40,10 @@ def main():
 	register_deserialization_handlers()
 
 	# Everything has been set up, connect to the server and begin the bootstrapping process
-	run(manifest,
-	    build_server,
-	    debug=opts['--debug'],
-	    dry_run=opts['--dry-run'])
+	with build_server.connect() as connection:
+		connection.run(manifest,
+		               debug=opts['--debug'],
+		               dry_run=['--dry-run'])
 
 
 def get_opts():
@@ -68,29 +68,3 @@ Options:
   -h, --help          show this help
 	"""
 	return docopt(usage)
-
-
-def run(manifest, build_server, debug=False, dry_run=False):
-	"""Connects to the remote build server, starts an RPC daemin
-	on the other side and initiates a remote bootstrapping procedure
-	"""
-	bootstrap_info = None
-	with build_server.connect() as (connection, callback_server):
-		# Replace the standard SIGINT handler with a remote call to the server
-		# so that it may abort the run.
-		def abort(signum, frame):
-			import logging
-			logging.getLogger(__name__).warn('SIGINT received, asking remote to abort.')
-			callback_server.abort_run()
-		import signal
-		orig_sigint = signal.signal(signal.SIGINT, abort)
-
-		# Everything has been set up, begin the bootstrapping process
-		bootstrap_info = connection.run(manifest,
-		                                debug=debug,
-		                                # We can't pause the bootstrapping process remotely, yet...
-		                                pause_on_error=False,
-		                                dry_run=dry_run)
-		# Restore the old SIGINT handler
-		signal.signal(signal.SIGINT, orig_sigint)
-	return bootstrap_info

bootstrapvz/remote/server.py

@@ -57,18 +57,51 @@ class Server(object):
 	def start(self):
 		Pyro4.config.COMMTIMEOUT = 0.5
 		daemon = Pyro4.Daemon('localhost', port=int(self.listen_port), unixsocket=None)
-
 		daemon.register(self, 'server')
+
 		daemon.requestLoop(loopCondition=lambda: not self.stop_serving)
 
 	@Pyro4.expose
-	def run(self, *args, **kwargs):
+	def run(self, manifest, debug=False, dry_run=False):
 
-		def abort_run():
-			return not self.callback_server.get_abort_run()
-		from bootstrapvz.base.main import run
-		kwargs['check_continue'] = abort_run
-		return run(*args, **kwargs)
+		def bootstrap(queue):
+			# setsid() creates a new session, making this process the group leader.
+			# We do that, so when the server calls killpg (kill process group)
+			# on us, it won't kill itself (this process was spawned from a
+			# thread under the server, meaning it's part of the same group).
+			# The process hierarchy looks like this:
+			# Pyro server (process - listening on a port)
+			# +- pool thread
+			# +- pool thread
+			# +- pool thread
+			# +- started thread (the one that got the "run()" call)
+			#    L bootstrap() process (us)
+			# Calling setsid() also fixes another problem:
+			#   SIGINTs sent to this process seem to be redirected
+			#   to the process leader. Since there is a thread between
+			#   us and the process leader, the signal will not be propagated
+			#   (signals are not propagated to threads), this means that any
+			#   subprocess we start (i.e. debootstrap) will not get a SIGINT.
+			import os
+			os.setsid()
+			from bootstrapvz.base.main import run
+			try:
+				bootstrap_info = run(manifest, debug=debug, dry_run=dry_run)
+				queue.put(bootstrap_info)
+			except (Exception, KeyboardInterrupt) as e:
+				queue.put(e)
+
+		from multiprocessing import Queue
+		from multiprocessing import Process
+		queue = Queue()
+		self.bootstrap_process = Process(target=bootstrap, args=(queue,))
+		self.bootstrap_process.start()
+		self.bootstrap_process.join()
+		del self.bootstrap_process
+		result = queue.get()
+		if isinstance(result, Exception):
+			raise result
+		return result
 
 	@Pyro4.expose
 	def set_callback_server(self, server):
@@ -82,4 +115,14 @@ class Server(object):
 
 	@Pyro4.expose
 	def stop(self):
+		if hasattr(self, 'bootstrap_process'):
+			log.warn('Sending SIGINT to bootstrapping process')
+			import os
+			import signal
+			os.killpg(self.bootstrap_process.pid, signal.SIGINT)
+			self.bootstrap_process.join()
+
+		# We can't send a SIGINT to the server,
+		# for some reason the Pyro4 shutdowns are rather unclean,
+		# throwing exceptions and such.
 		self.stop_serving = True

tests/integration/tools/__init__.py

@@ -17,7 +17,9 @@ def boot_manifest(manifest_data):
 	from bootstrapvz.base.manifest import Manifest
 	manifest = Manifest(data=manifest_data)
 
-	bootstrap_info = build_server.run(manifest)
+	bootstrap_info = None
+	with build_server.connect() as connection:
+		bootstrap_info = connection.run(manifest)
 
 	from ..images import initialize_image
 	image = initialize_image(manifest, build_server, bootstrap_info)