openstack
/
nova
Code Issues Proposed changes

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 0258cecaca)
This commit is contained in:
melanie witt 2018-01-31 20:45:51 +00:00 committed by Matt Riedemann
parent bbe81652f9
commit 6d1877bf1d
3 changed files with 23 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
nova/compute/api.py
View File

@ -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

4
nova/tests/unit/compute/test_compute.py
View File

@ -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')

21
nova/tests/unit/compute/test_compute_api.py
View File

@ -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,