diff --git a/nova/compute/api.py b/nova/compute/api.py index 145e675b5824..5868f12b4636 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2687,7 +2687,15 @@ class API(base.Base): self._record_action_start(context, instance, instance_actions.UNSHELVE) - self.compute_task_api.unshelve_instance(context, instance) + try: + request_spec = objects.RequestSpec.get_by_instance_uuid( + context, instance.uuid) + except exception.RequestSpecNotFound: + # Some old instances can still have no RequestSpec object attached + # to them, we need to support the old way + request_spec = None + self.compute_task_api.unshelve_instance(context, instance, + request_spec) @wrap_check_policy @check_instance_lock diff --git a/nova/conductor/api.py b/nova/conductor/api.py index b9be88204a3f..2400d31e8f7a 100644 --- a/nova/conductor/api.py +++ b/nova/conductor/api.py @@ -95,9 +95,9 @@ class LocalComputeTaskAPI(object): block_device_mapping=block_device_mapping, legacy_bdm=legacy_bdm) - def unshelve_instance(self, context, instance): + def unshelve_instance(self, context, instance, request_spec=None): utils.spawn_n(self._manager.unshelve_instance, context, - instance=instance) + instance=instance, request_spec=request_spec) def rebuild_instance(self, context, instance, orig_image_ref, image_ref, injected_files, new_pass, orig_sys_metadata, @@ -205,9 +205,9 @@ class ComputeTaskAPI(object): block_device_mapping=block_device_mapping, legacy_bdm=legacy_bdm) - def unshelve_instance(self, context, instance): + def unshelve_instance(self, context, instance, request_spec=None): self.conductor_compute_rpcapi.unshelve_instance(context, - instance=instance) + instance=instance, request_spec=None) def rebuild_instance(self, context, instance, orig_image_ref, image_ref, injected_files, new_pass, orig_sys_metadata, diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 7eb2aad0d079..82bfe455d92c 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -144,7 +144,7 @@ class ComputeTaskManager(base.Base): may involve coordinating activities on multiple compute nodes. """ - target = messaging.Target(namespace='compute_task', version='1.13') + target = messaging.Target(namespace='compute_task', version='1.14') def __init__(self): super(ComputeTaskManager, self).__init__() @@ -433,7 +433,7 @@ class ComputeTaskManager(base.Base): hosts = self.scheduler_client.select_destinations(context, spec_obj) return hosts - def unshelve_instance(self, context, instance): + def unshelve_instance(self, context, instance, request_spec=None): sys_meta = instance.system_metadata def safe_image_show(ctx, image_id): @@ -471,11 +471,24 @@ class ComputeTaskManager(base.Base): try: with compute_utils.EventReporter(context, 'schedule_instances', instance.uuid): - filter_properties = {} + if not request_spec: + # NOTE(sbauza): We were unable to find an original + # RequestSpec object - probably because the instance is + # old. We need to mock that the old way + filter_properties = {} + request_spec = scheduler_utils.build_request_spec( + context, image, [instance]) + else: + # TODO(sbauza): Provide directly the RequestSpec object + # when _schedule_instances(), + # populate_filter_properties and populate_retry() + # accept it + filter_properties = request_spec.\ + to_legacy_filter_properties_dict() + request_spec = request_spec.\ + to_legacy_request_spec_dict() scheduler_utils.populate_retry(filter_properties, instance.uuid) - request_spec = scheduler_utils.build_request_spec( - context, image, [instance]) hosts = self._schedule_instances( context, request_spec, filter_properties) host_state = hosts[0] diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index 5e818a66735b..0a300ce6f4d9 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -269,6 +269,7 @@ class ComputeTaskAPI(object): 1.11 - Added clean_shutdown to migrate_server() 1.12 - Added request_spec to rebuild_instance() 1.13 - Added request_spec to migrate_server() + 1.14 - Added request_spec to unshelve_instance() """ def __init__(self): @@ -337,9 +338,16 @@ class ComputeTaskAPI(object): cctxt = self.client.prepare(version=version) cctxt.cast(context, 'build_instances', **kw) - def unshelve_instance(self, context, instance): - cctxt = self.client.prepare(version='1.3') - cctxt.cast(context, 'unshelve_instance', instance=instance) + def unshelve_instance(self, context, instance, request_spec=None): + version = '1.14' + kw = {'instance': instance, + 'request_spec': request_spec + } + if not self.client.can_send_version(version): + version = '1.3' + del kw['request_spec'] + cctxt = self.client.prepare(version=version) + cctxt.cast(context, 'unshelve_instance', **kw) def rebuild_instance(self, ctxt, instance, new_pass, injected_files, image_ref, orig_image_ref, orig_sys_metadata, bdms, diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index 1cd962017e54..9746143131c2 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -476,7 +476,8 @@ class ShelveComputeAPITestCase(test_compute.BaseTestCase): db.instance_destroy(self.context, instance['uuid']) - def test_unshelve(self): + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + def test_unshelve(self, get_by_instance_uuid): # Ensure instance can be unshelved. instance = self._create_fake_instance_obj() @@ -488,7 +489,14 @@ class ShelveComputeAPITestCase(test_compute.BaseTestCase): instance.vm_state = vm_states.SHELVED instance.save() - self.compute_api.unshelve(self.context, instance) + fake_spec = objects.RequestSpec() + get_by_instance_uuid.return_value = fake_spec + with mock.patch.object(self.compute_api.compute_task_api, + 'unshelve_instance') as unshelve: + self.compute_api.unshelve(self.context, instance) + get_by_instance_uuid.assert_called_once_with(self.context, + instance.uuid) + unshelve.assert_called_once_with(self.context, instance, fake_spec) self.assertEqual(instance.task_state, task_states.UNSHELVING) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index a9fb9e086e61..5a7e2fdcab4d 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -50,6 +50,7 @@ from nova.tests.unit import cast_as_call from nova.tests.unit.compute import test_compute from nova.tests.unit import fake_instance from nova.tests.unit import fake_notifier +from nova.tests.unit import fake_request_spec from nova.tests.unit import fake_server_actions from nova.tests.unit import fake_utils from nova import utils @@ -643,6 +644,51 @@ class _BaseTaskTestCase(object): system_metadata['shelved_host'] = 'fake-mini' self.conductor_manager.unshelve_instance(self.context, instance) + def test_unshelve_offload_instance_on_host_with_request_spec(self): + instance = self._create_fake_instance_obj() + instance.vm_state = vm_states.SHELVED_OFFLOADED + instance.task_state = task_states.UNSHELVING + instance.save() + system_metadata = instance.system_metadata + + system_metadata['shelved_at'] = timeutils.utcnow() + system_metadata['shelved_image_id'] = 'fake_image_id' + system_metadata['shelved_host'] = 'fake-mini' + + fake_spec = fake_request_spec.fake_spec_obj() + # FIXME(sbauza): Modify the fake RequestSpec object to either add a + # non-empty SchedulerRetries object or nullify the field + fake_spec.retry = None + # FIXME(sbauza): Modify the fake RequestSpec object to either add a + # non-empty SchedulerLimits object or nullify the field + fake_spec.limits = None + # FIXME(sbauza): Modify the fake RequestSpec object to either add a + # non-empty InstanceGroup object or nullify the field + fake_spec.instance_group = None + + filter_properties = fake_spec.to_legacy_filter_properties_dict() + request_spec = fake_spec.to_legacy_request_spec_dict() + + host = {'host': 'host1', 'nodename': 'node1', 'limits': []} + + @mock.patch.object(self.conductor_manager.compute_rpcapi, + 'unshelve_instance') + @mock.patch.object(self.conductor_manager, '_schedule_instances') + def do_test(sched_instances, unshelve_instance): + sched_instances.return_value = [host] + self.conductor_manager.unshelve_instance(self.context, instance, + fake_spec) + scheduler_utils.populate_retry(filter_properties, instance.uuid) + scheduler_utils.populate_filter_properties(filter_properties, host) + sched_instances.assert_called_once_with(self.context, request_spec, + filter_properties) + unshelve_instance.assert_called_once_with( + self.context, instance, host['host'], image=mock.ANY, + filter_properties=filter_properties, node=host['nodename'] + ) + + do_test() + def test_unshelve_offloaded_instance_glance_image_not_found(self): shelved_image_id = "image_not_found"