From 686dbc3640f12a85f450ffa75c2a152e355d0b79 Mon Sep 17 00:00:00 2001 From: zhangbailin Date: Wed, 26 Sep 2018 23:04:46 -0400 Subject: [PATCH] Add compute API validation for when a volume_type is requested This adds validation in the compute API for when a block_device_mapping_v2 request specifies a volume_type. Note that this is all noop code right now since users cannot specify that field in the API until the microversion is added which enables that feature. In other words, this is just plumbing. Part of bp boot-instance-specific-storage-backend Change-Id: I45bd42908d44a0f05e1231febab926b23232b57b --- nova/compute/api.py | 56 +++++++++ nova/exception.py | 12 ++ nova/tests/unit/compute/test_compute_api.py | 121 ++++++++++++++++++++ nova/tests/unit/volume/test_cinder.py | 27 +++++ nova/volume/cinder.py | 20 ++++ 5 files changed, 236 insertions(+) diff --git a/nova/compute/api.py b/nova/compute/api.py index b9308278f89e..fdb67fe0f10e 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -106,6 +106,7 @@ CINDER_V3_ATTACH_MIN_COMPUTE_VERSION = 24 MIN_COMPUTE_MULTIATTACH = 27 MIN_COMPUTE_TRUSTED_CERTS = 31 MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION = 34 +MIN_COMPUTE_VOLUME_TYPE = 36 # FIXME(danms): Keep a global cache of the cells we find the # first time we look. This needs to be refreshed on a timer or @@ -1359,6 +1360,36 @@ class API(base.Base): bdm.update_or_create() + @staticmethod + def _check_requested_volume_type(bdm, volume_type_id_or_name, + volume_types): + """If we are specifying a volume type, we need to get the + volume type details from Cinder and make sure the ``volume_type`` + is available. + """ + + # NOTE(brinzhang): Verify that the specified volume type exists. + # And save the volume type name internally for consistency in the + # BlockDeviceMapping object. + for vol_type in volume_types: + if (volume_type_id_or_name == vol_type['id'] or + volume_type_id_or_name == vol_type['name']): + bdm.volume_type = vol_type['name'] + break + else: + raise exception.VolumeTypeNotFound( + id_or_name=volume_type_id_or_name) + + @staticmethod + def _check_compute_supports_volume_type(context): + # NOTE(brinzhang): Checking the minimum nova-compute service + # version across the deployment. Just make sure the volume + # type can be supported when the bdm.volume_type is requested. + min_compute_version = objects.service.get_minimum_version_all_cells( + context, ['nova-compute']) + if min_compute_version < MIN_COMPUTE_VOLUME_TYPE: + raise exception.VolumeTypeSupportNotYetAvailable() + def _validate_bdm(self, context, instance, instance_type, block_device_mappings, supports_multiattach=False): # Make sure that the boot indexes make sense. @@ -1379,7 +1410,32 @@ class API(base.Base): instance=instance) raise exception.InvalidBDMBootSequence() + volume_types = None + volume_type_is_supported = False for bdm in block_device_mappings: + volume_type = bdm.volume_type + if volume_type: + if not volume_type_is_supported: + # The following method raises + # VolumeTypeSupportNotYetAvailable if the minimum + # nova-compute service version across the deployment is + # not new enough to support creating volumes with a + # specific type. + self._check_compute_supports_volume_type(context) + # Set the flag to avoid calling + # _check_compute_supports_volume_type more than once in + # this for loop. + volume_type_is_supported = True + + if not volume_types: + # In order to reduce the number of hit cinder APIs, + # initialize our cache of volume types. + volume_types = self.volume_api.get_all_volume_types( + context) + # NOTE(brinzhang): Ensure the validity of volume_type. + self._check_requested_volume_type(bdm, volume_type, + volume_types) + # NOTE(vish): For now, just make sure the volumes are accessible. # Additionally, check that the volume can be attached to this # instance. diff --git a/nova/exception.py b/nova/exception.py index 4e5a5f79486c..10111e0f8339 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -283,6 +283,14 @@ class MultiattachNotSupportedOldMicroversion(Invalid): 'compute API version 2.60.') +class VolumeTypeSupportNotYetAvailable(NovaException): + # This exception indicates the deployment is not yet new enough to support + # volume type, so a 409 HTTPConflict response is generally used + # for handling this in the API. + msg_fmt = _("Volume type support is not yet available.") + code = 409 + + class MultiattachToShelvedNotSupported(Invalid): msg_fmt = _("Attaching multiattach volumes is not supported for " "shelved-offloaded instances.") @@ -631,6 +639,10 @@ class VolumeNotFound(NotFound): msg_fmt = _("Volume %(volume_id)s could not be found.") +class VolumeTypeNotFound(NotFound): + msg_fmt = _("Volume type %(id_or_name)s could not be found.") + + class UndefinedRootBDM(NovaException): msg_fmt = _("Undefined Block Device Mapping root: BlockDeviceMappingList " "contains Block Device Mappings from multiple instances.") diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 1b5af23c6c2a..e5a6e4399067 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -4455,6 +4455,92 @@ class _ComputeAPIUnitTestMixIn(object): self.context, objects.Instance(), objects.Flavor(), bdms) + @mock.patch.object(objects.service, 'get_minimum_version_all_cells', + return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE) + def test_validate_bdm_with_volume_type_name_is_specified( + self, mock_get_min_version): + """Test _check_requested_volume_type and + _check_compute_supports_volume_type methods are used. + """ + instance = self._create_instance_obj() + instance_type = self._create_flavor() + + volume_type = 'fake_lvm_1' + volume_types = [{'id': 'fake_volume_type_id_1', 'name': 'fake_lvm_1'}, + {'id': 'fake_volume_type_id_2', 'name': 'fake_lvm_2'}] + + bdm1 = objects.BlockDeviceMapping( + **fake_block_device.AnonFakeDbBlockDeviceDict( + { + 'uuid': uuids.image_id, + 'source_type': 'image', + 'destination_type': 'volume', + 'device_name': 'vda', + 'boot_index': 0, + 'volume_size': 3, + 'volume_type': 'fake_lvm_1' + })) + bdm2 = objects.BlockDeviceMapping( + **fake_block_device.AnonFakeDbBlockDeviceDict( + { + 'uuid': uuids.image_id, + 'source_type': 'snapshot', + 'destination_type': 'volume', + 'device_name': 'vdb', + 'boot_index': 1, + 'volume_size': 3, + 'volume_type': 'fake_lvm_1' + })) + bdms = [bdm1, bdm2] + + with test.nested( + mock.patch.object(cinder.API, 'get_all_volume_types', + return_value=volume_types), + mock.patch.object(compute_api.API, + '_check_compute_supports_volume_type'), + mock.patch.object(compute_api.API, + '_check_requested_volume_type')) as ( + get_all_vol_types, vol_type_supported, vol_type_requested): + + self.compute_api._validate_bdm(self.context, instance, + instance_type, bdms) + + vol_type_supported.assert_called_once_with(self.context) + get_all_vol_types.assert_called_once_with(self.context) + + vol_type_requested.assert_any_call(bdms[0], volume_type, + volume_types) + vol_type_requested.assert_any_call(bdms[1], volume_type, + volume_types) + + @mock.patch.object(objects.service, 'get_minimum_version_all_cells', + return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE) + def test_the_specified_volume_type_id_assignment_to_name( + self, mock_get_min_version): + """Test _check_requested_volume_type method is called, if the user + is using the volume type ID, assign volume_type to volume type name. + """ + volume_type = 'fake_volume_type_id_1' + volume_types = [{'id': 'fake_volume_type_id_1', 'name': 'fake_lvm_1'}, + {'id': 'fake_volume_type_id_2', 'name': 'fake_lvm_2'}] + + bdms = [objects.BlockDeviceMapping( + **fake_block_device.AnonFakeDbBlockDeviceDict( + { + 'uuid': uuids.image_id, + 'source_type': 'image', + 'destination_type': 'volume', + 'device_name': 'vda', + 'boot_index': 0, + 'volume_size': 3, + 'volume_type': 'fake_volume_type_id_1' + }))] + + self.compute_api._check_requested_volume_type(bdms[0], + volume_type, + volume_types) + self.assertEqual(bdms[0].volume_type, volume_types[0]['name']) + def _test_provision_instances_with_cinder_error(self, expected_exception): @mock.patch('nova.compute.utils.check_num_instances_quota') @@ -6207,6 +6293,41 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): self.compute_api.attach_volume, self.context, instance, uuids.volumeid) + @mock.patch('nova.objects.service.get_minimum_version_all_cells', + return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE - 1) + def test_check_compute_supports_volume_type_new_inst_old_compute( + self, get_min_version): + """Tests that _check_compute_supports_volume_type fails if trying to + specify a volume type to create a new instance but the nova-compute + service version are not all upgraded yet. + """ + self.assertRaises(exception.VolumeTypeSupportNotYetAvailable, + self.compute_api._check_compute_supports_volume_type, + self.context) + + @mock.patch('nova.objects.service.get_minimum_version_all_cells', + return_value=compute_api.MIN_COMPUTE_VOLUME_TYPE) + def test_validate_bdm_check_volume_type_raise_not_found( + self, get_min_version): + """Tests that _validate_bdm will fail if the requested volume type + name or id does not match the volume types in Cinder. + """ + volume_types = [{'id': 'fake_volume_type_id_1', 'name': 'fake_lvm_1'}, + {'id': 'fake_volume_type_id_2', 'name': 'fake_lvm_2'}] + bdm = objects.BlockDeviceMapping( + **fake_block_device.FakeDbBlockDeviceDict( + { + 'uuid': uuids.image_id, + 'source_type': 'image', + 'destination_type': 'volume', + 'device_name': 'vda', + 'boot_index': 0, + 'volume_size': 3, + 'volume_type': 'lvm-1'})) + self.assertRaises(exception.VolumeTypeNotFound, + self.compute_api._check_requested_volume_type, + bdm, 'lvm-1', volume_types) + @mock.patch.object(neutron_api.API, 'has_substr_port_filtering_extension') @mock.patch.object(neutron_api.API, 'list_ports') @mock.patch.object(objects.BuildRequestList, 'get_by_filters', diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py index 47f500615fcd..1451de952f0d 100644 --- a/nova/tests/unit/volume/test_cinder.py +++ b/nova/tests/unit/volume/test_cinder.py @@ -70,6 +70,12 @@ class FakeSnapshot(object): self.project_id = 'fake_project' +class FakeVolumeType(object): + def __init__(self, volume_type_name, volume_type_id): + self.id = volume_type_id + self.name = volume_type_name + + class FakeAttachment(object): def __init__(self): @@ -895,6 +901,27 @@ class CinderApiTestCase(test.NoDBTestCase): mock_volume_snapshots.update_snapshot_status.assert_called_once_with( 'snapshot_id', {'status': 'error', 'progress': '90%'}) + @mock.patch('nova.volume.cinder.cinderclient') + def test_get_all_volume_types(self, mock_cinderclient): + volume_type1 = FakeVolumeType('lvm_1', 'volume_type_id1') + volume_type2 = FakeVolumeType('lvm_2', 'volume_type_id2') + volume_type_list = [volume_type1, volume_type2] + + mock_volume_types = mock.MagicMock() + mock_cinderclient.return_value = mock.MagicMock( + volume_types=mock_volume_types) + mock_volume_types.list.return_value = volume_type_list + + volume_types = self.api.get_all_volume_types(self.ctx) + self.assertEqual(2, len(volume_types)) + self.assertEqual(['volume_type_id1', 'volume_type_id2'], + [vol_type['id'] for vol_type in volume_types]) + self.assertEqual(['lvm_1', 'lvm_2'], + [vol_type['name'] for vol_type in volume_types]) + + mock_cinderclient.assert_called_once_with(self.ctx) + mock_volume_types.list.assert_called_once_with() + @mock.patch('nova.volume.cinder.cinderclient') def test_get_volume_encryption_metadata(self, mock_cinderclient): mock_volumes = mock.MagicMock() diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py index a729349f81ff..90e27fee0a83 100644 --- a/nova/volume/cinder.py +++ b/nova/volume/cinder.py @@ -328,6 +328,16 @@ def _untranslate_volume_summary_view(context, vol): return d +def _untranslate_volume_type_view(volume_type): + """Maps keys for volume type view.""" + v = {} + + v['id'] = volume_type.id + v['name'] = volume_type.name + + return v + + def _untranslate_snapshot_summary_view(context, snapshot): """Maps keys for snapshots summary view.""" d = {} @@ -686,6 +696,16 @@ class API(object): def delete_snapshot(self, context, snapshot_id): cinderclient(context).volume_snapshots.delete(snapshot_id) + @translate_cinder_exception + def get_all_volume_types(self, context): + items = cinderclient(context).volume_types.list() + rvals = [] + + for item in items: + rvals.append(_untranslate_volume_type_view(item)) + + return rvals + @translate_cinder_exception def get_volume_encryption_metadata(self, context, volume_id): return cinderclient(context).volumes.get_encryption_metadata(volume_id)