From 53fc4ab1ddfd2aba3a32936f9f49b37a6e51a198 Mon Sep 17 00:00:00 2001
From: Surya Seetharaman <suryaseetharaman.9@gmail.com>
Date: Mon, 14 May 2018 13:50:12 +0200
Subject: [PATCH] Make nova service-list use scatter-gather routine

This patch makes nova service-list use the scatter-gather routine
so that if a cell is down, at least the services from other cells
are listed by ignoring the down cell instead of the whole command
failing with an API exception as is the current situation. Also
making this query parallel for all cells is more efficient.

Depends-On: https://review.openstack.org/569112/

Change-Id: I90b488102eb265d971cade29892279a22d3b5273
Closes-Bug: #1726310
(cherry picked from commit 64e76de43dc55e584c100005fa60da50dd06d352)
---
 nova/compute/api.py                      | 12 ++++++------
 nova/tests/unit/compute/test_host_api.py | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 031587db99bc..b86a16cbb54d 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -4524,13 +4524,13 @@ class HostAPI(base.Base):
         # and we should always iterate over the cells. However, certain
         # callers need the legacy behavior for now.
         if all_cells:
-            load_cells()
             services = []
-            for cell in CELLS:
-                with nova_context.target_cell(context, cell) as cctxt:
-                    cell_services = objects.ServiceList.get_all(
-                        cctxt, disabled, set_zones=set_zones)
-                services.extend(cell_services)
+            service_dict = nova_context.scatter_gather_all_cells(context,
+                objects.ServiceList.get_all, disabled, set_zones=set_zones)
+            for service in service_dict.values():
+                if service not in (nova_context.did_not_respond_sentinel,
+                                   nova_context.raised_exception_sentinel):
+                    services.extend(service)
         else:
             services = objects.ServiceList.get_all(context, disabled,
                                                    set_zones=set_zones)
diff --git a/nova/tests/unit/compute/test_host_api.py b/nova/tests/unit/compute/test_host_api.py
index a0fe540c0f67..1c06a1658aa9 100644
--- a/nova/tests/unit/compute/test_host_api.py
+++ b/nova/tests/unit/compute/test_host_api.py
@@ -186,6 +186,19 @@ class ComputeHostAPITestCase(test.TestCase):
         self.assertEqual(sorted(['host-%s' % cell.uuid for cell in cells]),
                          sorted([svc.host for svc in services]))
 
+    @mock.patch('nova.context.scatter_gather_cells')
+    def test_service_get_all_cells_with_failures(self, mock_sg):
+        service = objects.Service(binary='nova-compute',
+                                  host='host-%s' % uuids.cell1)
+        mock_sg.return_value = {
+            uuids.cell1: [service],
+            uuids.cell2: context.raised_exception_sentinel
+        }
+        services = self.host_api.service_get_all(self.ctxt, all_cells=True)
+        # returns the results from cell1 and ignores cell2.
+        self.assertEqual(['host-%s' % uuids.cell1],
+                         [svc.host for svc in services])
+
     def test_service_get_all_no_zones(self):
         services = [dict(test_service.fake_service,
                          id=1, topic='compute', host='host1'),
@@ -508,6 +521,10 @@ class ComputeHostAPICellsTestCase(ComputeHostAPITestCase):
     def test_service_get_all_cells(self):
         pass
 
+    @testtools.skip('cellsv1 does not use this')
+    def test_service_get_all_cells_with_failures(self):
+        pass
+
     @testtools.skip('cellsv1 does not use this')
     def test_service_delete_ambiguous(self):
         pass