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)
This commit is contained in:
Surya Seetharaman 2018-03-21 14:16:24 +01:00 committed by Matt Riedemann
parent cd50dcaf3e
commit 9fe847bdca
5 changed files with 61 additions and 10 deletions

View File

@ -27,6 +27,7 @@ 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
@ -42,6 +43,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):
# The API services are filtered out since they are not RPC services
@ -234,6 +236,13 @@ 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.
hm = objects.HostMapping.get_by_host(context, service.host)
hm.destroy()
self.host_api.service_delete(context, id)
except exception.ServiceNotFound:

View File

@ -56,7 +56,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):

View File

@ -18,6 +18,8 @@ from oslo_utils import fixture as utils_fixture
from nova import exception
from nova.tests.functional.api_sample_tests import api_sample_base
from nova.tests.unit.api.openstack.compute import test_services
from nova.tests.unit.objects import test_compute_node
from nova.tests.unit.objects import test_host_mapping
class ServicesJsonTest(api_sample_base.ApiSampleTestBaseV21):
@ -119,7 +121,29 @@ class ServicesV253JsonTest(ServicesV211JsonTest):
if svc['uuid'] == service_uuid:
return svc
raise exception.ServiceNotFound(service_id=service_uuid)
def fake_cn_get_all_by_host(context, host):
cn = test_compute_node.fake_compute_node
cn['uuid'] = test_services.FAKE_UUID_COMPUTE_HOST1
cn['host'] = host
return [cn]
def fake_hm_get_by_host(context, host):
hm = test_host_mapping.get_db_mapping()
hm['host'] = host
return hm
def fake_hm_destroy(context, host):
return 1
self.stub_out('nova.db.service_get_by_uuid', db_service_get_by_uuid)
self.stub_out('nova.db.compute_node_get_all_by_host',
fake_cn_get_all_by_host)
self.stub_out(
'nova.objects.host_mapping.HostMapping._get_by_host_from_db',
fake_hm_get_by_host)
self.stub_out('nova.objects.host_mapping.HostMapping._destroy_in_db',
fake_hm_destroy)
def test_service_enable(self):
"""Enable an existing service."""

View File

@ -13,6 +13,7 @@
import six
from nova import context as nova_context
from nova import exception
from nova import objects
from nova.objects import fields as rc_fields
from nova.tests.functional.api import client as api_client
@ -105,13 +106,13 @@ class TestServicesAPI(test_servers.ProviderUsageBaseTestCase):
'os-hypervisors?hypervisor_hostname_pattern=%s' % service['host'],
check_response_status=[404])
# FIXME(mriedem): This is bug 1756179 where the host mapping is not
# deleted. Once the bug is fixed, we should be able to change this
# to assert that HostMappingNotFound is raised.
objects.HostMapping.get_by_host(ctxt, service['host'])
# The host mapping should also be gone.
self.assertRaises(exception.HostMappingNotFound,
objects.HostMapping.get_by_host,
ctxt, service['host'])
# FIXME(mriedem): This is bug 1756179 where the compute node resource
# provider is not deleted. Once the bug is fixed, this should result
# in a 404 response.
# And finally, the resource provider should also be gone. The API
# will perform a cascading delete of the resource provider inventory
# and allocation information.
resp = self.placement_api.get('/resource_providers/%s' % rp_uuid)
self.assertEqual(200, resp.status)
self.assertEqual(404, resp.status)

View File

@ -405,12 +405,18 @@ class ComputeHostAPITestCase(test.TestCase):
self.assertFalse(service2.destroy.called)
self.assertFalse(set_target.called)
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)
@ -421,6 +427,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()
@mock.patch('nova.db.compute_node_statistics')
def test_compute_node_statistics(self, mock_cns):