From 11b678805e857f8b328605888e29f466858a5756 Mon Sep 17 00:00:00 2001 From: Yuriy Taraday Date: Wed, 7 Sep 2016 14:06:13 +0300 Subject: [PATCH] Switch main config object to be result of YAML parsing We still use oslo.config for CLI parsing and default values, so added copying of all values from oslo.config to YAML dict. To keep easy access, added AttrDict class that allows to access config dict via attribute access. Created a new fixture for tests that loads default values from oslo.config to reuse current definition. Change-Id: I921830890463f295f9e6a2467d6bc15504742e1b --- fuel_ccp/build.py | 2 +- fuel_ccp/config/__init__.py | 54 ++++++++++++++++++++------- fuel_ccp/config/_yaml.py | 55 ++++++++++++++++++++++++++++ fuel_ccp/tests/base.py | 7 +++- fuel_ccp/tests/conf_fixture.py | 13 +++++++ fuel_ccp/tests/test_build.py | 7 +--- fuel_ccp/tests/test_config.py | 61 +++++++++++++++++++++++++++++-- fuel_ccp/tests/test_fetch.py | 4 +- fuel_ccp/tests/test_kubernetes.py | 24 +++--------- 9 files changed, 181 insertions(+), 46 deletions(-) create mode 100644 fuel_ccp/config/_yaml.py create mode 100644 fuel_ccp/tests/conf_fixture.py diff --git a/fuel_ccp/build.py b/fuel_ccp/build.py index 32136aaa..dc2fa14e 100644 --- a/fuel_ccp/build.py +++ b/fuel_ccp/build.py @@ -301,7 +301,7 @@ def wait_futures(future_list, skip_errors=False): def _get_config(): - cfg = {'render': dict(CONF.images.items())} + cfg = {'render': dict(CONF.images._items())} if CONF.registry.address: cfg['render']['namespace'] = '%s/%s' % ( CONF.registry.address, cfg['render']['namespace']) diff --git a/fuel_ccp/config/__init__.py b/fuel_ccp/config/__init__.py index 309dcb81..5541a463 100644 --- a/fuel_ccp/config/__init__.py +++ b/fuel_ccp/config/__init__.py @@ -4,35 +4,41 @@ import os from oslo_config import cfg from oslo_log import log import six -import yaml + +from fuel_ccp.config import _yaml LOG = log.getLogger(__name__) -CONF = cfg.CONF -CONF.import_group('builder', 'fuel_ccp.config.builder') -CONF.import_opt("action", "fuel_ccp.config.cli") -CONF.import_opt("deploy_config", "fuel_ccp.config.cli") -CONF.import_group('images', 'fuel_ccp.config.images') -CONF.import_group('kubernetes', 'fuel_ccp.config.kubernetes') -CONF.import_group('registry', 'fuel_ccp.config.registry') -CONF.import_group('repositories', 'fuel_ccp.config.repositories') +cfg.CONF.import_group('builder', 'fuel_ccp.config.builder') +cfg.CONF.import_opt("action", "fuel_ccp.config.cli") +cfg.CONF.import_opt("deploy_config", "fuel_ccp.config.cli") +cfg.CONF.import_group('images', 'fuel_ccp.config.images') +cfg.CONF.import_group('kubernetes', 'fuel_ccp.config.kubernetes') +cfg.CONF.import_group('registry', 'fuel_ccp.config.registry') +cfg.CONF.import_group('repositories', 'fuel_ccp.config.repositories') + +_REAL_CONF = None def setup_config(): config_file, args = get_cli_config() if config_file is None: config_file = find_config() - log.register_options(CONF) + log.register_options(cfg.CONF) if config_file: yconf = parse_config(config_file) set_oslo_defaults(cfg.CONF, yconf) # Don't let oslo.config parse any config files - CONF(args, project='ccp', default_config_files=[]) - log.setup(CONF, 'fuel-ccp') + cfg.CONF(args, project='ccp', default_config_files=[]) + log.setup(cfg.CONF, 'fuel-ccp') if config_file: LOG.debug('Loaded config from file %s', config_file) else: LOG.debug('No config file loaded') + yconf = _yaml.AttrDict() + copy_values_from_oslo(cfg.CONF, yconf) + global _REAL_CONF + _REAL_CONF = yconf def get_cli_config(): @@ -66,7 +72,7 @@ def find_config(): def parse_config(config_file): with open(config_file) as f: - return yaml.load(f) + return _yaml.load(f) def set_oslo_defaults(oconf, yconf): @@ -78,7 +84,27 @@ def set_oslo_defaults(oconf, yconf): except KeyError: continue if isinstance(value, cfg.ConfigOpts.GroupAttr): - for subkey, subvalue in six.iteritems(yconf_value): + for subkey, subvalue in yconf_value._items(): oconf.set_default(group=key, name=subkey, default=subvalue) else: oconf.set_default(group=None, name=key, default=yconf_value) + + +def copy_values_from_oslo(oconf, yconf): + for key, value in six.iteritems(oconf): + if isinstance(value, cfg.ConfigOpts.GroupAttr): + try: + yconf_value = yconf[key] + except KeyError: + yconf[key] = _yaml.AttrDict(value) + else: + yconf_value._update(value) + else: + yconf[key] = value + + +class _Wrapper(object): + def __getattr__(self, name): + return getattr(_REAL_CONF, name) + +CONF = _Wrapper() diff --git a/fuel_ccp/config/_yaml.py b/fuel_ccp/config/_yaml.py new file mode 100644 index 00000000..a67054d7 --- /dev/null +++ b/fuel_ccp/config/_yaml.py @@ -0,0 +1,55 @@ +import six +import yaml + + +# NOTE(yorik-sar): Don't implement full dict interface to avoid name conflicts +class AttrDict(object): + def __init__(self, *args, **kwargs): + self._dict = dict(*args, **kwargs) + + def __getattr__(self, name): + try: + return self._dict[name] + except KeyError: + raise AttributeError(name) + + def __getitem__(self, name): + return self._dict[name] + + def __setitem__(self, name, value): + self._dict[name] = value + + def _update(self, *args, **kwargs): + self._dict.update(*args, **kwargs) + + def _items(self): + return six.iteritems(self._dict) + + def __eq__(self, other): + if isinstance(other, AttrDict): + return self._dict == other._dict + elif isinstance(other, dict): + return self._dict == other + else: + return NotImplemented + + def __repr__(self): + return 'AttrDict({})'.format(self._dict) + + +class Loader(yaml.SafeLoader): + pass + + +def construct_mapping(loader, node): + loader.flatten_mapping(node) + return AttrDict(loader.construct_pairs(node)) + +Loader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, + construct_mapping, +) + + +def load(stream): + return yaml.load(stream, Loader=Loader) diff --git a/fuel_ccp/tests/base.py b/fuel_ccp/tests/base.py index 1c30cdb5..0e795e8c 100644 --- a/fuel_ccp/tests/base.py +++ b/fuel_ccp/tests/base.py @@ -17,7 +17,12 @@ from oslotest import base +from fuel_ccp.tests import conf_fixture + class TestCase(base.BaseTestCase): - """Test case base class for all unit tests.""" + + def setUp(self): + super(TestCase, self).setUp() + self.conf = self.useFixture(conf_fixture.Config()).conf diff --git a/fuel_ccp/tests/conf_fixture.py b/fuel_ccp/tests/conf_fixture.py new file mode 100644 index 00000000..a00fef4d --- /dev/null +++ b/fuel_ccp/tests/conf_fixture.py @@ -0,0 +1,13 @@ +import fixtures +from oslo_config import cfg + +from fuel_ccp import config +from fuel_ccp.config import _yaml + + +class Config(fixtures.Fixture): + def _setUp(self): + self.conf = _yaml.AttrDict() + config.copy_values_from_oslo(cfg.CONF, self.conf) + self.useFixture( + fixtures.MockPatchObject(config, '_REAL_CONF', new=self.conf)) diff --git a/fuel_ccp/tests/test_build.py b/fuel_ccp/tests/test_build.py index 2fc63af9..a3d0e72c 100644 --- a/fuel_ccp/tests/test_build.py +++ b/fuel_ccp/tests/test_build.py @@ -2,7 +2,6 @@ import collections import io import mock -from oslo_config import fixture as conf_fixture from fuel_ccp import build from fuel_ccp.tests import base @@ -25,10 +24,6 @@ RUN apt-get -y install \ class TestBuild(base.TestCase): - def setUp(self): - super(TestBuild, self).setUp() - self.cfg = self.useFixture(conf_fixture.Config()) - @staticmethod def __create_dockerfile_objects(): return collections.OrderedDict([ @@ -168,7 +163,7 @@ class TestBuild(base.TestCase): } } - self.cfg.config(group="builder", keep_image_tree_consistency=False) + self.conf["builder"]["keep_image_tree_consistency"] = False for dockerfile in dockerfiles.values(): if dockerfile['parent']: diff --git a/fuel_ccp/tests/test_config.py b/fuel_ccp/tests/test_config.py index b3e90812..bdf9aabd 100644 --- a/fuel_ccp/tests/test_config.py +++ b/fuel_ccp/tests/test_config.py @@ -3,10 +3,11 @@ import functools import fixtures from oslo_config import cfg -from oslo_config import fixture as conf_fixture +import six import testscenarios from fuel_ccp import config +from fuel_ccp.config import _yaml from fuel_ccp.tests import base @@ -42,6 +43,16 @@ class TestGetCLIConfig(testscenarios.WithScenarios, base.TestCase): self.assertEqual(result, self.expected_result) +def nested_dict_to_attrdict(d): + if isinstance(d, dict): + return _yaml.AttrDict({k: nested_dict_to_attrdict(v) + for k, v in six.iteritems(d)}) + elif isinstance(d, list): + return list(map(nested_dict_to_attrdict, d)) + else: + return d + + class TestSetOsloDefaults(testscenarios.WithScenarios, base.TestCase): scenarios = [ ('empty', {'yconf': {}, 'expected_defaults': {}}), @@ -64,7 +75,6 @@ class TestSetOsloDefaults(testscenarios.WithScenarios, base.TestCase): self.conf = cfg.ConfigOpts() self.conf.register_opt(cfg.BoolOpt('debug', default=False)) self.conf.register_opt(cfg.IntOpt('count'), group='thegroup') - self.useFixture(conf_fixture.Config(self.conf)) def get_defaults(self): res = collections.defaultdict( @@ -80,5 +90,50 @@ class TestSetOsloDefaults(testscenarios.WithScenarios, base.TestCase): return res def test_set_oslo_defaults(self): - config.set_oslo_defaults(self.conf, self.yconf) + yconf = nested_dict_to_attrdict(self.yconf) + config.set_oslo_defaults(self.conf, yconf) self.assertEqual(self.get_defaults(), self.expected_defaults) + + +class TestCopyValuesFromOslo(testscenarios.WithScenarios, base.TestCase): + scenarios = [ + ('simple', { + 'yconf': {}, + 'oconf': {None: {'debug': True}}, + 'expected_result': {'debug': True, 'thegroup': {'count': None}}, + }), + ('overwrite', { + 'yconf': {'debug': False}, + 'oconf': {None: {'debug': True}}, + 'expected_result': {'debug': True, 'thegroup': {'count': None}}, + }), + ('deep', { + 'yconf': {'debug': False}, + 'oconf': {'thegroup': {'count': 3}}, + 'expected_result': {'debug': False, 'thegroup': {'count': 3}}, + }), + ('deep_overwrite_with_bogus', { + 'yconf': {'thegroup': {'bogus': 'value'}, 'other': 1}, + 'oconf': {'thegroup': {'count': 3}}, + 'expected_result': { + 'debug': False, + 'thegroup': {'count': 3, 'bogus': 'value'}, + 'other': 1, + }, + }), + ] + + yconf = None + oconf = None + expected_result = None + + def test_copy_values_from_oslo(self): + conf = cfg.ConfigOpts() + conf.register_opt(cfg.BoolOpt('debug', default=False)) + conf.register_opt(cfg.IntOpt('count'), group='thegroup') + for group, values in six.iteritems(self.oconf): + for key, value in six.iteritems(values): + conf.set_default(group=group, name=key, default=value) + yconf = nested_dict_to_attrdict(self.yconf) + config.copy_values_from_oslo(conf, yconf) + self.assertEqual(yconf, self.expected_result) diff --git a/fuel_ccp/tests/test_fetch.py b/fuel_ccp/tests/test_fetch.py index 616b9739..100dbf07 100644 --- a/fuel_ccp/tests/test_fetch.py +++ b/fuel_ccp/tests/test_fetch.py @@ -2,7 +2,6 @@ import os import fixtures import mock -from oslo_config import fixture as conf_fixture from fuel_ccp import fetch from fuel_ccp.tests import base @@ -17,8 +16,7 @@ class TestFetch(base.TestCase): tmp_dir = fixtures.TempDir() tmp_dir.setUp() self.tmp_path = tmp_dir.path - conf = self.useFixture(conf_fixture.Config()) - conf.config(path=self.tmp_path, group='repositories') + self.conf['repositories']['path'] = self.tmp_path # Create temporary directory for openstack-base to not clone it os.mkdir(os.path.join(self.tmp_path, 'ms-openstack-base')) diff --git a/fuel_ccp/tests/test_kubernetes.py b/fuel_ccp/tests/test_kubernetes.py index e1a991d0..f54a0dc7 100644 --- a/fuel_ccp/tests/test_kubernetes.py +++ b/fuel_ccp/tests/test_kubernetes.py @@ -1,20 +1,13 @@ import mock -from oslo_config import fixture as conf_fixture - from fuel_ccp import kubernetes from fuel_ccp.tests import base class TestKubernetes(base.TestCase): - def setUp(self): - super(TestKubernetes, self).setUp() - self.conf = self.useFixture(conf_fixture.Config()) - @mock.patch('k8sclient.client.api_client.ApiClient') def test_get_client_with_conf(self, api_client): - self.conf.config( - group='kubernetes', + self.conf['kubernetes']._update( key_file='test.key', ca_certs='ca.crt', cert_file='test.cert', @@ -27,8 +20,7 @@ class TestKubernetes(base.TestCase): @mock.patch('k8sclient.client.api_client.ApiClient') def test_get_client(self, api_client): - self.conf.config( - group='kubernetes', + self.conf['kubernetes']._update( key_file='test.key', ca_certs='ca.crt', cert_file='test.cert', @@ -44,10 +36,8 @@ class TestKubernetes(base.TestCase): @mock.patch( 'k8sclient.client.apis.apisextensionsvbeta_api.ApisextensionsvbetaApi') def test_create_deployment(self, api_beta): - # NOTE(yorik-sar): can't use self.conf.config() with 'action', but - # fixture will clear it up anyway - self.conf.conf.action.dry_run = False - self.conf.conf.action.export_dir = False + self.conf.action.dry_run = False + self.conf.action.export_dir = False api = mock.Mock() api.create_namespaced_deployment = mock.Mock() api_beta.return_value = api @@ -60,10 +50,8 @@ class TestKubernetes(base.TestCase): @mock.patch('k8sclient.client.apis.apiv_api.ApivApi') def test_create_service(self, api_v1): - # NOTE(yorik-sar): can't use self.conf.config() with 'action', but - # fixture will clear it up anyway - self.conf.conf.action.dry_run = False - self.conf.conf.action.export_dir = False + self.conf.action.dry_run = False + self.conf.action.export_dir = False api = mock.Mock() api.create_namespaced_service = mock.Mock() api_v1.return_value = api