Fill fault from all nova cells in GET servers/detail

nova/objects/instance.py

@@ -13,6 +13,7 @@
 #    under the License.
 
 import contextlib
+import itertools
 
 from oslo_config import cfg
 from oslo_db import exception as db_exc
@@ -31,6 +32,7 @@
 from nova.cells import utils as cells_utils
 from nova.compute import task_states
 from nova.compute import vm_states
+from nova import context as nova_context
 from nova import db
 from nova.db.sqlalchemy import api as db_api
 from nova.db.sqlalchemy import models
@@ -1426,11 +1428,29 @@ def fill_faults(self):
         :returns: A list of instance uuids for which faults were found.
         """
         uuids = [inst.uuid for inst in self]
-        faults = objects.InstanceFaultList.get_latest_by_instance_uuids(
+        inst_mappings = objects.InstanceMappingList.get_by_instance_uuids(
             self._context, uuids)
+
+        def _group_inst_mappings_by_cell_id(instance_mappings):
+            def _im_cell_id(instance_mapping):
+                cm = instance_mapping.cell_mapping
+                cell_id = getattr(cm, 'id', None) if cm else None
+                return cell_id
+
+            instance_mappings = sorted(instance_mappings, key=_im_cell_id)
+            return itertools.groupby(instance_mappings, key=_im_cell_id)
+
         faults_by_uuid = {}
-        for fault in faults:
-            faults_by_uuid[fault.instance_uuid] = fault
+        for cell_id, ims in _group_inst_mappings_by_cell_id(inst_mappings):
+            cm, inst_uuids = None, []
+            for im in ims:
+                inst_uuids.append(im.instance_uuid)
+                cm = im.cell_mapping
+            with nova_context.target_cell(self._context, cm) as cell_ctxt:
+                faults = (objects.InstanceFaultList.
+                    get_latest_by_instance_uuids(cell_ctxt, inst_uuids))
+                for fault in faults:
+                    faults_by_uuid[fault.instance_uuid] = fault
 
         for instance in self:
             if instance.uuid in faults_by_uuid:

nova/tests/unit/api/openstack/compute/test_extended_volumes.py

@@ -140,8 +140,9 @@ def test_show(self):
         actual = server.get('%svolumes_attached' % self.prefix)
         self.assertEqual(self.exp_volumes_show, actual)
 
+    @mock.patch.object(objects.InstanceList, 'fill_faults')
     @mock.patch.object(objects.InstanceMappingList, 'get_by_instance_uuids')
-    def test_detail(self, mock_get_by_instance_uuids):
+    def test_detail(self, mock_get_by_instance_uuids, mock_fill_faults):
         mock_get_by_instance_uuids.return_value = [
             objects.InstanceMapping(
                 instance_uuid=UUID1,
@@ -165,10 +166,12 @@ def test_detail(self, mock_get_by_instance_uuids):
             actual = server.get('%svolumes_attached' % self.prefix)
             self.assertEqual(self.exp_volumes_detail[i], actual)
 
+    @mock.patch.object(objects.InstanceList, 'fill_faults')
     @mock.patch.object(objects.InstanceMappingList, 'get_by_instance_uuids')
     @mock.patch('nova.context.scatter_gather_cells')
     def test_detail_with_cell_failures(self, mock_sg,
-                                        mock_get_by_instance_uuids):
+                                       mock_get_by_instance_uuids,
+                                       mock_fill_faults):
 
         mock_get_by_instance_uuids.return_value = [
             objects.InstanceMapping(

nova/tests/unit/objects/test_instance.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from contextlib import contextmanager
 import datetime
 
 import mock
@@ -1869,13 +1870,22 @@ def test_with_fault(self, mock_get_all, mock_fault_get):
         mock_fault_get.assert_called_once_with(self.context,
             [x['uuid'] for x in fake_insts])
 
+    @mock.patch('nova.objects.InstanceMappingList.get_by_instance_uuids')
     @mock.patch.object(db, 'instance_fault_get_by_instance_uuids')
-    def test_fill_faults(self, mock_fault_get):
+    def test_fill_faults(self, mock_fault_get, mock_get_inst_map_list):
         inst1 = objects.Instance(uuid=uuids.db_fault_1)
         inst2 = objects.Instance(uuid=uuids.db_fault_2)
         insts = [inst1, inst2]
         for inst in insts:
             inst.obj_reset_changes()
+
+        mock_get_inst_map_list.return_value = [
+            mock.Mock(cell_mapping=None,
+                      instance_uuid=uuids.db_fault_1),
+            mock.Mock(cell_mapping=None,
+                      instance_uuid=uuids.db_fault_2)
+        ]
+
         db_faults = {
             'uuid1': [{'id': 123,
                        'instance_uuid': uuids.db_fault_1,
@@ -1902,10 +1912,83 @@ def test_fill_faults(self, mock_fault_get):
         for inst in inst_list:
             self.assertEqual(set(), inst.obj_what_changed())
 
-        mock_fault_get.assert_called_once_with(self.context,
+        mock_fault_get.assert_called_once_with(mock.ANY,
                                                [x.uuid for x in insts],
                                                latest=True)
 
+    @mock.patch('nova.context.target_cell')
+    @mock.patch('nova.objects.InstanceMappingList.get_by_instance_uuids')
+    @mock.patch.object(db, 'instance_fault_get_by_instance_uuids')
+    def test_fill_faults_multicell(self,
+                                     mock_fault_get,
+                                     mock_get_inst_map_list,
+                                     mock_ctxt_target_cell):
+        # Prepare list with 2 instances
+        inst_list = objects.InstanceList()
+        inst_list._context = self.context
+        inst_list.objects = [
+            objects.Instance(uuid=uuids.inst_1),
+            objects.Instance(uuid=uuids.inst_2)
+        ]
+        for inst in inst_list.objects:
+            inst.obj_reset_changes()
+
+        # Define different cell mappings for the 2 instances
+        im1 = mock.Mock(instance_uuid=uuids.inst_1,
+                        cell_mapping=mock.Mock(
+                            database_connection=self.context.db_connection))
+        im2 = mock.Mock(instance_uuid=uuids.inst_2)
+        mock_get_inst_map_list.return_value = [im1, im2]
+
+        # Define faults for the instances in different cell DBs (mocked)
+        def _fake_db_faults(instance_uuid):
+            return {
+                instance_uuid: [{
+                    'id': 123,
+                    'instance_uuid': instance_uuid,
+                    'code': 456,
+                    'message': 'Fake message %s' % instance_uuid,
+                    'details': 'No details',
+                    'host': 'foo',
+                    'deleted': False,
+                    'deleted_at': None,
+                    'updated_at': None,
+                    'created_at': None
+                }]
+            }
+
+        db_1_faults = _fake_db_faults(uuids.inst_1)
+        db_2_faults = _fake_db_faults(uuids.inst_2)
+        cell_faults = [
+            (im1.cell_mapping, db_1_faults),
+            (im2.cell_mapping, db_2_faults)
+        ]
+
+        def _mock_cell_fault_get(context, instance_uuids, latest=False):
+            for cf in cell_faults:
+                if context.db_connection == cf[0].database_connection:
+                    return cf[1]
+            return None
+        mock_fault_get.side_effect = _mock_cell_fault_get
+
+        @contextmanager
+        def _mock_ctxt_target_cell(context, cell_mapping):
+            yield mock.Mock(
+                db_connection=
+                    (cell_mapping.database_connection
+                     if cell_mapping
+                        and hasattr(cell_mapping, 'database_connection')
+                     else None))
+        mock_ctxt_target_cell.side_effect = _mock_ctxt_target_cell
+
+        inst_list.fill_faults()
+        # Verify multicell fill_faults by comparing combined list of faults
+        # from different cell DBs to the list of instance faults as filled
+        self.assertEqual([db_1_faults[uuids.inst_1][0]['message'],
+                          db_2_faults[uuids.inst_2][0]['message']],
+                         [getattr(inst.fault, 'message', None)
+                          for inst in inst_list])
+
     @mock.patch('nova.objects.instance.Instance.obj_make_compatible')
     def test_get_by_security_group(self, mock_compat):
         fake_secgroup = dict(test_security_group.fake_secgroup)