diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 9f89a6c9df..ceee22dcb7 100644 --- a/nova/conductor/manager.py +++ b/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( diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index f329ddfe38..afcb7c6157 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/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 diff --git a/nova/tests/functional/regressions/test_bug_1823370.py b/nova/tests/functional/regressions/test_bug_1823370.py index c21c597a78..ffbbf1183b 100644 --- a/nova/tests/functional/regressions/test_bug_1823370.py +++ b/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']) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index fedb5c3c17..a0ee8dcdf7 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/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)