From 0a2f45a5b2e90b0a5650f5069b77dd2a8ddfc4a4 Mon Sep 17 00:00:00 2001
From: Takashi NATSUME <natsume.takashi@lab.ntt.co.jp>
Date: Thu, 3 Aug 2017 14:11:19 +0900
Subject: [PATCH] Fix getting instance bdms in multiple cells

In the 'detail' method of ExtendedVolumesController
related to 'GET /servers/detail' API,
multiple cells environemnt is not taken into account
when getting instance BDMs.
So fix it.

Change-Id: I9377e988fbdc822df46e91c0db6c8012697bc2ee
Closes-Bug: #1708210
---
 .../api/openstack/compute/extended_volumes.py | 27 ++++++++++++++++---
 .../compute/test_extended_volumes.py          | 21 ++++++++++++++-
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/nova/api/openstack/compute/extended_volumes.py b/nova/api/openstack/compute/extended_volumes.py
index fbe03fc4f105..13416b39dadf 100644
--- a/nova/api/openstack/compute/extended_volumes.py
+++ b/nova/api/openstack/compute/extended_volumes.py
@@ -15,6 +15,7 @@
 """The Extended Volumes API extension."""
 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
 
@@ -45,14 +46,34 @@ class ExtendedVolumesController(wsgi.Controller):
             instance_bdms = self._get_instance_bdms(bdms, server)
             self._extend_server(context, server, req, instance_bdms)
 
+    @staticmethod
+    def _get_instance_bdms_in_multiple_cells(ctxt, servers):
+        instance_uuids = [server['id'] for server in servers]
+        inst_maps = objects.InstanceMappingList.get_by_instance_uuids(
+                        ctxt, instance_uuids)
+
+        cell_mappings = {}
+        for inst_map in inst_maps:
+            if (inst_map.cell_mapping is not None and
+                    inst_map.cell_mapping.uuid not in cell_mappings):
+                cell_mappings.update(
+                    {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))
+
+        return bdms
+
     @wsgi.extends
     def detail(self, req, resp_obj):
         context = req.environ['nova.context']
         if context.can(ev_policies.BASE_POLICY_NAME, fatal=False):
             servers = list(resp_obj.obj['servers'])
-            instance_uuids = [server['id'] for server in servers]
-            bdms = objects.BlockDeviceMappingList.bdms_by_instance_uuid(
-                context, instance_uuids)
+            bdms = self._get_instance_bdms_in_multiple_cells(context, servers)
             for server in servers:
                 instance_bdms = self._get_instance_bdms(bdms, server)
                 self._extend_server(context, server, req, instance_bdms)
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 c59c43149873..a041e9694a58 100644
--- a/nova/tests/unit/api/openstack/compute/test_extended_volumes.py
+++ b/nova/tests/unit/api/openstack/compute/test_extended_volumes.py
@@ -20,12 +20,14 @@ from nova.api.openstack.compute import (extended_volumes
                                                    as extended_volumes_v21)
 from nova.api.openstack import wsgi as os_wsgi
 from nova import compute
+from nova import context as nova_context
 from nova import objects
 from nova.objects import instance as instance_obj
 from nova import test
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_block_device
 from nova.tests.unit import fake_instance
+from nova.tests import uuidsentinel as uuids
 from nova import volume
 
 UUID1 = '00000000-0000-0000-0000-000000000001'
@@ -138,8 +140,25 @@ class ExtendedVolumesTestV21(test.TestCase):
         actual = server.get('%svolumes_attached' % self.prefix)
         self.assertEqual(self.exp_volumes_show, actual)
 
-    def test_detail(self):
+    @mock.patch.object(objects.InstanceMappingList, 'get_by_instance_uuids')
+    def test_detail(self, 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.cell1,
+                    transport_url='fake://nowhere/',
+                    database_connection=uuids.cell1))]
+
         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)
         for i, server in enumerate(self._get_servers(res.body)):