From 6d1877bf1da098b3be17fbd2dd5e53eecd2f048c Mon Sep 17 00:00:00 2001 From: melanie witt Date: Wed, 31 Jan 2018 20:45:51 +0000 Subject: [PATCH] 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) --- nova/compute/api.py | 7 +++++-- nova/tests/unit/compute/test_compute.py | 4 ++-- nova/tests/unit/compute/test_compute_api.py | 21 ++++++++++++++++----- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 3eeeb69a7b54..8078add299ff 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 dbd6ffde8045..596236f85865 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 da9eefa059d7..f403d082be67 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,