openstack
/
nova
Code Issues Proposed changes

Merge "Remove old check_attach version check in API"

This commit is contained in:
Zuul 2018-08-31 15:02:50 +00:00 committed by Gerrit Code Review
parent 7cc1312ea5 fc5839b959
commit 3be8dbb68f
6 changed files with 48 additions and 138 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
nova/compute/api.py
View File

@ -102,7 +102,6 @@ AGGREGATE_ACTION_UPDATE = 'Update'
AGGREGATE_ACTION_UPDATE_META = 'UpdateMeta'
AGGREGATE_ACTION_DELETE = 'Delete'
AGGREGATE_ACTION_ADD = 'Add'
BFV_RESERVE_MIN_COMPUTE_VERSION = 17
CINDER_V3_ATTACH_MIN_COMPUTE_VERSION = 24
MIN_COMPUTE_MULTIATTACH = 27
MIN_COMPUTE_TRUSTED_CERTS = 31
@ -1400,28 +1399,10 @@ class API(base.Base):
"destination_type 'volume' need to have a non-zero "
"size specified"))
elif volume_id is not None:
# 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
# are running 'check_attach' which will fail if the volume
# is in 'attaching' state; if the compute service version
# is not high enough we will just perform the old check as
# opposed to reserving the volume here.
volume = self.volume_api.get(context, volume_id)
if (min_compute_version >=
BFV_RESERVE_MIN_COMPUTE_VERSION):
self._check_attach_and_reserve_volume(
context, volume, instance, bdm,
supports_multiattach)
else:
# NOTE(ildikov): This call is here only for backward
# compatibility can be removed after Ocata EOL.
self._check_attach(context, volume, instance)
self._check_attach_and_reserve_volume(
context, volume, instance, bdm, supports_multiattach)
bdm.volume_size = volume.get('size')
# NOTE(mnaser): If we end up reserving the volume, it will
@ -1478,20 +1459,6 @@ class API(base.Base):
if num_local > max_local:
raise exception.InvalidBDMLocalsLimit()
def _check_attach(self, context, volume, instance):
# TODO(ildikov): This check_attach code is kept only for backward
# compatibility and should be removed after Ocata EOL.
if volume['status'] != 'available':
msg = _("volume '%(vol)s' status must be 'available'. Currently "
"in '%(status)s'") % {'vol': volume['id'],
'status': volume['status']}
raise exception.InvalidVolume(reason=msg)
if volume['attach_status'] == 'attached':
msg = _("volume %s already attached") % volume['id']
raise exception.InvalidVolume(reason=msg)
self.volume_api.check_availability_zone(context, volume,
instance=instance)
def _populate_instance_names(self, instance, num_instances, index):
"""Populate instance display_name and hostname.

3
nova/tests/functional/regressions/test_bug_1404867.py
View File

@ -61,7 +61,8 @@ class DeleteWithReservedVolumes(integrated_helpers._IntegratedTestBase,
return self._wait_for_state_change(self.api, server, 'ERROR')
@mock.patch('nova.objects.service.get_minimum_version_all_cells',
return_value=compute_api.BFV_RESERVE_MIN_COMPUTE_VERSION)
return_value=
compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1)
def test_delete_with_reserved_volumes(self, mock_version_get=None):
self.cinder = self.useFixture(nova_fixtures.CinderFixture(self))

5
nova/tests/functional/wsgi/test_servers.py
View File

@ -248,8 +248,9 @@ class ServersPreSchedulingTestCase(test.TestCase,
self.assertEqual(0, len(list_resp))
@mock.patch('nova.objects.service.get_minimum_version_all_cells',
return_value=compute_api.BFV_RESERVE_MIN_COMPUTE_VERSION)
def test_bfv_delete_build_request_pre_scheduling_ocata(self, mock_get):
return_value=
compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1)
def test_bfv_delete_build_request_pre_scheduling_old_flow(self, mock_get):
cinder = self.useFixture(nova_fixtures.CinderFixture(self))
volume_id = nova_fixtures.CinderFixture.IMAGE_BACKED_VOL

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

@ -977,9 +977,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_all_cells',
return_value=17)
def test_validate_bdm(self, mock_get_min_ver):
def test_validate_bdm(self):
def fake_get(self, context, res_id):
return {'id': res_id, 'size': 4, 'multiattach': False}
@ -1205,52 +1203,13 @@ class ComputeVolumeTestCase(BaseTestCase):
self.context, self.instance,
instance_type, all_mappings)
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=16)
@mock.patch.object(nova.volume.cinder.API, 'get')
@mock.patch.object(cinder.API, 'check_availability_zone')
def test_validate_bdm_volume_old_compute(self, mock_check_av_zone,
mock_get, mock_get_min_ver):
instance = self._create_fake_instance_obj()
instance_type = {'ephemeral_gb': 2}
volume_id = uuids.volume_id
mappings = [
fake_block_device.FakeDbBlockDeviceDict({
'device_name': '/dev/sda1',
'source_type': 'volume',
'destination_type': 'volume',
'device_type': 'disk',
'volume_id': volume_id,
'guest_format': None,
'boot_index': 0,
}, anon=True)
]
mappings = block_device_obj.block_device_make_list_from_dicts(
self.context, mappings)
volume = {'id': volume_id,
'size': 4,
'status': 'available',
'attach_status': 'detached',
'multiattach': False}
mock_get.return_value = volume
self.compute_api._validate_bdm(self.context, instance,
instance_type, mappings)
self.assertEqual(4, mappings[0].volume_size)
mock_check_av_zone.assert_called_once_with(self.context, volume,
instance=instance)
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=17)
@mock.patch.object(cinder.API, 'get')
@mock.patch.object(cinder.API, 'check_availability_zone')
@mock.patch.object(cinder.API, 'reserve_volume',
side_effect=exception.InvalidVolume(reason='error'))
def test_validate_bdm_media_service_invalid_volume(self, mock_reserve_vol,
mock_check_av_zone,
mock_get,
mock_get_min_ver):
mock_get):
volume_id = uuids.volume_id
instance_type = {'swap': 1, 'ephemeral_gb': 1}
bdms = [fake_block_device.FakeDbBlockDeviceDict({
@ -1296,11 +1255,8 @@ class ComputeVolumeTestCase(BaseTestCase):
instance_type, bdms)
mock_get.assert_called_with(self.context, volume_id)
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=17)
@mock.patch.object(cinder.API, 'get')
def test_validate_bdm_media_service_volume_not_found(self, mock_get,
mock_get_min_ver):
def test_validate_bdm_media_service_volume_not_found(self, mock_get):
volume_id = uuids.volume_id
instance_type = {'swap': 1, 'ephemeral_gb': 1}
bdms = [fake_block_device.FakeDbBlockDeviceDict({
@ -1322,15 +1278,12 @@ class ComputeVolumeTestCase(BaseTestCase):
self.context, self.instance,
instance_type, bdms)
@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')
def test_validate_bdm_media_service_valid(self, mock_reserve_vol,
mock_check_av_zone,
mock_get,
mock_get_min_ver):
mock_get):
volume_id = uuids.volume_id
instance_type = {'swap': 1, 'ephemeral_gb': 1}
bdms = [fake_block_device.FakeDbBlockDeviceDict({

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

@ -4290,17 +4290,17 @@ class _ComputeAPIUnitTestMixIn(object):
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)
return_value=
compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1)
@mock.patch.object(cinder.API, 'get')
@mock.patch.object(cinder.API, 'reserve_volume')
def test_validate_bdm_returns_attachment_id(self, mock_reserve_volume,
mock_get, mock_get_min_ver,
mock_get,
mock_get_min_ver_all):
# Tests that bdm validation *always* returns an attachment_id even if
# it's None.
instance = self._create_instance_obj()
del instance.id
instance_type = self._create_flavor()
volume_id = 'e856840e-9f5b-4894-8bde-58c6e29ac1e8'
volume_info = {'status': 'available',
@ -4330,19 +4330,19 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, volume_id)
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
return_value=17)
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=17)
return_value=
compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1)
@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_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.
instance = self._create_instance_obj()
del instance.id
instance_type = self._create_flavor()
volume_id = 'e856840e-9f5b-4894-8bde-58c6e29ac1e8'
volume_info = {'status': 'error',
@ -4369,14 +4369,11 @@ class _ComputeAPIUnitTestMixIn(object):
mock_reserve_volume.assert_called_once_with(
self.context, volume_id)
@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'))
@mock.patch.object(cinder.API, 'get',
side_effect=exception.CinderConnectionFailed(reason='error'))
def test_validate_bdm_with_cinder_down(self, mock_get, mock_get_snapshot,
mock_get_min_ver):
def test_validate_bdm_with_cinder_down(self, mock_get, mock_get_snapshot):
instance = self._create_instance_obj()
instance_type = self._create_flavor()
bdm = [objects.BlockDeviceMapping(
@ -4410,19 +4407,17 @@ class _ComputeAPIUnitTestMixIn(object):
@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')
@mock.patch.object(cinder.API, 'attachment_create',
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_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.
instance = self._create_instance_obj()
del instance.id
instance_type = self._create_flavor()
volume_id = 'e856840e-9f5b-4894-8bde-58c6e29ac1e8'
volume_info = {'status': 'error',
@ -4529,19 +4524,12 @@ class _ComputeAPIUnitTestMixIn(object):
do_test()
@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'))
def test_provision_instances_with_cinder_down(self, mock_get,
mock_get_min_ver):
def test_provision_instances_with_cinder_down(self, mock_get):
self._test_provision_instances_with_cinder_error(
expected_exception=exception.CinderConnectionFailed)
@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',
return_value={'id': '1', 'multiattach': False})
@mock.patch.object(cinder.API, 'check_availability_zone')
@ -4550,9 +4538,7 @@ class _ComputeAPIUnitTestMixIn(object):
def test_provision_instances_with_error_volume(self,
mock_cinder_check_av_zone,
mock_reserve_volume,
mock_get,
mock_get_min_ver_cells,
mock_get_min_ver):
mock_get):
self._test_provision_instances_with_cinder_error(
expected_exception=exception.InvalidVolume)
@ -4614,21 +4600,15 @@ class _ComputeAPIUnitTestMixIn(object):
def test_provision_instances_creates_build_request(self):
@mock.patch.object(objects.Instance, 'create')
@mock.patch.object(self.compute_api, 'volume_api')
@mock.patch('nova.compute.utils.check_num_instances_quota')
@mock.patch.object(self.compute_api.security_group_api,
'ensure_default')
@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_get_min_ver_cells,
_mock_inst_mapping_create, mock_build_req,
def do_test(_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):
mock_check_num_inst_quota, mock_inst_create):
min_count = 1
max_count = 2
@ -4669,7 +4649,6 @@ class _ComputeAPIUnitTestMixIn(object):
'device_name': 'vda',
'boot_index': 0,
}))])
mock_volume.get.return_value = {'id': '1', 'multiattach': False}
instance_tags = objects.TagList(objects=[objects.Tag(tag='tag')])
shutdown_terminate = True
trusted_certs = None
@ -4678,13 +4657,20 @@ class _ComputeAPIUnitTestMixIn(object):
filter_properties = {'scheduler_hints': None,
'instance_type': flavor}
instances_to_build = self.compute_api._provision_instances(
ctxt, flavor,
min_count, max_count, base_options, boot_meta,
security_groups, block_device_mappings, shutdown_terminate,
instance_group, check_server_group_quota,
filter_properties, None, instance_tags, trusted_certs,
False)
with mock.patch.object(
self.compute_api,
'_bdm_validate_set_size_and_instance',
return_value=block_device_mappings) as validate_bdm:
instances_to_build = self.compute_api._provision_instances(
ctxt, flavor,
min_count, max_count, base_options, boot_meta,
security_groups, block_device_mappings,
shutdown_terminate, instance_group,
check_server_group_quota, filter_properties, None,
instance_tags, trusted_certs, False)
validate_bdm.assert_has_calls([mock.call(
ctxt, test.MatchType(objects.Instance), flavor,
block_device_mappings, False)] * max_count)
for rs, br, im in instances_to_build:
self.assertIsInstance(br.instance, objects.Instance)
@ -4777,10 +4763,9 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertEqual(ctxt.project_id, inst_mapping_mock.project_id)
do_test()
@mock.patch.object(objects.Service, 'get_minimum_version',
return_value=17)
@mock.patch.object(objects.service, 'get_minimum_version_all_cells',
return_value=17)
return_value=
compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1)
@mock.patch.object(cinder.API, 'get',
return_value={'id': '1', 'multiattach': False})
@mock.patch.object(cinder.API, 'check_availability_zone',)
@ -4789,7 +4774,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_cells, _mock_get_min_ver):
_mock_get_min_ver_cells):
@mock.patch('nova.compute.utils.check_num_instances_quota')
@mock.patch.object(objects, 'Instance')
@mock.patch.object(self.compute_api.security_group_api,

9
nova/utils.py
View File

@ -953,9 +953,12 @@ def get_image_metadata_from_volume(volume):
val = properties.pop(attr, None)
if attr in ('min_ram', 'min_disk'):
image_meta[attr] = int(val or 0)
# NOTE(yjiang5): Always set the image status as 'active'
# and depends on followed volume_api.check_attach() to
# verify it. This hack should be harmless with that check.
# NOTE(mriedem): Set the status to 'active' as a really old hack
# from when this method was in the compute API class and is
# needed for _check_requested_image which makes sure the image
# is 'active'. For volume-backed servers, if the volume is not
# available because the image backing the volume is not active,
# then the compute API trying to reserve the volume should fail.
image_meta['status'] = 'active'
return image_meta