openstack
/
nova
Code Issues Proposed changes

Merge "Fix and optimize external_events for multiple cells"

This commit is contained in:
Jenkins 2017-07-21 00:13:14 +00:00 committed by Gerrit Code Review
parent f590636b59 b8be61eb39
commit 49203d56d9
5 changed files with 173 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
nova/api/openstack/compute/server_external_events.py
View File

@ -21,7 +21,6 @@ from nova.api.openstack import wsgi
from nova.api import validation
from nova import compute
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova import objects
from nova.policies import server_external_events as see_policies
@ -42,6 +41,28 @@ class ServerExternalEventsController(wsgi.Controller):
return False
return True
def _get_instances_all_cells(self, context, instance_uuids,
instance_mappings):
cells = {}
instance_uuids_by_cell = {}
for im in instance_mappings:
if im.cell_mapping.uuid not in cells:
cells[im.cell_mapping.uuid] = im.cell_mapping
instance_uuids_by_cell.setdefault(im.cell_mapping.uuid, list())
instance_uuids_by_cell[im.cell_mapping.uuid].append(
im.instance_uuid)
instances = {}
for cell_uuid, cell in cells.items():
with nova_context.target_cell(context, cell) as cctxt:
instances.update(
{inst.uuid: inst for inst in
objects.InstanceList.get_by_filters(
cctxt, {'uuid': instance_uuids_by_cell[cell_uuid]},
expected_attrs=['migration_context', 'info_cache'])})
return instances
@extensions.expected_errors((400, 403, 404))
@wsgi.response(200)
@validation.schema(server_external_events.create, '2.1', '2.50')
@ -54,12 +75,17 @@ class ServerExternalEventsController(wsgi.Controller):
response_events = []
accepted_events = []
accepted_instances = set()
instances = {}
mappings = {}
result = 200
body_events = body['events']
# Fetch instance objects for all relevant instances
instance_uuids = set([event['server_uuid'] for event in body_events])
instance_mappings = objects.InstanceMappingList.get_by_instance_uuids(
context, list(instance_uuids))
instances = self._get_instances_all_cells(context, instance_uuids,
instance_mappings)
for _event in body_events:
client_event = dict(_event)
event = objects.InstanceExternalEvent(context)
@ -69,70 +95,54 @@ class ServerExternalEventsController(wsgi.Controller):
event.status = client_event.pop('status', 'completed')
event.tag = client_event.pop('tag', None)
response_events.append(_event)
instance = instances.get(event.instance_uuid)
if not instance:
try:
mapping = objects.InstanceMapping.get_by_instance_uuid(
context, event.instance_uuid)
cell_mapping = mapping.cell_mapping
mappings[event.instance_uuid] = cell_mapping
# Load migration_context and info_cache here in a single DB
# operation because we need them later on
with nova_context.target_cell(context,
cell_mapping) as cctxt:
instance = objects.Instance.get_by_uuid(
cctxt, event.instance_uuid,
expected_attrs=['migration_context', 'info_cache'])
instances[event.instance_uuid] = instance
except (exception.InstanceNotFound,
exception.InstanceMappingNotFound):
LOG.debug('Dropping event %(name)s:%(tag)s for unknown '
'instance %(instance_uuid)s',
{'name': event.name, 'tag': event.tag,
'instance_uuid': event.instance_uuid})
_event['status'] = 'failed'
_event['code'] = 404
result = 207
LOG.debug('Dropping event %(name)s:%(tag)s for unknown '
'instance %(instance_uuid)s',
{'name': event.name, 'tag': event.tag,
'instance_uuid': event.instance_uuid})
_event['status'] = 'failed'
_event['code'] = 404
result = 207
continue
# NOTE: before accepting the event, make sure the instance
# for which the event is sent is assigned to a host; otherwise
# it will not be possible to dispatch the event
if instance:
if not self._is_event_tag_present_when_required(event):
LOG.debug("Event tag is missing for instance "
"%(instance)s. Dropping event %(event)s",
{'instance': event.instance_uuid,
'event': event.name})
_event['status'] = 'failed'
_event['code'] = 400
result = 207
elif instance.host:
accepted_events.append(event)
accepted_instances.add(instance)
LOG.info('Creating event %(name)s:%(tag)s for '
'instance %(instance_uuid)s on %(host)s',
{'name': event.name, 'tag': event.tag,
'instance_uuid': event.instance_uuid,
'host': instance.host})
# NOTE: as the event is processed asynchronously verify
# whether 202 is a more suitable response code than 200
_event['status'] = 'completed'
_event['code'] = 200
else:
LOG.debug("Unable to find a host for instance "
"%(instance)s. Dropping event %(event)s",
{'instance': event.instance_uuid,
'event': event.name})
_event['status'] = 'failed'
_event['code'] = 422
result = 207
response_events.append(_event)
if not self._is_event_tag_present_when_required(event):
LOG.debug("Event tag is missing for instance "
"%(instance)s. Dropping event %(event)s",
{'instance': event.instance_uuid,
'event': event.name})
_event['status'] = 'failed'
_event['code'] = 400
result = 207
elif instance.host:
accepted_events.append(event)
accepted_instances.add(instance)
LOG.info('Creating event %(name)s:%(tag)s for '
'instance %(instance_uuid)s on %(host)s',
{'name': event.name, 'tag': event.tag,
'instance_uuid': event.instance_uuid,
'host': instance.host})
# NOTE: as the event is processed asynchronously verify
# whether 202 is a more suitable response code than 200
_event['status'] = 'completed'
_event['code'] = 200
else:
LOG.debug("Unable to find a host for instance "
"%(instance)s. Dropping event %(event)s",
{'instance': event.instance_uuid,
'event': event.name})
_event['status'] = 'failed'
_event['code'] = 422
result = 207
if accepted_events:
self.compute_api.external_instance_event(
context, accepted_instances, mappings, accepted_events)
context, accepted_instances, accepted_events)
else:
msg = _('No instances found for any event')
raise webob.exc.HTTPNotFound(explanation=msg)

34
nova/compute/api.py
View File

@ -4101,7 +4101,7 @@ class API(base.Base):
do_volume_snapshot_delete(self, context, bdm.instance)
def external_instance_event(self, context, instances, mappings, events):
def external_instance_event(self, api_context, instances, events):
# NOTE(danms): The external API consumer just provides events,
# but doesn't know where they go. We need to collate lists
# by the host the affected instance is on and dispatch them
@ -4109,8 +4109,23 @@ class API(base.Base):
instances_by_host = collections.defaultdict(list)
events_by_host = collections.defaultdict(list)
hosts_by_instance = collections.defaultdict(list)
cell_contexts_by_host = {}
for instance in instances:
for host in self._get_relevant_hosts(context, instance):
# 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):
# 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
instances_by_host[host].append(instance)
hosts_by_instance[instance.uuid].append(host)
@ -4119,24 +4134,23 @@ class API(base.Base):
# Volume extend is a user-initiated operation starting in the
# Block Storage service API. We record an instance action so
# the user can monitor the operation to completion.
host = hosts_by_instance[event.instance_uuid][0]
cell_context = cell_contexts_by_host[host]
objects.InstanceAction.action_start(
context, event.instance_uuid,
cell_context, event.instance_uuid,
instance_actions.EXTEND_VOLUME, want_result=False)
for host in hosts_by_instance[event.instance_uuid]:
events_by_host[host].append(event)
for host in instances_by_host:
# NOTE(danms): All instances on a host must have the same
# mapping, so just use that
cell_mapping = mappings[instances_by_host[host][0].uuid]
cell_context = cell_contexts_by_host[host]
# TODO(salv-orlando): Handle exceptions raised by the rpc api layer
# in order to ensure that a failure in processing events on a host
# will not prevent processing events on other hosts
with nova_context.target_cell(context, cell_mapping) as cctxt:
self.compute_rpcapi.external_instance_event(
cctxt, instances_by_host[host], events_by_host[host],
host=host)
self.compute_rpcapi.external_instance_event(
cell_context, instances_by_host[host], events_by_host[host],
host=host)
def _get_relevant_hosts(self, context, instance):
hosts = set()

9
nova/tests/fixtures.py
View File

