Merge "Allow conductor exceptions to pass over RPC silently"

3 files changed, 76 insertions, 24 deletions

diff --git a/nova/conductor/api.py b/nova/conductor/api.py
index b9013c4c6..397cd3fe6 100644
--- a/nova/conductor/api.py
+++ b/nova/conductor/api.py
@@ -14,10 +14,13 @@
 
 """Handles all requests to the conductor service"""
 
+import functools
+
 from nova.conductor import manager
 from nova.conductor import rpcapi
 from nova import exception as exc
 from nova.openstack.common import cfg
+from nova.openstack.common.rpc import common as rpc_common
 
 conductor_opts = [
     cfg.BoolOpt('use_local',
@@ -37,12 +40,33 @@ CONF.register_group(conductor_group)
 CONF.register_opts(conductor_opts, conductor_group)
 
 
+class ExceptionHelper(object):
+    """Class to wrap another and translate the ClientExceptions raised by its
+    function calls to the actual ones"""
+
+    def __init__(self, target):
+        self._target = target
+
+    def __getattr__(self, name):
+        func = getattr(self._target, name)
+
+        @functools.wraps(func)
+        def wrapper(*args, **kwargs):
+            try:
+                return func(*args, **kwargs)
+            except rpc_common.ClientException, e:
+                raise e._exc_info
+        return wrapper
+
+
 class LocalAPI(object):
     """A local version of the conductor API that does database updates
     locally instead of via RPC"""
 
     def __init__(self):
-        self._manager = manager.ConductorManager()
+        # TODO(danms): This needs to be something more generic for
+        # other/future users of this sort of functionality.
+        self._manager = ExceptionHelper(manager.ConductorManager())
 
     def instance_update(self, context, instance_uuid, **updates):
         """Perform an instance update in the database"""
diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 796b99360..b953e0704 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -14,10 +14,12 @@
 
 """Handles database requests from other nova services"""
 
+from nova import exception
 from nova import manager
 from nova import notifications
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
+from nova.openstack.common.rpc import common as rpc_common
 from nova.openstack.common import timeutils
 
 
@@ -47,6 +49,10 @@ class ConductorManager(manager.SchedulerDependentManager):
         super(ConductorManager, self).__init__(service_name='conductor',
                                                *args, **kwargs)
 
+    @rpc_common.client_exceptions(KeyError, ValueError,
+                                  exception.InvalidUUID,
+                                  exception.InstanceNotFound,
+                                  exception.UnexpectedTaskStateError)
     def instance_update(self, context, instance_uuid, updates):
         for key, value in updates.iteritems():
             if key not in allowed_updates:
@@ -61,6 +67,7 @@ class ConductorManager(manager.SchedulerDependentManager):
         notifications.send_update(context, old_ref, instance_ref)
         return jsonutils.to_primitive(instance_ref)
 
+    @rpc_common.client_exceptions(exception.InstanceNotFound)
     def instance_get_by_uuid(self, context, instance_uuid):
         return jsonutils.to_primitive(
             self.db.instance_get_by_uuid(context, instance_uuid))
@@ -69,23 +76,27 @@ class ConductorManager(manager.SchedulerDependentManager):
         return jsonutils.to_primitive(
             self.db.instance_get_all_by_host(context.elevated(), host))
 
+    @rpc_common.client_exceptions(exception.MigrationNotFound)
     def migration_get(self, context, migration_id):
         migration_ref = self.db.migration_get(context.elevated(),
                                               migration_id)
         return jsonutils.to_primitive(migration_ref)
 
+    @rpc_common.client_exceptions(exception.MigrationNotFound)
     def migration_update(self, context, migration, status):
         migration_ref = self.db.migration_update(context.elevated(),
                                                  migration['id'],
                                                  {'status': status})
         return jsonutils.to_primitive(migration_ref)
 
+    @rpc_common.client_exceptions(exception.AggregateHostExists)
     def aggregate_host_add(self, context, aggregate, host):
         host_ref = self.db.aggregate_host_add(context.elevated(),
                 aggregate['id'], host)
 
         return jsonutils.to_primitive(host_ref)
 
+    @rpc_common.client_exceptions(exception.AggregateHostNotFound)
     def aggregate_host_delete(self, context, aggregate, host):
         self.db.aggregate_host_delete(context.elevated(),
                 aggregate['id'], host)
diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py
index cace21984..e785fc54b 100644
--- a/nova/tests/conductor/test_conductor.py
+++ b/nova/tests/conductor/test_conductor.py
@@ -26,6 +26,7 @@ from nova.db.sqlalchemy import models
 from nova import exception as exc
 from nova import notifications
 from nova.openstack.common import jsonutils
+from nova.openstack.common.rpc import common as rpc_common
 from nova.openstack.common import timeutils
 from nova import test
 
@@ -33,14 +34,21 @@ from nova import test
 FAKE_IMAGE_REF = 'fake-image-ref'
 
 
-class BaseTestCase(test.TestCase):
+class _BaseTestCase(test.TestCase):
     def setUp(self):
-        super(BaseTestCase, self).setUp()
+        super(_BaseTestCase, self).setUp()
+        self.db = None
         self.user_id = 'fake'
         self.project_id = 'fake'
         self.context = context.RequestContext(self.user_id,
                                               self.project_id)
 
+    def stub_out_client_exceptions(self):
+        def passthru(exceptions, func, *args, **kwargs):
+            return func(*args, **kwargs)
+
+        self.stubs.Set(rpc_common, 'catch_client_exception', passthru)
+
     def _create_fake_instance(self, params=None, type_name='m1.tiny'):
         if not params:
             params = {}
@@ -65,14 +73,6 @@ class BaseTestCase(test.TestCase):
         inst.update(params)
         return db.instance_create(self.context, inst)
 
-
-class ConductorTestCase(BaseTestCase):
-    """Conductor Manager Tests"""
-    def setUp(self):
-        super(ConductorTestCase, self).setUp()
-        self.conductor = conductor_manager.ConductorManager()
-        self.db = None
-
     def _do_update(self, instance_uuid, **updates):
         return self.conductor.instance_update(self.context, instance_uuid,
                                               updates)
@@ -194,7 +194,15 @@ class ConductorTestCase(BaseTestCase):
         self.assertEqual(port, backdoor_port)
 
 
-class ConductorRPCAPITestCase(ConductorTestCase):
+class ConductorTestCase(_BaseTestCase):
+    """Conductor Manager Tests"""
+    def setUp(self):
+        super(ConductorTestCase, self).setUp()
+        self.conductor = conductor_manager.ConductorManager()
+        self.stub_out_client_exceptions()
+
+
+class ConductorRPCAPITestCase(_BaseTestCase):
     """Conductor RPC API Tests"""
     def setUp(self):
         super(ConductorRPCAPITestCase, self).setUp()
@@ -203,12 +211,14 @@ class ConductorRPCAPITestCase(ConductorTestCase):
         self.conductor = conductor_rpcapi.ConductorAPI()
 
 
-class ConductorLocalAPITestCase(ConductorTestCase):
-    """Conductor LocalAPI Tests"""
+class ConductorAPITestCase(_BaseTestCase):
+    """Conductor API Tests"""
     def setUp(self):
-        super(ConductorLocalAPITestCase, self).setUp()
-        self.conductor = conductor_api.LocalAPI()
-        self.db = db
+        super(ConductorAPITestCase, self).setUp()
+        self.conductor_service = self.start_service(
+            'conductor', manager='nova.conductor.manager.ConductorManager')
+        self.conductor = conductor_api.API()
+        self.db = None
 
     def _do_update(self, instance_uuid, **updates):
         # NOTE(danms): the public API takes actual keyword arguments,
@@ -229,14 +239,21 @@ class ConductorLocalAPITestCase(ConductorTestCase):
         self.assertEqual(result, 'foo')
 
 
-class ConductorAPITestCase(ConductorLocalAPITestCase):
-    """Conductor API Tests"""
+class ConductorLocalAPITestCase(ConductorAPITestCase):
+    """Conductor LocalAPI Tests"""
     def setUp(self):
-        super(ConductorAPITestCase, self).setUp()
-        self.conductor_service = self.start_service(
-            'conductor', manager='nova.conductor.manager.ConductorManager')
-        self.conductor = conductor_api.API()
-        self.db = None
+        super(ConductorLocalAPITestCase, self).setUp()
+        self.conductor = conductor_api.LocalAPI()
+        self.db = db
+        self.stub_out_client_exceptions()
+
+    def test_client_exceptions(self):
+        instance = self._create_fake_instance()
+        # NOTE(danms): The LocalAPI should not raise exceptions wrapped
+        # in ClientException. KeyError should be raised if an invalid
+        # update key is passed, so use that to validate.
+        self.assertRaises(KeyError,
+                          self._do_update, instance['uuid'], foo='bar')
 
 
 class ConductorImportTest(test.TestCase):