Skip to content

Commit 6d1877b

Browse files
melwittmriedem
authored andcommitted
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 0258cec)
1 parent bbe8165 commit 6d1877b

File tree

3 files changed

+23
-9
lines changed

3 files changed

+23
-9
lines changed

nova/compute/api.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,8 +1330,11 @@ def _subsequent_list(l):
13301330
"destination_type 'volume' need to have a non-zero "
13311331
"size specified"))
13321332
elif volume_id is not None:
1333-
min_compute_version = objects.Service.get_minimum_version(
1334-
context, 'nova-compute')
1333+
# The instance is being created and we don't know which
1334+
# cell it's going to land in, so check all cells.
1335+
min_compute_version = \
1336+
objects.service.get_minimum_version_all_cells(
1337+
context, ['nova-compute'])
13351338
try:
13361339
# NOTE(ildikov): The boot from volume operation did not
13371340
# reserve the volume before Pike and as the older computes

nova/tests/unit/compute/test_compute.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,7 +922,7 @@ def test_prepare_image_mapping(self):
922922
for expected, got in zip(expected_result, preped_bdm):
923923
self.assertThat(expected, matchers.IsSubDictOf(got))
924924

925-
@mock.patch.object(objects.Service, 'get_minimum_version',
925+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
926926
return_value=17)
927927
def test_validate_bdm(self, mock_get_min_ver):
928928
def fake_get(self, context, res_id):
@@ -1267,7 +1267,7 @@ def test_validate_bdm_media_service_volume_not_found(self, mock_get,
12671267
self.context, self.instance,
12681268
instance_type, bdms)
12691269

1270-
@mock.patch.object(objects.Service, 'get_minimum_version',
1270+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
12711271
return_value=17)
12721272
@mock.patch.object(cinder.API, 'get')
12731273
@mock.patch.object(cinder.API, 'check_availability_zone')

nova/tests/unit/compute/test_compute_api.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3487,13 +3487,16 @@ def test_get_bdm_image_metadata_with_cinder_down(self, mock_get):
34873487
self.context,
34883488
bdms, legacy_bdm=True)
34893489

3490+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
3491+
return_value=17)
34903492
@mock.patch.object(objects.Service, 'get_minimum_version',
34913493
return_value=17)
34923494
@mock.patch.object(cinder.API, 'get')
34933495
@mock.patch.object(cinder.API, 'reserve_volume',
34943496
side_effect=exception.InvalidInput(reason='error'))
34953497
def test_validate_bdm_with_error_volume(self, mock_reserve_volume,
3496-
mock_get, mock_get_min_ver):
3498+
mock_get, mock_get_min_ver,
3499+
mock_get_min_ver_all):
34973500
# Tests that an InvalidInput exception raised from
34983501
# volume_api.reserve_volume due to the volume status not being
34993502
# 'available' results in _validate_bdm re-raising InvalidVolume.
@@ -3523,7 +3526,7 @@ def test_validate_bdm_with_error_volume(self, mock_reserve_volume,
35233526
mock_reserve_volume.assert_called_once_with(
35243527
self.context, volume_id)
35253528

3526-
@mock.patch.object(objects.Service, 'get_minimum_version',
3529+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
35273530
return_value=17)
35283531
@mock.patch.object(cinder.API, 'get_snapshot',
35293532
side_effect=exception.CinderConnectionFailed(reason='error'))
@@ -3629,7 +3632,7 @@ def do_test(
36293632

36303633
do_test()
36313634

3632-
@mock.patch.object(objects.Service, 'get_minimum_version',
3635+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
36333636
return_value=17)
36343637
@mock.patch.object(cinder.API, 'get',
36353638
side_effect=exception.CinderConnectionFailed(reason='error'))
@@ -3640,6 +3643,8 @@ def test_provision_instances_with_cinder_down(self, mock_get,
36403643

36413644
@mock.patch.object(objects.Service, 'get_minimum_version',
36423645
return_value=17)
3646+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
3647+
return_value=17)
36433648
@mock.patch.object(cinder.API, 'get')
36443649
@mock.patch.object(cinder.API, 'check_availability_zone')
36453650
@mock.patch.object(cinder.API, 'reserve_volume',
@@ -3648,6 +3653,7 @@ def test_provision_instances_with_error_volume(self,
36483653
mock_cinder_check_av_zone,
36493654
mock_reserve_volume,
36503655
mock_get,
3656+
mock_get_min_ver_cells,
36513657
mock_get_min_ver):
36523658
self._test_provision_instances_with_cinder_error(
36533659
expected_exception=exception.InvalidVolume)
@@ -3700,9 +3706,12 @@ def test_provision_instances_creates_build_request(self):
37003706
@mock.patch.object(objects.RequestSpec, 'from_components')
37013707
@mock.patch.object(objects.BuildRequest, 'create')
37023708
@mock.patch.object(objects.InstanceMapping, 'create')
3709+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
3710+
return_value=17)
37033711
@mock.patch.object(objects.Service, 'get_minimum_version',
37043712
return_value=17)
3705-
def do_test(mock_get_min_ver, _mock_inst_mapping_create,
3713+
def do_test(mock_get_min_ver, mock_get_min_ver_cells,
3714+
_mock_inst_mapping_create,
37063715
mock_build_req, mock_req_spec_from_components,
37073716
_mock_ensure_default, mock_check_num_inst_quota,
37083717
mock_volume, mock_inst_create):
@@ -3851,14 +3860,16 @@ def do_test(mock_inst_mapping, mock_check_num_inst_quota):
38513860

38523861
@mock.patch.object(objects.Service, 'get_minimum_version',
38533862
return_value=17)
3863+
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
3864+
return_value=17)
38543865
@mock.patch.object(cinder.API, 'get')
38553866
@mock.patch.object(cinder.API, 'check_availability_zone',)
38563867
@mock.patch.object(cinder.API, 'reserve_volume',
38573868
side_effect=(None, exception.InvalidInput(reason='error')))
38583869
def test_provision_instances_cleans_up_when_volume_invalid(self,
38593870
_mock_cinder_reserve_volume,
38603871
_mock_cinder_check_availability_zone, _mock_cinder_get,
3861-
_mock_get_min_ver):
3872+
_mock_get_min_ver_cells, _mock_get_min_ver):
38623873
@mock.patch('nova.compute.utils.check_num_instances_quota')
38633874
@mock.patch.object(objects, 'Instance')
38643875
@mock.patch.object(self.compute_api.security_group_api,

0 commit comments

Comments
 (0)