Support cross-cell moves in external_instance_event
The external_instance_event method in the API assumed that all events for an instance would be routed to hosts within the same cell the instance lives in, but that is no longer the case when a cross-cell migration is happening. This changes the external_instance_event flow to check if the instance is undergoing a cross-cell migration and if so, gets the host mappings for the source and dest host to properly target the context for the cast to each compute in different cells. Part of blueprint cross-cell-resize Change-Id: Ie556b5837e7b45b314616fd4b19dc08c8193ed54
This commit is contained in:
parent
fbebfcaf34
commit
c2e315975c
|
@ -5122,17 +5122,25 @@ class API(base.Base):
|
|||
# instance._context is used here since it's already targeted to
|
||||
# the cell that the instance lives in, and we need to use that
|
||||
# cell context to lookup any migrations associated to the instance.
|
||||
for host in self._get_relevant_hosts(instance._context, instance):
|
||||
hosts, cross_cell_move = self._get_relevant_hosts(
|
||||
instance._context, instance)
|
||||
for host in hosts:
|
||||
# NOTE(danms): All instances on a host must have the same
|
||||
# mapping, so just use that
|
||||
# NOTE(mdbooth): We don't currently support migrations between
|
||||
# cells, and given that the Migration record is hosted in the
|
||||
# cell _get_relevant_hosts will likely have to change before we
|
||||
# do. Consequently we can currently assume that the context for
|
||||
# both the source and destination hosts of a migration is the
|
||||
# same.
|
||||
if host not in cell_contexts_by_host:
|
||||
cell_contexts_by_host[host] = instance._context
|
||||
# NOTE(mriedem): If the instance is being migrated across
|
||||
# cells then we have to get the host mapping to determine
|
||||
# which cell a given host is in.
|
||||
if cross_cell_move:
|
||||
hm = objects.HostMapping.get_by_host(api_context, host)
|
||||
ctxt = nova_context.get_admin_context()
|
||||
nova_context.set_target_cell(ctxt, hm.cell_mapping)
|
||||
cell_contexts_by_host[host] = ctxt
|
||||
else:
|
||||
# The instance is not migrating across cells so just
|
||||
# use the cell-targeted context already in the
|
||||
# instance since the host has to be in that same cell.
|
||||
cell_contexts_by_host[host] = instance._context
|
||||
|
||||
instances_by_host[host].append(instance)
|
||||
hosts_by_instance[instance.uuid].append(host)
|
||||
|
@ -5177,18 +5185,35 @@ class API(base.Base):
|
|||
host=host)
|
||||
|
||||
def _get_relevant_hosts(self, context, instance):
|
||||
"""Get the relevant hosts for an external server event on an instance.
|
||||
|
||||
:param context: nova auth request context targeted at the same cell
|
||||
that the instance lives in
|
||||
:param instance: Instance object which is the target of an external
|
||||
server event
|
||||
:returns: 2-item tuple of:
|
||||
- set of at least one host (the host where the instance lives); if
|
||||
the instance is being migrated the source and dest compute
|
||||
hostnames are in the returned set
|
||||
- boolean indicating if the instance is being migrated across cells
|
||||
"""
|
||||
hosts = set()
|
||||
hosts.add(instance.host)
|
||||
cross_cell_move = False
|
||||
if instance.migration_context is not None:
|
||||
migration_id = instance.migration_context.migration_id
|
||||
migration = objects.Migration.get_by_id(context, migration_id)
|
||||
cross_cell_move = migration.cross_cell_move
|
||||
hosts.add(migration.dest_compute)
|
||||
hosts.add(migration.source_compute)
|
||||
LOG.debug('Instance %(instance)s is migrating, '
|
||||
cells_msg = (
|
||||
'across cells' if cross_cell_move else 'within the same cell')
|
||||
LOG.debug('Instance %(instance)s is migrating %(cells_msg)s, '
|
||||
'copying events to all relevant hosts: '
|
||||
'%(hosts)s', {'instance': instance.uuid,
|
||||
'%(hosts)s', {'cells_msg': cells_msg,
|
||||
'instance': instance.uuid,
|
||||
'hosts': hosts})
|
||||
return hosts
|
||||
return hosts, cross_cell_move
|
||||
|
||||
def get_instance_host_status(self, instance):
|
||||
if instance.host:
|
||||
|
|
|
@ -4185,6 +4185,87 @@ class _ComputeAPIUnitTestMixIn(object):
|
|||
host='host2')
|
||||
self.assertEqual(2, method.call_count)
|
||||
|
||||
@mock.patch('nova.objects.Migration.get_by_id')
|
||||
@mock.patch('nova.objects.HostMapping.get_by_host')
|
||||
@mock.patch('nova.context.set_target_cell')
|
||||
@mock.patch('nova.context.get_admin_context')
|
||||
def test_external_instance_event_cross_cell_move(
|
||||
self, get_admin_context, set_target_cell, get_hm_by_host,
|
||||
get_mig_by_id):
|
||||
"""Tests a scenario where an external server event comes for an
|
||||
instance undergoing a cross-cell migration so the event is routed
|
||||
to both the source host in the source cell and dest host in dest cell
|
||||
using the properly targeted request contexts.
|
||||
"""
|
||||
migration = objects.Migration(
|
||||
id=1, source_compute='host1', dest_compute='host2',
|
||||
cross_cell_move=True)
|
||||
migration_context = objects.MigrationContext(
|
||||
instance_uuid=uuids.instance, migration_id=migration.id,
|
||||
migration_type='resize', cross_cell_move=True)
|
||||
instance = objects.Instance(
|
||||
self.context, uuid=uuids.instance, host=migration.source_compute,
|
||||
migration_context=migration_context)
|
||||
get_mig_by_id.return_value = migration
|
||||
source_cell_mapping = objects.CellMapping(name='source-cell')
|
||||
dest_cell_mapping = objects.CellMapping(name='dest-cell')
|
||||
|
||||
# Wrap _get_relevant_hosts and sort the result for predictable asserts.
|
||||
original_get_relevant_hosts = self.compute_api._get_relevant_hosts
|
||||
|
||||
def wrap_get_relevant_hosts(_self, *a, **kw):
|
||||
hosts, cross_cell_move = original_get_relevant_hosts(*a, **kw)
|
||||
return sorted(hosts), cross_cell_move
|
||||
self.stub_out('nova.compute.api.API._get_relevant_hosts',
|
||||
wrap_get_relevant_hosts)
|
||||
|
||||
def fake_hm_get_by_host(ctxt, host):
|
||||
if host == migration.source_compute:
|
||||
return objects.HostMapping(
|
||||
host=host, cell_mapping=source_cell_mapping)
|
||||
if host == migration.dest_compute:
|
||||
return objects.HostMapping(
|
||||
host=host, cell_mapping=dest_cell_mapping)
|
||||
raise Exception('Unexpected host: %s' % host)
|
||||
|
||||
get_hm_by_host.side_effect = fake_hm_get_by_host
|
||||
# get_admin_context should be called twice in order (source and dest)
|
||||
get_admin_context.side_effect = [
|
||||
mock.sentinel.source_context, mock.sentinel.dest_context]
|
||||
|
||||
event = objects.InstanceExternalEvent(
|
||||
instance_uuid=instance.uuid, name='network-vif-plugged')
|
||||
events = [event]
|
||||
|
||||
with mock.patch.object(self.compute_api.compute_rpcapi,
|
||||
'external_instance_event') as rpc_mock:
|
||||
self.compute_api.external_instance_event(
|
||||
self.context, [instance], events)
|
||||
|
||||
# We should have gotten the migration because of the migration_context.
|
||||
get_mig_by_id.assert_called_once_with(self.context, migration.id)
|
||||
# We should have gotten two host mappings (for source and dest).
|
||||
self.assertEqual(2, get_hm_by_host.call_count)
|
||||
get_hm_by_host.assert_has_calls([
|
||||
mock.call(self.context, migration.source_compute),
|
||||
mock.call(self.context, migration.dest_compute)])
|
||||
self.assertEqual(2, get_admin_context.call_count)
|
||||
# We should have targeted a context to both cells.
|
||||
self.assertEqual(2, set_target_cell.call_count)
|
||||
set_target_cell.assert_has_calls([
|
||||
mock.call(mock.sentinel.source_context, source_cell_mapping),
|
||||
mock.call(mock.sentinel.dest_context, dest_cell_mapping)])
|
||||
# We should have RPC cast to both hosts in different cells.
|
||||
self.assertEqual(2, rpc_mock.call_count)
|
||||
rpc_mock.assert_has_calls([
|
||||
mock.call(mock.sentinel.source_context, [instance], events,
|
||||
host=migration.source_compute),
|
||||
mock.call(mock.sentinel.dest_context, [instance], events,
|
||||
host=migration.dest_compute)],
|
||||
# The rpc calls are based on iterating over a dict which is not
|
||||
# ordered so we have to just assert the calls in any order.
|
||||
any_order=True)
|
||||
|
||||
def test_volume_ops_invalid_task_state(self):
|
||||
instance = self._create_instance_obj()
|
||||
self.assertEqual(instance.vm_state, vm_states.ACTIVE)
|
||||
|
|
Loading…
Reference in New Issue