openstack
/
nova
Code Issues Proposed changes

Merge "Plumb allow_cross_cell_resize into compute API resize()"

This commit is contained in:
Zuul 2019-11-12 02:42:44 +00:00 committed by Gerrit Code Review
parent 17b5a1ab85 a2e53cacd3
commit ee16ae1b39
5 changed files with 205 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

99
nova/compute/api.py
View File

@ -3639,6 +3639,74 @@ class API(base.Base):
migration,
migration.source_compute)
@staticmethod
def _allow_cross_cell_resize(context, instance):
"""Determine if the request can perform a cross-cell resize on this
instance.
:param context: nova auth request context for the resize operation
:param instance: Instance object being resized
:returns: True if cross-cell resize is allowed, False otherwise
"""
# TODO(mriedem): Uncomment this when confirm/revert are done. For now
# this method can just be used to internally enable the feature for
# functional testing.
# TODO(mriedem): If true, we should probably check if all of the
# computes are running a high enough service version and if not, do
# what? Raise an exception or just log something and return False?
# return context.can(
# server_policies.CROSS_CELL_RESIZE,
# target={'user_id': instance.user_id,
# 'project_id': instance.project_id},
# fatal=False)
return False
@staticmethod
def _validate_host_for_cold_migrate(
context, instance, host_name, allow_cross_cell_resize):
"""Validates a host specified for cold migration.
:param context: nova auth request context for the cold migration
:param instance: Instance object being cold migrated
:param host_name: User-specified compute service hostname for the
desired destination of the instance during the cold migration
:param allow_cross_cell_resize: If True, cross-cell resize is allowed
for this operation and the host could be in a different cell from
the one that the instance is currently in. If False, the speciifed
host must be in the same cell as the instance.
:returns: ComputeNode object of the requested host
:raises: CannotMigrateToSameHost if the host is the same as the
current instance.host
:raises: ComputeHostNotFound if the specified host cannot be found
"""
# Cannot migrate to the host where the instance exists
# because it is useless.
if host_name == instance.host:
raise exception.CannotMigrateToSameHost()
# Check whether host exists or not. If a cross-cell resize is
# allowed, the host could be in another cell from the one the
# instance is currently in, so we need to lookup the HostMapping
# to get the cell and lookup the ComputeNode in that cell.
if allow_cross_cell_resize:
try:
hm = objects.HostMapping.get_by_host(context, host_name)
except exception.HostMappingNotFound:
LOG.info('HostMapping not found for host: %s', host_name)
raise exception.ComputeHostNotFound(host=host_name)
with nova_context.target_cell(context, hm.cell_mapping) as cctxt:
node = objects.ComputeNode.\
get_first_node_by_host_for_old_compat(
cctxt, host_name, use_slave=True)
else:
node = objects.ComputeNode.get_first_node_by_host_for_old_compat(
context, host_name, use_slave=True)
return node
# TODO(mriedem): It looks like for resize (not cold migrate) the only
# possible kwarg here is auto_disk_config. Drop this dumb **kwargs and make
# it explicitly an auto_disk_config param
@ -3654,15 +3722,12 @@ class API(base.Base):
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:
# Cannot migrate to the host where the instance exists
# because it is useless.
if host_name == instance.host:
raise exception.CannotMigrateToSameHost()
allow_cross_cell_resize = self._allow_cross_cell_resize(
context, instance)
# Check whether host exists or not.
node = objects.ComputeNode.get_first_node_by_host_for_old_compat(
context, host_name, use_slave=True)
if host_name is not None:
node = self._validate_host_for_cold_migrate(
context, instance, host_name, allow_cross_cell_resize)
self._check_auto_disk_config(instance, **extra_instance_updates)
@ -3762,19 +3827,29 @@ class API(base.Base):
if host_name is None:
# If 'host_name' is not specified,
# clear the 'requested_destination' field of the RequestSpec.
request_spec.requested_destination = None
# clear the 'requested_destination' field of the RequestSpec
# except set the allow_cross_cell_move flag since conductor uses
# it prior to scheduling.
request_spec.requested_destination = objects.Destination(
allow_cross_cell_move=allow_cross_cell_resize)
else:
# Set the host and the node so that the scheduler will
# validate them.
request_spec.requested_destination = objects.Destination(
host=node.host, node=node.hypervisor_hostname)
host=node.host, node=node.hypervisor_hostname,
allow_cross_cell_move=allow_cross_cell_resize)
# This is by default a synchronous RPC call to conductor which does not
# return until the MigrationTask in conductor RPC casts to the
# prep_resize method on the selected destination nova-compute service.
# However, if we are allowed to do a cross-cell resize, then we
# asynchronously RPC cast since the CrossCellMigrationTask is blocking.
self.compute_task_api.resize_instance(context, instance,
scheduler_hint=scheduler_hint,
flavor=new_instance_type,
clean_shutdown=clean_shutdown,
request_spec=request_spec)
request_spec=request_spec,
do_cast=allow_cross_cell_resize)
@check_instance_lock
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED,

