From 87b6072b8401528c5582f48ec3cc5b52049a3774 Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Tue, 18 Aug 2015 20:21:56 -0400 Subject: [PATCH] Extra specs may not be in volume types Get volume types no longer returns extra specs for non-admin users. This breaks volume creation which needs extra specs. This patch fixes it as follows: * In volume/api.py, get extra specs when checking for retype. * In scheduler/filter_scheduler.py, get extra specs if they are not already in volume type. Closes-Bug: 1495764 Change-Id: I1eca87d14cce84596628f0e8b3ea1602914bd883 --- cinder/tests/unit/test_volume.py | 94 ++++++++++++++++---------------- cinder/volume/api.py | 30 +++++++--- 2 files changed, 68 insertions(+), 56 deletions(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 3087358a2..c4189115b 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -1183,30 +1183,30 @@ class VolumeTestCase(BaseVolumeTestCase): **snapshot) # Make sure the case of specifying a type that # doesn't match the snapshots type fails + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + volume_type=foo_type, + snapshot=snapshot_obj) + + # Make sure that trying to specify a type + # when the snapshots type is None fails + snapshot_obj.volume_type_id = None + self.assertRaises(exception.InvalidVolumeType, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + volume_type=foo_type, + snapshot=snapshot_obj) + with mock.patch.object(cinder.volume.volume_types, 'get_volume_type') as mock_get_type: mock_get_type.return_value = biz_type - self.assertRaises(exception.InvalidInput, - volume_api.create, - self.context, - size=1, - name='fake_name', - description='fake_desc', - volume_type=foo_type, - snapshot=snapshot_obj) - - # Make sure that trying to specify a type - # when the snapshots type is None fails - snapshot_obj.volume_type_id = None - self.assertRaises(exception.InvalidInput, - volume_api.create, - self.context, - size=1, - name='fake_name', - description='fake_desc', - volume_type=foo_type, - snapshot=snapshot_obj) - snapshot_obj.volume_type_id = foo_type['id'] volume_api.create(self.context, size=1, name='fake_name', description='fake_desc', volume_type=foo_type, @@ -1242,41 +1242,41 @@ class VolumeTestCase(BaseVolumeTestCase): 'volume_type': biz_type, 'volume_type_id': biz_type['id']} + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + volume_type=foo_type, + source_volume=source_vol) + + # Make sure that trying to specify a type + # when the source type is None fails + source_vol['volume_type_id'] = None + source_vol['volume_type'] = None + self.assertRaises(exception.InvalidVolumeType, + volume_api.create, + self.context, + size=1, + name='fake_name', + description='fake_desc', + volume_type=foo_type, + source_volume=source_vol) + with mock.patch.object(cinder.volume.volume_types, 'get_volume_type') as mock_get_type: mock_get_type.return_value = biz_type - self.assertRaises(exception.InvalidInput, - volume_api.create, - self.context, - size=1, - name='fake_name', - description='fake_desc', - volume_type=foo_type, - source_volume=source_vol) - - # Make sure that trying to specify a type - # when the source type is None fails - source_vol['volume_type_id'] = None - source_vol['volume_type'] = None - self.assertRaises(exception.InvalidInput, - volume_api.create, - self.context, - size=1, - name='fake_name', - description='fake_desc', - volume_type=foo_type, - source_volume=source_vol) - source_vol['volume_type_id'] = biz_type['id'] source_vol['volume_type'] = biz_type volume_api.create(self.context, size=1, name='fake_name', description='fake_desc', volume_type=biz_type, source_volume=source_vol) - db.volume_type_destroy(context.get_admin_context(), - foo_type['id']) - db.volume_type_destroy(context.get_admin_context(), - biz_type['id']) + db.volume_type_destroy(context.get_admin_context(), + foo_type['id']) + db.volume_type_destroy(context.get_admin_context(), + biz_type['id']) @mock.patch('cinder.volume.flows.api.create_volume.get_flow') def test_create_volume_from_source_with_same_backend(self, _get_flow): diff --git a/cinder/volume/api.py b/cinder/volume/api.py index e41d03dae..03f1d74fa 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -170,20 +170,21 @@ class API(base.Base): first_type_id, second_type_id, first_type=None, second_type=None): safe = False - services = objects.ServiceList.get_all_by_topic(context, + elevated = context.elevated() + services = objects.ServiceList.get_all_by_topic(elevated, 'cinder-volume', disabled=True) if len(services.objects) == 1: safe = True else: type_a = first_type or volume_types.get_volume_type( - context, + elevated, first_type_id) type_b = second_type or volume_types.get_volume_type( - context, + elevated, second_type_id) - if(volume_utils.matching_backend_name(type_a['extra_specs'], - type_b['extra_specs'])): + if (volume_utils.matching_backend_name(type_a['extra_specs'], + type_b['extra_specs'])): safe = True return safe @@ -235,6 +236,11 @@ class API(base.Base): "group).") % volume_type raise exception.InvalidInput(reason=msg) + if volume_type and 'extra_specs' not in volume_type: + extra_specs = volume_types.get_volume_type_extra_specs( + volume_type['id']) + volume_type['extra_specs'] = extra_specs + if source_volume and volume_type: if volume_type['id'] != source_volume['volume_type_id']: if not self._retype_is_possible( @@ -1340,7 +1346,8 @@ class API(base.Base): volume_type = {} volume_type_id = volume['volume_type_id'] if volume_type_id: - volume_type = volume_types.get_volume_type(context, volume_type_id) + volume_type = volume_types.get_volume_type(context.elevated(), + volume_type_id) request_spec = {'volume_properties': volume, 'volume_type': volume_type, 'volume_id': volume['id']} @@ -1427,10 +1434,11 @@ class API(base.Base): # Support specifying volume type by ID or name try: if uuidutils.is_uuid_like(new_type): - vol_type = volume_types.get_volume_type(context, new_type) + vol_type = volume_types.get_volume_type(context.elevated(), + new_type) else: - vol_type = volume_types.get_volume_type_by_name(context, - new_type) + vol_type = volume_types.get_volume_type_by_name( + context.elevated(), new_type) except exception.InvalidVolumeType: msg = _('Invalid volume_type passed: %s.') % new_type LOG.error(msg) @@ -1500,6 +1508,10 @@ class API(base.Base): def manage_existing(self, context, host, ref, name=None, description=None, volume_type=None, metadata=None, availability_zone=None, bootable=False): + if volume_type and 'extra_specs' not in volume_type: + extra_specs = volume_types.get_volume_type_extra_specs( + volume_type['id']) + volume_type['extra_specs'] = extra_specs if availability_zone is None: elevated = context.elevated() try: -- 2.45.2