Query all cells for service version in _validate_bdm

We call _validate_bdm during instance creation to validate block device
mappings boot indexes, accessibility, attachability, and so on. We need
to query the service version in order to decide which Cinder APIs to
call and because we're in the middle of creating the instance, we
don't yet know which cell it's going to land in.

This changes the service version query to check all cells so that
_validate_bdm will use the 'reserve_volume' Cinder API in a multi-cell
environment. Use of the 'reserve_volume' API is based on the service
version check and without targeting any cells, the service version will
be 0 and we'll use the old 'check_attach' API.

Conflicts:
      nova/tests/unit/compute/test_compute_api.py

NOTE(mriedem): Conflicts are due to not having change
Ifc01dbf98545104c998ab96f65ff8623a6db0f28 in Pike which added
a test and updated some other tests which we now have to do
in this change.

Closes-Bug: #1746634

Change-Id: I68d5398d2a6d85c833e46ce682672008dbd5c4c1
(cherry picked from commit 0258cecaca88d4a305e99c5a17e2230361ef1235)

3 files changed, 23 insertions, 9 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 3eeeb69a7b..8078add299 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1330,8 +1330,11 @@ class API(base.Base):
                         "destination_type 'volume' need to have a non-zero "
                         "size specified"))
             elif volume_id is not None:
-                min_compute_version = objects.Service.get_minimum_version(
-                    context, 'nova-compute')
+                # The instance is being created and we don't know which
+                # cell it's going to land in, so check all cells.
+                min_compute_version = \
+                    objects.service.get_minimum_version_all_cells(
+                        context, ['nova-compute'])
                 try:
                     # NOTE(ildikov): The boot from volume operation did not
                     # reserve the volume before Pike and as the older computes
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index dbd6ffde80..596236f858 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -922,7 +922,7 @@ class ComputeVolumeTestCase(BaseTestCase):
         for expected, got in zip(expected_result, preped_bdm):
             self.assertThat(expected, matchers.IsSubDictOf(got))
 
-    @mock.patch.object(objects.Service, 'get_minimum_version',
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
         return_value=17)
     def test_validate_bdm(self, mock_get_min_ver):
         def fake_get(self, context, res_id):
@@ -1267,7 +1267,7 @@ class ComputeVolumeTestCase(BaseTestCase):
                           self.context, self.instance,
                           instance_type, bdms)
 
-    @mock.patch.object(objects.Service, 'get_minimum_version',
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
                        return_value=17)
     @mock.patch.object(cinder.API, 'get')
     @mock.patch.object(cinder.API, 'check_availability_zone')
diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py
index da9eefa059..f403d082be 100644
--- a/nova/tests/unit/compute/test_compute_api.py
+++ b/nova/tests/unit/compute/test_compute_api.py
@@ -3487,13 +3487,16 @@ class _ComputeAPIUnitTestMixIn(object):
                           self.context,
                           bdms, legacy_bdm=True)
 
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
+                       return_value=17)
     @mock.patch.object(objects.Service, 'get_minimum_version',
                        return_value=17)
     @mock.patch.object(cinder.API, 'get')
     @mock.patch.object(cinder.API, 'reserve_volume',
                        side_effect=exception.InvalidInput(reason='error'))
     def test_validate_bdm_with_error_volume(self, mock_reserve_volume,
-                                            mock_get, mock_get_min_ver):
+                                            mock_get, mock_get_min_ver,
+                                            mock_get_min_ver_all):
         # Tests that an InvalidInput exception raised from
         # volume_api.reserve_volume due to the volume status not being
         # 'available' results in _validate_bdm re-raising InvalidVolume.
@@ -3523,7 +3526,7 @@ class _ComputeAPIUnitTestMixIn(object):
         mock_reserve_volume.assert_called_once_with(
             self.context, volume_id)
 
-    @mock.patch.object(objects.Service, 'get_minimum_version',
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
                        return_value=17)
     @mock.patch.object(cinder.API, 'get_snapshot',
              side_effect=exception.CinderConnectionFailed(reason='error'))
@@ -3629,7 +3632,7 @@ class _ComputeAPIUnitTestMixIn(object):
 
         do_test()
 
-    @mock.patch.object(objects.Service, 'get_minimum_version',
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
                        return_value=17)
     @mock.patch.object(cinder.API, 'get',
              side_effect=exception.CinderConnectionFailed(reason='error'))
@@ -3640,6 +3643,8 @@ class _ComputeAPIUnitTestMixIn(object):
 
     @mock.patch.object(objects.Service, 'get_minimum_version',
                        return_value=17)
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
+                       return_value=17)
     @mock.patch.object(cinder.API, 'get')
     @mock.patch.object(cinder.API, 'check_availability_zone')
     @mock.patch.object(cinder.API, 'reserve_volume',
@@ -3648,6 +3653,7 @@ class _ComputeAPIUnitTestMixIn(object):
                                                    mock_cinder_check_av_zone,
                                                    mock_reserve_volume,
                                                    mock_get,
+                                                   mock_get_min_ver_cells,
                                                    mock_get_min_ver):
         self._test_provision_instances_with_cinder_error(
             expected_exception=exception.InvalidVolume)
@@ -3700,9 +3706,12 @@ class _ComputeAPIUnitTestMixIn(object):
         @mock.patch.object(objects.RequestSpec, 'from_components')
         @mock.patch.object(objects.BuildRequest, 'create')
         @mock.patch.object(objects.InstanceMapping, 'create')
+        @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
+                           return_value=17)
         @mock.patch.object(objects.Service, 'get_minimum_version',
                 return_value=17)
-        def do_test(mock_get_min_ver, _mock_inst_mapping_create,
+        def do_test(mock_get_min_ver, mock_get_min_ver_cells,
+                    _mock_inst_mapping_create,
                     mock_build_req, mock_req_spec_from_components,
                     _mock_ensure_default, mock_check_num_inst_quota,
                     mock_volume, mock_inst_create):
@@ -3851,6 +3860,8 @@ class _ComputeAPIUnitTestMixIn(object):
 
     @mock.patch.object(objects.Service, 'get_minimum_version',
                        return_value=17)
+    @mock.patch.object(objects.service, 'get_minimum_version_all_cells',
+                       return_value=17)
     @mock.patch.object(cinder.API, 'get')
     @mock.patch.object(cinder.API, 'check_availability_zone',)
     @mock.patch.object(cinder.API, 'reserve_volume',
@@ -3858,7 +3869,7 @@ class _ComputeAPIUnitTestMixIn(object):
     def test_provision_instances_cleans_up_when_volume_invalid(self,
             _mock_cinder_reserve_volume,
             _mock_cinder_check_availability_zone, _mock_cinder_get,
-            _mock_get_min_ver):
+            _mock_get_min_ver_cells, _mock_get_min_ver):
         @mock.patch('nova.compute.utils.check_num_instances_quota')
         @mock.patch.object(objects, 'Instance')
         @mock.patch.object(self.compute_api.security_group_api,