]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Extra specs may not be in volume types
authorXing Yang <xing.yang@emc.com>
Wed, 19 Aug 2015 00:21:56 +0000 (20:21 -0400)
committerXing Yang <xing.yang@emc.com>
Wed, 19 Aug 2015 13:06:52 +0000 (09:06 -0400)
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
cinder/volume/api.py

index 3087358a22c623d2fece1712a0d801b3b50f27fb..c4189115bfd1c1a2b46a48ec6c675c1e390ee7d6 100644 (file)
@@ -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):
index e41d03daea8928d1d4d68a6de76492dc31eae3df..03f1d74fae2d75295fc02e209fb01173c542e322 100644 (file)
@@ -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: