From 3838d5e10fac87531102d03cb69bd6640a0737ab Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 23 May 2017 17:11:43 -0400 Subject: [PATCH] Handle uuid in HostAPI._find_service This allows looking up a service in the HostAPI by its uuid value to uniquely identify it in a list of cells. A later REST API microversion change will rely on this to avoid the ServiceNotUnique scenario. Note that the existing service_get_by_id and service_delete methods are used to minimize the impact to cells v1. Since we don't have a service mapping table in the API DB we still need to loop over cells. A unit test is provided to show that the loop over the cells is broken once we find the cell that the service lives in and that the context is targeted to that cell. Part of blueprint service-hyper-uuid-in-api Change-Id: Ib82432f125649bc96b386c8f5895813cfbaa57dd --- nova/compute/api.py | 18 +++++--- nova/tests/unit/compute/test_host_api.py | 54 +++++++++++++++++++++++- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5d63e04398e7..1e65d56a6f9b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4438,7 +4438,7 @@ class HostAPI(base.Base): are found, raise an exception. :param context: A context.RequestContext - :param service_id: The DB ID of the service to find + :param service_id: The DB ID (or UUID) of the service to find :returns: An objects.Service :raises: ServiceNotUnique if multiple matches are found :raises: ServiceNotFound if no matches are found @@ -4449,12 +4449,20 @@ class HostAPI(base.Base): # which means we really can't do something efficient here service = None found_in_cell = None + is_uuid = uuidutils.is_uuid_like(service_id) for cell in CELLS: # NOTE(danms): Services can be in cell0, so don't skip it here try: with nova_context.target_cell(context, cell) as cctxt: - cell_service = objects.Service.get_by_id(cctxt, - service_id) + if is_uuid: + service = objects.Service.get_by_uuid(cctxt, + service_id) + found_in_cell = cell + # Service uuids are unique so we can break the loop now + break + else: + cell_service = objects.Service.get_by_id(cctxt, + service_id) except exception.ServiceNotFound: # NOTE(danms): Keep looking in other cells continue @@ -4472,7 +4480,7 @@ class HostAPI(base.Base): raise exception.ServiceNotFound(service_id=service_id) def service_get_by_id(self, context, service_id): - """Get service entry for the given service id.""" + """Get service entry for the given service id or uuid.""" return self._find_service(context, service_id) @target_host_cell @@ -4503,7 +4511,7 @@ class HostAPI(base.Base): service.destroy() def service_delete(self, context, service_id): - """Deletes the specified service.""" + """Deletes the specified service found via id or uuid.""" self._service_delete(context, service_id) @target_host_cell diff --git a/nova/tests/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py index 376273c5f5ac..02c7fe3fc1fd 100644 --- a/nova/tests/unit/compute/test_host_api.py +++ b/nova/tests/unit/compute/test_host_api.py @@ -16,6 +16,7 @@ import copy +import fixtures import mock from oslo_serialization import jsonutils import testtools @@ -28,11 +29,12 @@ from nova import context from nova import exception from nova import objects from nova import test -from nova.tests import fixtures +from nova.tests import fixtures as nova_fixtures from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_notifier from nova.tests.unit.objects import test_objects from nova.tests.unit.objects import test_service +from nova.tests import uuidsentinel as uuids class ComputeHostAPITestCase(test.TestCase): @@ -45,7 +47,7 @@ class ComputeHostAPITestCase(test.TestCase): self.addCleanup(fake_notifier.reset) self.req = fakes.HTTPRequest.blank('') self.controller = services.ServiceController() - self.useFixture(fixtures.SingleCellSimple()) + self.useFixture(nova_fixtures.SingleCellSimple()) def _compare_obj(self, obj, db_obj): test_objects.compare_obj(self, obj, db_obj, @@ -313,6 +315,54 @@ class ComputeHostAPITestCase(test.TestCase): _do_test() + @mock.patch.object(objects.CellMappingList, 'get_all', + return_value=objects.CellMappingList(objects=[ + objects.CellMapping( + uuid=uuids.cell1_uuid, + transport_url='mq://fake1', + database_connection='db://fake1'), + objects.CellMapping( + uuid=uuids.cell2_uuid, + transport_url='mq://fake2', + database_connection='db://fake2'), + objects.CellMapping( + uuid=uuids.cell3_uuid, + transport_url='mq://fake3', + database_connection='db://fake3')])) + @mock.patch.object(objects.Service, 'get_by_uuid', + side_effect=[ + exception.ServiceNotFound( + service_id=uuids.service_uuid), + objects.Service(uuid=uuids.service_uuid)]) + def test_service_get_by_id_using_uuid(self, service_get_by_uuid, + cell_mappings_get_all): + """Tests that we can lookup a service in the HostAPI using a uuid. + There are two calls to objects.Service.get_by_uuid and the first + raises ServiceNotFound so that we ensure we keep looping over the + cells. We'll find the service in the second cell and break the loop + so that we don't needlessly check in the third cell. + """ + + def _fake_set_target_cell(ctxt, cell_mapping): + if cell_mapping: + # These aren't really what would be set for values but let's + # keep this simple so we can assert something is set when a + # mapping is provided. + ctxt.db_connection = cell_mapping.database_connection + ctxt.mq_connection = cell_mapping.transport_url + + # We have to override the SingleCellSimple fixture. + self.useFixture(fixtures.MonkeyPatch( + 'nova.context.set_target_cell', _fake_set_target_cell)) + ctxt = context.get_admin_context() + self.assertIsNone(ctxt.db_connection) + self.host_api.service_get_by_id(ctxt, uuids.service_uuid) + # We should have broken the loop over the cells and set the target cell + # on the context. + service_get_by_uuid.assert_has_calls( + [mock.call(ctxt, uuids.service_uuid)] * 2) + self.assertEqual('db://fake2', ctxt.db_connection) + @mock.patch('nova.context.set_target_cell') @mock.patch('nova.compute.api.load_cells') @mock.patch('nova.objects.Service.get_by_id')