No API cell up-call to delete consoleauth tokens

One goal of Cells v2 is to remove all up-calls from compute to API
cells.  This change removes the up-call to delete consoleauth tokens
during instance deletion and live migration.  Instead, the tokens are
deleted near the start of these processes in the API cell.  To close
the loop, new consoleauth tokens are not granted to instances in
deleting or migrating task states.

Change-Id: Ic7c68b75e22bad2b0237a2f861254611a86516f3

nova/compute/api.py

@@ -154,6 +154,39 @@ def check_instance_state(vm_state=None, task_state=(None,),
     return outer
 
 
+def _set_or_none(q):
+    return q if q is None or isinstance(q, set) else set(q)
+
+
+def reject_instance_state(vm_state=None, task_state=None):
+    """Decorator.  Raise InstanceInvalidState if instance is in any of the
+    given states.
+    """
+
+    vm_state = _set_or_none(vm_state)
+    task_state = _set_or_none(task_state)
+
+    def outer(f):
+        @six.wraps(f)
+        def inner(self, context, instance, *args, **kw):
+            _InstanceInvalidState = functools.partial(
+                exception.InstanceInvalidState,
+                instance_uuid=instance.uuid,
+                method=f.__name__)
+
+            if vm_state is not None and instance.vm_state in vm_state:
+                raise _InstanceInvalidState(
+                    attr='vm_state', state=instance.vm_state)
+
+            if task_state is not None and instance.task_state in task_state:
+                raise _InstanceInvalidState(
+                    attr='task_state', state=instance.task_state)
+
+            return f(self, context, instance, *args, **kw)
+        return inner
+    return outer
+
+
 def check_instance_host(function):
     @six.wraps(function)
     def wrapped(self, context, instance, *args, **kwargs):
@@ -1854,6 +1887,12 @@ class API(base.Base):
                                                original_task_state,
                                                project_id, user_id)
 
+            # NOTE(dtp): cells.enable = False means "use cells v2".
+            # Run everywhere except v1 compute cells.
+            if not CONF.cells.enable or self.cell_type == 'api':
+                self.consoleauth_rpcapi.delete_tokens_for_instance(
+                    context, instance.uuid)
+
             if self.cell_type == 'api':
                 # NOTE(comstud): If we're in the API cell, we need to
                 # skip all remaining logic and just call the callback,
