From 76016fffc946301ba4df6b2b58713dcb41d45dff Mon Sep 17 00:00:00 2001 From: Patrick East Date: Tue, 28 Jun 2016 14:49:11 -0700 Subject: [PATCH] Add support for shared "backend_defaults" config This gives a new config section that is shared for all enabled_backends in a cinder.conf. This is optional but can be used like: [DEFAULT] ... enabled_backends=lvm-1,lvm-2 ... [backend_defaults] image_volume_cache_enabled = True volume_clear = none iscsi_helper = tgtadm volume_driver = cinder.volume.drivers.lvm.LVMVolumeDriver [lvm-1] volume_group = stack-volumes-lvm-1 [lvm-2] volume_group = stack-volumes-lvmd-2 lvm_type = thin This also sets up the config helper to be easily refactored to remove support for DEFAULT stanza configured backends. DocImpact: Need to document upgrade path and new recommended config. Implements: blueprint shared-backend-config Change-Id: I2f94118b32076f264b3cf21caa259785fd415167 --- .../tests/unit/test_volume_configuration.py | 38 +++++- cinder/tests/unit/volume/test_volume.py | 4 - cinder/volume/configuration.py | 112 +++++++++++++++--- ...hared-backend-config-d841b806354ad5be.yaml | 5 + 4 files changed, 134 insertions(+), 25 deletions(-) create mode 100644 releasenotes/notes/shared-backend-config-d841b806354ad5be.yaml diff --git a/cinder/tests/unit/test_volume_configuration.py b/cinder/tests/unit/test_volume_configuration.py index 733706e68ba..2abb78b1281 100644 --- a/cinder/tests/unit/test_volume_configuration.py +++ b/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.bool_opt, CONF.foo.bool_opt) + self.assertEqual(c.str_opt, 'STR_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.bool_opt, CONF.foo.bool_opt) - self.assertEqual(c.int_opt, CONF.foo.int_opt) + self.assertEqual(c.str_opt, 'STR_OPT') + self.assertEqual(c.bool_opt, False) + 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) diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index 33afdc6d379..8a143182ca3 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/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): diff --git a/cinder/volume/configuration.py b/cinder/volume/configuration.py index 61ff8fbfdaa..7a5710cbc6c 100644 --- a/cinder/volume/configuration.py +++ b/cinder/volume/configuration.py @@ -46,25 +46,15 @@ CONF = cfg.CONF SHARED_CONF_GROUP = 'backend_defaults' -class Configuration(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 - """ - self.config_group = config_group +class DefaultGroupConfiguration(object): + """Get config options from only DEFAULT.""" + def __init__(self): # set the local conf so that __call__'s know what to use - if self.config_group: - self._ensure_config_values(volume_opts) - self.local_conf = CONF._get(self.config_group) - else: - self.local_conf = CONF + 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) diff --git a/releasenotes/notes/shared-backend-config-d841b806354ad5be.yaml b/releasenotes/notes/shared-backend-config-d841b806354ad5be.yaml new file mode 100644 index 00000000000..6af993787a8 --- /dev/null +++ b/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. \ No newline at end of file