From 26335270bf528a2b9c48c976110c1672137df336 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 8 Feb 2018 11:58:38 -0800 Subject: [PATCH] Clean up reservations in migrate_task call path Note that this now begs for a conductor RPC bump, which this stops short of for now. Change-Id: I6e4cd9b08e6b3798c5a6d0784654b20125c8f7be --- nova/conductor/manager.py | 12 ++++++----- nova/conductor/tasks/migrate.py | 3 +-- .../unit/conductor/tasks/test_migrate.py | 2 +- nova/tests/unit/conductor/test_conductor.py | 21 +++++++------------ 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 98efa2336b8e..f0327422fd73 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -239,6 +239,8 @@ class ComputeTaskManager(base.Base): # TODO(tdurakov): remove `live` parameter here on compute task api RPC # version bump to 2.x + # TODO(danms): remove the `reservations` parameter here on compute task api + # RPC version bump to 2.x @messaging.expected_exceptions( exception.NoValidHost, exception.ComputeServiceUnavailable, @@ -282,13 +284,13 @@ class ComputeTaskManager(base.Base): instance_uuid): self._cold_migrate(context, instance, flavor, scheduler_hint['filter_properties'], - reservations, clean_shutdown, request_spec, + clean_shutdown, request_spec, host_list) else: raise NotImplementedError() def _cold_migrate(self, context, instance, flavor, filter_properties, - reservations, clean_shutdown, request_spec, host_list): + clean_shutdown, request_spec, host_list): image = utils.get_image_from_system_metadata( instance.system_metadata) @@ -309,7 +311,7 @@ class ComputeTaskManager(base.Base): request_spec.flavor = flavor task = self._build_cold_migrate_task(context, instance, flavor, - request_spec, reservations, clean_shutdown, host_list) + request_spec, clean_shutdown, host_list) try: task.execute() except exception.NoValidHost as ex: @@ -463,10 +465,10 @@ class ComputeTaskManager(base.Base): request_spec) def _build_cold_migrate_task(self, context, instance, flavor, request_spec, - reservations, clean_shutdown, host_list): + clean_shutdown, host_list): return migrate.MigrationTask(context, instance, flavor, request_spec, - reservations, clean_shutdown, + clean_shutdown, self.compute_rpcapi, self.scheduler_client, host_list) diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index 45ff3b326391..3a654375d875 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -110,12 +110,11 @@ def should_do_migration_allocation(context): class MigrationTask(base.TaskBase): def __init__(self, context, instance, flavor, - request_spec, reservations, clean_shutdown, compute_rpcapi, + request_spec, clean_shutdown, compute_rpcapi, scheduler_client, host_list): super(MigrationTask, self).__init__(context, instance) self.clean_shutdown = clean_shutdown self.request_spec = request_spec - self.reservations = reservations self.flavor = flavor self.compute_rpcapi = compute_rpcapi diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index 968ce37a3656..0d4957f153fe 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -53,7 +53,7 @@ class MigrationTaskTestCase(test.NoDBTestCase): def _generate_task(self): return migrate.MigrationTask(self.context, self.instance, self.flavor, - self.request_spec, self.reservations, + self.request_spec, self.clean_shutdown, compute_rpcapi.ComputeAPI(), scheduler_client.SchedulerClient(), diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index a7c82eeb1ac4..4a69918b3e7c 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -2186,7 +2186,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): pci_requests=None, numa_topology=None, project_id=self.context.project_id) - resvs = 'fake-resvs' image = 'fake-image' fake_spec = objects.RequestSpec(image=objects.ImageMeta()) spec_fc_mock.return_value = fake_spec @@ -2203,7 +2202,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, inst_obj, - flavor, {}, [resvs], + flavor, {}, True, None, None) metadata_mock.assert_called_with({}) sig_mock.assert_called_once_with(self.context, fake_spec) @@ -2238,7 +2237,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): availability_zone=None, project_id=self.context.project_id) image = 'fake-image' - resvs = 'fake-resvs' fake_spec = objects.RequestSpec(image=objects.ImageMeta()) spec_fc_mock.return_value = fake_spec @@ -2255,7 +2253,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, inst_obj, - flavor, {}, [resvs], + flavor, {}, True, None, None) metadata_mock.assert_called_with({}) sig_mock.assert_called_once_with(self.context, fake_spec) @@ -2275,7 +2273,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): uuid=uuids.instance, user_id=fakes.FAKE_USER_ID) fake_spec = fake_request_spec.fake_spec_obj() - resvs = 'fake-resvs' image = 'fake-image' with test.nested( @@ -2290,7 +2287,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): task_rollback_mock): nvh = self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, - inst_obj, flavor, {}, [resvs], + inst_obj, flavor, {}, True, fake_spec, None) self.assertIn('cold migrate', nvh.message) @@ -2318,7 +2315,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): numa_topology=None, pci_requests=None, availability_zone=None) - resvs = 'fake-resvs' image = 'fake-image' exception = exc.UnsupportedPolicyException(reason='') fake_spec = fake_request_spec.fake_spec_obj() @@ -2329,7 +2325,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(exc.UnsupportedPolicyException, self.conductor._cold_migrate, self.context, - inst_obj, flavor, {}, [resvs], True, None, None) + inst_obj, flavor, {}, True, None, None) updates = {'vm_state': vm_states.STOPPED, 'task_state': None} set_vm_mock.assert_called_once_with(self.context, inst_obj.uuid, @@ -2363,7 +2359,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): numa_topology=None, project_id=self.context.project_id) image = 'fake-image' - resvs = 'fake-resvs' fake_spec = objects.RequestSpec(image=objects.ImageMeta()) legacy_request_spec = fake_spec.to_legacy_request_spec_dict() spec_fc_mock.return_value = fake_spec @@ -2383,7 +2378,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertRaises(test.TestingException, self.conductor._cold_migrate, self.context, inst_obj, flavor, - {}, [resvs], True, None, None) + {}, True, None, None) # Filter properties are populated during code execution legacy_filter_props = {'retry': {'num_attempts': 1, @@ -2425,7 +2420,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): availability_zone=None, pci_requests=None, numa_topology=None) - resvs = 'fake-resvs' image = 'fake-image' fake_spec = fake_request_spec.fake_spec_obj() @@ -2434,7 +2428,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): # the new one self.assertNotEqual(flavor, fake_spec.flavor) self.conductor._cold_migrate(self.context, inst_obj, flavor, {}, - [resvs], True, fake_spec, None) + True, fake_spec, None) # Now the RequestSpec should be updated... self.assertEqual(flavor, fake_spec.flavor) @@ -2453,7 +2447,6 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): user_id=fakes.FAKE_USER_ID) fake_spec = fake_request_spec.fake_spec_obj() - resvs = 'fake-resvs' image = 'fake-image' with test.nested( @@ -2471,7 +2464,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): nvh = self.assertRaises(exc.NoValidHost, self.conductor._cold_migrate, self.context, inst_obj, flavor_new, {}, - [resvs], True, fake_spec, None) + True, fake_spec, None) self.assertIn('resize', nvh.message) @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid')