From ee2f65d91fb632f04be3572967029cd58be4c614 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 1 Feb 2018 18:56:10 +0100 Subject: [PATCH] Extending delete_cell --force to delete instance_mappings Currently the nova-manage delete_cell command with the --force option allows us to destroy hosts_mappings of the cell which does not have any instances or instance_mappings. This patch extends this command with the --force option for destroying instance_mappings in addition to the host_mappings of the cell which is to be deleted. Note that the instance mappings and host mappings are only removed if there are no living instances in the cell. So a --force delete here will not work if there are non-deleted instances. Change-Id: Ibefa0465224bec9a22431c7d3b5c8f5d91fc7732 Closes-Bug: #1745375 --- doc/source/cli/nova-manage.rst | 17 +++++----- nova/cmd/manage.py | 35 +++++++++++++-------- nova/tests/unit/test_nova_manage.py | 48 +++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 21 deletions(-) diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index 39dc3105277f..ecb51137d906 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -212,14 +212,15 @@ Nova Cells v2 ``nova-manage cell_v2 delete_cell [--force] --cell_uuid `` - Delete a cell by the given uuid. Returns 0 if the empty cell is - found and deleted successfully or the cell that has hosts is found and - the cell and the hosts are deleted successfully with ``--force`` option, - 1 if a cell with that uuid could not be found, 2 if host mappings were - found for the cell (cell not empty) without ``--force`` option, and 3 - if there are instances mapped to the cell (cell not empty), 4 if there are - instance mappings to the cell but all instances have been deleted - in the cell. + Delete a cell by the given uuid. Returns 0 if the empty cell is found and + deleted successfully or the cell that has hosts is found and the cell, hosts + and the instance_mappings are deleted successfully with ``--force`` option + (this happens if there are no living instances), 1 if a cell with that uuid + could not be found, 2 if host mappings were found for the cell (cell not empty) + without ``--force`` option, 3 if there are instances mapped to the cell + (cell not empty) irrespective of the ``--force`` option, and 4 if there are + instance mappings to the cell but all instances have been deleted in the cell, + again without the ``--force`` option. ``nova-manage cell_v2 list_hosts [--cell_uuid ]`` diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 330cef307b69..9a62f928d2f6 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -1368,7 +1368,8 @@ class CellV2Commands(object): return 0 @args('--force', action='store_true', default=False, - help=_('Delete hosts that belong to the cell as well.')) + help=_('Delete hosts and instance_mappings that belong ' + 'to the cell as well.')) @args('--cell_uuid', metavar='', dest='cell_uuid', required=True, help=_('The uuid of the cell to delete.')) def delete_cell(self, cell_uuid, force=False): @@ -1378,15 +1379,17 @@ class CellV2Commands(object): * The cell is not found by uuid. * It has hosts and force is False. - * It has instance mappings. + * It has instance mappings and force is False. - If force is True and the cell has host, hosts are deleted as well. + If force is True and the cell has hosts and/or instance_mappings, they + are deleted as well (as long as there are no living instances). Returns 0 in the following cases. * The empty cell is found and deleted successfully. - * The cell has hosts and force is True and the cell and the hosts are - deleted successfully. + * The cell has hosts and force is True then the cell, hosts and + instance_mappings are deleted successfully; if there are no + living instances. """ ctxt = context.get_admin_context() # Find the CellMapping given the uuid. @@ -1415,14 +1418,20 @@ class CellV2Commands(object): print(_('There are existing instances mapped to cell with ' 'uuid %s.') % cell_uuid) return 3 - # There are no instances in the cell but the records remains - # in the 'instance_mappings' table. - print(_("There are instance mappings to cell with uuid %s, " - "but all instances have been deleted " - "in the cell.") % cell_uuid) - print(_("So execute 'nova-manage db archive_deleted_rows' to " - "delete the instance mappings.")) - return 4 + else: + if not force: + # There are no instances in the cell but the records remain + # in the 'instance_mappings' table. + print(_("There are instance mappings to cell with uuid " + "%s, but all instances have been deleted " + "in the cell.") % cell_uuid) + print(_("So execute 'nova-manage db archive_deleted_rows' " + "to delete the instance mappings.")) + return 4 + + # Delete instance_mappings of the deleted instances + for instance_mapping in instance_mappings: + instance_mapping.destroy() # Delete hosts mapped to the cell. for host_mapping in host_mappings: diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index ad9d1836761c..ab925d5d442e 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -1802,6 +1802,54 @@ class CellV2CommandsTestCase(test.NoDBTestCase): mock_hm_destroy.assert_called_once_with() mock_cell_destroy.assert_called_once_with() + @mock.patch.object(context, 'target_cell') + @mock.patch.object(objects.InstanceMapping, 'destroy') + @mock.patch.object(objects.HostMapping, 'destroy') + @mock.patch.object(objects.CellMapping, 'destroy') + def test_delete_cell_force_with_inst_mappings_of_deleted_instances(self, + mock_cell_destroy, mock_hm_destroy, + mock_im_destroy, mock_target_cell): + + # Test for verifying the deletion of instance_mappings + # of deleted instances when using the --force option + ctxt = context.get_admin_context() + # create the cell mapping + cm = objects.CellMapping( + context=ctxt, uuid=uuidsentinel.cell1, + database_connection='fake:///db', transport_url='fake:///mq') + cm.create() + mock_target_cell.return_value.__enter__.return_value = ctxt + # create a host mapping in this cell + hm = objects.HostMapping( + context=ctxt, host='fake-host', cell_mapping=cm) + hm.create() + # create an instance and its mapping. + inst_uuid = uuidutils.generate_uuid() + proj_uuid = uuidutils.generate_uuid() + instance = objects.Instance(ctxt, project_id=proj_uuid, + uuid=inst_uuid) + instance.create() + + im = objects.InstanceMapping(ctxt, project_id=proj_uuid, + cell_mapping=cm, + instance_uuid=inst_uuid) + im.create() + + res = self.commands.delete_cell(uuidsentinel.cell1, force=True) + self.assertEqual(3, res) + output = self.output.getvalue().strip() + self.assertIn('There are existing instances mapped to cell', output) + + # delete the instance such that we now have only its mapping + instance.destroy() + + res = self.commands.delete_cell(uuidsentinel.cell1, force=True) + self.assertEqual(0, res) + mock_hm_destroy.assert_called_once_with() + mock_cell_destroy.assert_called_once_with() + mock_im_destroy.assert_called_once_with() + self.assertEqual(2, mock_target_cell.call_count) + def test_update_cell_not_found(self): self.assertEqual(1, self.commands.update_cell( uuidsentinel.cell1, 'foo', 'fake://new', 'fake:///new'))