Merge "Stop providing force_hosts to the scheduler for move ops" into stable/mitaka

6 files changed, 47 insertions, 3 deletions

diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 199d87078a..2f6d89e9d8 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 93560c2347..a105835bf4 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 0a0ff944b2..573d96c0a7 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 ff48f98c42..44cb4781d6 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 d23c2bc08c..b89e82fac5 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 f5fc439353..836d6a5588 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):