Merge "Add support for shared "backend_defaults" config"

cinder/tests/unit/test_volume_configuration.py

@@ -15,7 +15,6 @@
 
 """Tests for the configuration wrapper in volume drivers."""
 
-
 from oslo_config import cfg
 
 from cinder import test
@@ -39,8 +38,12 @@ class VolumeConfigurationTest(test.TestCase):
 
     def test_group_grafts_opts(self):
         c = configuration.Configuration(volume_opts, config_group='foo')
-        self.assertEqual(c.str_opt, CONF.foo.str_opt)
+        self.assertEqual(c.str_opt, 'STR_OPT')
-        self.assertEqual(c.bool_opt, CONF.foo.bool_opt)
+        self.assertEqual(c.bool_opt, False)
+        self.assertEqual(c.str_opt, CONF.backend_defaults.str_opt)
+        self.assertEqual(c.bool_opt, CONF.backend_defaults.bool_opt)
+        self.assertIsNone(CONF.foo.str_opt)
+        self.assertIsNone(CONF.foo.bool_opt)
 
     def test_opts_no_group(self):
         c = configuration.Configuration(volume_opts)
@@ -50,10 +53,33 @@ class VolumeConfigurationTest(test.TestCase):
     def test_grafting_multiple_opts(self):
         c = configuration.Configuration(volume_opts, config_group='foo')
         c.append_config_values(more_volume_opts)
-        self.assertEqual(c.str_opt, CONF.foo.str_opt)
+        self.assertEqual(c.str_opt, 'STR_OPT')
-        self.assertEqual(c.bool_opt, CONF.foo.bool_opt)
+        self.assertEqual(c.bool_opt, False)
-        self.assertEqual(c.int_opt, CONF.foo.int_opt)
+        self.assertEqual(c.int_opt, 1)
+
+        # We get the right values, but they are coming from the backend_default
+        # group of CONF no the 'foo' one.
+        self.assertEqual(c.str_opt, CONF.backend_defaults.str_opt)
+        self.assertEqual(c.bool_opt, CONF.backend_defaults.bool_opt)
+        self.assertEqual(c.int_opt, CONF.backend_defaults.int_opt)
+        self.assertIsNone(CONF.foo.str_opt)
+        self.assertIsNone(CONF.foo.bool_opt)
+        self.assertIsNone(CONF.foo.int_opt)
 
     def test_safe_get(self):
         c = configuration.Configuration(volume_opts, config_group='foo')
         self.assertIsNone(c.safe_get('none_opt'))
+
+    def test_backend_specific_value(self):
+        c = configuration.Configuration(volume_opts, config_group='foo')
+
+        # Set some new non-default value
+        CONF.set_override('str_opt', 'bar', group='backend_defaults')
+        actual_value = c.str_opt
+        self.assertEqual('bar', actual_value)
+
+        CONF.set_override('str_opt', 'notbar', group='foo')
+        actual_value = c.str_opt
+        # Make sure that we pick up the backend value and not the shared group
+        # value...
+        self.assertEqual('notbar', actual_value)

cinder/tests/unit/volume/test_volume.py

@@ -2682,10 +2682,6 @@ class VolumeTestCase(base.BaseVolumeTestCase):
             self.context)
         self.assertEqual(expected_result, result)
 
-    def test_backup_use_temp_snapshot_config(self):
-        local_conf = self.volume.driver.configuration.local_conf
-        self.assertFalse(local_conf.backup_use_temp_snapshot)
-
     @mock.patch('cinder.tests.fake_driver.FakeLoggingVolumeDriver.'
                 'SUPPORTS_ACTIVE_ACTIVE', True)
     def test_set_resource_host_different(self):

cinder/volume/configuration.py

@@ -46,25 +46,15 @@ CONF = cfg.CONF
 SHARED_CONF_GROUP = 'backend_defaults'
 
 
-class Configuration(object):
+class DefaultGroupConfiguration(object):
-
+    """Get config options from only DEFAULT."""
-    def __init__(self, volume_opts, config_group=None):
-        """Initialize configuration.
-
-        This takes care of grafting the implementation's config
-        values into the config group
-        """
-        self.config_group = config_group
 
+    def __init__(self):
         # set the local conf so that __call__'s know what to use
