diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 199d87078a6e..2f6d89e9d8c0 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -482,6 +482,10 @@ class ComputeTaskManager(base.Base): request_spec = scheduler_utils.build_request_spec( context, image, [instance]) else: + # 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 + request_spec.reset_forced_destinations() # TODO(sbauza): Provide directly the RequestSpec object # when _schedule_instances(), # populate_filter_properties and populate_retry() @@ -543,6 +547,10 @@ class ComputeTaskManager(base.Base): # the source host for avoiding the scheduler to pick it request_spec.ignore_hosts = request_spec.ignore_hosts or [] request_spec.ignore_hosts.append(instance.host) + # 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 + request_spec.reset_forced_destinations() # TODO(sbauza): Provide directly the RequestSpec object # when _schedule_instances() and _set_vm_state_and_notify() # accept it diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index 93560c2347c2..a105835bf4e3 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -184,6 +184,10 @@ class LiveMigrationTask(base.TaskBase): ) else: request_spec = self.request_spec + # 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 + request_spec.reset_forced_destinations() host = None while host is None: diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index 0a0ff944b29b..573d96c0a73d 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -450,6 +450,18 @@ class RequestSpec(base.NovaObject): self._from_db_object(self._context, self, db_spec) self.obj_reset_changes() + def reset_forced_destinations(self): + """Clears the forced destination fields from the RequestSpec object. + + This method is for making sure we don't ask the scheduler to give us + again the same destination(s) without persisting the modifications. + """ + self.force_hosts = None + self.force_nodes = None + # NOTE(sbauza): Make sure we don't persist this, we need to keep the + # original request for the forced hosts + self.obj_reset_changes(['force_hosts', 'force_nodes']) + @base.NovaObjectRegistry.register class SchedulerRetries(base.NovaObject): diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index ff48f98c4279..44cb4781d641 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -254,6 +254,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): def test_find_destination_works(self): self.mox.StubOutWithMock(utils, 'get_image_from_system_metadata') self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group') + self.mox.StubOutWithMock(objects.RequestSpec, + 'reset_forced_destinations') self.mox.StubOutWithMock(self.task.scheduler_client, 'select_destinations') self.mox.StubOutWithMock(self.task, @@ -265,6 +267,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): fake_props = {'instance_properties': {'uuid': self.instance_uuid}} scheduler_utils.setup_instance_group( self.context, fake_props, {'ignore_hosts': [self.instance_host]}) + self.fake_spec.reset_forced_destinations() self.task.scheduler_client.select_destinations( self.context, self.fake_spec).AndReturn( [{'host': 'host1'}]) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index d23c2bc08c75..b89e82fac5ca 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -678,13 +678,16 @@ class _BaseTaskTestCase(object): @mock.patch.object(objects.RequestSpec, 'to_legacy_request_spec_dict') @mock.patch.object(objects.RequestSpec, 'to_legacy_filter_properties_dict') - def do_test(to_filtprops, to_reqspec, sched_instances, + @mock.patch.object(objects.RequestSpec, 'reset_forced_destinations') + def do_test(reset_forced_destinations, + to_filtprops, to_reqspec, sched_instances, populate_retry, populate_filter_properties, unshelve_instance): to_filtprops.return_value = filter_properties to_reqspec.return_value = request_spec sched_instances.return_value = [host] self.conductor.unshelve_instance(self.context, instance, fake_spec) + reset_forced_destinations.assert_called_once_with() sched_instances.assert_called_once_with(self.context, request_spec, filter_properties) # NOTE(sbauza): Since the instance is dehydrated when passing thru @@ -1038,15 +1041,17 @@ class _BaseTaskTestCase(object): return_value=[{'host': expected_host, 'nodename': expected_node, 'limits': expected_limits}]), + mock.patch.object(fake_spec, 'reset_forced_destinations'), mock.patch.object(fake_spec, 'to_legacy_request_spec_dict', return_value=request_spec), mock.patch.object(fake_spec, 'to_legacy_filter_properties_dict', return_value=filter_properties), - ) as (rebuild_mock, sig_mock, fp_mock, select_dest_mock, to_reqspec, - to_filtprops): + ) as (rebuild_mock, sig_mock, fp_mock, select_dest_mock, reset_fd, + to_reqspec, to_filtprops): self.conductor_manager.rebuild_instance(context=self.context, instance=inst_obj, **rebuild_args) + reset_fd.assert_called_once_with() to_reqspec.assert_called_once_with() to_filtprops.assert_called_once_with() fp_mock.assert_called_once_with(self.context, request_spec, diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index f5fc43935322..836d6a55884d 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -531,6 +531,18 @@ class _TestRequestSpecObject(object): _test_save_args): req_obj.save() + def test_reset_forced_destinations(self): + req_obj = fake_request_spec.fake_spec_obj() + # Making sure the fake object has forced hosts and nodes + self.assertIsNotNone(req_obj.force_hosts) + self.assertIsNotNone(req_obj.force_nodes) + + with mock.patch.object(req_obj, 'obj_reset_changes') as mock_reset: + req_obj.reset_forced_destinations() + self.assertIsNone(req_obj.force_hosts) + self.assertIsNone(req_obj.force_nodes) + mock_reset.assert_called_once_with(['force_hosts', 'force_nodes']) + class TestRequestSpecObject(test_objects._LocalTest, _TestRequestSpecObject):