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 589c495c1a)
(cherry picked from commit dede2de2bd)
(cherry picked from commit 9fe847bdca)

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:

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):

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

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):