From 94848ca40f9bce9c53b820bbfe66eb3d2d181afc Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 21 Mar 2018 14:16:24 +0100 Subject: [PATCH] Cleanup RP and HM records while deleting a compute service. Currently when deleting a nova-compute service via the API, we will (soft) delete the service and compute_node records in the DB, but the placement resource provider and host mapping records will be orphaned. This patch deletes the resource provider and host_mapping records before deleting the service/compute node. Change-Id: I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 Closes-Bug: #1756179 (cherry picked from commit 589c495c1ae62129e20ab5e2641e330541eee01f) (cherry picked from commit dede2de2bd482d0378a7acd81b65d93b1635e825) (cherry picked from commit 9fe847bdca39627b4d1741d2c5807ebca7101d2e) --- nova/api/openstack/compute/services.py | 13 +++++++++++ nova/tests/functional/api/client.py | 12 +++++++++- .../api/openstack/compute/test_services.py | 22 ++++++++++++++++++- nova/tests/unit/compute/test_host_api.py | 10 ++++++++- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/compute/services.py b/nova/api/openstack/compute/services.py index 09667dde29d6..17006fb9e051 100644 --- a/nova/api/openstack/compute/services.py +++ b/nova/api/openstack/compute/services.py @@ -23,7 +23,9 @@ from nova.api import validation from nova import compute from nova import exception from nova.i18n import _ +from nova import objects from nova.policies import services as services_policies +from nova.scheduler.client import report from nova import servicegroup from nova import utils @@ -39,6 +41,7 @@ class ServiceController(wsgi.Controller): self.actions = {"enable": self._enable, "disable": self._disable, "disable-log-reason": self._disable_log_reason} + self.placementclient = report.SchedulerReportClient() def _get_services(self, req): api_services = ('nova-osapi_compute', 'nova-ec2', 'nova-metadata') @@ -189,6 +192,16 @@ class ServiceController(wsgi.Controller): self.aggregate_api.remove_host_from_aggregate(context, ag.id, service.host) + # remove the corresponding resource provider record from + # placement for this compute node + self.placementclient.delete_resource_provider( + context, service.compute_node, cascade=True) + # remove the host_mapping of this host. + try: + hm = objects.HostMapping.get_by_host(context, service.host) + hm.destroy() + except exception.HostMappingNotFound: + pass self.host_api.service_delete(context, id) except exception.ServiceNotFound: diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py index e328eeb863c1..64a9a0bb1ab9 100644 --- a/nova/tests/functional/api/client.py +++ b/nova/tests/functional/api/client.py @@ -57,7 +57,17 @@ class APIResponse(object): self.status = response.status_code self.content = response.content if self.content: - self.body = jsonutils.loads(self.content) + # The Compute API and Placement API handle error responses a bit + # differently so we need to check the content-type header to + # figure out what to do. + content_type = response.headers.get('content-type') + if 'application/json' in content_type: + self.body = response.json() + elif 'text/html' in content_type: + self.body = response.text + else: + raise ValueError('Unexpected response content-type: %s' % + content_type) self.headers = response.headers def __str__(self): diff --git a/nova/tests/unit/api/openstack/compute/test_services.py b/nova/tests/unit/api/openstack/compute/test_services.py index 4d09658c25ee..76ea33095af9 100644 --- a/nova/tests/unit/api/openstack/compute/test_services.py +++ b/nova/tests/unit/api/openstack/compute/test_services.py @@ -31,6 +31,7 @@ from nova import compute from nova import context from nova import exception from nova import objects +from nova.scheduler.client import report as scheduler_report from nova.servicegroup.drivers import db as db_driver from nova import test from nova.tests.unit.api.openstack import fakes @@ -554,7 +555,9 @@ class ServicesTestV21(test.TestCase): self.controller.update, self.req, "disable-log-reason", body=body) - def test_services_delete(self): + @mock.patch.object(scheduler_report.SchedulerReportClient, + 'delete_resource_provider') + def test_services_delete(self, mock_delete_rp): self.ext_mgr.extensions['os-extended-services-delete'] = True compute = self.host_api.db.service_create(self.ctxt, @@ -563,12 +566,29 @@ class ServicesTestV21(test.TestCase): 'topic': 'compute', 'report_count': 0}) + self.host_api.db.compute_node_create(self.ctxt, + { + 'uuid': 'f1891d93-37a1-4230-aa10-433f63fb7d67', + 'host': 'fake-compute-host', + 'vcpus': 64, + 'memory_mb': 1024, + 'local_gb': 500, + 'vcpus_used': 0, + 'memory_mb_used': 128, + 'local_gb_used': 20, + 'hypervisor_type': 'QEMU', + 'hypervisor_version': 2009000, + 'cpu_info': '{"vendor": "Intel", "model": "Broadwell", ' + '"arch": "x86_64"}' + }) + with mock.patch.object(self.controller.host_api, 'service_delete') as service_delete: self.controller.delete(self.req, compute.id) service_delete.assert_called_once_with( self.req.environ['nova.context'], compute.id) self.assertEqual(self.controller.delete.wsgi_code, 204) + mock_delete_rp.assert_called_once() def test_services_delete_not_found(self): self.ext_mgr.extensions['os-extended-services-delete'] = True diff --git a/nova/tests/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py index 3f20816850b5..9a1fb0717321 100644 --- a/nova/tests/unit/compute/test_host_api.py +++ b/nova/tests/unit/compute/test_host_api.py @@ -31,6 +31,7 @@ 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 import testtools @@ -312,12 +313,18 @@ class ComputeHostAPITestCase(test.TestCase): get_by_id.assert_called_once_with(self.ctxt, 1) destroy.assert_called_once_with() - def test_service_delete_compute_in_aggregate(self): + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') + @mock.patch.object(objects.HostMapping, 'get_by_host') + def test_service_delete_compute_in_aggregate(self, mock_hm, mock_get_cn): compute = self.host_api.db.service_create(self.ctxt, {'host': 'fake-compute-host', 'binary': 'nova-compute', 'topic': 'compute', 'report_count': 0}) + # This is needed because of lazy-loading service.compute_node + cn = objects.ComputeNode(uuid=uuids.cn, host="fake-compute-host", + hypervisor_hostname="fake-compute-host") + mock_get_cn.return_value = [cn] aggregate = self.aggregate_api.create_aggregate(self.ctxt, 'aggregate', None) @@ -328,6 +335,7 @@ class ComputeHostAPITestCase(test.TestCase): result = self.aggregate_api.get_aggregate(self.ctxt, aggregate.id).hosts self.assertEqual([], result) + mock_hm.return_value.destroy.assert_called_once_with() class ComputeHostAPICellsTestCase(ComputeHostAPITestCase):