@ -288,6 +288,9 @@ class SingleCellSimple(fixtures.Fixture):
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.InstanceMapping._get_by_instance_uuid_from_db',
self._fake_instancemapping_get))
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.InstanceMappingList._get_by_instance_uuids_from_db',
self._fake_instancemapping_get_uuids))
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.InstanceMapping._save_in_db',
self._fake_instancemapping_get))
@ -310,13 +313,17 @@ class SingleCellSimple(fixtures.Fixture):
'id': 1,
'updated_at': None,
'created_at': None,
'instance_uuid': uuidsentinel.instance,
'instance_uuid': args[-1],
'cell_id': (self.instances_created and 1 or None),
'project_id': 'project',
'cell_mapping': (
self.instances_created and self._fake_cell_get() or None),
}
def _fake_instancemapping_get_uuids(self, *args):
return [self._fake_instancemapping_get(uuid)
for uuid in args[-1]]
def _fake_cell_get(self, *args):
return self._fake_cell_list()[0]

69
nova/tests/unit/api/openstack/compute/test_server_external_events.py
View File

@ -22,36 +22,55 @@ from nova import objects
from nova.objects import instance as instance_obj
from nova import test
from nova.tests.unit.api.openstack import fakes
from nova.tests import uuidsentinel as uuids
fake_instances = {
'00000000-0000-0000-0000-000000000001': objects.Instance(
'00000000-0000-0000-0000-000000000001': objects.Instance(id=1,
uuid='00000000-0000-0000-0000-000000000001', host='host1'),
'00000000-0000-0000-0000-000000000002': objects.Instance(
'00000000-0000-0000-0000-000000000002': objects.Instance(id=2,
uuid='00000000-0000-0000-0000-000000000002', host='host1'),
'00000000-0000-0000-0000-000000000003': objects.Instance(
'00000000-0000-0000-0000-000000000003': objects.Instance(id=3,
uuid='00000000-0000-0000-0000-000000000003', host='host2'),
'00000000-0000-0000-0000-000000000004': objects.Instance(
'00000000-0000-0000-0000-000000000004': objects.Instance(id=4,
uuid='00000000-0000-0000-0000-000000000004', host=None),
}
fake_instance_uuids = sorted(fake_instances.keys())
MISSING_UUID = '00000000-0000-0000-0000-000000000005'
fake_cells = [objects.CellMapping(uuid=uuids.cell1, database_connection="db1"),
objects.CellMapping(uuid=uuids.cell2, database_connection="db2")]
fake_instance_mappings = [
objects.InstanceMapping(cell_mapping=fake_cells[instance.id % 2],
instance_uuid=instance.uuid)
for instance in fake_instances.values()]
@classmethod
def fake_get_by_uuid(cls, context, uuid, **kwargs):
if 'expected_attrs' in kwargs:
expected_attrs_set = set(kwargs['expected_attrs'])
def fake_get_by_filters(cls, context, filters, expected_attrs=None):
if expected_attrs:
# This is a regression check for bug 1645479.
expected_attrs_set = set(expected_attrs)
full_expected_attrs_set = set(instance_obj.INSTANCE_OPTIONAL_ATTRS)
assert expected_attrs_set.issubset(full_expected_attrs_set), \
('%s is not a subset of %s' % (expected_attrs_set,
full_expected_attrs_set))
try:
return fake_instances[uuid]
except KeyError:
raise exception.InstanceNotFound(instance_id=uuid)
l = objects.InstanceList(objects=[
inst for inst in fake_instances.values()
if inst.uuid in filters['uuid']])
return l
@mock.patch('nova.objects.instance.Instance.get_by_uuid', fake_get_by_uuid)
@classmethod
def fake_get_by_instance_uuids(cls, context, uuids):
mappings = [im for im in fake_instance_mappings
if im.instance_uuid in uuids]
return objects.InstanceMappingList(objects=mappings)
@mock.patch('nova.objects.InstanceMappingList.get_by_instance_uuids',
fake_get_by_instance_uuids)
@mock.patch('nova.objects.InstanceList.get_by_filters',
fake_get_by_filters)
class ServerExternalEventsTestV21(test.NoDBTestCase):
server_external_events = server_external_events_v21
invalid_error = exception.ValidationError
@ -87,12 +106,21 @@ class ServerExternalEventsTestV21(test.NoDBTestCase):
code = response._code
self.assertEqual(1, api_method.call_count)
for inst in api_method.call_args_list[0][0][1]:
expected_uuids.remove(inst.uuid)
self.assertEqual([], expected_uuids)
for event in api_method.call_args_list[0][0][3]:
expected_events.remove(event.name)
self.assertEqual([], expected_events)
call = api_method.call_args_list[0]
args = call[0]
call_instances = args[1]
call_events = args[2]
self.assertEqual(set(expected_uuids),
set([instance.uuid for instance in call_instances]))
self.assertEqual(len(expected_uuids), len(call_instances))
self.assertEqual(set(expected_events),
set([event.name for event in call_events]))
self.assertEqual(len(expected_events),
len(call_events))
return result, code
def test_create(self):
@ -161,7 +189,10 @@ class ServerExternalEventsTestV21(test.NoDBTestCase):
self.api.create, self.req, body=body)
@mock.patch('nova.objects.instance.Instance.get_by_uuid', fake_get_by_uuid)
@mock.patch('nova.objects.InstanceMappingList.get_by_instance_uuids',
fake_get_by_instance_uuids)
@mock.patch('nova.objects.InstanceList.get_by_filters',
fake_get_by_filters)
class ServerExternalEventsTestV251(ServerExternalEventsTestV21):
wsgi_api_version = '2.51'

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

@ -3310,9 +3310,14 @@ class _ComputeAPIUnitTestMixIn(object):
objects.Instance(uuid=uuids.instance_4, host='host2',
migration_context=None),
]
mappings = {inst.uuid: objects.InstanceMapping.get_by_instance_uuid(
self.context, inst.uuid)
for inst in instances}
# Create a single cell context and associate it with all instances
mapping = objects.InstanceMapping.get_by_instance_uuid(
self.context, instances[0].uuid)
with context.target_cell(self.context, mapping.cell_mapping) as cc:
cell_context = cc
for instance in instances:
instance._context = cell_context
volume_id = uuidutils.generate_uuid()
events = [
objects.InstanceExternalEvent(
@ -3331,11 +3336,11 @@ class _ComputeAPIUnitTestMixIn(object):
]
self.compute_api.compute_rpcapi = mock.MagicMock()
self.compute_api.external_instance_event(self.context,
instances, mappings, events)
instances, events)
method = self.compute_api.compute_rpcapi.external_instance_event
method.assert_any_call(self.context, instances[0:2], events[0:2],
method.assert_any_call(cell_context, instances[0:2], events[0:2],
host='host1')
method.assert_any_call(self.context, instances[2:], events[2:],
method.assert_any_call(cell_context, instances[2:], events[2:],
host='host2')
mock_action_start.assert_called_once_with(
self.context, uuids.instance_4, instance_actions.EXTEND_VOLUME,
@ -3371,8 +3376,15 @@ class _ComputeAPIUnitTestMixIn(object):
objects.Instance(uuid=uuids.instance_3, host='host2',
migration_context=None)
]
mappings = {inst.uuid: objects.InstanceMapping.get_by_instance_uuid(
self.context, inst.uuid) for inst in instances}
# Create a single cell context and associate it with all instances
mapping = objects.InstanceMapping.get_by_instance_uuid(
self.context, instances[0].uuid)
with context.target_cell(self.context, mapping.cell_mapping) as cc:
cell_context = cc
for instance in instances:
instance._context = cell_context
events = [
objects.InstanceExternalEvent(
instance_uuid=uuids.instance_1,
@ -3388,12 +3400,11 @@ class _ComputeAPIUnitTestMixIn(object):
with mock.patch('nova.db.sqlalchemy.api.migration_get', migration_get):
self.compute_api.compute_rpcapi = mock.MagicMock()
self.compute_api.external_instance_event(self.context,
instances, mappings,
events)
instances, events)
method = self.compute_api.compute_rpcapi.external_instance_event
method.assert_any_call(self.context, instances[0:2], events[0:2],
method.assert_any_call(cell_context, instances[0:2], events[0:2],
host='host1')
method.assert_any_call(self.context, instances[1:], events[1:],
method.assert_any_call(cell_context, instances[1:], events[1:],
host='host2')
self.assertEqual(2, method.call_count)