Don't use rpc to lock/unlock an instance.

Instead of converting this method to send a full instance over rpc
instead of just an instance UUID, this patch removes the usage of rpc
for this operation entirely.  All it's doing is a database update.  RPC
is expensive, so cut out the middle-man.

One functional difference with this approach is that the db update is
now synchronous on the API node, instead of kicking off an async message
to a compute node to handle it.  This seems fine, though.

Part of blueprint no-db-messaging.

Change-Id: I15ceb7625425ab097eebd5b7dd3606a171329f97

5 files changed, 32 insertions, 24 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 7cdb8d167..0571c7313 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -130,6 +130,15 @@ class API(base.Base):
         self.compute_rpcapi = compute_rpcapi.ComputeAPI()
         super(API, self).__init__(**kwargs)
 
+    def _instance_update(self, context, instance_uuid, **kwargs):
+        """Update an instance in the database using kwargs as value."""
+
+        (old_ref, instance_ref) = self.db.instance_update_and_get_original(
+                context, instance_uuid, kwargs)
+        notifications.send_update(context, old_ref, instance_ref)
+
+        return instance_ref
+
     def _check_injected_file_quota(self, context, injected_files):
         """Enforce quota limits on injected files.
 
@@ -1531,12 +1540,18 @@ class API(base.Base):
     @wrap_check_policy
     def lock(self, context, instance):
         """Lock the given instance."""
-        self.compute_rpcapi.lock_instance(context, instance=instance)
+        context = context.elevated()
+        instance_uuid = instance['uuid']
+        LOG.debug(_('Locking'), context=context, instance_uuid=instance_uuid)
+        self._instance_update(context, instance_uuid, locked=True)
 
     @wrap_check_policy
     def unlock(self, context, instance):
         """Unlock the given instance."""
-        self.compute_rpcapi.unlock_instance(context, instance=instance)
+        context = context.elevated()
+        instance_uuid = instance['uuid']
+        LOG.debug(_('Unlocking'), context=context, instance_uuid=instance_uuid)
+        self._instance_update(context, instance_uuid, locked=False)
 
     @wrap_check_policy
     def get_lock(self, context, instance):
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 067bc584b..cced8509b 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1821,7 +1821,12 @@ class ComputeManager(manager.SchedulerDependentManager):
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @wrap_instance_fault
     def lock_instance(self, context, instance_uuid):
-        """Lock the given instance."""
+        """Lock the given instance.
+
+        This isn't actually used in the current code.  The same thing is now
+        done directly in nova.compute.api.  This must stay here for backwards
+        compatibility of the rpc API.
+        """
         context = context.elevated()
 
         LOG.debug(_('Locking'), context=context, instance_uuid=instance_uuid)
@@ -1830,7 +1835,12 @@ class ComputeManager(manager.SchedulerDependentManager):
     @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id())
     @wrap_instance_fault
     def unlock_instance(self, context, instance_uuid):
-        """Unlock the given instance."""
+        """Unlock the given instance.
+
+        This isn't actually used in the current code.  The same thing is now
+        done directly in nova.compute.api.  This must stay here for backwards
+        compatibility of the rpc API.
+        """
         context = context.elevated()
 
         LOG.debug(_('Unlocking'), context=context, instance_uuid=instance_uuid)
diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py
index 296ee476c..e480ee20d 100644
--- a/nova/compute/rpcapi.py
+++ b/nova/compute/rpcapi.py
@@ -203,11 +203,6 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
                 instance_uuid=instance['uuid']),
                 topic=_compute_topic(self.topic, ctxt, None, instance))
 
-    def lock_instance(self, ctxt, instance):
-        self.cast(ctxt, self.make_msg('lock_instance',
-                instance_uuid=instance['uuid']),
-                topic=_compute_topic(self.topic, ctxt, None, instance))
-
     def post_live_migration_at_destination(self, ctxt, instance,
             block_migration, host):
         return self.call(ctxt,
@@ -365,11 +360,6 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
                 instance_uuid=instance['uuid']),
                 topic=_compute_topic(self.topic, ctxt, None, instance))
 
-    def unlock_instance(self, ctxt, instance):
-        self.cast(ctxt, self.make_msg('unlock_instance',
-                instance_uuid=instance['uuid']),
-                topic=_compute_topic(self.topic, ctxt, None, instance))
-
     def unpause_instance(self, ctxt, instance):
         self.cast(ctxt, self.make_msg('unpause_instance',
                 instance_uuid=instance['uuid']),
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 93eb644c6..c9110176d 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -213,6 +213,7 @@ class ComputeTestCase(BaseTestCase):
                        fake_get_nw_info)
         self.stubs.Set(nova.network.API, 'allocate_for_instance',
                        fake_get_nw_info)
+        self.compute_api = compute.API()
 
     def tearDown(self):
         super(ComputeTestCase, self).tearDown()
@@ -1038,13 +1039,13 @@ class ComputeTestCase(BaseTestCase):
                                                    is_admin=False)
 
         # decorator should return False (fail) with locked nonadmin context
-        self.compute.lock_instance(self.context, instance_uuid)
+        self.compute_api.lock(self.context, instance)
         ret_val = self.compute.reboot_instance(non_admin_context,
                 instance=jsonutils.to_primitive(instance))
         self.assertEqual(ret_val, False)
 
         # decorator should return None (success) with unlocked nonadmin context
-        self.compute.unlock_instance(self.context, instance_uuid)
+        self.compute_api.unlock(self.context, instance)
         ret_val = self.compute.reboot_instance(non_admin_context,
                 instance=jsonutils.to_primitive(instance))
         self.assertEqual(ret_val, None)
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index 080128f47..488119116 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -195,10 +195,6 @@ class ComputeRpcAPITestCase(test.TestCase):
         self._test_compute_api('inject_network_info', 'cast',
                 instance=self.fake_instance)
 
-    def test_lock_instance(self):
-        self._test_compute_api('lock_instance', 'cast',
-                instance=self.fake_instance)
-
     def test_post_live_migration_at_destination(self):
         self._test_compute_api('post_live_migration_at_destination', 'call',
                 instance=self.fake_instance, block_migration='block_migration',
@@ -318,10 +314,6 @@ class ComputeRpcAPITestCase(test.TestCase):
         self._test_compute_api('terminate_instance', 'cast',
                 instance=self.fake_instance)
 
-    def test_unlock_instance(self):
-        self._test_compute_api('unlock_instance', 'cast',
-                instance=self.fake_instance)
-
     def test_unpause_instance(self):
         self._test_compute_api('unpause_instance', 'cast',
                 instance=self.fake_instance)