@@ -3412,6 +3451,8 @@ class API(base.Base):
                                                new_pass=password)
 
     @check_instance_host
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_vnc_console(self, context, instance, console_type):
         """Get a url to an instance Console."""
         connect_info = self.compute_rpcapi.get_vnc_console(context,
@@ -3433,6 +3474,8 @@ class API(base.Base):
         return connect_info
 
     @check_instance_host
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_spice_console(self, context, instance, console_type):
         """Get a url to an instance Console."""
         connect_info = self.compute_rpcapi.get_spice_console(context,
@@ -3453,6 +3496,8 @@ class API(base.Base):
         return connect_info
 
     @check_instance_host
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_rdp_console(self, context, instance, console_type):
         """Get a url to an instance Console."""
         connect_info = self.compute_rpcapi.get_rdp_console(context,
@@ -3473,6 +3518,8 @@ class API(base.Base):
         return connect_info
 
     @check_instance_host
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_serial_console(self, context, instance, console_type):
         """Get a url to a serial console."""
         connect_info = self.compute_rpcapi.get_serial_console(context,
@@ -3493,6 +3540,8 @@ class API(base.Base):
         return connect_info
 
     @check_instance_host
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_mks_console(self, context, instance, console_type):
         """Get a url to a MKS console."""
         connect_info = self.compute_rpcapi.get_mks_console(context,
@@ -3831,6 +3880,10 @@ class API(base.Base):
 
         self._record_action_start(context, instance,
                                   instance_actions.LIVE_MIGRATION)
+
+        self.consoleauth_rpcapi.delete_tokens_for_instance(
+            context, instance.uuid)
+
         try:
             request_spec = objects.RequestSpec.get_by_instance_uuid(
                 context, instance.uuid)

nova/compute/cells_api.py

@@ -24,6 +24,7 @@ from nova.cells import rpcapi as cells_rpcapi
 from nova.cells import utils as cells_utils
 from nova.compute import api as compute_api
 from nova.compute import rpcapi as compute_rpcapi
+from nova.compute import task_states
 from nova.compute import vm_states
 from nova import exception
 from nova import objects
@@ -32,6 +33,7 @@ from nova import rpc
 
 
 check_instance_state = compute_api.check_instance_state
+reject_instance_state = compute_api.reject_instance_state
 check_instance_lock = compute_api.check_instance_lock
 check_instance_cell = compute_api.check_instance_cell
 
@@ -373,6 +375,8 @@ class ComputeCellsAPI(compute_api.API):
         self._cast_to_cells(context, instance, 'unshelve')
 
     @check_instance_cell
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_vnc_console(self, context, instance, console_type):
         """Get a url to a VNC Console."""
         if not instance.host:
@@ -388,6 +392,8 @@ class ComputeCellsAPI(compute_api.API):
         return {'url': connect_info['access_url']}
 
     @check_instance_cell
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_spice_console(self, context, instance, console_type):
         """Get a url to a SPICE Console."""
         if not instance.host:
@@ -403,6 +409,8 @@ class ComputeCellsAPI(compute_api.API):
         return {'url': connect_info['access_url']}
 
     @check_instance_cell
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_rdp_console(self, context, instance, console_type):
         """Get a url to a RDP Console."""
         if not instance.host:
@@ -418,6 +426,8 @@ class ComputeCellsAPI(compute_api.API):
         return {'url': connect_info['access_url']}
 
     @check_instance_cell
+    @reject_instance_state(
+        task_state=[task_states.DELETING, task_states.MIGRATING])
     def get_serial_console(self, context, instance, console_type):
         """Get a url to a serial console."""
         if not instance.host:

nova/compute/manager.py

@@ -67,7 +67,6 @@ from nova.compute.utils import wrap_instance_event
 from nova.compute import vm_states
 from nova import conductor
 import nova.conf
-from nova import consoleauth
 import nova.context
 from nova import exception
 from nova import exception_wrapper
@@ -511,7 +510,6 @@ class ComputeManager(manager.Manager):
         self.compute_task_api = conductor.ComputeTaskAPI()
         self.is_neutron_security_groups = (
             openstack_driver.is_neutron_security_groups())
-        self.consoleauth_rpcapi = consoleauth.rpcapi.ConsoleAuthAPI()
         self.cells_rpcapi = cells_rpcapi.CellsAPI()
         self.scheduler_client = scheduler_client.SchedulerClient()
         self._resource_tracker = None
@@ -730,7 +728,6 @@ class ComputeManager(manager.Manager):
         compute_utils.notify_about_instance_action(context, instance,
                 self.host, action=fields.NotificationAction.DELETE,
                 phase=fields.NotificationPhase.END)
-        self._clean_instance_console_tokens(context, instance)
         self._delete_scheduler_instance_info(context, instance.uuid)
 
     def _create_reservations(self, context, instance, project_id, user_id):
@@ -5627,7 +5624,6 @@ class ComputeManager(manager.Manager):
                      "This error can be safely ignored."),
                  instance=instance)
 
-        self._clean_instance_console_tokens(ctxt, instance)
         if migrate_data and migrate_data.obj_attr_is_set('migration'):
             migrate_data.migration.status = 'completed'
             migrate_data.migration.save()
@@ -5638,16 +5634,6 @@ class ComputeManager(manager.Manager):
                 CONF.rdp.enabled or CONF.serial_console.enabled or
                 CONF.mks.enabled)
 
-    def _clean_instance_console_tokens(self, ctxt, instance):
-        """Clean console tokens stored for an instance."""
-        if self._consoles_enabled():
-            if CONF.cells.enable:
-                self.cells_rpcapi.consoleauth_delete_tokens(
-                    ctxt, instance.uuid)
-            else:
-                self.consoleauth_rpcapi.delete_tokens_for_instance(
-                    ctxt, instance.uuid)
-
     @wrap_exception()
     @wrap_instance_event(prefix='compute')
     @wrap_instance_fault

nova/tests/unit/compute/test_compute.py

@@ -25,6 +25,8 @@ import time
 import traceback
 import uuid
 
+import ddt
+
 import mock
 from neutronclient.common import exceptions as neutron_exceptions
 from oslo_log import log as logging
@@ -4268,42 +4270,6 @@ class ComputeTestCase(BaseTestCase):
                                              mock.ANY)
         mock_deallocate.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)
 
-    def test_delete_instance_deletes_console_auth_tokens(self):
-        instance = self._create_fake_instance_obj()
-        self.flags(enabled=True, group='vnc')
-
-        self.tokens_deleted = False
-
-        def fake_delete_tokens(*args, **kwargs):
-            self.tokens_deleted = True
-
-        self.stub_out('nova.consoleauth.rpcapi.ConsoleAuthAPI.'
-                       'delete_tokens_for_instance',
-                       fake_delete_tokens)
-
-        self.compute._delete_instance(self.context, instance, [],
-                                      self.none_quotas)
-
-        self.assertTrue(self.tokens_deleted)
-
-    def test_delete_instance_deletes_console_auth_tokens_cells(self):
-        instance = self._create_fake_instance_obj()
-        self.flags(enabled=True, group='vnc')
-        self.flags(enable=True, group='cells')
-
-        self.tokens_deleted = False
-
-        def fake_delete_tokens(*args, **kwargs):
-            self.tokens_deleted = True
-
-        self.stub_out('nova.cells.rpcapi.CellsAPI.consoleauth_delete_tokens',
-                       fake_delete_tokens)
-
-        self.compute._delete_instance(self.context, instance,
-                                      [], self.none_quotas)
-
-        self.assertTrue(self.tokens_deleted)
-
     def test_delete_instance_changes_power_state(self):
         """Test that the power state is NOSTATE after deleting an instance."""
         instance = self._create_fake_instance_obj()
