Handle ObjectActionError during cells instance delete

There was logic in place to handle an ObjectActionError being raised
when deleting an instance with cellsv1, however a second
instance.destroy() call was added which did not have the same logic
applied. This adds appropriate handling for that case.

Change-Id: I007f249f772dbb53cf24a9bf06d2962001cb8384
Closes-Bug: 1621574

nova/compute/cells_api.py

@@ -221,7 +221,23 @@ 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.
-            if self._delete_while_booting(context, instance):
+            try:
+                if self._delete_while_booting(context, instance):
+                    return
+            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.  We handle this similarly to how the
+                # ObjectActionError is handled below.
+                with excutils.save_and_reraise_exception() as exc:
+                    instance = self._lookup_instance(context, instance.uuid)
+                    if instance is None:
+                        exc.reraise = False
+                    elif instance.cell_name:
+                        exc.reraise = False
+                        self._handle_cell_delete(context, instance,
+                                                 method_name)
                 return
             # If instance.cell_name was not set it's possible that the Instance
             # object here was pulled from a BuildRequest object and is not

nova/tests/unit/compute/test_compute_cells.py

@@ -15,6 +15,7 @@
 """
 Tests For Compute w/ Cells
 """
+import copy
 import functools
 import inspect
 
@@ -295,6 +296,60 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
     def test_force_delete_instance_no_cell(self):
         self._test_delete_instance_no_cell('force_delete')
 
+    @mock.patch.object(compute_api.API, '_delete_while_booting',
+                       side_effect=exception.ObjectActionError(
+                           action='delete', reason='host now set'))
+    @mock.patch.object(compute_api.API, '_local_delete')
+    @mock.patch.object(compute_api.API, '_lookup_instance')
+    @mock.patch.object(compute_api.API, 'delete')
+    def test_delete_instance_no_cell_then_cell(self, mock_delete,
+                                               mock_lookup_instance,
+                                               mock_local_delete,
+                                               mock_delete_while_booting):
+        # This checks the case where initially an instance has no cell_name,
+        # and therefore no host, set but instance.destroy fails because
+        # there is now a host.
+        instance = self._create_fake_instance_obj()
+        instance_with_cell = copy.deepcopy(instance)
+        instance_with_cell.cell_name = 'foo'
+        mock_lookup_instance.return_value = instance_with_cell
+
+        cells_rpcapi = self.compute_api.cells_rpcapi
+
+        @mock.patch.object(cells_rpcapi, 'instance_delete_everywhere')
+        def test(mock_inst_delete_everywhere):
+            self.compute_api.delete(self.context, instance)
+            mock_local_delete.assert_not_called()
+            mock_delete.assert_called_once_with(self.context,
+                                                instance_with_cell)
+
+        test()
+
+    @mock.patch.object(compute_api.API, '_delete_while_booting',
+                       side_effect=exception.ObjectActionError(
+                           action='delete', reason='host now set'))
+    @mock.patch.object(compute_api.API, '_local_delete')
+    @mock.patch.object(compute_api.API, '_lookup_instance')
+    @mock.patch.object(compute_api.API, 'delete')
+    def test_delete_instance_no_cell_then_no_instance(self,
+            mock_delete, mock_lookup_instance, mock_local_delete,
+            mock_delete_while_booting):
+        # This checks the case where initially an instance has no cell_name,
+        # and therefore no host, set but instance.destroy fails because
+        # there is now a host. And then the instance can't be looked up.
+        instance = self._create_fake_instance_obj()
+        mock_lookup_instance.return_value = None
+
+        cells_rpcapi = self.compute_api.cells_rpcapi
+
+        @mock.patch.object(cells_rpcapi, 'instance_delete_everywhere')
+        def test(mock_inst_delete_everywhere):
+            self.compute_api.delete(self.context, instance)
+            mock_local_delete.assert_not_called()
+            mock_delete.assert_not_called()
+
+        test()
+
     def test_get_migrations(self):
         filters = {'cell_name': 'ChildCell', 'status': 'confirmed'}
         migrations = {'migrations': [{'id': 1234}]}