5
nova/conductor/api.py
View File

@ -90,12 +90,13 @@ class ComputeTaskAPI(object):
# reservations anymore
def resize_instance(self, context, instance, scheduler_hint, flavor,
reservations=None, clean_shutdown=True,
request_spec=None, host_list=None):
request_spec=None, host_list=None, do_cast=False):
self.conductor_compute_rpcapi.migrate_server(
context, instance, scheduler_hint, live=False, rebuild=False,
flavor=flavor, block_migration=None, disk_over_commit=None,
reservations=reservations, clean_shutdown=clean_shutdown,
request_spec=request_spec, host_list=host_list)
request_spec=request_spec, host_list=host_list,
do_cast=do_cast)
def live_migrate_instance(self, context, instance, host_name,
block_migration, disk_over_commit,

4
nova/conductor/rpcapi.py
View File

@ -311,7 +311,7 @@ class ComputeTaskAPI(object):
def migrate_server(self, context, instance, scheduler_hint, live, rebuild,
flavor, block_migration, disk_over_commit,
reservations=None, clean_shutdown=True, request_spec=None,
host_list=None):
host_list=None, do_cast=False):
kw = {'instance': instance, 'scheduler_hint': scheduler_hint,
'live': live, 'rebuild': rebuild, 'flavor': flavor,
'block_migration': block_migration,
@ -342,6 +342,8 @@ class ComputeTaskAPI(object):
version=version,
call_monitor_timeout=CONF.rpc_response_timeout,
timeout=CONF.long_rpc_timeout)
if do_cast:
return cctxt.cast(context, 'migrate_server', **kw)
return cctxt.call(context, 'migrate_server', **kw)
def build_instances(self, context, instances, image, filter_properties,

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

@ -1775,7 +1775,8 @@ class _ComputeAPIUnitTestMixIn(object):
clean_shutdown=True,
host_name=None,
request_spec=True,
requested_destination=False):
requested_destination=False,
allow_cross_cell_resize=False):
self.flags(allow_resize_to_same_host=allow_same_host)
@ -1841,6 +1842,11 @@ class _ComputeAPIUnitTestMixIn(object):
scheduler_hint = {'filter_properties': filter_properties}
mock_allow_cross_cell_resize = self.useFixture(
fixtures.MockPatchObject(
self.compute_api, '_allow_cross_cell_resize')).mock
mock_allow_cross_cell_resize.return_value = allow_cross_cell_resize
if flavor_id_passed:
self.compute_api.resize(self.context, fake_inst,
flavor_id='new-flavor-id',
@ -1865,7 +1871,7 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertEqual([fake_inst['host']], fake_spec.ignore_hosts)
if host_name is None:
self.assertIsNone(fake_spec.requested_destination)
self.assertIsNotNone(fake_spec.requested_destination)
else:
self.assertIn('host', fake_spec.requested_destination)
self.assertEqual(host_name,
@ -1873,6 +1879,9 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertIn('node', fake_spec.requested_destination)
self.assertEqual('hypervisor_host',
fake_spec.requested_destination.node)
self.assertEqual(
allow_cross_cell_resize,
fake_spec.requested_destination.allow_cross_cell_move)
if host_name:
mock_get_all_by_host.assert_called_once_with(
@ -1932,7 +1941,7 @@ class _ComputeAPIUnitTestMixIn(object):
scheduler_hint=scheduler_hint,
flavor=test.MatchType(objects.Flavor),
clean_shutdown=clean_shutdown,
request_spec=fake_spec)
request_spec=fake_spec, do_cast=allow_cross_cell_resize)
else:
mock_resize.assert_not_called()
@ -1954,6 +1963,9 @@ class _ComputeAPIUnitTestMixIn(object):
def test_resize_forced_shutdown(self):
self._test_resize(clean_shutdown=False)
def test_resize_allow_cross_cell_resize_true(self):
self._test_resize(allow_cross_cell_resize=True)
@mock.patch('nova.compute.flavors.get_flavor_by_flavor_id')
@mock.patch('nova.objects.Quotas.count_as_dict')
@mock.patch('nova.objects.Quotas.limit_check_project_and_user')
@ -2007,6 +2019,10 @@ class _ComputeAPIUnitTestMixIn(object):
def test_migrate_with_host_name(self):
self._test_migrate(host_name='target_host')
def test_migrate_with_host_name_allow_cross_cell_resize_true(self):
self._test_migrate(host_name='target_host',
allow_cross_cell_resize=True)
@mock.patch.object(objects.ComputeNodeList, 'get_all_by_host',
side_effect=exception.ComputeHostNotFound(
host='nonexistent_host'))
@ -2016,12 +2032,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.compute_api.resize, self.context,
fake_inst, host_name='nonexistent_host')
def test_migrate_to_same_host(self):
fake_inst = self._create_instance_obj()
self.assertRaises(exception.CannotMigrateToSameHost,
self.compute_api.resize, self.context,
fake_inst, host_name='fake_host')
@mock.patch.object(objects.Instance, 'save')
@mock.patch.object(compute_api.API, '_record_action_start')
@mock.patch.object(quotas_obj.Quotas, 'limit_check_project_and_user')
@ -6602,6 +6612,67 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase):
self.compute_api.placementclient
mock_report_client.assert_called_once_with()
def test_validate_host_for_cold_migrate_same_host_fails(self):
"""Asserts CannotMigrateToSameHost is raised when trying to cold
migrate to the same host.
"""
instance = fake_instance.fake_instance_obj(self.context)
self.assertRaises(exception.CannotMigrateToSameHost,
self.compute_api._validate_host_for_cold_migrate,
self.context, instance, instance.host,
allow_cross_cell_resize=False)
@mock.patch('nova.objects.ComputeNode.'
'get_first_node_by_host_for_old_compat')
def test_validate_host_for_cold_migrate_diff_host_no_cross_cell(
self, mock_cn_get):
"""Tests the scenario where allow_cross_cell_resize=False and the
host is found in the same cell as the instance.
"""
instance = fake_instance.fake_instance_obj(self.context)
node = self.compute_api._validate_host_for_cold_migrate(
self.context, instance, uuids.host, allow_cross_cell_resize=False)
self.assertIs(node, mock_cn_get.return_value)
mock_cn_get.assert_called_once_with(
self.context, uuids.host, use_slave=True)
@mock.patch('nova.objects.HostMapping.get_by_host',
side_effect=exception.HostMappingNotFound(name=uuids.host))
def test_validate_host_for_cold_migrate_cross_cell_host_mapping_not_found(
self, mock_hm_get):
"""Tests the scenario where allow_cross_cell_resize=True but the
HostMapping for the given host could not be found.
"""
instance = fake_instance.fake_instance_obj(self.context)
self.assertRaises(exception.ComputeHostNotFound,
self.compute_api._validate_host_for_cold_migrate,
self.context, instance, uuids.host,
allow_cross_cell_resize=True)
mock_hm_get.assert_called_once_with(self.context, uuids.host)
@mock.patch('nova.objects.HostMapping.get_by_host',
return_value=objects.HostMapping(
cell_mapping=objects.CellMapping(uuid=uuids.cell2)))
@mock.patch('nova.context.target_cell')
@mock.patch('nova.objects.ComputeNode.'
'get_first_node_by_host_for_old_compat')
def test_validate_host_for_cold_migrate_cross_cell(
self, mock_cn_get, mock_target_cell, mock_hm_get):
"""Tests the scenario where allow_cross_cell_resize=True and the
ComputeNode is pulled from the target cell defined by the HostMapping.
"""
instance = fake_instance.fake_instance_obj(self.context)
node = self.compute_api._validate_host_for_cold_migrate(
self.context, instance, uuids.host,
allow_cross_cell_resize=True)
self.assertIs(node, mock_cn_get.return_value)
mock_hm_get.assert_called_once_with(self.context, uuids.host)
# get_first_node_by_host_for_old_compat is called with a temporarily
# cell-targeted context
mock_cn_get.assert_called_once_with(
mock_target_cell.return_value.__enter__.return_value,
uuids.host, use_slave=True)
class DiffDictTestCase(test.NoDBTestCase):
"""Unit tests for _diff_dict()."""

32
nova/tests/unit/conductor/test_conductor.py
View File

@ -3699,6 +3699,38 @@ class ConductorTaskRPCAPITestCase(_BaseTaskTestCase,
_test()
def test_migrate_server_cast(self):
"""Tests that if calling migrate_server() with do_cast=True an RPC
cast is performed rather than a call.
"""
instance = objects.Instance()
scheduler_hint = {}
live = rebuild = False
flavor = objects.Flavor()
block_migration = disk_over_commit = None
@mock.patch.object(self.conductor.client, 'can_send_version',
return_value=True)
@mock.patch.object(self.conductor.client, 'prepare')
def _test(prepare_mock, can_send_mock):
self.conductor.migrate_server(
self.context, instance, scheduler_hint, live, rebuild,
flavor, block_migration, disk_over_commit, do_cast=True)
kw = {'instance': instance, 'scheduler_hint': scheduler_hint,
'live': live, 'rebuild': rebuild, 'flavor': flavor,
'block_migration': block_migration,
'disk_over_commit': disk_over_commit,
'reservations': None, 'clean_shutdown': True,
'request_spec': None, 'host_list': None}
prepare_mock.assert_called_once_with(
version=test.MatchType(str), # version
call_monitor_timeout=CONF.rpc_response_timeout,
timeout=CONF.long_rpc_timeout)
prepare_mock.return_value.cast.assert_called_once_with(
self.context, 'migrate_server', **kw)
_test()
class ConductorTaskAPITestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
"""Compute task API Tests."""