@@ -7824,6 +7790,7 @@ class ComputeTestCase(BaseTestCase):
         self.assertEqual('Preserve this', instance.fault.message)
 
 
+@ddt.ddt
 class ComputeAPITestCase(BaseTestCase):
     def setUp(self):
         def fake_get_nw_info(cls, ctxt, instance):
@@ -9494,6 +9461,25 @@ class ComputeAPITestCase(BaseTestCase):
                           self.compute_api.get_spice_console,
                           self.context, instance, 'spice')
 
+    @ddt.data(('spice', task_states.DELETING),
+              ('spice', task_states.MIGRATING),
+              ('rdp', task_states.DELETING),
+              ('rdp', task_states.MIGRATING),
+              ('vnc', task_states.DELETING),
+              ('vnc', task_states.MIGRATING),
+              ('mks', task_states.DELETING),
+              ('mks', task_states.MIGRATING),
+              ('serial', task_states.DELETING),
+              ('serial', task_states.MIGRATING))
+    @ddt.unpack
+    def test_get_console_invalid_task_state(self, console_type, task_state):
+        instance = self._create_fake_instance_obj(
+            params={'task_state': task_state})
+        self.assertRaises(
+            exception.InstanceInvalidState,
+            getattr(self.compute_api, 'get_%s_console' % console_type),
+            self.context, instance, console_type)
+
     @mock.patch.object(compute_api.consoleauth_rpcapi.ConsoleAuthAPI,
                        'authorize_console')
     @mock.patch.object(compute_rpcapi.ComputeAPI, 'get_rdp_console')
@@ -10213,14 +10199,17 @@ class ComputeAPITestCase(BaseTestCase):
         instance, instance_uuid = self._run_instance()
 
         rpcapi = self.compute_api.compute_task_api
+        consoleauth_rpcapi = self.compute_api.consoleauth_rpcapi
         fake_spec = objects.RequestSpec()
 
+        @mock.patch.object(consoleauth_rpcapi, 'delete_tokens_for_instance')
         @mock.patch.object(rpcapi, 'live_migrate_instance')
         @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host')
         @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid')
         @mock.patch.object(self.compute_api, '_record_action_start')
         def do_test(record_action_start, get_by_instance_uuid,
-                    get_all_by_host, live_migrate_instance):
+                    get_all_by_host, live_migrate_instance,
+                    delete_tokens_for_instance):
             get_by_instance_uuid.return_value = fake_spec
             get_all_by_host.return_value = objects.ComputeNodeList(
                 objects=[objects.ComputeNode(
@@ -10245,6 +10234,9 @@ class ComputeAPITestCase(BaseTestCase):
                 disk_over_commit=True,
                 request_spec=fake_spec, async=False)
 
+            delete_tokens_for_instance.assert_called_once_with(
+                self.context, instance.uuid)
+
         do_test()
         instance.refresh()
         self.assertEqual(instance['task_state'], task_states.MIGRATING)

nova/tests/unit/compute/test_compute_api.py

@@ -992,7 +992,8 @@ class _ComputeAPIUnitTestMixIn(object):
         self.mox.StubOutWithMock(quota.QUOTAS, 'rollback')
         rpcapi = self.compute_api.compute_rpcapi
         self.mox.StubOutWithMock(rpcapi, 'confirm_resize')
-
+        self.mox.StubOutWithMock(self.compute_api.consoleauth_rpcapi,
+                                 'delete_tokens_for_instance')
         if (inst.vm_state in
             (vm_states.SHELVED, vm_states.SHELVED_OFFLOADED)):
             self._test_delete_shelved_part(inst)
@@ -1080,6 +1081,10 @@ class _ComputeAPIUnitTestMixIn(object):
                                 project_id=inst.project_id,
                                 user_id=inst.user_id)
 
+        if self.cell_type is None or self.cell_type == 'api':
+            self.compute_api.consoleauth_rpcapi.delete_tokens_for_instance(
+                self.context, inst.uuid)
+
         self.mox.ReplayAll()
 
         getattr(self.compute_api, delete_type)(self.context, inst)