Merge "Restrict RequestSpec to cell when evacuating"

nova/conductor/manager.py

@@ -860,6 +860,32 @@ class ComputeTaskManager(base.Base):
                   'instance(s).', timer.elapsed(), len(instance_uuids))
         return host_lists
 
+    @staticmethod
+    def _restrict_request_spec_to_cell(context, instance, request_spec):
+        """Sets RequestSpec.requested_destination.cell for the move operation
+
+        Move operations, e.g. evacuate and unshelve, must be restricted to the
+        cell in which the instance already exists, so this method is used to
+        target the RequestSpec, which is sent to the scheduler via the
+        _schedule_instances method, to the instance's current cell.
+
+        :param context: nova auth RequestContext
+        """
+        instance_mapping = \
+            objects.InstanceMapping.get_by_instance_uuid(
+                context, instance.uuid)
+        LOG.debug('Requesting cell %(cell)s during scheduling',
+                  {'cell': instance_mapping.cell_mapping.identity},
+                  instance=instance)
+        if ('requested_destination' in request_spec and
+                request_spec.requested_destination):
+            request_spec.requested_destination.cell = (
+                instance_mapping.cell_mapping)
+        else:
+            request_spec.requested_destination = (
+                objects.Destination(
+                    cell=instance_mapping.cell_mapping))
+
     # TODO(mriedem): Make request_spec required in ComputeTaskAPI RPC v2.0.
     @targets_cell
     def unshelve_instance(self, context, instance, request_spec=None):
@@ -918,20 +944,8 @@ class ComputeTaskManager(base.Base):
 
                     # NOTE(cfriesen): Ensure that we restrict the scheduler to
                     # the cell specified by the instance mapping.
-                    instance_mapping = \
-                        objects.InstanceMapping.get_by_instance_uuid(
-                            context, instance.uuid)
-                    LOG.debug('Requesting cell %(cell)s while unshelving',
-                              {'cell': instance_mapping.cell_mapping.identity},
-                              instance=instance)
-                    if ('requested_destination' in request_spec and
-                            request_spec.requested_destination):
-                        request_spec.requested_destination.cell = (
-                            instance_mapping.cell_mapping)
-                    else:
-                        request_spec.requested_destination = (
-                            objects.Destination(
-                                cell=instance_mapping.cell_mapping))
+                    self._restrict_request_spec_to_cell(
+                        context, instance, request_spec)
 
                     request_spec.ensure_project_and_user_id(instance)
                     request_spec.ensure_network_metadata(instance)
@@ -1114,6 +1128,8 @@ class ComputeTaskManager(base.Base):
                         self._validate_image_traits_for_rebuild(context,
                                                                 instance,
                                                                 image_ref)
+                    self._restrict_request_spec_to_cell(
+                        context, instance, request_spec)
                     request_spec.ensure_project_and_user_id(instance)
                     request_spec.ensure_network_metadata(instance)
                     compute_utils.heal_reqspec_is_bfv(

nova/tests/functional/notification_sample_tests/test_instance.py

@@ -1269,7 +1269,8 @@ class TestInstanceNotificationSample(
                 'image.tags': [],
                 'scheduler_hints': {'_nova_check_type': ['rebuild']},
                 'force_hosts': 'compute',
-                'force_nodes': 'fake-mini'})
+                'force_nodes': 'fake-mini',
+                'requested_destination': self._build_destination_payload()})
 
         # 0. instance.rebuild_scheduled
         # 1. instance.exists
@@ -1365,7 +1366,8 @@ class TestInstanceNotificationSample(
                 'image.tags': [],
                 'scheduler_hints': {'_nova_check_type': ['rebuild']},
                 'force_hosts': 'compute',
-                'force_nodes': 'fake-mini'})
+                'force_nodes': 'fake-mini',
+                'requested_destination': self._build_destination_payload()})
 
         # 0. instance.rebuild_scheduled
         # 1. instance.exists

nova/tests/functional/regressions/test_bug_1823370.py

@@ -73,8 +73,4 @@ class MultiCellEvacuateTestCase(integrated_helpers._IntegratedTestBase,
         self.api.post_server_action(server['id'], req)
         self._wait_for_migration_status(server, ['done'])
         server = self._wait_for_state_change(self.api, server, 'ACTIVE')
-        # FIXME(mriedem): This is bug 1823370 where conductor does not restrict
-        # the RequestSpec to the origin cell before calling the scheduler to
-        # pick a new host so the host (host2) in the other cell (cell2) is
-        # incorrectly picked.
-        self.assertEqual('host2', server['OS-EXT-SRV-ATTR:host'])
+        self.assertEqual('host3', server['OS-EXT-SRV-ATTR:host'])

nova/tests/unit/conductor/test_conductor.py

@@ -1735,6 +1735,10 @@ class _BaseTaskTestCase(object):
             reset_fd.assert_called_once_with()
             # The RequestSpec.ignore_hosts field should be overwritten.
             self.assertEqual([inst_obj.host], fake_spec.ignore_hosts)
+            # The RequestSpec.requested_destination.cell field should be set.
+            self.assertIn('requested_destination', fake_spec)
+            self.assertIn('cell', fake_spec.requested_destination)
+            self.assertIsNotNone(fake_spec.requested_destination.cell)
             select_dest_mock.assert_called_once_with(self.context,
                     fake_spec, [inst_obj.uuid], return_objects=True,
                     return_alternates=False)