openstack
/
nova
Code Issues Proposed changes

Merge "Enable cold migration with target host(1/2)"

This commit is contained in:
Zuul 2017-11-28 21:00:28 +00:00 committed by Gerrit Code Review
parent 379bbf5183 81ffd2065d
commit 05c6b54eec
5 changed files with 177 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
nova/compute/api.py
View File

@ -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,

16
nova/conductor/tasks/migrate.py
View File

@ -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)

9
nova/exception.py
View File

@ -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.")

108
nova/tests/unit/compute/test_compute_api.py
View File

@ -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.

26
nova/tests/unit/conductor/tasks/test_migrate.py
View File

@ -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')