-        if self.config_group:
+        self.local_conf = CONF
-            self._ensure_config_values(volume_opts)
-            self.local_conf = CONF._get(self.config_group)
-        else:
-            self.local_conf = CONF
 
     def _ensure_config_values(self, volume_opts):
-        CONF.register_opts(volume_opts, group=self.config_group)
+        CONF.register_opts(volume_opts, group=None)
 
     def append_config_values(self, volume_opts):
         self._ensure_config_values(volume_opts)
@@ -79,3 +69,95 @@ class Configuration(object):
         # Don't use self.local_conf to avoid reentrant call to __getattr__()
         local_conf = object.__getattribute__(self, 'local_conf')
         return getattr(local_conf, value)
+
+
+class BackendGroupConfiguration(object):
+
+    def __init__(self, volume_opts, config_group=None):
+        """Initialize configuration.
+
+        This takes care of grafting the implementation's config
+        values into the config group and shared defaults. We will try to
+        pull values from the specified 'config_group', but fall back to
+        defaults from the SHARED_CONF_GROUP.
+        """
+        self.config_group = config_group
+
+        # set the local conf so that __call__'s know what to use
+        self._ensure_config_values(volume_opts)
+        self.backend_conf = CONF._get(self.config_group)
+        self.shared_backend_conf = CONF._get(SHARED_CONF_GROUP)
+
+    def _safe_register(self, opt, group):
+        try:
+            CONF.register_opt(opt, group=group)
+        except cfg.DuplicateOptError:
+            pass  # If it's already registered ignore it
+
+    def _ensure_config_values(self, volume_opts):
+        """Register the options in the shared group.
+
+        When we go to get a config option we will try the backend specific
+        group first and fall back to the shared group. We override the default
+        from all the config options for the backend group so we can know if it
+        was set or not.
+        """
+        for opt in volume_opts:
+            self._safe_register(opt, SHARED_CONF_GROUP)
+            # Assuming they aren't the same groups, graft on the options into
+            # the backend group and override its default value.
+            if self.config_group != SHARED_CONF_GROUP:
+                self._safe_register(opt, self.config_group)
+                CONF.set_default(opt.name, None, group=self.config_group)
+
+    def append_config_values(self, volume_opts):
+        self._ensure_config_values(volume_opts)
+
+    def set_default(self, opt_name, default):
+        CONF.set_default(opt_name, default, group=SHARED_CONF_GROUP)
+
+    def get(self, key, default=None):
+        return getattr(self, key, default)
+
+    def safe_get(self, value):
+        try:
+            return self.__getattr__(value)
+        except cfg.NoSuchOptError:
+            return None
+
+    def __getattr__(self, opt_name):
+        # Don't use self.X to avoid reentrant call to __getattr__()
+        backend_conf = object.__getattribute__(self, 'backend_conf')
+        opt_value = getattr(backend_conf, opt_name)
+        if opt_value is None:
+            shared_conf = object.__getattribute__(self, 'shared_backend_conf')
+            opt_value = getattr(shared_conf, opt_name)
+        return opt_value
+
+
+class Configuration(object):
+
+    def __init__(self, volume_opts, config_group=None):
+        """Initialize configuration.
+
+        This shim will allow for compatibility with the DEFAULT
+        style of backend configuration which is used by some of the users
+        of this configuration helper, or by the volume drivers that have
+        all been forced over to the config_group style.
+        """
+        self.config_group = config_group
+        if config_group:
+            self.conf = BackendGroupConfiguration(volume_opts, config_group)
+        else:
+            self.conf = DefaultGroupConfiguration()
+
+    def append_config_values(self, volume_opts):
+        self.conf.append_config_values(volume_opts)
+
+    def safe_get(self, value):
+        return self.conf.safe_get(value)
+
+    def __getattr__(self, value):
+        # Don't use self.conf to avoid reentrant call to __getattr__()
+        conf = object.__getattribute__(self, 'conf')
+        return getattr(conf, value)

releasenotes/notes/shared-backend-config-d841b806354ad5be.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - New config format to allow for using shared Volume Driver configuration
+    defaults via the [backend_defaults] stanza. Config options defined there
+    will be used as defaults for each backend enabled via enabled_backends.