From 0d9a2b9de2304d44c4fab3c20af6a239f862db52 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Fri, 2 May 2014 09:25:50 -0700 Subject: [PATCH] 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 | 5 +++- nova/db/api.py | 10 ++----- nova/objects/instance_fault.py | 17 ++++++++++++ nova/tests/cells/test_cells_messaging.py | 33 ++++++++++++++--------- nova/tests/objects/test_instance_fault.py | 20 +++++++++++++- 5 files changed, 62 insertions(+), 23 deletions(-) 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()