From 650fe118d128f09f78552b82abc114bb4b84930e Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 3 May 2019 15:23:57 -0400 Subject: [PATCH] Delete resource providers for all nodes when deleting compute service Change I7b8622b178d5043ed1556d7bdceaf60f47e5ac80 started deleting the compute node resource provider associated with a compute node when deleting a nova-compute service. However, it would only delete the first compute node associated with the service which means for an ironic compute service that is managing multiple nodes, the resource providers were not cleaned up in placement. This fixes the issue by iterating all the compute nodes and cleaning up their providers. Note this could be potentially a lot of nodes, but we don't really have many good options here but to iterate them and clean them up one at a time. Note that this is best-effort but because of how the SchedulerReportClient.delete_resource_provider method ignores ResourceProviderInUse errors, and we could have stale allocations on the host for which delete_resource_provider is not accounting, namely allocations from evacuated instances (or incomplete migrations though you can't migrate baremetal instances today), we could still delete the compute service and orphan those in-use providers. That, however, is no worse than before this change where we did not try to cleanup all providers. The issue described above is being tracked with bug 1829479 and will be dealt with separately. Change-Id: I9e852e25ea89f32bf19cdaeb1f5dac8f749f5dbc Closes-Bug: #1811726 --- nova/api/openstack/compute/services.py | 11 +++++++--- .../api/openstack/compute/test_services.py | 20 +++++++++++++------ ...26-multi-node-delete-2ba17f02c6171fbb.yaml | 10 ++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/bug-1811726-multi-node-delete-2ba17f02c6171fbb.yaml diff --git a/nova/api/openstack/compute/services.py b/nova/api/openstack/compute/services.py index f39d9b68ef3d..a779e96656ef 100644 --- a/nova/api/openstack/compute/services.py +++ b/nova/api/openstack/compute/services.py @@ -266,9 +266,14 @@ class ServiceController(wsgi.Controller): 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) + # placement for the compute nodes managed by this service; + # remember that an ironic compute service can manage multiple + # nodes + compute_nodes = objects.ComputeNodeList.get_all_by_host( + context, service.host) + for compute_node in compute_nodes: + self.placementclient.delete_resource_provider( + context, compute_node, cascade=True) # remove the host_mapping of this host. try: hm = objects.HostMapping.get_by_host(context, service.host) diff --git a/nova/tests/unit/api/openstack/compute/test_services.py b/nova/tests/unit/api/openstack/compute/test_services.py index 458e673956f5..8e7c4597b793 100644 --- a/nova/tests/unit/api/openstack/compute/test_services.py +++ b/nova/tests/unit/api/openstack/compute/test_services.py @@ -713,25 +713,33 @@ class ServicesTestV21(test.TestCase): """Tests that we are still able to successfully delete a nova-compute service even if the HostMapping is not found. """ + @mock.patch('nova.objects.ComputeNodeList.get_all_by_host', + return_value=objects.ComputeNodeList(objects=[ + objects.ComputeNode(host='host1', + hypervisor_hostname='node1'), + objects.ComputeNode(host='host1', + hypervisor_hostname='node2')])) @mock.patch.object(self.controller.host_api, 'service_get_by_id', return_value=objects.Service( - host='host1', binary='nova-compute', - compute_node=objects.ComputeNode())) + host='host1', binary='nova-compute')) @mock.patch.object(self.controller.aggregate_api, 'get_aggregates_by_host', return_value=objects.AggregateList()) @mock.patch.object(self.controller.placementclient, 'delete_resource_provider') def _test(delete_resource_provider, - get_aggregates_by_host, service_get_by_id): + get_aggregates_by_host, service_get_by_id, + cn_get_all_by_host): 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_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, - cascade=True) + self.assertEqual(2, delete_resource_provider.call_count) + nodes = cn_get_all_by_host.return_value + delete_resource_provider.assert_has_calls([ + mock.call(ctxt, node, cascade=True) for node in nodes + ], any_order=True) get_hm.assert_called_once_with(ctxt, 'host1') service_delete.assert_called_once_with() _test() diff --git a/releasenotes/notes/bug-1811726-multi-node-delete-2ba17f02c6171fbb.yaml b/releasenotes/notes/bug-1811726-multi-node-delete-2ba17f02c6171fbb.yaml new file mode 100644 index 000000000000..66fe6a5f9945 --- /dev/null +++ b/releasenotes/notes/bug-1811726-multi-node-delete-2ba17f02c6171fbb.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + `Bug 1811726`_ is fixed by deleting the resource provider (in placement) + associated with each compute node record managed by a ``nova-compute`` + service when that service is deleted via the + ``DELETE /os-services/{service_id}`` API. This is particularly important + for compute services managing ironic baremetal nodes. + + .. _Bug 1811726: https://bugs.launchpad.net/nova/+bug/1811726