]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Add default volume type flag
authorZhiteng Huang <zhiteng.huang@intel.com>
Wed, 3 Oct 2012 09:10:57 +0000 (17:10 +0800)
committerZhiteng Huang <zhiteng.huang@intel.com>
Fri, 26 Oct 2012 08:25:48 +0000 (16:25 +0800)
Where using volume type scheduler, 'volume type' is an essential input for
scheduler to perform sophysticated scheduling.  This patch adds a new
flag 'default_volume_type to allow admin to set one volume type as the
default type for volume creation if user doesn't specify any.

Also add more clear 404 message to API if create request provided non-existing
volume type.

Note: Setting the 'default_volume_type' flag to non-existing volume type
is problematic and should be avoided, and the outcome of such behavior is
equivlant to setting default volume type to None.

This is part of patch set implementing 'volume-type-scheduler'.

Change-Id: Ib2b2bbdc9bc96ab097cca2821c9aef04c65c2857

cinder/api/openstack/volume/volumes.py
cinder/flags.py
cinder/tests/fake_flags.py
cinder/tests/test_volume.py
cinder/tests/test_volume_types.py
cinder/volume/api.py
cinder/volume/volume_types.py

index 69d55c6bd8e3f0cab1a25c54e91b6c3872196155..5b80d4922bdcf6ffa1f26cd165d7d190d1cc9b70 100644 (file)
@@ -289,8 +289,9 @@ class VolumeController(wsgi.Controller):
             try:
                 kwargs['volume_type'] = volume_types.get_volume_type_by_name(
                         context, req_volume_type)
-            except exception.NotFound:
-                raise exc.HTTPNotFound()
+            except exception.VolumeTypeNotFound:
+                explanation = 'Volume type not found.'
+                raise exc.HTTPNotFound(explanation=explanation)
 
         kwargs['metadata'] = volume.get('metadata', None)
 
index 1c61cad7acfa183e62160313f1cc63d4f12ecd5f..6c418399ad88a0ed7c60493245a08768cf1d546d 100644 (file)
@@ -184,6 +184,9 @@ global_opts = [
     cfg.ListOpt('memcached_servers',
                 default=None,
                 help='Memcached servers or None for in process cache.'),
+    cfg.StrOpt('default_volume_type',
+               default=None,
+               help='default volume type to use'),
     cfg.StrOpt('volume_usage_audit_period',
                default='month',
                help='time period to generate volume usages for.  '
index 15f17114c5e1b80a8f185a3970dbd03ddc04d776..e2147c63c582d3708bf54d8ebef0dfaf32437e25 100644 (file)
@@ -25,8 +25,11 @@ flags.DECLARE('policy_file', 'cinder.policy')
 flags.DECLARE('volume_driver', 'cinder.volume.manager')
 flags.DECLARE('xiv_proxy', 'cinder.volume.xiv')
 
+def_vol_type = 'fake_vol_type'
+
 
 def set_defaults(conf):
+    conf.set_default('default_volume_type', def_vol_type)
     conf.set_default('volume_driver', 'cinder.volume.driver.FakeISCSIDriver')
     conf.set_default('connection_type', 'fake')
     conf.set_default('fake_rabbit', True)
index 52f980b9511c00b8063c460f3334ecc0147edf3c..bb848d8d4f89610d443f6dda7716bfb129b25822 100644 (file)
@@ -40,6 +40,7 @@ import cinder.policy
 from cinder import quota
 from cinder import test
 from cinder.volume import iscsi
+from cinder.tests import fake_flags
 
 QUOTAS = quota.QUOTAS
 FLAGS = flags.FLAGS
@@ -138,6 +139,60 @@ class VolumeTestCase(test.TestCase):
                           self.context,
                           volume_id)
 
+    def test_create_volume_with_volume_type(self):
+        """Test volume creation with default volume type."""
+        def fake_reserve(context, expire=None, **deltas):
+            return ["RESERVATION"]
+
+        def fake_commit(context, reservations):
+            pass
+
+        def fake_rollback(context, reservations):
+            pass
+
+        self.stubs.Set(QUOTAS, "reserve", fake_reserve)
+        self.stubs.Set(QUOTAS, "commit", fake_commit)
+        self.stubs.Set(QUOTAS, "rollback", fake_rollback)
+
+        volume_api = cinder.volume.api.API()
+
+        # Create volume with default volume type while default
+        # volume type doesn't exist, volume_type_id should be NULL
+        volume = volume_api.create(self.context,
+                                   1,
+                                   'name',
+                                   'description')
+        self.assertEquals(volume['volume_type_id'], None)
+
+        # Create default volume type
+        vol_type = fake_flags.def_vol_type
+        db.volume_type_create(context.get_admin_context(),
+                              dict(name=vol_type, extra_specs={}))
+
+        db_vol_type = db.volume_type_get_by_name(context.get_admin_context(),
+                                                 vol_type)
+
+        # Create volume with default volume type
+        volume = volume_api.create(self.context,
+                                   1,
+                                   'name',
+                                   'description')
+        self.assertEquals(volume['volume_type_id'], db_vol_type.get('id'))
+
+        # Create volume with specific volume type
+        vol_type = 'test'
+        db.volume_type_create(context.get_admin_context(),
+                              dict(name=vol_type, extra_specs={}))
+        db_vol_type = db.volume_type_get_by_name(context.get_admin_context(),
+                                                 vol_type)
+
+        volume = volume_api.create(self.context,
+                                   1,
+                                   'name',
+                                   'description',
+                                   volume_type=db_vol_type)
+        self.assertEquals(volume['volume_type_id'], db_vol_type.get('id'))
+
     def test_delete_busy_volume(self):
         """Test volume survives deletion if driver reports it as busy."""
         volume = self._create_volume()
index 8650b4c0592c227194da5741d4e8ef79ab46638b..847ff8814180ba17724cc68e44c9f0a5b932c9b0 100644 (file)
@@ -26,6 +26,7 @@ from cinder import test
 from cinder.volume import volume_types
 from cinder.db.sqlalchemy import session as sql_session
 from cinder.db.sqlalchemy import models
+from cinder.tests import fake_flags
 
 FLAGS = flags.FLAGS
 LOG = logging.getLogger(__name__)
@@ -80,6 +81,22 @@ class VolumeTypeTestCase(test.TestCase):
         vol_types = volume_types.get_all_types(self.ctxt)
         self.assertEqual(total_volume_types, len(vol_types))
 
+    def test_get_default_volume_type(self):
+        """ Ensures default volume type can be retrieved """
+        volume_types.create(self.ctxt,
+                            fake_flags.def_vol_type,
+                            {})
+        default_vol_type = volume_types.get_default_volume_type()
+        self.assertEqual(default_vol_type.get('name'),
+                         fake_flags.def_vol_type)
+
+    def test_default_volume_type_missing_in_db(self):
+        """ Ensures proper exception raised if default volume type
+        is not in database. """
+        session = sql_session.get_session()
+        default_vol_type = volume_types.get_default_volume_type()
+        self.assertEqual(default_vol_type, {})
+
     def test_non_existent_vol_type_shouldnt_delete(self):
         """Ensures that volume type creation fails with invalid args"""
         self.assertRaises(exception.VolumeTypeNotFoundByName,
index b983409647f552f95d8bd285dbacb35658c8660f..418b1ef78e516e3036a94c1b9747012b87391ac9 100644 (file)
@@ -31,6 +31,7 @@ from cinder.image import glance
 from cinder.openstack.common import log as logging
 from cinder.openstack.common import rpc
 from cinder.openstack.common import timeutils
+from cinder.volume import volume_types
 import cinder.policy
 from cinder import quota
 
@@ -146,10 +147,10 @@ class API(base.Base):
         if availability_zone is None:
             availability_zone = FLAGS.storage_availability_zone
 
-        if volume_type is None:
-            volume_type_id = None
-        else:
-            volume_type_id = volume_type.get('id', None)
+        if not volume_type:
+            volume_type = volume_types.get_default_volume_type()
+
+        volume_type_id = volume_type.get('id')
 
         options = {
             'size': size,
index 3c8bb826434025bd1ab1d791b453b8550ee1df43..f4ac4dc14a08f5f9640f4726c39bb13b1292fa00 100644 (file)
@@ -111,6 +111,25 @@ def get_volume_type_by_name(context, name):
     return db.volume_type_get_by_name(context, name)
 
 
+def get_default_volume_type():
+    """Get the default volume type."""
+    name = FLAGS.default_volume_type
+    vol_type = {}
+
+    if name is not None:
+        ctxt = context.get_admin_context()
+        try:
+            vol_type = get_volume_type_by_name(ctxt, name)
+        except exception.VolumeTypeNotFoundByName, e:
+            # Couldn't find volume type with the name in default_volume_type
+            # flag, record this issue and move on
+            #TODO(zhiteng) consider add notification to warn admin
+            LOG.exception(_('Default volume type is not found, '
+                            'please check default_volume_type config: %s'), e)
+
+    return vol_type
+
+
 def is_key_value_present(volume_type_id, key, value, volume_type=None):
     if volume_type_id is None:
         return False