diff --git a/cinderlib/cinderlib.py b/cinderlib/cinderlib.py index 9a4766e..a0322a7 100644 --- a/cinderlib/cinderlib.py +++ b/cinderlib/cinderlib.py @@ -250,24 +250,94 @@ class Backend(object): 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.""" + # Dynamically loading the driver triggers adding the specific + # configuration options to the backend_defaults section + if 'volume_driver' in kvs: + driver_ns = kvs['volume_driver'].rsplit('.', 1)[0] + __import__(driver_ns) + opts = configuration.CONF._groups['backend_defaults']._opts + return opts + + @classmethod + def __val_to_str(cls, val): + """Convert an oslo config value to its string representation. + + 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): - for key, val in kvs.items(): - # We receive list or tuple for multiopt and ConfigParser doesn't - # support repeating the same entry multiple times, so we hack our - # way around it - if isinstance(val, (list, tuple)): - 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:])) + """Set Oslo configuration options in our parser. - if not isinstance(val, six.string_types): - val = six.text_type(val) - cls._parser.set(section, key, val) + 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): backend_name = driver_cfg['volume_backend_name'] diff --git a/cinderlib/tests/unit/test_cinderlib.py b/cinderlib/tests/unit/test_cinderlib.py index 8ba4d28..9434a8d 100644 --- a/cinderlib/tests/unit/test_cinderlib.py +++ b/cinderlib/tests/unit/test_cinderlib.py @@ -13,8 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +import collections + import mock from oslo_config import cfg +from oslo_config import types import cinderlib from cinderlib import objects @@ -72,6 +75,7 @@ class TestCinderlib(base.BaseTest): driver.get_volume_stats.assert_not_called() self.assertEqual(('default',), backend.pool_names) + @mock.patch('cinderlib.Backend._Backend__convert_option_to_string') @mock.patch('urllib3.disable_warnings') @mock.patch('cinder.coordination.COORDINATOR') @mock.patch('cinderlib.Backend._set_priv_helper') @@ -79,7 +83,9 @@ class TestCinderlib(base.BaseTest): @mock.patch('cinderlib.cinderlib.serialization') @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_sudo, mock_coord, mock_disable_warn, + mock_convert_to_str): + mock_convert_to_str.side_effect = lambda *args: args[1] cls = objects.Backend cls.global_initialization = False cinder_cfg = {'k': 'v', 'k2': 'v2'} @@ -247,3 +253,135 @@ class TestCinderlib(base.BaseTest): self.backend._volumes = None 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() + for i in range(0, len(args), 2): + 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) + + 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) diff --git a/releasenotes/notes/support-comple-config-options-39ba50c6c9165cc3.yaml b/releasenotes/notes/support-comple-config-options-39ba50c6c9165cc3.yaml new file mode 100644 index 0000000..95a8c0f --- /dev/null +++ b/releasenotes/notes/support-comple-config-options-39ba50c6c9165cc3.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Bug #1854188: Work with complex configuration options like ListOpt, + DictOpt, and MultiOpt with dict items.