diff --git a/nova/compute/api.py b/nova/compute/api.py index 73349ca9fb..a2ec43c632 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3090,13 +3090,25 @@ class API(base.Base): @check_instance_cell @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED]) def resize(self, context, instance, flavor_id=None, clean_shutdown=True, - **extra_instance_updates): + host_name=None, **extra_instance_updates): """Resize (ie, migrate) a running instance. If flavor_id is None, the process is considered a migration, keeping the original flavor_id. If flavor_id is not None, the instance should be migrated to a new host and resized to the new flavor_id. + host_name is always None in the resize case. + host_name can be set in the cold migration case only. """ + if host_name is not None: + # Check whether host exists or not. + node = objects.ComputeNode.get_first_node_by_host_for_old_compat( + context, host_name, use_slave=True) + + # Cannot migrate to the host where the instance exists + # because it is useless. + if host_name == instance.host: + raise exception.CannotMigrateToSameHost() + self._check_auto_disk_config(instance, **extra_instance_updates) current_instance_type = instance.get_flavor() @@ -3178,6 +3190,10 @@ class API(base.Base): except exception.RequestSpecNotFound: # Some old instances can still have no RequestSpec object attached # to them, we need to support the old way + if host_name is not None: + # If there is no request spec we cannot honor the request + # and we need to fail. + raise exception.CannotMigrateWithTargetHost() request_spec = None # TODO(melwitt): We're not rechecking for strict quota here to guard @@ -3185,6 +3201,22 @@ class API(base.Base): # resource consumption for this operation is written to the database # by compute. scheduler_hint = {'filter_properties': filter_properties} + + if request_spec: + if host_name is None: + # If 'host_name' is not specified, + # clear the 'requested_destination' field of the RequestSpec. + request_spec.requested_destination = None + else: + # Set the host and the node so that the scheduler will + # validate them. + # TODO(takashin): It will be added to check whether + # the specified host is within the same cell as + # the instance or not. If not, raise specific error message + # that is clear to the caller. + request_spec.requested_destination = objects.Destination( + host=node.host, node=node.hypervisor_hostname) + self.compute_task_api.resize_instance(context, instance, extra_instance_updates, scheduler_hint=scheduler_hint, flavor=new_instance_type, diff --git a/nova/conductor/tasks/migrate.py b/nova/conductor/tasks/migrate.py index d039754d86..4d2b37da26 100644 --- a/nova/conductor/tasks/migrate.py +++ b/nova/conductor/tasks/migrate.py @@ -202,8 +202,13 @@ class MigrationTask(base.TaskBase): legacy_spec = self.request_spec.to_legacy_request_spec_dict() legacy_props = self.request_spec.to_legacy_filter_properties_dict() scheduler_utils.setup_instance_group(self.context, self.request_spec) - scheduler_utils.populate_retry(legacy_props, - self.instance.uuid) + # If a target host is set in a requested destination, + # 'populate_retry' need not be executed. + if not ('requested_destination' in self.request_spec and + self.request_spec.requested_destination and + 'host' in self.request_spec.requested_destination): + scheduler_utils.populate_retry(legacy_props, + self.instance.uuid) # NOTE(sbauza): Force_hosts/nodes needs to be reset # if we want to make sure that the next destination @@ -222,6 +227,13 @@ class MigrationTask(base.TaskBase): self.request_spec.requested_destination): 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 + # the cold migration to the same host. So make sure that + # reschedule will not occur. + if 'host' in self.request_spec.requested_destination: + legacy_props.pop('retry', None) + self.request_spec.retry = None else: self.request_spec.requested_destination = objects.Destination( cell=instance_mapping.cell_mapping) diff --git a/nova/exception.py b/nova/exception.py index daa47d5dc2..52f56aeb76 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -2183,3 +2183,12 @@ class TraitCannotDeleteStandard(Invalid): class TraitInUse(Invalid): msg_fmt = _("The trait %(name)s is in use by a resource provider.") + + +class CannotMigrateWithTargetHost(NovaException): + msg_fmt = _("Cannot migrate with target host. Retry without a host " + "specified.") + + +class CannotMigrateToSameHost(NovaException): + msg_fmt = _("Cannot migrate to the host where the server exists.") diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index afe7e93bab..abc5ded184 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1641,12 +1641,19 @@ class _ComputeAPIUnitTestMixIn(object): self.context, fake_inst['uuid'], 'finished') mock_inst_save.assert_called_once_with(expected_task_state=[None]) - def _test_resize(self, flavor_id_passed=True, + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') + def _test_resize(self, mock_get_all_by_host, + mock_get_by_instance_uuid, + flavor_id_passed=True, same_host=False, allow_same_host=False, project_id=None, extra_kwargs=None, same_flavor=False, - clean_shutdown=True): + clean_shutdown=True, + host_name=None, + request_spec=True, + requested_destination=False): if extra_kwargs is None: extra_kwargs = {} @@ -1665,10 +1672,13 @@ class _ComputeAPIUnitTestMixIn(object): self.mox.StubOutWithMock(quotas_obj.Quotas, 'limit_check_project_and_user') self.mox.StubOutWithMock(self.compute_api, '_record_action_start') - self.mox.StubOutWithMock(objects.RequestSpec, 'get_by_instance_uuid') self.mox.StubOutWithMock(self.compute_api.compute_task_api, 'resize_instance') + if host_name: + mock_get_all_by_host.return_value = [objects.ComputeNode( + host=host_name, hypervisor_hostname='hypervisor_host')] + current_flavor = fake_inst.get_flavor() if flavor_id_passed: new_flavor = self._create_flavor(id=200, flavorid='new-flavor-id', @@ -1755,9 +1765,17 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api._record_action_start(self.context, fake_inst, 'migrate') - fake_spec = objects.RequestSpec() - objects.RequestSpec.get_by_instance_uuid( - self.context, fake_inst.uuid).AndReturn(fake_spec) + if request_spec: + fake_spec = objects.RequestSpec() + if requested_destination: + cell1 = objects.CellMapping(uuid=uuids.cell1, name='cell1') + fake_spec.requested_destination = objects.Destination( + host='dummy_host', node='dummy_node', cell=cell1) + mock_get_by_instance_uuid.return_value = fake_spec + else: + mock_get_by_instance_uuid.side_effect = ( + exception.RequestSpecNotFound(instance_uuid=fake_inst.id)) + fake_spec = None scheduler_hint = {'filter_properties': filter_properties} @@ -1774,16 +1792,29 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.resize(self.context, fake_inst, flavor_id='new-flavor-id', clean_shutdown=clean_shutdown, + host_name=host_name, **extra_kwargs) else: self.compute_api.resize(self.context, fake_inst, clean_shutdown=clean_shutdown, + host_name=host_name, **extra_kwargs) - if allow_same_host: - self.assertEqual([], fake_spec.ignore_hosts) - else: - self.assertEqual([fake_inst['host']], fake_spec.ignore_hosts) + if request_spec: + if allow_same_host: + self.assertEqual([], fake_spec.ignore_hosts) + else: + self.assertEqual([fake_inst['host']], fake_spec.ignore_hosts) + + if host_name is None: + self.assertIsNone(fake_spec.requested_destination) + else: + self.assertIn('host', fake_spec.requested_destination) + self.assertEqual(host_name, + fake_spec.requested_destination.host) + self.assertIn('node', fake_spec.requested_destination) + self.assertEqual('hypervisor_host', + fake_spec.requested_destination.node) def _test_migrate(self, *args, **kwargs): self._test_resize(*args, flavor_id_passed=False, **kwargs) @@ -1852,6 +1883,63 @@ class _ComputeAPIUnitTestMixIn(object): def test_migrate_different_project_id(self): self._test_migrate(project_id='different') + def test_migrate_request_spec_not_found(self): + self._test_migrate(request_spec=False) + + @mock.patch.object(objects.Migration, 'create') + @mock.patch.object(objects.InstanceAction, 'action_start') + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + return_value=[objects.ComputeNode( + host='target_host', + hypervisor_hostname='hypervisor_host')]) + def test_migrate_request_spec_not_found_with_target_host( + self, mock_get_all_by_host, mock_get_by_instance_uuid, mock_save, + mock_action_start, mock_migration_create): + fake_inst = self._create_instance_obj() + mock_get_by_instance_uuid.side_effect = ( + exception.RequestSpecNotFound(instance_uuid=fake_inst.uuid)) + self.assertRaises(exception.CannotMigrateWithTargetHost, + self.compute_api.resize, self.context, + fake_inst, host_name='target_host') + mock_get_all_by_host.assert_called_once_with( + self.context, 'target_host', True) + mock_get_by_instance_uuid.assert_called_once_with(self.context, + fake_inst.uuid) + mock_save.assert_called_once_with(expected_task_state=[None]) + mock_action_start.assert_called_once_with( + self.context, fake_inst.uuid, instance_actions.MIGRATE, + want_result=False) + if self.cell_type == 'api': + mock_migration_create.assert_called_once_with() + + def test_migrate_with_requested_destination(self): + # RequestSpec has requested_destination + self._test_migrate(requested_destination=True) + + def test_migrate_with_host_name(self): + self._test_migrate(host_name='target_host') + + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + side_effect=exception.ComputeHostNotFound( + host='nonexistent_host')) + def test_migrate_nonexistent_host(self, mock_get_all_by_host): + fake_inst = self._create_instance_obj() + self.assertRaises(exception.ComputeHostNotFound, + self.compute_api.resize, self.context, + fake_inst, host_name='nonexistent_host') + + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + return_value=[objects.ComputeNode( + host='fake_host', + hypervisor_hostname='hypervisor_host')]) + def test_migrate_to_same_host(self, mock_get_all_by_host): + fake_inst = self._create_instance_obj() + self.assertRaises(exception.CannotMigrateToSameHost, + self.compute_api.resize, self.context, + fake_inst, host_name='fake_host') + def test_resize_invalid_flavor_fails(self): self.mox.StubOutWithMock(flavors, 'get_flavor_by_flavor_id') # Should never reach these. diff --git a/nova/tests/unit/conductor/tasks/test_migrate.py b/nova/tests/unit/conductor/tasks/test_migrate.py index dcf6d66e07..3dea062ef4 100644 --- a/nova/tests/unit/conductor/tasks/test_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_migrate.py @@ -92,10 +92,21 @@ class MigrationTaskTestCase(test.NoDBTestCase): @mock.patch.object(scheduler_utils, 'setup_instance_group') @mock.patch.object(scheduler_client.SchedulerClient, 'select_destinations') @mock.patch.object(compute_rpcapi.ComputeAPI, 'prep_resize') - def test_execute(self, prep_resize_mock, sel_dest_mock, sig_mock, az_mock, - gmv_mock, cm_mock, sm_mock, cn_mock, rc_mock): + def _test_execute(self, prep_resize_mock, sel_dest_mock, sig_mock, az_mock, + gmv_mock, cm_mock, sm_mock, cn_mock, rc_mock, + requested_destination=False): sel_dest_mock.return_value = self.hosts az_mock.return_value = 'myaz' + + if requested_destination: + self.request_spec.requested_destination = objects.Destination( + host='target_host', node=None) + self.request_spec.retry = objects.SchedulerRetries.from_dict( + self.context, self.filter_properties['retry']) + self.filter_properties.pop('retry') + self.filter_properties['requested_destination'] = ( + self.request_spec.requested_destination) + task = self._generate_task() legacy_request_spec = self.request_spec.to_legacy_request_spec_dict() gmv_mock.return_value = 23 @@ -140,10 +151,19 @@ class MigrationTaskTestCase(test.NoDBTestCase): task._migration.create.assert_called_once_with() + if requested_destination: + self.assertIsNone(self.request_spec.retry) + + def test_execute(self): + self._test_execute() + + def test_execute_with_destination(self): + self._test_execute(requested_destination=True) + def test_execute_resize(self): self.flavor = self.flavor.obj_clone() self.flavor.id = 3 - self.test_execute() + self._test_execute() @mock.patch('nova.conductor.tasks.migrate.revert_allocation_for_migration') @mock.patch('nova.scheduler.client.report.SchedulerReportClient')