Make cells use Fault obj for create

Add support into InstanceFault.create() to handle where we need to
duplicate any faults in the API cells. Convert the cells code to use
InstanceFault.create()

NOTE: This happens to fix an oversight where we accidentally left the
update_cells=False kwarg off of the db call in the cells code.

Change-Id: Iec11b8259aaecba055a6d5636ab60dfa0dbb6c83

nova/cells/messaging.py

@@ -47,6 +47,7 @@ from nova import exception
 from nova.network import model as network_model
 from nova.objects import base as objects_base
 from nova.objects import instance as instance_obj
+from nova.objects import instance_fault as instance_fault_obj
 from nova.openstack.common import excutils
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import importutils
@@ -1099,7 +1100,9 @@ class _BroadcastMessageMethods(_BaseMessageMethods):
         log_str = _("Got message to create instance fault: "
                     "%(instance_fault)s")
         LOG.debug(log_str, {'instance_fault': instance_fault})
-        self.db.instance_fault_create(message.ctxt, instance_fault)
+        fault = instance_fault_obj.InstanceFault(context=message.ctxt)
+        fault.update(instance_fault)
+        fault.create()
 
     def bw_usage_update_at_top(self, message, bw_update_info, **kwargs):
         """Update Bandwidth usage in the DB if we're a top level cell."""

nova/db/api.py

@@ -1828,15 +1828,9 @@ def aggregate_host_delete(context, aggregate_id, host):
 ####################
 
 
-def instance_fault_create(context, values, update_cells=True):
+def instance_fault_create(context, values):
     """Create a new Instance Fault."""
-    rv = IMPL.instance_fault_create(context, values)
-    if update_cells:
-        try:
-            cells_rpcapi.CellsAPI().instance_fault_create_at_top(context, rv)
-        except Exception:
-            LOG.exception(_("Failed to notify cells of instance fault"))
-    return rv
+    return IMPL.instance_fault_create(context, values)
 
 
 def instance_fault_get_by_instance_uuids(context, instance_uuids):

nova/objects/instance_fault.py

@@ -14,10 +14,17 @@
 
 import itertools
 
+from nova.cells import opts as cells_opts
+from nova.cells import rpcapi as cells_rpcapi
 from nova import db
 from nova import exception
 from nova.objects import base
 from nova.objects import fields
+from nova.openstack.common.gettextutils import _LE
+from nova.openstack.common import log as logging
+
+
+LOG = logging.getLogger(__name__)
 
 
 class InstanceFault(base.NovaPersistentObject, base.NovaObject):
@@ -67,6 +74,16 @@ class InstanceFault(base.NovaPersistentObject, base.NovaObject):
         db_fault = db.instance_fault_create(context, values)
         self._from_db_object(context, self, db_fault)
         self.obj_reset_changes()
+        # Cells should only try sending a message over to nova-cells
+        # if cells is enabled and we're not the API cell. Otherwise,
+        # if the API cell is calling this, we could end up with
+        # infinite recursion.
+        if cells_opts.get_cell_type() == 'compute':
+            try:
+                cells_rpcapi.CellsAPI().instance_fault_create_at_top(
+                    context, db_fault)
+            except Exception:
+                LOG.exception(_LE("Failed to notify cells of instance fault"))
 
 
 class InstanceFaultList(base.ObjectListBase, base.NovaObject):

nova/tests/cells/test_cells_messaging.py

@@ -17,6 +17,7 @@
 Tests For Cells Messaging module
 """
 
+import mock
 import mox
 from oslo.config import cfg
 from oslo import messaging as oslo_messaging
@@ -32,6 +33,7 @@ from nova.network import model as network_model
 from nova.objects import base as objects_base
 from nova.objects import fields as objects_fields
 from nova.objects import instance as instance_obj
+from nova.objects import instance_fault as instance_fault_obj
 from nova.openstack.common import jsonutils
 from nova.openstack.common import timeutils
 from nova.openstack.common import uuidutils
@@ -1608,22 +1610,27 @@ class CellsBroadcastMethodsTestCase(test.TestCase):
 
     def test_instance_fault_create_at_top(self):
         fake_instance_fault = {'id': 1,
-                               'other stuff': 2,
-                               'more stuff': 3}
-        expected_instance_fault = {'other stuff': 2,
-                                   'more stuff': 3}
+                               'message': 'fake-message',
+                               'details': 'fake-details'}
 
-        # Shouldn't be called for these 2 cells
-        self.mox.StubOutWithMock(self.src_db_inst, 'instance_fault_create')
-        self.mox.StubOutWithMock(self.mid_db_inst, 'instance_fault_create')
+        if_mock = mock.Mock(spec_set=instance_fault_obj.InstanceFault)
 
-        self.mox.StubOutWithMock(self.tgt_db_inst, 'instance_fault_create')
-        self.tgt_db_inst.instance_fault_create(self.ctxt,
-                                               expected_instance_fault)
-        self.mox.ReplayAll()
+        def _check_create():
+            self.assertEqual('fake-message', if_mock.message)
+            self.assertEqual('fake-details', if_mock.details)
+            # Should not be set
+            self.assertNotEqual(1, if_mock.id)
 
-        self.src_msg_runner.instance_fault_create_at_top(self.ctxt,
-                fake_instance_fault)
+        if_mock.create.side_effect = _check_create
+
+        with mock.patch.object(instance_fault_obj,
+                               'InstanceFault') as if_obj_mock:
+            if_obj_mock.return_value = if_mock
+            self.src_msg_runner.instance_fault_create_at_top(
+                    self.ctxt, fake_instance_fault)
+
+        if_obj_mock.assert_called_once_with(context=self.ctxt)
+        if_mock.create.assert_called_once_with()
 
     def test_bw_usage_update_at_top(self):
         fake_bw_update_info = {'uuid': 'fake_uuid',

nova/tests/objects/test_instance_fault.py

@@ -73,8 +73,9 @@ class _TestInstanceFault(object):
             self.context, ['fake-uuid'])
         self.assertEqual(0, len(faults))
 
+    @mock.patch('nova.cells.rpcapi.CellsAPI.instance_fault_create_at_top')
     @mock.patch('nova.db.instance_fault_create')
-    def test_create(self, mock_create):
+    def _test_create(self, update_cells, mock_create, cells_fault_create):
         mock_create.return_value = fake_faults['fake-uuid'][1]
         fault = instance_fault.InstanceFault()
         fault.instance_uuid = 'fake-uuid'
@@ -90,6 +91,23 @@ class _TestInstanceFault(object):
                                              'message': 'foo',
                                              'details': 'you screwed up',
                                              'host': 'myhost'})
+        if update_cells:
+            cells_fault_create.assert_called_once_with(
+                    self.context, fake_faults['fake-uuid'][1])
+        else:
+            self.assertFalse(cells_fault_create.called)
+
+    def test_create_no_cells(self):
+        self.flags(enable=False, group='cells')
+        self._test_create(False)
+
+    def test_create_api_cell(self):
+        self.flags(cell_type='api', enable=True, group='cells')
+        self._test_create(False)
+
+    def test_create_compute_cell(self):
+        self.flags(cell_type='compute', enable=True, group='cells')
+        self._test_create(True)
 
     def test_create_already_created(self):
         fault = instance_fault.InstanceFault()