]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
coraid: allow setting default repository
authorRoman Bogorodskiy <rbogorodskiy@mirantis.com>
Wed, 1 Oct 2014 08:15:14 +0000 (08:15 +0000)
committerRoman Bogorodskiy <rbogorodskiy@mirantis.com>
Mon, 6 Oct 2014 15:33:35 +0000 (15:33 +0000)
Currently, the Coraid driver requires storing Storage Repository
name in volume type key named 'coraid_repository' or different key name
if a value of the 'coraid_repository_key' option changed in the cinder
configuration file.

In case of creating a volume without volume type associated or creating
a volume with volume type that doesn't have an appropriate key with
the repository name, volume creation fails.

That is an inconvenient behaviour, especially in case of using that with
Tempest that creates new volume types but obviously doesn't know
that it should add keys with the repository name.

In order to fix that, introduce a 'coraid_default_repository' config
option that allows to set a default repository to use if one is not
specified in the volume type key.

In case if neither volume type nor config file provide a repository
name, throw CoraidException with description of the problem.

DocImpact

Change-Id: Ib205553cdee0af0036a774f6926433f4452ec023

cinder/tests/test_coraid.py
cinder/volume/drivers/coraid.py
etc/cinder/cinder.conf.sample

index 7ee9573bee2789e9f4f923caa24f7bc7fc497438..24dc64ccb875119b9de29643c907ebcb2347ef80 100644 (file)
@@ -16,6 +16,7 @@
 
 import math
 
+import mock
 import mox
 from oslo.config import cfg
 
@@ -233,6 +234,7 @@ class CoraidDriverTestCase(test.TestCase):
         super(CoraidDriverTestCase, self).setUp()
         configuration = mox.MockObject(conf.Configuration)
         configuration.append_config_values(mox.IgnoreArg())
+        configuration.coraid_default_repository = 'default_repository'
         configuration.coraid_esm_address = fake_esm_ipaddress
         configuration.coraid_user = fake_esm_username
         configuration.coraid_group = fake_esm_group
@@ -344,6 +346,52 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
 
         self.mox.VerifyAll()
 
+    @mock.patch.object(volume_types, 'get_volume_type_extra_specs')
+    def test_create_volume_volume_type_no_repo_key(self, volume_specs_mock):
+        """Test volume creation without repo specified in volume type."""
+        volume_specs_mock.return_value = None
+
+        create_volume_request = {'addr': 'cms',
+                                 'data': {
+                                     'servers': [],
+                                     'size':
+                                         coraid_volume_size(fake_volume_size),
+                                     'repoName': 'default_repository',
+                                     'lvName': fake_volume_name},
+                                 'op': 'orchStrLun',
+                                 'args': 'add'}
+        pack_data(create_volume_request)
+
+        self.fake_rpc.handle('configure', {}, [create_volume_request],
+                             {'configState': 'completedSuccessfully',
+                              'firstParam': 'fake_first_param'})
+
+        self.driver.create_volume(fake_volume)
+
+    @mock.patch.object(volume_types, 'get_volume_type_extra_specs')
+    def test_create_volume_volume_type_no_repo_data(self, volume_specs_mock):
+        """Test volume creation w/o repo in volume type nor config."""
+        volume_specs_mock.return_value = None
+        self.driver.configuration.coraid_default_repository = None
+
+        create_volume_request = {'addr': 'cms',
+                                 'data': {
+                                     'servers': [],
+                                     'size':
+                                         coraid_volume_size(fake_volume_size),
+                                     'repoName': 'default_repository',
+                                     'lvName': fake_volume_name},
+                                 'op': 'orchStrLun',
+                                 'args': 'add'}
+        pack_data(create_volume_request)
+
+        self.fake_rpc.handle('configure', {}, [create_volume_request],
+                             {'configState': 'completedSuccessfully',
+                              'firstParam': 'fake_first_param'})
+
+        self.assertRaises(exception.CoraidException,
+                          self.driver.create_volume, fake_volume)
+
     def test_delete_volume(self):
         delete_volume_request = {'addr': 'cms',
                                  'data': {
index 652ae43a95ff0454a79a46f960f4d07aefb1caa6..caee1003f7878306eec60080329b53ba548cb1cf 100644 (file)
@@ -57,6 +57,9 @@ coraid_opts = [
     cfg.StrOpt('coraid_repository_key',
                default='coraid_repository',
                help='Volume Type key name to store ESM Repository Name'),
+    cfg.StrOpt('coraid_default_repository',
+               help='ESM Repository Name to use if not specified in '
+                    'Volume Type keys'),
 ]
 
 CONF = cfg.CONF
@@ -446,8 +449,18 @@ class CoraidDriver(driver.VolumeDriver):
         """
         volume_type_id = volume_type['id']
         repository_key_name = self.configuration.coraid_repository_key
-        repository = volume_types.get_volume_type_extra_specs(
-            volume_type_id, repository_key_name)
+        repository = (volume_types.get_volume_type_extra_specs(
+            volume_type_id, repository_key_name) or
+            self.configuration.coraid_default_repository)
+
+        # if there's no repository still, we cannot move forward
+        if not repository:
+            message = ("The Coraid repository not specified neither "
+                       "in Volume Type '%s' key nor in the "
+                       "'coraid_default_repository' config option" %
+                       self.configuration.coraid_repository_key)
+            raise exception.CoraidException(message=message)
+
         # Remove <in> keyword from repository name if needed
         if repository.startswith('<in> '):
             return repository[len('<in> '):]
index de342bd3712f383a0f7b4f9e3bc3ce639ecc13c5..c7a5e616fc366cf5bb89d9835088649ea1e65e00 100644 (file)
 # value)
 #coraid_repository_key=coraid_repository
 
+# ESM Repository Name to use if not specified in Volume Type
+# keys (string value)
+#coraid_default_repository=<None>
+
 
 #
 # Options defined in cinder.volume.drivers.datera