diff --git a/nova/compute/api.py b/nova/compute/api.py index 19d286475d77..a2c97e4d1d64 100644 --- a/nova/compute/api.py +++ b/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): @@ -1871,6 +1904,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, @@ -3421,6 +3460,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, @@ -3442,6 +3483,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, @@ -3462,6 +3505,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, @@ -3482,6 +3527,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, @@ -3502,6 +3549,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, @@ -3840,6 +3889,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) diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py index 4a7ea1537a50..91ead953e55a 100644 --- a/nova/compute/cells_api.py +++ b/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: diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 1c706fc4fb6e..b8a18faea465 100644 --- a/nova/compute/manager.py +++ b/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 diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index e59f668cac6a..d83e736492f0 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/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() @@ -7918,6 +7884,7 @@ class ComputeTestCase(BaseTestCase): notify, instance.uuid, self.compute_api.notifier, self.context) +@ddt.ddt class ComputeAPITestCase(BaseTestCase): def setUp(self): def fake_get_nw_info(cls, ctxt, instance): @@ -9588,6 +9555,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') @@ -10307,14 +10293,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( @@ -10339,6 +10328,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) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 791ed1306e70..32727a46a5fd 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/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)