From 8e59a7d8be23d928290b7fcfa2a1c7f37b45632f Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 10 Apr 2019 16:51:52 -0400 Subject: [PATCH] Use InstanceList.get_count_by_hosts when deleting a compute service This resolves the TODO in the delete service API code by using the InstanceList.get_count_by_hosts method which is a simpler COUNT query function rather than get_uuids_by_host. Change-Id: I7de2526b72c2c080f314419792e4c6766907872d --- nova/api/openstack/compute/services.py | 8 +++----- nova/tests/unit/api/openstack/compute/test_services.py | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/nova/api/openstack/compute/services.py b/nova/api/openstack/compute/services.py index f39d9b68ef3d..91c4fe2ad6e1 100644 --- a/nova/api/openstack/compute/services.py +++ b/nova/api/openstack/compute/services.py @@ -249,11 +249,9 @@ class ServiceController(wsgi.Controller): # related compute_nodes record) delete since it will impact # resource accounting in Placement and orphan the compute node # resource provider. - # TODO(mriedem): Use a COUNT SQL query-based function instead - # of InstanceList.get_uuids_by_host for performance. - instance_uuids = objects.InstanceList.get_uuids_by_host( - context, service['host']) - if instance_uuids: + num_instances = objects.InstanceList.get_count_by_hosts( + context, [service['host']]) + if num_instances: raise webob.exc.HTTPConflict( explanation=_('Unable to delete compute service that ' 'is hosting instances. Migrate or ' diff --git a/nova/tests/unit/api/openstack/compute/test_services.py b/nova/tests/unit/api/openstack/compute/test_services.py index 458e673956f5..799a8300366a 100644 --- a/nova/tests/unit/api/openstack/compute/test_services.py +++ b/nova/tests/unit/api/openstack/compute/test_services.py @@ -703,13 +703,13 @@ class ServicesTestV21(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.delete, self.req, 1234) - @mock.patch('nova.objects.InstanceList.get_uuids_by_host', - return_value=objects.InstanceList()) + @mock.patch('nova.objects.InstanceList.get_count_by_hosts', + return_value=0) @mock.patch('nova.objects.HostMapping.get_by_host', side_effect=exception.HostMappingNotFound(name='host1')) @mock.patch('nova.objects.Service.destroy') def test_compute_service_delete_host_mapping_not_found( - self, service_delete, get_instances, get_hm): + self, service_delete, get_hm, get_count_by_hosts): """Tests that we are still able to successfully delete a nova-compute service even if the HostMapping is not found. """ @@ -727,7 +727,7 @@ class ServicesTestV21(test.TestCase): self.controller.delete(self.req, 2) ctxt = self.req.environ['nova.context'] service_get_by_id.assert_called_once_with(ctxt, 2) - get_instances.assert_called_once_with(ctxt, 'host1') + get_count_by_hosts.assert_called_once_with(ctxt, ['host1']) get_aggregates_by_host.assert_called_once_with(ctxt, 'host1') delete_resource_provider.assert_called_once_with( ctxt, service_get_by_id.return_value.compute_node,