diff --git a/bin/nova-import-canonical-imagestore b/bin/nova-import-canonical-imagestore index e6931d9d..2bc61cf0 100755 --- a/bin/nova-import-canonical-imagestore +++ b/bin/nova-import-canonical-imagestore @@ -56,21 +56,21 @@ def download(img): for f in img['files']: if f['kind'] == 'kernel': dest = os.path.join(tempdir, 'kernel') - subprocess.call(['curl', f['url'], '-o', dest]) + subprocess.call(['curl', '--fail', f['url'], '-o', dest]) kernel_id = image.Image.add(dest, description='kernel/' + img['title'], kernel=True) for f in img['files']: if f['kind'] == 'ramdisk': dest = os.path.join(tempdir, 'ramdisk') - subprocess.call(['curl', f['url'], '-o', dest]) + subprocess.call(['curl', '--fail', f['url'], '-o', dest]) ramdisk_id = image.Image.add(dest, description='ramdisk/' + img['title'], ramdisk=True) for f in img['files']: if f['kind'] == 'image': dest = os.path.join(tempdir, 'image') - subprocess.call(['curl', f['url'], '-o', dest]) + subprocess.call(['curl', '--fail', f['url'], '-o', dest]) ramdisk_id = image.Image.add(dest, description=img['title'], kernel=kernel_id, ramdisk=ramdisk_id) diff --git a/bin/nova-manage b/bin/nova-manage index 33141a49..145294d3 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -56,7 +56,8 @@ class VpnCommands(object): vpn = self._vpn_for(project.id) if vpn: command = "ping -c1 -w1 %s > /dev/null; echo $?" - out, _err = utils.execute(command % vpn['private_dns_name']) + out, _err = utils.execute( command % vpn['private_dns_name'], + check_exit_code=False) if out.strip() == '0': net = 'up' else: diff --git a/nova/process.py b/nova/process.py index 86f29e2c..425d9f16 100644 --- a/nova/process.py +++ b/nova/process.py @@ -2,6 +2,7 @@ # Copyright 2010 United States Government as represented by the # Administrator of the National Aeronautics and Space Administration. +# Copyright 2010 FathomDB Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -20,17 +21,12 @@ Process pool, still buggy right now. """ -import logging -import multiprocessing import StringIO from twisted.internet import defer from twisted.internet import error -from twisted.internet import process from twisted.internet import protocol from twisted.internet import reactor -from twisted.internet import threads -from twisted.python import failure from nova import flags @@ -55,111 +51,100 @@ class UnexpectedErrorOutput(IOError): IOError.__init__(self, "got stdout: %r\nstderr: %r" % (stdout, stderr)) -# NOTE(termie): this too -class _BackRelay(protocol.ProcessProtocol): +# This is based on _BackRelay from twister.internal.utils, but modified to +# capture both stdout and stderr, without odd stderr handling, and also to +# handle stdin +class BackRelayWithInput(protocol.ProcessProtocol): """ Trivial protocol for communicating with a process and turning its output into the result of a L{Deferred}. @ivar deferred: A L{Deferred} which will be called back with all of stdout - and, if C{errortoo} is true, all of stderr as well (mixed together in - one string). If C{errortoo} is false and any bytes are received over - stderr, this will fire with an L{_UnexpectedErrorOutput} instance and - the attribute will be set to C{None}. + and all of stderr as well (as a tuple). C{terminate_on_stderr} is true + and any bytes are received over stderr, this will fire with an + L{_UnexpectedErrorOutput} instance and the attribute will be set to + C{None}. - @ivar onProcessEnded: If C{errortoo} is false and bytes are received over - stderr, this attribute will refer to a L{Deferred} which will be called - back when the process ends. This C{Deferred} is also associated with - the L{_UnexpectedErrorOutput} which C{deferred} fires with earlier in - this case so that users can determine when the process has actually - ended, in addition to knowing when bytes have been received via stderr. + @ivar onProcessEnded: If C{terminate_on_stderr} is false and bytes are + received over stderr, this attribute will refer to a L{Deferred} which + will be called back when the process ends. This C{Deferred} is also + associated with the L{_UnexpectedErrorOutput} which C{deferred} fires + with earlier in this case so that users can determine when the process + has actually ended, in addition to knowing when bytes have been received + via stderr. """ - def __init__(self, deferred, errortoo=0): + def __init__(self, deferred, started_deferred=None, + terminate_on_stderr=False, check_exit_code=True, + process_input=None): self.deferred = deferred - self.s = StringIO.StringIO() - if errortoo: - self.errReceived = self.errReceivedIsGood - else: - self.errReceived = self.errReceivedIsBad - - def errReceivedIsBad(self, text): - if self.deferred is not None: - self.onProcessEnded = defer.Deferred() - err = UnexpectedErrorOutput(text, self.onProcessEnded) - self.deferred.errback(failure.Failure(err)) + self.stdout = StringIO.StringIO() + self.stderr = StringIO.StringIO() + self.started_deferred = started_deferred + self.terminate_on_stderr = terminate_on_stderr + self.check_exit_code = check_exit_code + self.process_input = process_input + self.on_process_ended = None + + def errReceived(self, text): + self.stderr.write(text) + if self.terminate_on_stderr and (self.deferred is not None): + self.on_process_ended = defer.Deferred() + self.deferred.errback(UnexpectedErrorOutput( + stdout=self.stdout.getvalue(), + stderr=self.stderr.getvalue())) self.deferred = None self.transport.loseConnection() - def errReceivedIsGood(self, text): - self.s.write(text) - def outReceived(self, text): - self.s.write(text) + self.stdout.write(text) def processEnded(self, reason): if self.deferred is not None: - self.deferred.callback(self.s.getvalue()) - elif self.onProcessEnded is not None: - self.onProcessEnded.errback(reason) - - -class BackRelayWithInput(_BackRelay): - def __init__(self, deferred, startedDeferred=None, error_ok=0, - input=None): - # Twisted doesn't use new-style classes in most places :( - _BackRelay.__init__(self, deferred, errortoo=error_ok) - self.error_ok = error_ok - self.input = input - self.stderr = StringIO.StringIO() - self.startedDeferred = startedDeferred - - def errReceivedIsBad(self, text): - self.stderr.write(text) - self.transport.loseConnection() - - def errReceivedIsGood(self, text): - self.stderr.write(text) - - def connectionMade(self): - if self.startedDeferred: - self.startedDeferred.callback(self) - if self.input: - self.transport.write(self.input) - self.transport.closeStdin() - - def processEnded(self, reason): - if self.deferred is not None: - stdout, stderr = self.s.getvalue(), self.stderr.getvalue() + stdout, stderr = self.stdout.getvalue(), self.stderr.getvalue() try: - # NOTE(termie): current behavior means if error_ok is True - # we won't throw an error even if the process - # exited with a non-0 status, so you can't be - # okay with stderr output and not with bad exit - # codes. - if not self.error_ok: + if self.check_exit_code: reason.trap(error.ProcessDone) self.deferred.callback((stdout, stderr)) except: + # NOTE(justinsb): This logic is a little suspicious to me... + # If the callback throws an exception, then errback will be + # called also. However, this is what the unit tests test for... self.deferred.errback(UnexpectedErrorOutput(stdout, stderr)) + elif self.on_process_ended is not None: + self.on_process_ended.errback(reason) -def getProcessOutput(executable, args=None, env=None, path=None, reactor=None, - error_ok=0, input=None, startedDeferred=None): - if reactor is None: - from twisted.internet import reactor + def connectionMade(self): + if self.started_deferred: + self.started_deferred.callback(self) + if self.process_input: + self.transport.write(self.process_input) + self.transport.closeStdin() + +def get_process_output(executable, args=None, env=None, path=None, + process_reactor=None, check_exit_code=True, + process_input=None, started_deferred=None, + terminate_on_stderr=False): + if process_reactor is None: + process_reactor = reactor args = args and args or () env = env and env and {} - d = defer.Deferred() - p = BackRelayWithInput( - d, startedDeferred=startedDeferred, error_ok=error_ok, input=input) + deferred = defer.Deferred() + process_handler = BackRelayWithInput( + deferred, + started_deferred=started_deferred, + check_exit_code=check_exit_code, + process_input=process_input, + terminate_on_stderr=terminate_on_stderr) # NOTE(vish): commands come in as unicode, but self.executes needs # strings or process.spawn raises a deprecation warning executable = str(executable) if not args is None: args = [str(x) for x in args] - reactor.spawnProcess(p, executable, (executable,)+tuple(args), env, path) - return d + process_reactor.spawnProcess( process_handler, executable, + (executable,)+tuple(args), env, path) + return deferred class ProcessPool(object): @@ -185,26 +170,26 @@ class ProcessPool(object): return self.execute(executable, args, **kw) def execute(self, *args, **kw): - d = self._pool.acquire() + deferred = self._pool.acquire() - def _associateProcess(proto): - d.process = proto.transport + def _associate_process(proto): + deferred.process = proto.transport return proto.transport started = defer.Deferred() - started.addCallback(_associateProcess) - kw.setdefault('startedDeferred', started) + started.addCallback(_associate_process) + kw.setdefault('started_deferred', started) - d.process = None - d.started = started + deferred.process = None + deferred.started = started - d.addCallback(lambda _: getProcessOutput(*args, **kw)) - d.addBoth(self._release) - return d + deferred.addCallback(lambda _: get_process_output(*args, **kw)) + deferred.addBoth(self._release) + return deferred - def _release(self, rv=None): + def _release(self, retval=None): self._pool.release() - return rv + return retval class SharedPool(object): diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 993bfacc..34b68f1e 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -166,7 +166,6 @@ class NetworkTestCase(test.TrialTestCase): release_ip(mac3, address3, hostname, net.bridge_name) net = model.get_project_network(self.projects[0].id, "default") self.service.deallocate_fixed_ip(firstaddress) - release_ip(mac, firstaddress, hostname, net.bridge_name) def test_vpn_ip_and_port_looks_valid(self): """Ensure the vpn ip and port are reasonable""" diff --git a/nova/tests/process_unittest.py b/nova/tests/process_unittest.py index 75187e1f..25c60c61 100644 --- a/nova/tests/process_unittest.py +++ b/nova/tests/process_unittest.py @@ -48,7 +48,7 @@ class ProcessTestCase(test.TrialTestCase): def test_execute_stderr(self): pool = process.ProcessPool(2) - d = pool.simple_execute('cat BAD_FILE', error_ok=1) + d = pool.simple_execute('cat BAD_FILE', check_exit_code=False) def _check(rv): self.assertEqual(rv[0], '') self.assert_('No such file' in rv[1])