Handle target host cross-cell cold migration in conductor

Because of change If7ea02df42d220c5042947efdef4777509492a0b,
when cold migrating with a target host, the cell on the
requested destination will be used to lookup that host
(compute node) to set the in_tree attribute on the
RequestGroup. When doing a targeted cold migration across
cells, the target host could be in another cell from the
Destination.cell (which is the source cell per the
MigrationTask._restrict_request_spec_to_cell method).
In this scenario, we aren't "preferring" the source cell
necessarily if the target host is in another cell.

This change simply removes the Destination.cell attribute
during a cold migration with a target host so that the
scheduler will look in all enabled cells for the host.
Alternatively, we could lookup the HostMapping for the
requested target host and set the Destination.cell attribute
appropriately but HostManager.get_compute_nodes_by_host_or_node
is going to do that for us anyway.

This also implements one of the TODO functional tests for
cross-cell migrate by targeting a specific host in another
cell for the cold migration. The functional test will fail
without the fix in conductor.

Part of blueprint cross-cell-resize

Change-Id: I2f61a4513135c9b514d938ca18c2c9c87f24403a

nova/conductor/tasks/migrate.py

@@ -178,8 +178,15 @@ class MigrationTask(base.TaskBase):
         # cell.
         cross_cell_allowed = (
             self.request_spec.requested_destination.allow_cross_cell_move)
-        self.request_spec.requested_destination.cell = (
-            instance_mapping.cell_mapping)
+        if targeted and cross_cell_allowed:
+            # If a target host is specified it might be in another cell so
+            # we cannot restrict the cell in this case. We would not prefer
+            # the source cell in that case either since we know where the
+            # user wants it to go. We just let the scheduler figure it out.
+            self.request_spec.requested_destination.cell = None
+        else:
+            self.request_spec.requested_destination.cell = (
+                instance_mapping.cell_mapping)
 
         # NOTE(takashin): In the case that the target host is specified,
         # if the migration is failed, it is not necessary to retry
@@ -190,7 +197,10 @@ class MigrationTask(base.TaskBase):
             self.request_spec.retry = None
 
         # Log our plan before calling the scheduler.
-        if cross_cell_allowed:
+        if cross_cell_allowed and targeted:
+            LOG.debug('Not restricting cell for targeted cold migration.',
+                      instance=self.instance)
+        elif cross_cell_allowed:
             LOG.debug('Allowing migration from cell %(cell)s',
                       {'cell': instance_mapping.cell_mapping.identity},
                       instance=self.instance)

nova/tests/functional/test_cross_cell_migrate.py

@@ -176,15 +176,21 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase):
         # Before resizing make sure quota usage is only 1 for total instances.
         self.assert_quota_usage(expected_num_instances=1)
 
-        # Resize it which should migrate the server to the host in the
-        # other cell.
-        new_flavor = flavors[1]
-        body = {'resize': {'flavorRef': new_flavor['id']}}
+        if target_host:
+            # Cold migrate the server to the target host.
+            new_flavor = old_flavor  # flavor does not change for cold migrate
+            body = {'migrate': {'host': target_host}}
+            expected_host = target_host
+        else:
+            # Resize it which should migrate the server to the host in the
+            # other cell.
+            new_flavor = flavors[1]
+            body = {'resize': {'flavorRef': new_flavor['id']}}
+            expected_host = 'host1' if original_host == 'host2' else 'host2'
         self.api.post_server_action(server['id'], body)
         # Wait for the server to be resized and then verify the host has
         # changed to be the host in the other cell.
         server = self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE')
-        expected_host = 'host1' if original_host == 'host2' else 'host2'
         self.assertEqual(expected_host, server['OS-EXT-SRV-ATTR:host'])
         # Assert that the instance is only listed one time from the API (to
         # make sure it's not listed out of both cells).
@@ -256,7 +262,12 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase):
                 inst.new_flavor,
                 'instance.new_flavor not saved in target cell')
             self.assertEqual(inst.flavor.flavorid, inst.new_flavor.flavorid)
-            self.assertNotEqual(inst.flavor.flavorid, inst.old_flavor.flavorid)
+            if target_host:  # cold migrate so flavor does not change
+                self.assertEqual(
+                    inst.flavor.flavorid, inst.old_flavor.flavorid)
+            else:
+                self.assertNotEqual(
+                    inst.flavor.flavorid, inst.old_flavor.flavorid)
             self.assertEqual(old_flavor['id'], inst.old_flavor.flavorid)
             self.assertEqual(new_flavor['id'], inst.new_flavor.flavorid)
             # Assert the ComputeManager._set_instance_info fields
@@ -349,8 +360,17 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase):
             instance = objects.Instance.get_by_uuid(cctxt, server['id'])
             self.assertTrue(instance.hidden)
 
-    # TODO(mriedem): Test cold migration with a specified target host in
-    # another cell.
+    def test_cold_migrate_target_host_in_other_cell(self):
+        """Tests cold migrating to a target host in another cell. This is
+        mostly just to ensure the API does not restrict the target host to
+        the source cell when cross-cell resize is allowed by policy.
+        """
+        # _resize_and_validate creates the server on host1 which is in cell1.
+        # To make things interesting, start a third host but in cell1 so we can
+        # be sure the requested host from cell2 is honored.
+        self._start_compute(
+            'host3', cell_name=self.host_to_cell_mappings['host1'])
+        self._resize_and_validate(target_host='host2')
 
     # TODO(mriedem): Test cross-cell list where the source cell has two
     # hosts so the CrossCellWeigher picks the other host in the source cell

nova/tests/unit/conductor/tasks/test_migrate.py

@@ -876,6 +876,27 @@ class MigrationTaskTestCase(test.NoDBTestCase):
         mock_debug.assert_called_once()
         self.assertIn('Allowing migration from cell',
                       mock_debug.call_args[0][0])
+        self.assertEqual(mock_get_im.return_value.cell_mapping,
+                         self.request_spec.requested_destination.cell)
+
+    @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid',
+                return_value=objects.InstanceMapping(
+                    cell_mapping=objects.CellMapping(uuid=uuids.cell1)))
+    @mock.patch('nova.conductor.tasks.migrate.LOG.debug')
+    def test_set_requested_destination_cell_allow_cross_cell_resize_true_host(
+            self, mock_debug, mock_get_im):
+        """Tests the scenario that the RequestSpec is configured for
+        allow_cross_cell_resize=True and there is a requested target host.
+        """
+        task = self._generate_task()
+        legacy_props = self.request_spec.to_legacy_filter_properties_dict()
+        self.request_spec.requested_destination = objects.Destination(
+            allow_cross_cell_move=True, host='fake-host')
+        task._set_requested_destination_cell(legacy_props)
+        mock_get_im.assert_called_once_with(self.context, self.instance.uuid)
+        mock_debug.assert_called_once()
+        self.assertIn('Not restricting cell', mock_debug.call_args[0][0])
+        self.assertIsNone(self.request_spec.requested_destination.cell)
 
     @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid',
                 return_value=objects.InstanceMapping(