diff --git a/nova/cells/messaging.py b/nova/cells/messaging.py index 411259eb00..b77781bbb4 100644 --- a/nova/cells/messaging.py +++ b/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.""" diff --git a/nova/db/api.py b/nova/db/api.py index 852563b16e..36bd756b3d 100644 --- a/nova/db/api.py +++ b/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): diff --git a/nova/objects/instance_fault.py b/nova/objects/instance_fault.py index 06e8fe53ce..794673cc43 100644 --- a/nova/objects/instance_fault.py +++ b/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): diff --git a/nova/tests/cells/test_cells_messaging.py b/nova/tests/cells/test_cells_messaging.py index f203f3e74c..ebd4cc749f 100644 --- a/nova/tests/cells/test_cells_messaging.py +++ b/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', diff --git a/nova/tests/objects/test_instance_fault.py b/nova/tests/objects/test_instance_fault.py index 2d7e454fd1..f1bf217d80 100644 --- a/nova/tests/objects/test_instance_fault.py +++ b/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()