Merge "Make cells use Fault obj for create"

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()