Retry a cell delete if host constraint fails

The instance.destroy method has a constraint that fails during a local
delete if the host is not None, because it should be in that case.
Cells calls local_delete when there's no cell_name on the instance, but
when this fails it's likely that we have a host and cell_name now so the
delete method should be attempted again.

Closes-Bug: 1454839
Change-Id: I412fab6f43cf2c64239a0292946c6c3bb9837110

nova/compute/cells_api.py

@@ -17,6 +17,7 @@
 """Compute API that proxies via Cells Service."""
 
 import oslo_messaging as messaging
+from oslo_utils import excutils
 
 from nova import availability_zones
 from nova import block_device
@@ -221,9 +222,21 @@ class ComputeCellsAPI(compute_api.API):
                     context, instance.uuid))
             # NOTE(danms): If we try to delete an instance with no cell,
             # there isn't anything to salvage, so we can hard-delete here.
-            super(ComputeCellsAPI, self)._local_delete(context, instance, bdms,
-                                                       method_name,
-                                                       self._do_delete)
+            try:
+                super(ComputeCellsAPI, self)._local_delete(context, instance,
+                                                           bdms, method_name,
+                                                           self._do_delete)
+            except exception.ObjectActionError:
+                # NOTE(alaski): We very likely got here because the host
+                # constraint in instance.destroy() failed.  This likely means
+                # that an update came up from a child cell and cell_name is
+                # set now.  If so try the delete again.
+                with excutils.save_and_reraise_exception() as exc:
+                    instance.refresh()
+                    if instance.cell_name:
+                        exc.reraise = False
+                        self._handle_cell_delete(context, instance,
+                                method_name)
             return
 
         method = getattr(super(ComputeCellsAPI, self), method_name)

nova/tests/unit/compute/test_compute_cells.py

@@ -30,6 +30,7 @@ from nova.compute import flavors
 from nova.compute import vm_states
 from nova import context
 from nova import db
+from nova import exception
 from nova import objects
 from nova import quota
 from nova import test
@@ -149,6 +150,45 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
                        lambda *a, **kw: None)
         self.compute_api.delete(self.context, inst)
 
+    def test_delete_instance_no_cell_constraint_failure_does_not_loop(self):
+        with mock.patch.object(self.compute_api.cells_rpcapi,
+                'instance_delete_everywhere'):
+            inst = self._create_fake_instance_obj()
+            inst.cell_name = None
+
+            inst.destroy = mock.MagicMock()
+            inst.destroy.side_effect = exception.ObjectActionError(action='',
+                    reason='')
+            inst.refresh = mock.MagicMock()
+
+            self.assertRaises(exception.ObjectActionError,
+                    self.compute_api.delete, self.context, inst)
+            inst.destroy.assert_called_once_with()
+
+    def test_delete_instance_no_cell_constraint_failure_corrects_itself(self):
+
+        def add_cell_name(context, instance, delete_type):
+            instance.cell_name = 'fake_cell_name'
+
+        @mock.patch.object(compute_api.API, 'delete')
+        @mock.patch.object(self.compute_api.cells_rpcapi,
+                'instance_delete_everywhere', side_effect=add_cell_name)
+        def _test(mock_delete_everywhere, mock_compute_delete):
+            inst = self._create_fake_instance_obj()
+            inst.cell_name = None
+
+            inst.destroy = mock.MagicMock()
+            inst.destroy.side_effect = exception.ObjectActionError(action='',
+                    reason='')
+            inst.refresh = mock.MagicMock()
+
+            self.compute_api.delete(self.context, inst)
+            inst.destroy.assert_called_once_with()
+
+            mock_compute_delete.assert_called_once_with(self.context, inst)
+
+        _test()
+
     def test_soft_delete_instance_no_cell(self):
         cells_rpcapi = self.compute_api.cells_rpcapi
         self.mox.StubOutWithMock(cells_rpcapi,