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.

Closes-Bug: #1746634

Change-Id: I68d5398d2a6d85c833e46ce682672008dbd5c4c1
This commit is contained in:
melanie witt 2018-01-31 20:45:51 +00:00
parent c1442d3c8c
commit 0258cecaca
3 changed files with 17 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
nova/compute/api.py
View File

@ -1323,8 +1323,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

@ -988,7 +988,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):
@ -1333,7 +1333,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')

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

@ -3891,13 +3891,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.
@ -3928,7 +3931,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'))
@ -3967,6 +3970,8 @@ class _ComputeAPIUnitTestMixIn(object):
self.context,
instance, instance_type, bdms)
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
return_value=COMPUTE_VERSION_NEW_ATTACH_FLOW)
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=COMPUTE_VERSION_NEW_ATTACH_FLOW)
@mock.patch.object(cinder.API, 'get')
@ -3974,7 +3979,8 @@ class _ComputeAPIUnitTestMixIn(object):
side_effect=exception.InvalidInput(reason='error'))
def test_validate_bdm_with_error_volume_new_flow(self, mock_attach_create,
mock_get,
mock_get_min_ver):
mock_get_min_ver,
mock_get_min_ver_all):
# Tests that an InvalidInput exception raised from
# volume_api.attachment_create due to the volume status not being
# 'available' results in _validate_bdm re-raising InvalidVolume.
@ -4071,7 +4077,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'))