From 76dfb4ba9fa0fed1350021591956c4e8143b1ce9 Mon Sep 17 00:00:00 2001 From: Sylvain Bauza Date: Tue, 29 Mar 2016 12:02:07 +0200 Subject: [PATCH] Cold migrate using the RequestSpec object Cold migrate and resize can now get the Spec object and pass it down the wire to the conductor so that it could use it for the move. Partially-Implements: blueprint check-destination-on-migrations-newton Co-Authored-By: Pawel Koniszewski Change-Id: Ic3968721d257a167f3f946e5387cd227a7eeec6c --- nova/cells/rpcapi.py | 7 +- nova/compute/api.py | 16 +- nova/conductor/api.py | 10 +- nova/conductor/manager.py | 30 ++- nova/conductor/tasks/migrate.py | 54 +++-- nova/tests/unit/cells/test_cells_rpcapi.py | 18 ++ nova/tests/unit/compute/test_compute_api.py | 17 +- nova/tests/unit/compute/test_compute_cells.py | 3 +- .../unit/conductor/tasks/test_migrate.py | 43 ++-- nova/tests/unit/conductor/test_conductor.py | 191 ++++++++---------- 10 files changed, 227 insertions(+), 162 deletions(-) diff --git a/nova/cells/rpcapi.py b/nova/cells/rpcapi.py index f95c56444c..5c11e45a9a 100644 --- a/nova/cells/rpcapi.py +++ b/nova/cells/rpcapi.py @@ -558,7 +558,12 @@ class CellsAPI(object): def resize_instance(self, ctxt, instance, extra_instance_updates, scheduler_hint, flavor, reservations, - clean_shutdown=True): + clean_shutdown=True, + request_spec=None): + # NOTE(sbauza): Since Cells v1 is quite feature-frozen, we don't want + # to pass down request_spec to the manager and rather keep the + # cell conductor providing a new RequestSpec like the original + # behaviour flavor_p = jsonutils.to_primitive(flavor) version = '1.33' msg_args = {'instance': instance, diff --git a/nova/compute/api.py b/nova/compute/api.py index 2c3d495dd6..9a7d570673 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2729,12 +2729,26 @@ class API(base.Base): self._record_action_start(context, instance, instance_actions.RESIZE) + # NOTE(sbauza): The migration script we provided in Newton should make + # sure that all our instances are currently migrated to have an + # attached RequestSpec object but let's consider that the operator only + # half migrated all their instances in the meantime. + try: + request_spec = objects.RequestSpec.get_by_instance_uuid( + context, instance.uuid) + request_spec.ignore_hosts = filter_properties['ignore_hosts'] + 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 + scheduler_hint = {'filter_properties': filter_properties} self.compute_task_api.resize_instance(context, instance, extra_instance_updates, scheduler_hint=scheduler_hint, flavor=new_instance_type, reservations=quotas.reservations or [], - clean_shutdown=clean_shutdown) + clean_shutdown=clean_shutdown, + request_spec=request_spec) @wrap_check_policy @check_instance_lock diff --git a/nova/conductor/api.py b/nova/conductor/api.py index 627d97b84c..535277c1d7 100644 --- a/nova/conductor/api.py +++ b/nova/conductor/api.py @@ -65,14 +65,15 @@ class LocalComputeTaskAPI(object): def resize_instance(self, context, instance, extra_instance_updates, scheduler_hint, flavor, reservations, - clean_shutdown=True): + clean_shutdown=True, request_spec=None): # NOTE(comstud): 'extra_instance_updates' is not used here but is # needed for compatibility with the cells_rpcapi version of this # method. self._manager.migrate_server( context, instance, scheduler_hint, live=False, rebuild=False, flavor=flavor, block_migration=None, disk_over_commit=None, - reservations=reservations, clean_shutdown=clean_shutdown) + reservations=reservations, clean_shutdown=clean_shutdown, + request_spec=request_spec) def live_migrate_instance(self, context, instance, host_name, block_migration, disk_over_commit, @@ -176,14 +177,15 @@ class ComputeTaskAPI(object): def resize_instance(self, context, instance, extra_instance_updates, scheduler_hint, flavor, reservations, - clean_shutdown=True): + clean_shutdown=True, request_spec=None): # NOTE(comstud): 'extra_instance_updates' is not used here but is # needed for compatibility with the cells_rpcapi version of this # method. self.conductor_compute_rpcapi.migrate_server( context, instance, scheduler_hint, live=False, rebuild=False, flavor=flavor, block_migration=None, disk_over_commit=None, - reservations=reservations, clean_shutdown=clean_shutdown) + reservations=reservations, clean_shutdown=clean_shutdown, + request_spec=request_spec) def live_migrate_instance(self, context, instance, host_name, block_migration, disk_over_commit, diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 77df46aad5..014267e7f8 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -200,20 +200,30 @@ class ComputeTaskManager(base.Base): instance_uuid): self._cold_migrate(context, instance, flavor, scheduler_hint['filter_properties'], - reservations, clean_shutdown) + reservations, clean_shutdown, request_spec) else: raise NotImplementedError() def _cold_migrate(self, context, instance, flavor, filter_properties, - reservations, clean_shutdown): + reservations, clean_shutdown, request_spec): image = utils.get_image_from_system_metadata( instance.system_metadata) - request_spec = scheduler_utils.build_request_spec( - context, image, [instance], instance_type=flavor) + # NOTE(sbauza): If a reschedule occurs when prep_resize(), then + # it only provides filter_properties legacy dict back to the + # conductor with no RequestSpec part of the payload. + if not request_spec: + request_spec = objects.RequestSpec.from_components( + context, instance.uuid, image, + instance.flavor, instance.numa_topology, instance.pci_requests, + filter_properties, None, instance.availability_zone) + task = self._build_cold_migrate_task(context, instance, flavor, - filter_properties, request_spec, + request_spec, reservations, clean_shutdown) + # TODO(sbauza): Provide directly the RequestSpec object once + # _set_vm_state_and_notify() accepts it + legacy_spec = request_spec.to_legacy_request_spec_dict() try: task.execute() except exception.NoValidHost as ex: @@ -223,7 +233,7 @@ class ComputeTaskManager(base.Base): updates = {'vm_state': vm_state, 'task_state': None} self._set_vm_state_and_notify(context, instance.uuid, 'migrate_server', - updates, ex, request_spec) + updates, ex, legacy_spec) # if the flavor IDs match, it's migrate; otherwise resize if flavor.id == instance.instance_type_id: @@ -239,14 +249,14 @@ class ComputeTaskManager(base.Base): updates = {'vm_state': vm_state, 'task_state': None} self._set_vm_state_and_notify(context, instance.uuid, 'migrate_server', - updates, ex, request_spec) + updates, ex, legacy_spec) except Exception as ex: with excutils.save_and_reraise_exception(): updates = {'vm_state': instance.vm_state, 'task_state': None} self._set_vm_state_and_notify(context, instance.uuid, 'migrate_server', - updates, ex, request_spec) + updates, ex, legacy_spec) def _set_vm_state_and_notify(self, context, instance_uuid, method, updates, ex, request_spec): @@ -351,10 +361,10 @@ class ComputeTaskManager(base.Base): request_spec) def _build_cold_migrate_task(self, context, instance, flavor, - filter_properties, request_spec, reservations, + request_spec, reservations, clean_shutdown): return migrate.MigrationTask(context, instance, flavor, - filter_properties, request_spec, + request_spec, reservations, clean_shutdown, self.compute_rpcapi, self.scheduler_client) diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 96768733fd..9ac4bdd4b5 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -10,20 +10,21 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_serialization import jsonutils + from nova.conductor.tasks import base from nova import objects from nova.scheduler import utils as scheduler_utils class MigrationTask(base.TaskBase): - def __init__(self, context, instance, flavor, filter_properties, + def __init__(self, context, instance, flavor, request_spec, reservations, clean_shutdown, compute_rpcapi, scheduler_client): super(MigrationTask, self).__init__(context, instance) self.clean_shutdown = clean_shutdown self.request_spec = request_spec self.reservations = reservations - self.filter_properties = filter_properties self.flavor = flavor self.quotas = None @@ -31,33 +32,52 @@ class MigrationTask(base.TaskBase): self.scheduler_client = scheduler_client def _execute(self): - image = self.request_spec.get('image') + image = self.request_spec.image self.quotas = objects.Quotas.from_reservations(self.context, self.reservations, instance=self.instance) - scheduler_utils.setup_instance_group(self.context, self.request_spec, - self.filter_properties) - scheduler_utils.populate_retry(self.filter_properties, + # TODO(sbauza): Remove that once prep_resize() accepts a RequestSpec + # object in the signature and all the scheduler.utils methods too + legacy_spec = self.request_spec.to_legacy_request_spec_dict() + legacy_props = self.request_spec.to_legacy_filter_properties_dict() + scheduler_utils.setup_instance_group(self.context, legacy_spec, + legacy_props) + scheduler_utils.populate_retry(legacy_props, self.instance.uuid) - # TODO(sbauza): Hydrate here the object until we modify the - # scheduler.utils methods to directly use the RequestSpec object - spec_obj = objects.RequestSpec.from_primitives( - self.context, self.request_spec, self.filter_properties) + + # TODO(sbauza): Remove that RequestSpec rehydratation once + # scheduler.utils methods use directly the NovaObject. + self.request_spec = objects.RequestSpec.from_components( + self.context, self.instance.uuid, image, + self.instance.flavor, self.instance.numa_topology, + self.instance.pci_requests, legacy_props, None, + self.instance.availability_zone) + # NOTE(sbauza): Force_hosts/nodes needs to be reset + # if we want to make sure that the next destination + # is not forced to be the original host + self.request_spec.reset_forced_destinations() hosts = self.scheduler_client.select_destinations( - self.context, spec_obj) + self.context, self.request_spec) host_state = hosts[0] - scheduler_utils.populate_filter_properties(self.filter_properties, + scheduler_utils.populate_filter_properties(legacy_props, host_state) # context is not serializable - self.filter_properties.pop('context', None) + legacy_props.pop('context', None) (host, node) = (host_state['host'], host_state['nodename']) + + # FIXME(sbauza): Serialize/Unserialize the legacy dict because of + # oslo.messaging #1529084 to transform datetime values into strings. + # tl;dr: datetimes in dicts are not accepted as correct values by the + # rpc fake driver. + legacy_spec = jsonutils.loads(jsonutils.dumps(legacy_spec)) + self.compute_rpcapi.prep_resize( - self.context, image, self.instance, self.flavor, host, - self.reservations, request_spec=self.request_spec, - filter_properties=self.filter_properties, node=node, - clean_shutdown=self.clean_shutdown) + self.context, legacy_spec['image'], self.instance, + self.flavor, host, self.reservations, + request_spec=legacy_spec, filter_properties=legacy_props, + node=node, clean_shutdown=self.clean_shutdown) def rollback(self): if self.quotas: diff --git a/nova/tests/unit/cells/test_cells_rpcapi.py b/nova/tests/unit/cells/test_cells_rpcapi.py index 63813837f2..a2dd666681 100644 --- a/nova/tests/unit/cells/test_cells_rpcapi.py +++ b/nova/tests/unit/cells/test_cells_rpcapi.py @@ -668,6 +668,24 @@ class CellsAPITestCase(test.NoDBTestCase): self._check_result(call_info, 'resize_instance', expected_args, version='1.33') + def test_resize_instance_not_passing_request_spec(self): + call_info = self._stub_rpc_method('cast', None) + + self.cells_rpcapi.resize_instance(self.fake_context, + 'fake-instance', + dict(cow='moo'), + 'fake-hint', + 'fake-flavor', + 'fake-reservations', + clean_shutdown=True, + request_spec='fake-spec') + expected_args = {'instance': 'fake-instance', + 'flavor': 'fake-flavor', + 'extra_instance_updates': dict(cow='moo'), + 'clean_shutdown': True} + self._check_result(call_info, 'resize_instance', + expected_args, version='1.33') + def test_live_migrate_instance(self): call_info = self._stub_rpc_method('cast', None) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index c5f533a7c5..d0c36e9cbe 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1487,6 +1487,7 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(fake_inst, 'save') self.mox.StubOutWithMock(quota.QUOTAS, 'commit') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') + self.mox.StubOutWithMock(objects.RequestSpec, 'get_by_instance_uuid') self.mox.StubOutWithMock(self.compute_api.compute_task_api, 'resize_instance') @@ -1571,6 +1572,10 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api._record_action_start(self.context, fake_inst, 'migrate') + fake_spec = objects.RequestSpec() + objects.RequestSpec.get_by_instance_uuid( + self.context, fake_inst.uuid).AndReturn(fake_spec) + scheduler_hint = {'filter_properties': filter_properties} self.compute_api.compute_task_api.resize_instance( @@ -1578,7 +1583,8 @@ class _ComputeAPIUnitTestMixIn(object): scheduler_hint=scheduler_hint, flavor=mox.IsA(objects.Flavor), reservations=expected_reservations, - clean_shutdown=clean_shutdown) + clean_shutdown=clean_shutdown, + request_spec=fake_spec) self.mox.ReplayAll() @@ -1592,6 +1598,11 @@ class _ComputeAPIUnitTestMixIn(object): clean_shutdown=clean_shutdown, **extra_kwargs) + if allow_same_host: + self.assertEqual([], fake_spec.ignore_hosts) + else: + self.assertEqual([fake_inst['host']], fake_spec.ignore_hosts) + def _test_migrate(self, *args, **kwargs): self._test_resize(*args, flavor_id_passed=False, **kwargs) @@ -1690,6 +1701,7 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.resize, self.context, fake_inst, flavor_id='flavor-id') + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch('nova.compute.api.API._record_action_start') @mock.patch('nova.compute.api.API._resize_cells_support') @mock.patch('nova.conductor.conductor_api.ComputeTaskAPI.resize_instance') @@ -1698,7 +1710,8 @@ class _ComputeAPIUnitTestMixIn(object): get_flavor_by_flavor_id, resize_instance_mock, cells_support_mock, - record_mock): + record_mock, + get_by_inst): params = dict(image_ref='') fake_inst = self._create_instance_obj(params=params) diff --git a/nova/tests/unit/compute/test_compute_cells.py b/nova/tests/unit/compute/test_compute_cells.py index d6591732c1..f115a67780 100644 --- a/nova/tests/unit/compute/test_compute_cells.py +++ b/nova/tests/unit/compute/test_compute_cells.py @@ -352,6 +352,7 @@ class CellsConductorAPIRPCRedirect(test.NoDBTestCase): # args since this is verified in compute test code. self.assertTrue(self.cells_rpcapi.build_instances.called) + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(compute_api.API, '_record_action_start') @mock.patch.object(compute_api.API, '_resize_cells_support') @mock.patch.object(compute_utils, 'reserve_quota_delta') @@ -361,7 +362,7 @@ class CellsConductorAPIRPCRedirect(test.NoDBTestCase): @mock.patch.object(compute_api.API, '_check_auto_disk_config') @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') def test_resize_instance(self, _bdms, _check, _extract, _save, _upsize, - _reserve, _cells, _record): + _reserve, _cells, _record, _spec_get_by_uuid): flavor = objects.Flavor(**test_flavor.fake_flavor) _extract.return_value = flavor orig_system_metadata = {} diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index 46417e457e..7190ab7644 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -15,7 +15,6 @@ import mock from nova.compute import rpcapi as compute_rpcapi from nova.conductor.tasks import migrate from nova import objects -from nova.objects import base as obj_base from nova.scheduler import client as scheduler_client from nova.scheduler import utils as scheduler_utils from nova import test @@ -30,16 +29,18 @@ class MigrationTaskTestCase(test.NoDBTestCase): self.user_id = 'fake' self.project_id = 'fake' self.context = FakeContext(self.user_id, self.project_id) - inst = fake_instance.fake_db_instance(image_ref='image_ref') - self.instance = objects.Instance._from_db_object( - self.context, objects.Instance(), inst, []) - self.instance.system_metadata = {'image_hw_disk_bus': 'scsi'} self.flavor = fake_flavor.fake_flavor_obj(self.context) self.flavor.extra_specs = {'extra_specs': 'fake'} - self.request_spec = {'instance_type': - obj_base.obj_to_primitive(self.flavor), - 'instance_properties': {}, - 'image': 'image'} + inst = fake_instance.fake_db_instance(image_ref='image_ref', + instance_type=self.flavor) + inst_object = objects.Instance( + flavor=self.flavor, + numa_topology=None, + pci_requests=None, + system_metadata={'image_hw_disk_bus': 'scsi'}) + self.instance = objects.Instance._from_db_object( + self.context, inst_object, inst, []) + self.request_spec = objects.RequestSpec(image=objects.ImageMeta()) self.hosts = [dict(host='host1', nodename=None, limits={})] self.filter_properties = {'limits': {}, 'retry': {'num_attempts': 1, 'hosts': [['host1', None]]}} @@ -48,37 +49,35 @@ class MigrationTaskTestCase(test.NoDBTestCase): def _generate_task(self): return migrate.MigrationTask(self.context, self.instance, self.flavor, - self.filter_properties, self.request_spec, - self.reservations, self.clean_shutdown, + self.request_spec, self.reservations, + self.clean_shutdown, compute_rpcapi.ComputeAPI(), scheduler_client.SchedulerClient()) - @mock.patch.object(scheduler_utils, 'build_request_spec') + @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(scheduler_utils, 'setup_instance_group') - @mock.patch.object(objects.RequestSpec, 'from_primitives') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize') @mock.patch.object(objects.Quotas, 'from_reservations') def test_execute(self, quotas_mock, prep_resize_mock, - sel_dest_mock, spec_fp_mock, sig_mock, brs_mock): - brs_mock.return_value = self.request_spec - fake_spec = objects.RequestSpec() - spec_fp_mock.return_value = fake_spec + sel_dest_mock, sig_mock, request_spec_from_components): sel_dest_mock.return_value = self.hosts task = self._generate_task() + request_spec_from_components.return_value = self.request_spec + legacy_request_spec = self.request_spec.to_legacy_request_spec_dict() task.execute() quotas_mock.assert_called_once_with(self.context, self.reservations, instance=self.instance) - sig_mock.assert_called_once_with(self.context, self.request_spec, + sig_mock.assert_called_once_with(self.context, legacy_request_spec, self.filter_properties) task.scheduler_client.select_destinations.assert_called_once_with( - self.context, fake_spec) + self.context, self.request_spec) prep_resize_mock.assert_called_once_with( - self.context, 'image', self.instance, self.flavor, - self.hosts[0]['host'], self.reservations, - request_spec=self.request_spec, + self.context, legacy_request_spec['image'], self.instance, + self.flavor, self.hosts[0]['host'], self.reservations, + request_spec=legacy_request_spec, filter_properties=self.filter_properties, node=self.hosts[0]['nodename'], clean_shutdown=self.clean_shutdown) self.assertFalse(quotas_mock.return_value.rollback.called) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index e57f9b85b8..a32a513360 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -355,33 +355,22 @@ class _BaseTaskTestCase(object): self.assertEqual('destination', migration.dest_compute) self.assertEqual(inst_obj.host, migration.source_compute) - def _test_cold_migrate(self, clean_shutdown=True): - self.mox.StubOutWithMock(utils, 'get_image_from_system_metadata') - self.mox.StubOutWithMock(scheduler_utils, 'build_request_spec') - self.mox.StubOutWithMock(migrate.MigrationTask, 'execute') + @mock.patch.object(migrate.MigrationTask, 'execute') + @mock.patch.object(utils, 'get_image_from_system_metadata') + @mock.patch.object(objects.RequestSpec, 'from_components') + def _test_cold_migrate(self, spec_from_components, get_image_from_metadata, + migration_task_execute, clean_shutdown=True): + get_image_from_metadata.return_value = 'image' inst = fake_instance.fake_db_instance(image_ref='image_ref') inst_obj = objects.Instance._from_db_object( self.context, objects.Instance(), inst, []) inst_obj.system_metadata = {'image_hw_disk_bus': 'scsi'} flavor = flavors.get_default_flavor() flavor.extra_specs = {'extra_specs': 'fake'} - filter_properties = {'limits': {}, - 'retry': {'num_attempts': 1, - 'hosts': [['host1', None]]}} - request_spec = {'instance_type': obj_base.obj_to_primitive(flavor), - 'instance_properties': {}} - utils.get_image_from_system_metadata( - inst_obj.system_metadata).AndReturn('image') + inst_obj.flavor = flavor - scheduler_utils.build_request_spec( - self.context, 'image', - [mox.IsA(objects.Instance)], - instance_type=mox.IsA(objects.Flavor)).AndReturn(request_spec) - task = self.conductor_manager._build_cold_migrate_task( - self.context, inst_obj, flavor, filter_properties, - request_spec, [], clean_shutdown=clean_shutdown) - task.execute() - self.mox.ReplayAll() + fake_spec = fake_request_spec.fake_spec_obj() + spec_from_components.return_value = fake_spec scheduler_hint = {'filter_properties': {}} @@ -398,6 +387,10 @@ class _BaseTaskTestCase(object): False, False, flavor, None, None, [], clean_shutdown) + get_image_from_metadata.assert_called_once_with( + inst_obj.system_metadata) + migration_task_execute.assert_called_once_with() + def test_cold_migrate(self): self._test_cold_migrate() @@ -1355,9 +1348,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.conductor._set_vm_state_and_notify( self.context, 1, 'method', 'updates', 'ex', 'request_spec') - @mock.patch.object(scheduler_utils, 'build_request_spec') @mock.patch.object(scheduler_utils, 'setup_instance_group') - @mock.patch.object(objects.RequestSpec, 'from_primitives') @mock.patch.object(utils, 'get_image_from_system_metadata') @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @@ -1366,7 +1357,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(migrate.MigrationTask, 'rollback') def test_cold_migrate_no_valid_host_back_in_active_state( self, rollback_mock, notify_mock, select_dest_mock, quotas_mock, - metadata_mock, spec_fp_mock, sig_mock, brs_mock): + metadata_mock, sig_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( image_ref='fake-image_ref', @@ -1374,41 +1365,42 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): vm_state=vm_states.ACTIVE, system_metadata={}, uuid=uuids.instance, - user_id=fakes.FAKE_USER_ID) - request_spec = dict(instance_type=dict(extra_specs=dict()), - instance_properties=dict()) - filter_props = dict(context=None) + user_id=fakes.FAKE_USER_ID, + flavor=flavor, + availability_zone=None, + pci_requests=None, + numa_topology=None) resvs = 'fake-resvs' image = 'fake-image' - fake_spec = objects.RequestSpec() + fake_spec = objects.RequestSpec(image=objects.ImageMeta()) + legacy_request_spec = fake_spec.to_legacy_request_spec_dict() metadata_mock.return_value = image - brs_mock.return_value = request_spec - spec_fp_mock.return_value = fake_spec exc_info = exc.NoValidHost(reason="") select_dest_mock.side_effect = exc_info updates = {'vm_state': vm_states.ACTIVE, 'task_state': None} + + # Filter properties are populated during code execution + legacy_filter_props = {'retry': {'num_attempts': 1, + 'hosts': []}} + self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, inst_obj, - flavor, filter_props, [resvs], - clean_shutdown=True) + flavor, {}, [resvs], + True, fake_spec) metadata_mock.assert_called_with({}) - brs_mock.assert_called_once_with(self.context, image, - [inst_obj], - instance_type=flavor) quotas_mock.assert_called_once_with(self.context, [resvs], instance=inst_obj) - sig_mock.assert_called_once_with(self.context, request_spec, - filter_props) + sig_mock.assert_called_once_with(self.context, legacy_request_spec, + legacy_filter_props) notify_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, - exc_info, request_spec) + exc_info, legacy_request_spec) rollback_mock.assert_called_once_with() - @mock.patch.object(scheduler_utils, 'build_request_spec') @mock.patch.object(scheduler_utils, 'setup_instance_group') - @mock.patch.object(objects.RequestSpec, 'from_primitives') + @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(utils, 'get_image_from_system_metadata') @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @@ -1417,7 +1409,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(migrate.MigrationTask, 'rollback') def test_cold_migrate_no_valid_host_back_in_stopped_state( self, rollback_mock, notify_mock, select_dest_mock, quotas_mock, - metadata_mock, spec_fp_mock, sig_mock, brs_mock): + metadata_mock, spec_fc_mock, sig_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( image_ref='fake-image_ref', @@ -1425,18 +1417,23 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): instance_type_id=flavor['id'], system_metadata={}, uuid=uuids.instance, - user_id=fakes.FAKE_USER_ID) + user_id=fakes.FAKE_USER_ID, + flavor=flavor, + numa_topology=None, + pci_requests=None, + availability_zone=None) image = 'fake-image' - request_spec = dict(instance_type=dict(extra_specs=dict()), - instance_properties=dict(), - image=image) - filter_props = dict(context=None) resvs = 'fake-resvs' - fake_spec = objects.RequestSpec() + + fake_spec = objects.RequestSpec(image=objects.ImageMeta()) + spec_fc_mock.return_value = fake_spec + legacy_request_spec = fake_spec.to_legacy_request_spec_dict() + + # Filter properties are populated during code execution + legacy_filter_props = {'retry': {'num_attempts': 1, + 'hosts': []}} metadata_mock.return_value = image - brs_mock.return_value = request_spec - spec_fp_mock.return_value = fake_spec exc_info = exc.NoValidHost(reason="") select_dest_mock.side_effect = exc_info updates = {'vm_state': vm_states.STOPPED, @@ -1444,19 +1441,16 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, inst_obj, - flavor, filter_props, [resvs], - clean_shutdown=True) + flavor, {}, [resvs], + True, fake_spec) metadata_mock.assert_called_with({}) - brs_mock.assert_called_once_with(self.context, image, - [inst_obj], - instance_type=flavor) quotas_mock.assert_called_once_with(self.context, [resvs], instance=inst_obj) - sig_mock.assert_called_once_with(self.context, request_spec, - filter_props) + sig_mock.assert_called_once_with(self.context, legacy_request_spec, + legacy_filter_props) notify_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, - exc_info, request_spec) + exc_info, legacy_request_spec) rollback_mock.assert_called_once_with() def test_cold_migrate_no_valid_host_error_msg(self): @@ -1468,32 +1462,27 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): system_metadata={}, uuid=uuids.instance, user_id=fakes.FAKE_USER_ID) - request_spec = dict(instance_type=dict(extra_specs=dict()), - instance_properties=dict()) - filter_props = dict(context=None) + fake_spec = fake_request_spec.fake_spec_obj() resvs = 'fake-resvs' image = 'fake-image' with test.nested( mock.patch.object(utils, 'get_image_from_system_metadata', return_value=image), - mock.patch.object(scheduler_utils, 'build_request_spec', - return_value=request_spec), mock.patch.object(self.conductor, '_set_vm_state_and_notify'), mock.patch.object(migrate.MigrationTask, 'execute', side_effect=exc.NoValidHost(reason="")), mock.patch.object(migrate.MigrationTask, 'rollback') - ) as (image_mock, brs_mock, set_vm_mock, task_execute_mock, + ) as (image_mock, set_vm_mock, task_execute_mock, task_rollback_mock): nvh = self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, - inst_obj, flavor, filter_props, [resvs], - clean_shutdown=True) + inst_obj, flavor, {}, [resvs], + True, fake_spec) self.assertIn('cold migrate', nvh.message) @mock.patch.object(utils, 'get_image_from_system_metadata') - @mock.patch('nova.scheduler.utils.build_request_spec') @mock.patch.object(migrate.MigrationTask, 'execute') @mock.patch.object(migrate.MigrationTask, 'rollback') @mock.patch.object(conductor_manager.ComputeTaskManager, @@ -1502,7 +1491,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): set_vm_mock, task_rollback_mock, task_exec_mock, - brs_mock, image_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( @@ -1512,30 +1500,26 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): system_metadata={}, uuid=uuids.instance, user_id=fakes.FAKE_USER_ID) - request_spec = dict(instance_type=dict(extra_specs=dict()), - instance_properties=dict()) - filter_props = dict(context=None) resvs = 'fake-resvs' image = 'fake-image' exception = exc.UnsupportedPolicyException(reason='') + fake_spec = fake_request_spec.fake_spec_obj() + legacy_request_spec = fake_spec.to_legacy_request_spec_dict() image_mock.return_value = image - brs_mock.return_value = request_spec task_exec_mock.side_effect = exception self.assertRaises(exc.UnsupportedPolicyException, self.conductor._cold_migrate, self.context, - inst_obj, flavor, filter_props, [resvs], - clean_shutdown=True) + inst_obj, flavor, {}, [resvs], True, fake_spec) updates = {'vm_state': vm_states.STOPPED, 'task_state': None} set_vm_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, - exception, request_spec) + exception, legacy_request_spec) - @mock.patch.object(scheduler_utils, 'build_request_spec') @mock.patch.object(scheduler_utils, 'setup_instance_group') - @mock.patch.object(objects.RequestSpec, 'from_primitives') + @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(utils, 'get_image_from_system_metadata') @mock.patch.object(objects.Quotas, 'from_reservations') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @@ -1545,8 +1529,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize') def test_cold_migrate_exception_host_in_error_state_and_raise( self, prep_resize_mock, rollback_mock, notify_mock, - select_dest_mock, quotas_mock, metadata_mock, spec_fp_mock, - sig_mock, brs_mock): + select_dest_mock, quotas_mock, metadata_mock, spec_fc_mock, + sig_mock): flavor = flavors.get_flavor_by_name('m1.tiny') inst_obj = objects.Instance( image_ref='fake-image_ref', @@ -1554,19 +1538,19 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): instance_type_id=flavor['id'], system_metadata={}, uuid=uuids.instance, - user_id=fakes.FAKE_USER_ID) + user_id=fakes.FAKE_USER_ID, + flavor=flavor, + availability_zone=None, + pci_requests=None, + numa_topology=None) image = 'fake-image' - request_spec = dict(instance_type=dict(), - instance_properties=dict(), - image=image) - filter_props = dict(context=None) resvs = 'fake-resvs' - fake_spec = objects.RequestSpec() + fake_spec = objects.RequestSpec(image=objects.ImageMeta()) + legacy_request_spec = fake_spec.to_legacy_request_spec_dict() + spec_fc_mock.return_value = fake_spec hosts = [dict(host='host1', nodename=None, limits={})] metadata_mock.return_value = image - brs_mock.return_value = request_spec - spec_fp_mock.return_value = fake_spec exc_info = test.TestingException('something happened') select_dest_mock.return_value = hosts @@ -1576,28 +1560,29 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(test.TestingException, self.conductor._cold_migrate, self.context, inst_obj, flavor, - filter_props, [resvs], - clean_shutdown=True) + {}, [resvs], True, fake_spec) + + # Filter properties are populated during code execution + legacy_filter_props = {'retry': {'num_attempts': 1, + 'hosts': [['host1', None]]}, + 'limits': {}} metadata_mock.assert_called_with({}) - brs_mock.assert_called_once_with(self.context, image, - [inst_obj], - instance_type=flavor) quotas_mock.assert_called_once_with(self.context, [resvs], instance=inst_obj) - sig_mock.assert_called_once_with(self.context, request_spec, - filter_props) + sig_mock.assert_called_once_with(self.context, legacy_request_spec, + legacy_filter_props) select_dest_mock.assert_called_once_with( self.context, fake_spec) prep_resize_mock.assert_called_once_with( - self.context, image, inst_obj, flavor, - hosts[0]['host'], [resvs], - request_spec=request_spec, - filter_properties=filter_props, + self.context, legacy_request_spec['image'], + inst_obj, flavor, hosts[0]['host'], [resvs], + request_spec=legacy_request_spec, + filter_properties=legacy_filter_props, node=hosts[0]['nodename'], clean_shutdown=True) notify_mock.assert_called_once_with(self.context, inst_obj.uuid, 'migrate_server', updates, - exc_info, request_spec) + exc_info, legacy_request_spec) rollback_mock.assert_called_once_with() def test_resize_no_valid_host_error_msg(self): @@ -1611,9 +1596,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): uuid=uuids.instance, user_id=fakes.FAKE_USER_ID) - request_spec = dict(instance_type=dict(extra_specs=dict()), - instance_properties=dict()) - filter_props = dict(context=None) + fake_spec = fake_request_spec.fake_spec_obj() resvs = 'fake-resvs' image = 'fake-image' @@ -1621,7 +1604,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock.patch.object(utils, 'get_image_from_system_metadata', return_value=image), mock.patch.object(scheduler_utils, 'build_request_spec', - return_value=request_spec), + return_value=fake_spec), mock.patch.object(self.conductor, '_set_vm_state_and_notify'), mock.patch.object(migrate.MigrationTask, 'execute', @@ -1631,8 +1614,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): task_rb_mock): nvh = self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, - inst_obj, flavor_new, filter_props, - [resvs], clean_shutdown=True) + inst_obj, flavor_new, {}, + [resvs], True, fake_spec) self.assertIn('resize', nvh.message) def test_build_instances_instance_not_found(self):