diff --git a/cinderlib/cinderlib.py b/cinderlib/cinderlib.py index a0322a7..26222b9 100644 --- a/cinderlib/cinderlib.py +++ b/cinderlib/cinderlib.py @@ -18,7 +18,6 @@ import json as json_lib import logging import multiprocessing import os -import six from cinder import coordination from cinder.db import api as db_api @@ -31,7 +30,7 @@ cinder_objects.register_all() # noqa from cinder.interface import util as cinder_interface_util from cinder import utils from cinder.volume import configuration -from cinder.volume import manager +from cinder.volume import manager # noqa We need to import config options from oslo_config import cfg from oslo_log import log as oslo_logging from oslo_utils import importutils @@ -76,7 +75,8 @@ class Backend(object): driver_cfg['volume_backend_name'] = volume_backend_name Backend.backends[volume_backend_name] = self - conf = self._set_backend_config(driver_cfg) + conf = self._get_backend_config(driver_cfg) + self._apply_backend_workarounds(conf) self.driver = importutils.import_object( conf.volume_driver, configuration=conf, @@ -190,164 +190,59 @@ class Backend(object): # the persistence plugin. db_api.IMPL = cls.persistence.db - # NOTE(geguileo): Staticmethod used instead of classmethod to make it work - # on Python3 when assigning the unbound method. - @staticmethod - def _config_parse(self): - """Replacer oslo_config.cfg.ConfigParser.parse for in-memory cfg.""" - res = super(cfg.ConfigParser, self).parse(Backend._config_string_io) - return res - - @classmethod - def _update_cinder_config(cls): - """Parse in-memory file to update OSLO configuration used by Cinder.""" - cls._config_string_io.seek(0) - cls._parser.write(cls._config_string_io) - - # Check if we have any multiopt - cls._config_string_io.seek(0) - current_cfg = cls._config_string_io.read() - if '\n\t' in current_cfg: - cls._config_string_io.seek(0) - cls._config_string_io.write(current_cfg.replace('\n\t', '\n')) - - cls._config_string_io.seek(0) - cfg.CONF.reload_config_files() - @classmethod def _set_cinder_config(cls, host, locks_path, cinder_config_params): """Setup the parser with all the known Cinder configuration.""" cfg.CONF.set_default('state_path', os.getcwd()) cfg.CONF.set_default('lock_path', '$state_path', 'oslo_concurrency') - - cls._parser = six.moves.configparser.SafeConfigParser() - cls._parser.set('DEFAULT', 'enabled_backends', '') + cfg.CONF.version = cinderlib.__version__ if locks_path: - cls._parser.add_section('oslo_concurrency') - cls._parser.set('oslo_concurrency', 'lock_path', locks_path) - cls._parser.add_section('coordination') - cls._parser.set('coordination', - 'backend_url', - 'file://' + locks_path) + cfg.CONF.oslo_concurrency.lock_path = locks_path + cfg.CONF.coordination.backend_url = 'file://' + locks_path + if host: - cls._parser.set('DEFAULT', 'host', host) + cfg.CONF.host = host - # All other configuration options go into the DEFAULT section - cls.__set_parser_kv(cinder_config_params, 'DEFAULT') - - # We replace the OSLO's default parser to read from a StringIO instead - # of reading from a file. - cls._config_string_io = six.moves.StringIO() - cfg.ConfigParser.parse = six.create_unbound_method(cls._config_parse, - cfg.ConfigParser) + cls._validate_options(cinder_config_params) + for k, v in cinder_config_params.items(): + setattr(cfg.CONF, k, v) # Replace command line arg parser so we ignore caller's args cfg._CachedArgumentParser.parse_args = lambda *a, **kw: None - # Update the configuration with the options we have configured - cfg.CONF(project='cinder', version=cinderlib.__version__, - default_config_files=['in_memory_file']) - cls._update_cinder_config() - - @staticmethod - def __get_options_types(kvs): - """Get loaded oslo options and load driver if we know it.""" + @classmethod + def _validate_options(cls, kvs, group=None): # Dynamically loading the driver triggers adding the specific # configuration options to the backend_defaults section - if 'volume_driver' in kvs: + if kvs.get('volume_driver'): driver_ns = kvs['volume_driver'].rsplit('.', 1)[0] __import__(driver_ns) - opts = configuration.CONF._groups['backend_defaults']._opts - return opts + group = group or 'backend_defaults' - @classmethod - def __val_to_str(cls, val): - """Convert an oslo config value to its string representation. + for k, v in kvs.items(): + try: + # set_override does the validation + cfg.CONF.set_override(k, v, group) + # for correctness, don't leave it there + cfg.CONF.clear_override(k, group) + except cfg.NoSuchOptError: + # Don't fail on unknown variables, behave like cinder + LOG.warning('Unknown config option %s', k) - Lists and tuples are treated as ListOpt classes and converted to - "[value1,value2]" instead of the standard string representation of - "['value1','value2']". - - Dictionaries are treated as DictOpt and converted to 'k1:v1,k2:v2" - instead of the standard representation of "{'k1':'v1','k2':'v2'}". - - Anything else is converted to a string. - """ - if isinstance(val, six.string_types): - return val - - # Recursion is used to to handle options like u4p_failover_target that - # is a MultiOpt where each entry is a dictionary. - if isinstance(val, (list, tuple)): - return '[' + ','.join((cls.__val_to_str(v) for v in val)) + ']' - - if isinstance(val, dict): - return ','.join('%s:%s' % (k, cls.__val_to_str(v)) - for k, v in val.items()) - - return six.text_type(val) - - @classmethod - def __convert_option_to_string(cls, key, val, opts): - """Convert a Cinder driver cfg option into oslo config file format. - - A single Python object represents multiple Oslo config types. For - example a list can be a ListOpt or a MultOpt, and their string - representations on a file are different. - - This method uses the configuration option types to return the right - string representation. - """ - opt = opts[key]['opt'] - - # Convert to a list for ListOpt opts were the caller didn't pass a list - if (isinstance(opt, cfg.ListOpt) and - not isinstance(val, (list, tuple))): - val = [val] - - # For MultiOpt we need multiple entries in the file but ConfigParser - # doesn't support repeating the same entry multiple times, so we hack - # our way around it - elif isinstance(opt, cfg.MultiOpt): - if not isinstance(val, (list, tuple)): - val = [val] if val else [] - val = [cls.__val_to_str(v) for v in val] - if not val: - val = '' - elif len(val) == 1: - val = val[0] - else: - val = (('%s\n' % val[0]) + - '\n'.join('%s = %s' % (key, v) for v in val[1:])) - - # This will handle DictOpt and ListOpt - if not isinstance(val, six.string_types): - val = cls.__val_to_str(val) - return val - - @classmethod - def __set_parser_kv(cls, kvs, section): - """Set Oslo configuration options in our parser. - - The Oslo parser needs to have the configuration options like they are - in a file, but we have them as Python objects, so we need to set them - for the parser in the format it is expecting them, strings. - """ - opts = cls.__get_options_types(kvs) - for key, val in kvs.items(): - string_val = cls.__convert_option_to_string(key, val, opts) - cls._parser.set(section, key, string_val) - - def _set_backend_config(self, driver_cfg): + def _get_backend_config(self, driver_cfg): + # Create the group for the backend backend_name = driver_cfg['volume_backend_name'] - self._parser.add_section(backend_name) - self.__set_parser_kv(driver_cfg, backend_name) - self._parser.set('DEFAULT', 'enabled_backends', - ','.join(self.backends.keys())) - self._update_cinder_config() - config = configuration.Configuration(manager.volume_backend_opts, - config_group=backend_name) + cfg.CONF.register_group(cfg.OptGroup(backend_name)) + + # Validate and set config options + backend_group = getattr(cfg.CONF, backend_name) + self._validate_options(driver_cfg) + for key, value in driver_cfg.items(): + setattr(backend_group, key, value) + + # Return the Configuration that will be passed to the driver + config = configuration.Configuration([], config_group=backend_name) return config @classmethod @@ -384,6 +279,14 @@ class Backend(object): cls.global_initialization = True cls.output_all_backend_info = output_all_backend_info + def _apply_backend_workarounds(self, config): + """Apply workarounds for drivers that do bad stuff.""" + if 'netapp' in config.volume_driver: + # Workaround NetApp's weird replication stuff that makes it reload + # config sections in get_backend_configuration. OK since we don't + # support replication. + cfg.CONF.list_all_sections = lambda: config.volume_backend_name + @classmethod def _set_logging(cls, disable_logs): if disable_logs: diff --git a/cinderlib/tests/unit/test_cinderlib.py b/cinderlib/tests/unit/test_cinderlib.py index 9434a8d..03c47e6 100644 --- a/cinderlib/tests/unit/test_cinderlib.py +++ b/cinderlib/tests/unit/test_cinderlib.py @@ -14,16 +14,18 @@ # under the License. import collections +import os +import ddt import mock from oslo_config import cfg -from oslo_config import types import cinderlib from cinderlib import objects from cinderlib.tests.unit import base +@ddt.ddt class TestCinderlib(base.BaseTest): def test_list_supported_drivers(self): expected_keys = {'version', 'class_name', 'supported', 'ci_wiki_name', @@ -40,10 +42,12 @@ class TestCinderlib(base.BaseTest): self.assertEqual(cinderlib.Backend, cinderlib.objects.Object.backend_class) + @mock.patch('cinderlib.Backend._apply_backend_workarounds') @mock.patch('oslo_utils.importutils.import_object') - @mock.patch('cinderlib.Backend._set_backend_config') + @mock.patch('cinderlib.Backend._get_backend_config') @mock.patch('cinderlib.Backend.global_setup') - def test_init(self, mock_global_setup, mock_config, mock_import): + def test_init(self, mock_global_setup, mock_config, mock_import, + mock_workarounds): cfg.CONF.set_override('host', 'host') driver_cfg = {'k': 'v', 'k2': 'v2', 'volume_backend_name': 'Test'} cinderlib.Backend.global_initialization = False @@ -74,8 +78,34 @@ class TestCinderlib(base.BaseTest): self.assertIsNone(backend._volumes) driver.get_volume_stats.assert_not_called() self.assertEqual(('default',), backend.pool_names) + mock_workarounds.assert_called_once_with(mock_config.return_value) - @mock.patch('cinderlib.Backend._Backend__convert_option_to_string') + @mock.patch('cinderlib.Backend._validate_options') + @mock.patch.object(cfg, 'CONF') + def test__set_cinder_config(self, conf_mock, validate_mock): + cinder_cfg = {'debug': True} + + objects.Backend._set_cinder_config('host', 'locks_path', cinder_cfg) + + self.assertEqual(2, conf_mock.set_default.call_count) + conf_mock.set_default.assert_has_calls( + [mock.call('state_path', os.getcwd()), + mock.call('lock_path', '$state_path', 'oslo_concurrency')]) + + self.assertEqual(cinderlib.__version__, cfg.CONF.version) + + self.assertEqual('locks_path', cfg.CONF.oslo_concurrency.lock_path) + self.assertEqual('file://locks_path', + cfg.CONF.coordination.backend_url) + self.assertEqual('host', cfg.CONF.host) + + validate_mock.assert_called_once_with(cinder_cfg) + + self.assertEqual(True, cfg.CONF.debug) + + self.assertIsNone(cfg._CachedArgumentParser().parse_args()) + + @mock.patch('cinderlib.Backend._set_cinder_config') @mock.patch('urllib3.disable_warnings') @mock.patch('cinder.coordination.COORDINATOR') @mock.patch('cinderlib.Backend._set_priv_helper') @@ -84,13 +114,12 @@ class TestCinderlib(base.BaseTest): @mock.patch('cinderlib.Backend.set_persistence') def test_global_setup(self, mock_set_pers, mock_serial, mock_log, mock_sudo, mock_coord, mock_disable_warn, - mock_convert_to_str): - mock_convert_to_str.side_effect = lambda *args: args[1] + mock_set_config): cls = objects.Backend cls.global_initialization = False cinder_cfg = {'k': 'v', 'k2': 'v2'} - cls.global_setup('file_locks', + cls.global_setup(mock.sentinel.locks_path, mock.sentinel.root_helper, mock.sentinel.ssl_warnings, mock.sentinel.disable_logs, @@ -100,23 +129,21 @@ class TestCinderlib(base.BaseTest): mock.sentinel.user_id, mock.sentinel.pers_cfg, mock.sentinel.fail_missing_backend, - 'mock.sentinel.host', + mock.sentinel.host, **cinder_cfg) - self.assertEqual('file_locks', cfg.CONF.oslo_concurrency.lock_path) - self.assertEqual('file://file_locks', - cfg.CONF.coordination.backend_url) + mock_set_config.assert_called_once_with(mock.sentinel.host, + mock.sentinel.locks_path, + cinder_cfg) + self.assertEqual(mock.sentinel.fail_missing_backend, cls.fail_on_missing_backend) self.assertEqual(mock.sentinel.root_helper, cls.root_helper) self.assertEqual(mock.sentinel.project_id, cls.project_id) self.assertEqual(mock.sentinel.user_id, cls.user_id) self.assertEqual(mock.sentinel.non_uuid_ids, cls.non_uuid_ids) - self.assertEqual('mock.sentinel.host', cfg.CONF.host) mock_set_pers.assert_called_once_with(mock.sentinel.pers_cfg) - self.assertEqual(cinderlib.__version__, cfg.CONF.version) - mock_serial.setup.assert_called_once_with(cls) mock_log.assert_called_once_with(mock.sentinel.disable_logs) mock_sudo.assert_called_once_with(mock.sentinel.root_helper) @@ -127,6 +154,41 @@ class TestCinderlib(base.BaseTest): self.assertEqual(mock.sentinel.backend_info, cls.output_all_backend_info) + @mock.patch('cinderlib.cinderlib.LOG.warning') + def test__validate_options(self, warning_mock): + # Validate default group config with Boolean and MultiStrOpt + self.backend._validate_options( + {'debug': True, + 'osapi_volume_extension': ['a', 'b', 'c'], + }) + # Test driver options with String, ListOpt, PortOpt + self.backend._validate_options( + {'volume_driver': 'cinder.volume.drivers.lvm.LVMVolumeDriver', + 'volume_group': 'cinder-volumes', + 'iscsi_secondary_ip_addresses': ['w.x.y.z', 'a.b.c.d'], + 'target_port': 12345, + }) + + warning_mock.assert_not_called() + + @ddt.data( + ('debug', 'sure', None), + ('target_port', 'abc', 'cinder.volume.drivers.lvm.LVMVolumeDriver')) + @ddt.unpack + def test__validate_options_failures(self, option, value, driver): + self.assertRaises( + ValueError, + self.backend._validate_options, + {'volume_driver': driver, + option: value}) + + @mock.patch('cinderlib.cinderlib.LOG.warning') + def test__validate_options_unkown(self, warning_mock): + self.backend._validate_options( + {'volume_driver': 'cinder.volume.drivers.lvm.LVMVolumeDriver', + 'vmware_cluster_name': 'name'}) + self.assertEqual(1, warning_mock.call_count) + def test_pool_names(self): pool_names = [mock.sentinel._pool_names] self.backend._pool_names = pool_names @@ -254,32 +316,6 @@ class TestCinderlib(base.BaseTest): self.backend.refresh() self.persistence.get_volumes.assert_not_called() - def test___get_options_types(self): - # Before knowing the driver we don't have driver specific options. - opts = self.backend._Backend__get_options_types({}) - self.assertNotIn('volume_group', opts) - # But we do have the basic options - self.assertIn('volume_driver', opts) - - # When we know the driver the method can load it to retrieve its - # specific options. - cfg = {'volume_driver': 'cinder.volume.drivers.lvm.LVMVolumeDriver'} - opts = self.backend._Backend__get_options_types(cfg) - self.assertIn('volume_group', opts) - - def test___val_to_str_integer(self): - res = self.backend._Backend__val_to_str(1) - self.assertEqual('1', res) - - def test___val_to_str_list_tuple(self): - val = ['hola', 'hello'] - expected = '[hola,hello]' - res = self.backend._Backend__val_to_str(val) - self.assertEqual(expected, res) - # Same with a tuple - res = self.backend._Backend__val_to_str(tuple(val)) - self.assertEqual(expected, res) - @staticmethod def odict(*args): res = collections.OrderedDict() @@ -287,101 +323,16 @@ class TestCinderlib(base.BaseTest): res[args[i]] = args[i + 1] return res - def test___val_to_str_dict(self): - val = self.odict('k1', 'v1', 'k2', 2) - res = self.backend._Backend__val_to_str(val) - self.assertEqual('k1:v1,k2:2', res) + @mock.patch('cinderlib.cinderlib.cfg.CONF') + def test__apply_backend_workarounds(self, mock_conf): + cfg = mock.Mock(volume_driver='cinder.volume.drivers.netapp...') + self.backend._apply_backend_workarounds(cfg) + self.assertEqual(cfg.volume_backend_name, + mock_conf.list_all_sections()) - def test___val_to_str_recursive(self): - val = self.odict('k1', ['hola', 'hello'], 'k2', [1, 2]) - expected = 'k1:[hola,hello],k2:[1,2]' - res = self.backend._Backend__val_to_str(val) - self.assertEqual(expected, res) - - @mock.patch('cinderlib.Backend._Backend__val_to_str') - def test___convert_option_to_string_int(self, convert_mock): - PortType = types.Integer(1, 65535) - opt = cfg.Opt('port', type=PortType, default=9292, help='Port number') - opts = {'port': {'opt': opt, 'cli': False}} - res = self.backend._Backend__convert_option_to_string( - 'port', 12345, opts) - self.assertEqual(convert_mock.return_value, res) - convert_mock.assert_called_once_with(12345) - - @mock.patch('cinderlib.Backend._Backend__val_to_str') - def test___convert_option_to_string_listopt(self, convert_mock): - opt = cfg.ListOpt('my_opt', help='my cool option') - opts = {'my_opt': {'opt': opt, 'cli': False}} - res = self.backend._Backend__convert_option_to_string( - 'my_opt', [mock.sentinel.value], opts) - self.assertEqual(convert_mock.return_value, res) - convert_mock.assert_called_once_with([mock.sentinel.value]) - - # Same result if we don't pass a list, since method converts it - convert_mock.reset_mock() - res = self.backend._Backend__convert_option_to_string( - 'my_opt', mock.sentinel.value, opts) - self.assertEqual(convert_mock.return_value, res) - convert_mock.assert_called_once_with([mock.sentinel.value]) - - @mock.patch('cinderlib.Backend._Backend__val_to_str') - def test___convert_option_to_string_multistr_one(self, convert_mock): - convert_mock.side_effect = lambda x: x - opt = cfg.MultiStrOpt('my_opt', default=['default'], help='help') - opts = {'my_opt': {'opt': opt, 'cli': False}} - res = self.backend._Backend__convert_option_to_string( - 'my_opt', ['value1'], opts) - self.assertEqual('value1', res) - convert_mock.assert_called_once_with('value1') - - # Same result if we don't pass a list, since method converts it - convert_mock.reset_mock() - res = self.backend._Backend__convert_option_to_string( - 'my_opt', 'value1', opts) - self.assertEqual('value1', res) - convert_mock.assert_called_once_with('value1') - - @mock.patch('cinderlib.Backend._Backend__val_to_str') - def test___convert_option_to_string_multistr(self, convert_mock): - convert_mock.side_effect = lambda x: x - opt = cfg.MultiStrOpt('my_opt', default=['default'], help='help') - opts = {'my_opt': {'opt': opt, 'cli': False}} - res = self.backend._Backend__convert_option_to_string( - 'my_opt', ['value1', 'value2'], opts) - self.assertEqual('value1\nmy_opt = value2', res) - self.assertEqual(2, convert_mock.call_count) - convert_mock.assert_has_calls([mock.call('value1'), - mock.call('value2')]) - - def test___convert_option_to_string_multiopt(self): - opt = cfg.MultiOpt('my_opt', item_type=types.Dict()) - opts = {'my_opt': {'opt': opt, 'cli': False}} - elem1 = self.odict('v1_k1', 'v1_v1', 'v1_k2', 'v1_v2') - elem2 = self.odict('v2_k1', 'v2_v1', 'v2_k2', 'v2_v2') - res = self.backend._Backend__convert_option_to_string( - 'my_opt', [elem1, elem2], opts) - expect = 'v1_k1:v1_v1,v1_k2:v1_v2\nmy_opt = v2_k1:v2_v1,v2_k2:v2_v2' - self.assertEqual(expect, res) - - @mock.patch('cinderlib.Backend._parser') - @mock.patch('cinderlib.Backend._Backend__convert_option_to_string') - @mock.patch('cinderlib.Backend._Backend__get_options_types') - def test___set_parser_kv(self, types_mock, convert_mock, parser_mock): - convert_mock.side_effect = [mock.sentinel.res1, mock.sentinel.res2] - section = mock.sentinel.section - kvs = self.odict('k1', 1, 'k2', 2) - - self.backend._Backend__set_parser_kv(kvs, section) - - types_mock.assert_called_once_with(kvs) - self.assertEqual(2, convert_mock.call_count) - convert_mock.assert_has_calls( - [mock.call('k1', 1, types_mock.return_value), - mock.call('k2', 2, types_mock.return_value)], - any_order=True) - - self.assertEqual(2, parser_mock.set.call_count) - parser_mock.set.assert_has_calls( - [mock.call(section, 'k1', mock.sentinel.res1), - mock.call(section, 'k2', mock.sentinel.res2)], - any_order=True) + @mock.patch('cinderlib.cinderlib.cfg.CONF') + def test__apply_backend_workarounds_do_nothing(self, mock_conf): + cfg = mock.Mock(volume_driver='cinder.volume.drivers.lvm...') + self.backend._apply_backend_workarounds(cfg) + self.assertEqual(mock_conf.list_all_sections.return_value, + mock_conf.list_all_sections()) diff --git a/doc/source/validated.rst b/doc/source/validated.rst index 5a69606..1f7da4e 100644 --- a/doc/source/validated.rst +++ b/doc/source/validated.rst @@ -223,12 +223,12 @@ VMAX hpe3par_api_url: https://w.x.y.z:8080/api/v1 hpe3par_username: user hpe3par_password: toomanysecrets - hpe3par_cpg: CPG_name + hpe3par_cpg: [CPG_name] san_ip: w.x.y.z san_login: user san_password: toomanysecrets volume_driver: cinder.volume.drivers.hpe.hpe_3par_iscsi.HPE3PARISCSIDriver - hpe3par_iscsi_ips: w.x.y2.z2,w.x.y2.z3,w.x.y2.z4,w.x.y2.z4 + hpe3par_iscsi_ips: [w.x.y2.z2,w.x.y2.z3,w.x.y2.z4,w.x.y2.z4] hpe3par_debug: false hpe3par_iscsi_chap_enabled: false hpe3par_snapshot_retention: 0