From 0ef4ed96d19a3576d67fdc9c1958610887d57ed4 Mon Sep 17 00:00:00 2001 From: Chen Date: Thu, 12 Jul 2018 20:33:14 +0800 Subject: [PATCH] fix cellv2 delete_host When trying to delete host that can be found in host_mappings but not in compute_nodes, current cellv2 delete_host will throw an exception but does not really handle it. This patch tries to handle this exception and allow the delete operation to continue since it shows the host has gone anyway. Change-Id: I99bd79fb45777edc0e33d846ba478b0a94a1191e Closes-Bug: #1781391 --- nova/cmd/manage.py | 5 ++++- nova/tests/unit/test_nova_manage.py | 34 ++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index c696393f0f..ef55895537 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -1725,7 +1725,10 @@ class CellV2Commands(object): with context.target_cell(ctxt, cell_mapping) as cctxt: instances = objects.InstanceList.get_by_host(cctxt, host) - nodes = objects.ComputeNodeList.get_all_by_host(cctxt, host) + try: + nodes = objects.ComputeNodeList.get_all_by_host(cctxt, host) + except exception.ComputeHostNotFound: + nodes = [] if instances: print(_('There are instances on the host %s.') % host) diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index ff02b2ebeb..94a919722f 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -2374,7 +2374,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase): @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host') def test_delete_host_success(self, mock_get_cn, mock_destroy, mock_get_by_host): - """Tests trying to delete a host that has not instances.""" + """Tests trying to delete a host that has no instances.""" ctxt = context.get_admin_context() # create the cell mapping cm1 = objects.CellMapping( @@ -2399,6 +2399,38 @@ class CellV2CommandsTestCase(test.NoDBTestCase): self.assertEqual(0, node.mapped) node.save.assert_called_once_with() + @mock.patch.object(objects.InstanceList, 'get_by_host', + return_value=[]) + @mock.patch.object(objects.HostMapping, 'destroy') + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + side_effect=exception.ComputeHostNotFound(host='fake-host')) + def test_delete_host_success_compute_host_not_found(self, mock_get_cn, + mock_destroy, + mock_get_by_host): + """Tests trying to delete a host that has no instances, but cannot + be found by ComputeNodeList.get_all_by_host. + """ + ctxt = context.get_admin_context() + # create the cell mapping + cm1 = objects.CellMapping( + context=ctxt, uuid=uuidsentinel.cell1, + database_connection='fake:///db', transport_url='fake:///mq') + cm1.create() + # create a host mapping in the cell + hm = objects.HostMapping( + context=ctxt, host='fake-host', cell_mapping=cm1) + hm.create() + + self.assertEqual(0, self.commands.delete_host(uuidsentinel.cell1, + 'fake-host')) + output = self.output.getvalue().strip() + self.assertEqual('', output) + mock_get_by_host.assert_called_once_with( + test.MatchType(context.RequestContext), 'fake-host') + mock_destroy.assert_called_once_with() + mock_get_cn.assert_called_once_with( + test.MatchType(context.RequestContext), 'fake-host') + @ddt.ddt class TestNovaManagePlacement(test.NoDBTestCase):