Unmap compute nodes when deleting host mappings in delete cell operation

Presently when a host_mapping is deleted during the nova-manage cell_v2
delete_cell operation, the compute node record still remains with its
mapped field set to 1. Due to this, if we delete a cell, and later
create a new cell, the discover_hosts command will not work and the
compute node will not get mapped to the new cell. This patch fixes this
issue by setting it back to 0.

Conflicts:
    nova/tests/unit/test_nova_manage.py

NOTE(tssurya): The conflict is due to not having the change
Ibefa0465224bec9a22431c7d3b5c8f5d91fc7732 in Queens.

Change-Id: Iaf13f11bcd0bb25f673fc28f252cb9391409b9b5
Closes-Bug: #1747936
(cherry picked from commit dc2d15129d)

nova/cmd/manage.py

@@ -1396,10 +1396,16 @@ class CellV2Commands(object):
         # Check to see if there are any HostMappings for this cell.
         host_mappings = objects.HostMappingList.get_by_cell_id(
             ctxt, cell_mapping.id)
-        if host_mappings and not force:
-            print(_('There are existing hosts mapped to cell with uuid %s.') %
-                  cell_uuid)
-            return 2
+        nodes = []
+        if host_mappings:
+            if not force:
+                print(_('There are existing hosts mapped to cell with uuid '
+                        '%s.') % cell_uuid)
+                return 2
+            # We query for the compute nodes in the cell,
+            # so that they can be unmapped.
+            with context.target_cell(ctxt, cell_mapping) as cctxt:
+                nodes = objects.ComputeNodeList.get_all(cctxt)
 
         # Check to see if there are any InstanceMappings for this cell.
         instance_mappings = objects.InstanceMappingList.get_by_cell_id(
@@ -1421,6 +1427,12 @@ class CellV2Commands(object):
                     "delete the instance mappings."))
             return 4
 
+        # Unmap the compute nodes so that they can be discovered
+        # again in future, if needed.
+        for node in nodes:
+            node.mapped = 0
+            node.save()
+
         # Delete hosts mapped to the cell.
         for host_mapping in host_mappings:
             host_mapping.destroy()

nova/tests/functional/test_nova_manage.py

@@ -317,3 +317,34 @@ class NovaManageCellV2Test(test.TestCase):
         cns = objects.ComputeNodeList.get_all(self.context)
         self.assertEqual(1, len(cns))
         self.assertEqual(0, cns[0].mapped)
+
+    def test_delete_cell_force_unmaps_computes(self):
+        cells = objects.CellMappingList.get_all(self.context)
+
+        self.commands.discover_hosts()
+
+        # We should have one host mapping
+        hms = objects.HostMappingList.get_all(self.context)
+        self.assertEqual(1, len(hms))
+
+        # We should have one mapped node
+        cns = objects.ComputeNodeList.get_all(self.context)
+        self.assertEqual(1, len(cns))
+        self.assertEqual(1, cns[0].mapped)
+
+        for cell in cells:
+            res = self.commands.delete_cell(cell.uuid, force=True)
+            self.assertEqual(0, res)
+
+        # The host mapping should be deleted since the force option is used
+        hms = objects.HostMappingList.get_all(self.context)
+        self.assertEqual(0, len(hms))
+
+        # All our cells should be deleted
+        cells = objects.CellMappingList.get_all(self.context)
+        self.assertEqual(0, len(cells))
+
+        # Our node should now be unmapped
+        cns = objects.ComputeNodeList.get_all(self.context)
+        self.assertEqual(1, len(cns))
+        self.assertEqual(0, cns[0].mapped)

nova/tests/unit/test_nova_manage.py

@@ -1706,7 +1706,8 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         self.assertEqual('Cell with uuid %s was not found.' % cell_uuid,
                          output)
 
-    def test_delete_cell_host_mappings_exist(self):
+    @mock.patch.object(objects.ComputeNodeList, 'get_all')
+    def test_delete_cell_host_mappings_exist(self, mock_get_cn):
         """Tests trying to delete a cell which has host mappings."""
         cell_uuid = uuidutils.generate_uuid()
         ctxt = context.get_admin_context()
@@ -1719,6 +1720,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         hm = objects.HostMapping(
             context=ctxt, host='fake-host', cell_mapping=cm)
         hm.create()
+        mock_get_cn.return_value = []
         self.assertEqual(2, self.commands.delete_cell(cell_uuid))
         output = self.output.getvalue().strip()
         self.assertIn('There are existing hosts mapped to cell', output)
@@ -1783,10 +1785,11 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         output = self.output.getvalue().strip()
         self.assertEqual('', output)
 
+    @mock.patch.object(objects.ComputeNodeList, 'get_all')
     @mock.patch.object(objects.HostMapping, 'destroy')
     @mock.patch.object(objects.CellMapping, 'destroy')
     def test_delete_cell_success_with_host_mappings(self, mock_cell_destroy,
-                                                    mock_hm_destroy):
+                                            mock_hm_destroy, mock_get_cn):
         """Tests trying to delete a cell with host."""
         ctxt = context.get_admin_context()
         # create the cell mapping
@@ -1798,6 +1801,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         hm = objects.HostMapping(
             context=ctxt, host='fake-host', cell_mapping=cm)
         hm.create()
+        mock_get_cn.return_value = []
         self.assertEqual(0, self.commands.delete_cell(uuidsentinel.cell1,
                                                       force=True))
         output = self.output.getvalue().strip()