From ea6757092df85b6cce38980ae5b7f54b76657e01 Mon Sep 17 00:00:00 2001
From: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Date: Tue, 6 Feb 2018 12:36:50 +0100
Subject: [PATCH] Make bdms querying in multi-cell use scatter-gather and
 ignore down cell

This patch makes the querying of bdms from multiple cells in the
_get_instance_bdms_in_multiple_cells function of extended_volumes use
scatter_gather_cells thus making the process, parallel. It also adds
warnings in case a cell is not available; which the operator can later
tweak, if an exception needs to be raised. So for now, cells that are
not reachable are ignored and it proceeds to the next cell.

Change-Id: I0e05eb1e2ad37962968b79100bf4a96c7d6ddd8f
Closes-Bug: #1747650
---
 .../api/openstack/compute/extended_volumes.py | 23 +++++++---
 .../compute/test_extended_volumes.py          | 43 +++++++++++++++++++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/nova/api/openstack/compute/extended_volumes.py b/nova/api/openstack/compute/extended_volumes.py
index 13416b39dadf..a88a45875355 100644
--- a/nova/api/openstack/compute/extended_volumes.py
+++ b/nova/api/openstack/compute/extended_volumes.py
@@ -13,12 +13,16 @@
 #   under the License.
 
 """The Extended Volumes API extension."""
+from oslo_log import log as logging
+
 from nova.api.openstack import api_version_request
 from nova.api.openstack import wsgi
 from nova import context
 from nova import objects
 from nova.policies import extended_volumes as ev_policies
 
+LOG = logging.getLogger(__name__)
+
 
 class ExtendedVolumesController(wsgi.Controller):
     def _extend_server(self, context, server, req, bdms):
@@ -60,12 +64,19 @@ class ExtendedVolumesController(wsgi.Controller):
                     {inst_map.cell_mapping.uuid: inst_map.cell_mapping})
 
         bdms = {}
-        for cell_mapping in cell_mappings.values():
-            with context.target_cell(ctxt, cell_mapping) as cctxt:
-                bdms.update(
-                    objects.BlockDeviceMappingList.bdms_by_instance_uuid(
-                        cctxt, instance_uuids))
-
+        results = context.scatter_gather_cells(
+                        ctxt, cell_mappings.values(), 60,
+                        objects.BlockDeviceMappingList.bdms_by_instance_uuid,
+                        instance_uuids)
+        for cell_uuid, result in results.items():
+            if result is context.raised_exception_sentinel:
+                LOG.warning('Failed to get block device mappings for cell %s',
+                            cell_uuid)
+            elif result is context.did_not_respond_sentinel:
+                LOG.warning('Timeout getting block device mappings for cell '
+                            '%s', cell_uuid)
+            else:
+                bdms.update(result)
         return bdms
 
     @wsgi.extends
diff --git a/nova/tests/unit/api/openstack/compute/test_extended_volumes.py b/nova/tests/unit/api/openstack/compute/test_extended_volumes.py
index a041e9694a58..c7b2d6c13569 100644
--- a/nova/tests/unit/api/openstack/compute/test_extended_volumes.py
+++ b/nova/tests/unit/api/openstack/compute/test_extended_volumes.py
@@ -165,6 +165,49 @@ class ExtendedVolumesTestV21(test.TestCase):
             actual = server.get('%svolumes_attached' % self.prefix)
             self.assertEqual(self.exp_volumes_detail[i], actual)
 
+    @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.return_value = [
+            objects.InstanceMapping(
+                instance_uuid=UUID1,
+                cell_mapping=objects.CellMapping(
+                    uuid=uuids.cell1,
+                    transport_url='fake://nowhere/',
+                    database_connection=uuids.cell1)),
+            objects.InstanceMapping(
+                instance_uuid=UUID2,
+                cell_mapping=objects.CellMapping(
+                    uuid=uuids.cell2,
+                    transport_url='fake://nowhere/',
+                    database_connection=uuids.cell2))
+            ]
+        bdm = fake_bdms_get_all_by_instance_uuids()
+        fake_bdm = fake_block_device.fake_bdm_object(
+                                        nova_context.RequestContext, bdm[0])
+        mock_sg.return_value = {
+            uuids.cell1: {UUID1: [fake_bdm]},
+            uuids.cell2: nova_context.raised_exception_sentinel
+        }
+
+        res = self._make_request('/detail')
+        mock_get_by_instance_uuids.assert_called_once_with(
+            test.MatchType(nova_context.RequestContext), [UUID1, UUID2])
+
+        self.assertEqual(200, res.status_int)
+
+        # we would get an empty list for the second instance
+        # which is in the down cell, however this would printed
+        # in the logs.
+        for i, server in enumerate(self._get_servers(res.body)):
+            actual = server.get('%svolumes_attached' % self.prefix)
+            if i == 0:
+                self.assertEqual(self.exp_volumes_detail[i], actual)
+            else:
+                self.assertEqual([], actual)
+
 
 class ExtendedVolumesTestV23(ExtendedVolumesTestV21):