diff --git a/nova/compute/api.py b/nova/compute/api.py index c12579c4df20..7a27ae352d8e 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4304,6 +4304,65 @@ def target_host_cell(fn): return targeted +def _find_service_in_cell(context, service_id=None, service_host=None): + """Find a service by id or hostname by searching all cells. + + If one matching service is found, return it. If none or multiple + are found, raise an exception. + + :param context: A context.RequestContext + :param service_id: If not none, the DB ID of the service to find + :param service_host: If not None, the hostname of the service to find + :returns: An objects.Service + :raises: ServiceNotUnique if multiple matching IDs are found + :raises: NotFound if no matches are found + :raises: NovaException if called with neither search option + """ + + load_cells() + service = None + found_in_cell = None + + is_uuid = False + if service_id is not None: + is_uuid = uuidutils.is_uuid_like(service_id) + if is_uuid: + lookup_fn = lambda c: objects.Service.get_by_uuid(c, service_id) + else: + lookup_fn = lambda c: objects.Service.get_by_id(c, service_id) + elif service_host is not None: + lookup_fn = lambda c: ( + objects.Service.get_by_compute_host(c, service_host)) + else: + LOG.exception('_find_service_in_cell called with no search parameters') + # This is intentionally cryptic so we don't leak implementation details + # out of the API. + raise exception.NovaException() + + 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): + cell_service = lookup_fn(context) + except exception.NotFound: + # NOTE(danms): Keep looking in other cells + continue + if service and cell_service: + raise exception.ServiceNotUnique() + service = cell_service + found_in_cell = cell + if service and is_uuid: + break + + if service: + # NOTE(danms): Set the cell on the context so it remains + # when we return to our caller + nova_context.set_target_cell(context, found_in_cell) + return service + else: + raise exception.NotFound() + + class HostAPI(base.Base): """Sub-set of the Compute Manager API for managing host operations.""" @@ -4418,57 +4477,12 @@ class HostAPI(base.Base): ret_services.append(service) return ret_services - def _find_service(self, context, service_id): - """Find a service by id by searching all cells. - - If one matching service is found, return it. If none or multiple - are found, raise an exception. - - :param context: A context.RequestContext - :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 - """ - - load_cells() - # NOTE(danms): Unfortunately this API exposes database identifiers - # 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: - 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 - if service and cell_service: - raise exception.ServiceNotUnique() - service = cell_service - found_in_cell = cell - - if service: - # NOTE(danms): Set the cell on the context so it remains - # when we return to our caller - nova_context.set_target_cell(context, found_in_cell) - return service - else: - raise exception.ServiceNotFound(service_id=service_id) - def service_get_by_id(self, context, service_id): """Get service entry for the given service id or uuid.""" - return self._find_service(context, service_id) + try: + return _find_service_in_cell(context, service_id=service_id) + except exception.NotFound: + raise exception.ServiceNotFound(service_id=service_id) @target_host_cell def service_get_by_compute_host(self, context, host_name): @@ -4494,7 +4508,10 @@ class HostAPI(base.Base): def _service_delete(self, context, service_id): """Performs the actual Service deletion operation.""" - service = self._find_service(context, service_id) + try: + service = _find_service_in_cell(context, service_id=service_id) + except exception.NotFound: + raise exception.ServiceNotFound(service_id=service_id) service.destroy() def service_delete(self, context, service_id): @@ -4761,9 +4778,16 @@ class AggregateAPI(base.Base): aggregate_payload) # validates the host; HostMappingNotFound or ComputeHostNotFound # is raised if invalid - mapping = objects.HostMapping.get_by_host(context, host_name) - nova_context.set_target_cell(context, mapping.cell_mapping) - objects.Service.get_by_compute_host(context, host_name) + try: + mapping = objects.HostMapping.get_by_host(context, host_name) + nova_context.set_target_cell(context, mapping.cell_mapping) + objects.Service.get_by_compute_host(context, host_name) + except exception.HostMappingNotFound: + try: + # NOTE(danms): This targets our cell + _find_service_in_cell(context, service_host=host_name) + except exception.NotFound: + raise exception.ComputeHostNotFound(host=host_name) aggregate = objects.Aggregate.get_by_id(context, aggregate_id) self.is_safe_to_update_az(context, aggregate.metadata, diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py index 9bf9ecd60acb..1abe5008e8d8 100644 --- a/nova/tests/functional/api/client.py +++ b/nova/tests/functional/api/client.py @@ -398,6 +398,10 @@ class TestOpenStackClient(object): def delete_aggregate(self, aggregate_id): self.api_delete('/os-aggregates/%s' % aggregate_id) + def add_host_to_aggregate(self, aggregate_id, host): + return self.api_post('/os-aggregates/%s/action' % aggregate_id, + {'add_host': {'host': host}}) + def get_limits(self): return self.api_get('/limits').body['limits'] @@ -416,3 +420,6 @@ class TestOpenStackClient(object): def detach_interface(self, server_id, port_id): return self.api_delete('/servers/%s/os-interface/%s' % (server_id, port_id)) + + def get_services(self): + return self.api_get('/os-services').body['services'] diff --git a/nova/tests/functional/test_aggregates.py b/nova/tests/functional/test_aggregates.py new file mode 100644 index 000000000000..06d6ae441a7d --- /dev/null +++ b/nova/tests/functional/test_aggregates.py @@ -0,0 +1,41 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from nova.tests.functional import integrated_helpers + + +class AggregatesTest(integrated_helpers._IntegratedTestBase): + api_major_version = 'v2' + ADMIN_API = True + + def _add_hosts_to_aggregate(self): + """List all compute services and add them all to an aggregate.""" + + compute_services = [s for s in self.api.get_services() + if s['binary'] == 'nova-compute'] + agg = {'aggregate': {'name': 'test-aggregate'}} + agg = self.api.post_aggregate(agg) + for service in compute_services: + self.api.add_host_to_aggregate(agg['id'], service['host']) + return len(compute_services) + + def test_add_hosts(self): + # Default case with one compute, mapped for us + self.assertEqual(1, self._add_hosts_to_aggregate()) + + def test_add_unmapped_host(self): + """Ensure that hosts without mappings are still found and added""" + + # Add another compute, but nuke its HostMapping + self.start_service('compute', host='compute2') + self.host_mappings['compute2'].destroy() + self.assertEqual(2, self._add_hosts_to_aggregate()) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 64b10d50e064..5fcbfbeadac1 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -11354,11 +11354,11 @@ class ComputeAPIAggrTestCase(BaseTestCase): self.assertEqual(len(aggr.hosts), len(values[0][1])) def test_add_host_to_aggregate_raise_not_found(self): - # Ensure HostMappingNotFound is raised when adding invalid host. + # Ensure ComputeHostNotFound is raised when adding invalid host. aggr = self.api.create_aggregate(self.context, 'fake_aggregate', 'fake_zone') fake_notifier.NOTIFICATIONS = [] - self.assertRaises(exception.HostMappingNotFound, + self.assertRaises(exception.ComputeHostNotFound, self.api.add_host_to_aggregate, self.context, aggr.id, 'invalid_host') self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index d1bd018238df..d7fab7136ca0 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -5100,6 +5100,10 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): self.assertRaises(exception.CannotResizeToSameFlavor, self._test_resize, same_flavor=True) + def test_find_service_in_cell_error_case(self): + self.assertRaises(exception.NovaException, + compute_api._find_service_in_cell, self.context) + def test_validate_and_build_base_options_translate_neutron_secgroup(self): """Tests that _check_requested_secgroups will return a uuid for a requested Neutron security group and that will be returned from