Proposing merge to get feedback on orm refactoring. I am very interested in feedback to all of these changes.

This is a huge set of changes, that touches almost all of the files.  I'm sure I have broken quite a bit, but better to take the plunge now than to postpone this until later.  The idea is to allow for pluggable backends throughout the code.

Brief Overview
For compute/volume/network, there are multiple classes
service - responsible for rpc
  this currently uses the existing cast and call in rpc.py and a little bit of magic
  to call public methods on the manager class.
  each service also reports its state into the database every 10 seconds
manager - responsible for managing respective object classes
  all the business logic for the classes go here
db (db_driver) - responsible for abstracting database access
driver (domain_driver) - responsible for executing actual shell commands and implementation

Compute hasn't been fully cleaned up, but to get an idea of how it works, take a look
at volume and network

Known issues/Things to be done:

* nova-api accesses db objects directly
  It seems cleaner to have only the managers dealing with their respective objects.  This would
  mean code for 'run_instances' would move into the manager class and it would do the initial
  setup and cast out to the remote service

* db code uses flat methods to define its interface
  In my mind this is a little prettier as an abstract base class, but driver loading code
  can load a module or a class.  It works, so I'm not sure it needs to be changed but feel
  free to debate it.

* Service classes have no code in them
  Not sure if this is a problem for people, but the magic of calling the manager's methods is
  done in the base class.  We could remove the magic from the base class and explicitly
  wrap methods that we want to make available via rpc if this seems nasty.

* AuthManager Projects/Users/Roles are not integrated into this system.
  In order for everything to live happily in the backend, we need some type
  of adaptor for LDAP

* Context is not passed properly across rabbit
  Context should probably be changed to a simple dictionary so that it can be
  passed properly through the queue

* No authorization checks on access to objects
  We need to decide on which layer auth checks should happen.

* Some of the methods in ComputeManager need to be moved into other layers/managers
* Compute driver layer should be abstracted more cleanly
* Flat networking is untested and may need to be reworked
* Some of the api commands are not working yet
* Nova Swift Authentication needs to be refactored(Todd is working on this)

1 files changed, 47 insertions, 48 deletions

diff --git a/nova/process.py b/nova/process.py
index 425d9f162..74725c157 100644
--- a/nova/process.py
+++ b/nova/process.py
@@ -18,9 +18,10 @@
 #    under the License.
 
 """
-Process pool, still buggy right now.
+Process pool using twisted threading
 """
 
+import logging
 import StringIO
 
 from twisted.internet import defer
@@ -29,30 +30,14 @@ from twisted.internet import protocol
 from twisted.internet import reactor
 
 from nova import flags
+from nova.utils import ProcessExecutionError
 
 FLAGS = flags.FLAGS
 flags.DEFINE_integer('process_pool_size', 4,
                      'Number of processes to use in the process pool')
 
-
-# NOTE(termie): this is copied from twisted.internet.utils but since
-#               they don't export it I've copied and modified
-class UnexpectedErrorOutput(IOError):
-    """
-    Standard error data was received where it was not expected.  This is a
-    subclass of L{IOError} to preserve backward compatibility with the previous
-    error behavior of L{getProcessOutput}.
-
-    @ivar processEnded: A L{Deferred} which will fire when the process which
-        produced the data on stderr has ended (exited and all file descriptors
-        closed).
-    """
-    def __init__(self, stdout=None, stderr=None):
-        IOError.__init__(self, "got stdout: %r\nstderr: %r" % (stdout, stderr))
-
-
-# This is based on _BackRelay from twister.internal.utils, but modified to 
-#  capture both stdout and stderr, without odd stderr handling, and also to 
+# 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):
     """
@@ -62,22 +47,23 @@ class BackRelayWithInput(protocol.ProcessProtocol):
     @ivar deferred: A L{Deferred} which will be called back with all of stdout
         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 
+        L{_ProcessExecutionError} instance and the attribute will be set to
         C{None}.
 
-    @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 
+    @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{_ProcessExecutionError} 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, started_deferred=None, 
-                    terminate_on_stderr=False, check_exit_code=True, 
-                    process_input=None):
+    def __init__(self, deferred, cmd, started_deferred=None,
+                 terminate_on_stderr=False, check_exit_code=True,
+                 process_input=None):
         self.deferred = deferred
+        self.cmd = cmd
         self.stdout = StringIO.StringIO()
         self.stderr = StringIO.StringIO()
         self.started_deferred = started_deferred
@@ -85,14 +71,18 @@ class BackRelayWithInput(protocol.ProcessProtocol):
         self.check_exit_code = check_exit_code
         self.process_input = process_input
         self.on_process_ended = None
-    
+
+    def _build_execution_error(self, exit_code=None):
+        return ProcessExecutionError(cmd=self.cmd,
+                                     exit_code=exit_code,
+                                     stdout=self.stdout.getvalue(),
+                                     stderr=self.stderr.getvalue())
+
     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.errback(self._build_execution_error())
             self.deferred = None
             self.transport.loseConnection()
 
@@ -102,15 +92,19 @@ class BackRelayWithInput(protocol.ProcessProtocol):
     def processEnded(self, reason):
         if self.deferred is not None:
             stdout, stderr = self.stdout.getvalue(), self.stderr.getvalue()
-            try:
-                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))
+            exit_code = reason.value.exitCode
+            if self.check_exit_code and exit_code <> 0:
+                self.deferred.errback(self._build_execution_error(exit_code))
+            else:
+                try:
+                    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(self._build_execution_error(exit_code))
         elif self.on_process_ended is not None:
             self.on_process_ended.errback(reason)
 
@@ -122,8 +116,8 @@ class BackRelayWithInput(protocol.ProcessProtocol):
             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, 
+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:
@@ -131,10 +125,15 @@ def get_process_output(executable, args=None, env=None, path=None,
     args = args and args or ()
     env = env and env and {}
     deferred = defer.Deferred()
+    cmd = executable
+    if args:
+        cmd = cmd + " " + ' '.join(args)
+    logging.debug("Running cmd: %s", cmd)
     process_handler = BackRelayWithInput(
-            deferred, 
-            started_deferred=started_deferred, 
-            check_exit_code=check_exit_code, 
+            deferred,
+            cmd,
+            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
@@ -142,7 +141,7 @@ def get_process_output(executable, args=None, env=None, path=None,
     executable = str(executable)
     if not args is None:
         args = [str(x) for x in args]
-    process_reactor.spawnProcess(   process_handler, executable, 
+    process_reactor.spawnProcess(   process_handler, executable,
                                     (executable,)+tuple(args), env, path)
     return deferred