From 94bed0ece28fd32117bc6c5b219d9bddec273fb9 Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Thu, 28 Feb 2019 11:31:47 +0100 Subject: [PATCH] Refactor reading kolla config in image builder Now we read config file(s) only once Change-Id: I46893e83cb1d354a17d14ceaf264071823eeb0a3 --- tripleoclient/tests/test_utils.py | 14 +++++------- .../tests/v1/test_container_image.py | 16 +++++++++----- tripleoclient/utils.py | 19 ++++++++++------ tripleoclient/v1/container_image.py | 22 +++++++------------ 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index d4626ae85..6e86eab3f 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -1372,6 +1372,7 @@ class TestCheckEnvForProxy(TestCase): class TestConfigParser(TestCase): + def setUp(self): self.tmp_dir = tempfile.mkdtemp() @@ -1381,14 +1382,10 @@ class TestConfigParser(TestCase): self.tmp_dir = None def test_get_config_value(self): - cfile, cfile_name = tempfile.mkstemp(dir=self.tmp_dir) - cfp = open(cfile_name, 'w') cfg = ConfigParser() cfg.add_section('foo') cfg.set('foo', 'bar', 'baz') - cfg.write(cfp) - cfp.close() - config = utils.get_config_value(cfile_name, 'foo', 'bar') + config = utils.get_from_cfg(cfg, 'bar', 'foo') self.assertEqual(config, 'baz') def test_get_config_value_multiple_files(self): @@ -1403,13 +1400,14 @@ class TestConfigParser(TestCase): cfg.set('foo', 'bar', 'boop') with open(cfile2_name, 'w') as fp: cfg.write(fp) - config = utils.get_config_value(cfiles, 'foo', 'bar') + cfgs = utils.get_read_config(cfiles) + config = utils.get_from_cfg(cfgs, 'bar', 'foo') self.assertEqual(config, 'boop') def test_get_config_value_bad_file(self): self.assertRaises(exceptions.NotFound, - utils.get_config_value, - 'does-not-exist', 'foo', 'bar') + utils.get_from_cfg, + 'does-not-exist', 'bar', 'foo') class TestAnsibleSymlink(TestCase): diff --git a/tripleoclient/tests/v1/test_container_image.py b/tripleoclient/tests/v1/test_container_image.py index 9a3af7370..8d7a95bf3 100644 --- a/tripleoclient/tests/v1/test_container_image.py +++ b/tripleoclient/tests/v1/test_container_image.py @@ -507,13 +507,16 @@ class TestContainerImageBuild(TestPluginV1): @mock.patch('os.fdopen', autospec=True) @mock.patch('tempfile.mkdtemp') @mock.patch('tempfile.mkstemp') - @mock.patch('tripleoclient.v1.container_image.BuildImage.kolla_cfg') + @mock.patch( + 'tripleoclient.utils.get_from_cfg') @mock.patch('tripleo_common.image.builder.buildah.BuildahBuilder', autospec=True) @mock.patch('tripleo_common.image.kolla_builder.KollaImageBuilder', autospec=True) + @mock.patch('tripleoclient.utils.get_read_config') @mock.patch('os.remove') def test_container_image_build_with_buildah(self, mock_remove, + mock_read_conf, mock_builder, mock_buildah, mock_kolla_cfg, mock_mkstemp, mock_mkdtemp, mock_fdopen): @@ -539,12 +542,13 @@ class TestContainerImageBuild(TestPluginV1): cfg_files = list(parsed_args.kolla_config_files) cfg_files.append('/tmp/whatever_file') + mock_read_conf.assert_any_call(cfg_files) cfg_calls = [ - mock.call(cfg_files, 'base'), - mock.call(cfg_files, 'type'), - mock.call(cfg_files, 'tag'), - mock.call(cfg_files, 'namespace'), - mock.call(cfg_files, 'registry'), + mock.call(mock_read_conf.return_value, 'base'), + mock.call(mock_read_conf.return_value, 'type'), + mock.call(mock_read_conf.return_value, 'tag'), + mock.call(mock_read_conf.return_value, 'namespace'), + mock.call(mock_read_conf.return_value, 'registry'), ] mock_kolla_cfg.assert_has_calls(cfg_calls) mock_bb.build_all.assert_called_once() diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index bd4a09b5d..3f26fe836 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1541,17 +1541,22 @@ def check_env_for_proxy(no_proxy_hosts=None): raise RuntimeError(message) -def get_config_value(cfg, section, option): - """Return the value of an option in ini config file(s)""" +def get_read_config(cfg): + """Return the config read from ini config file(s)""" config = ConfigParser() config.read(cfg) + return config + + +def get_from_cfg(cfg, param, section="DEFAULT"): + """Return a parameter from Kolla config""" try: - val = config.get(section, option) + val = cfg.get(section, param) except Exception: - raise exceptions.NotFound(_('Unable to find {section}/{option} in ' - '{config}').format(section=section, - option=option, - config=cfg)) + raise exceptions.NotFound(_("Unable to find {section}/{option} in " + "{config}").format(section=param, + option=section, + config=cfg)) return val diff --git a/tripleoclient/v1/container_image.py b/tripleoclient/v1/container_image.py index 23644e968..43ea0dcc9 100644 --- a/tripleoclient/v1/container_image.py +++ b/tripleoclient/v1/container_image.py @@ -181,11 +181,6 @@ class BuildImage(command.Command): ) return parser - @staticmethod - def kolla_cfg(cfg, param): - """Return a parameter from Kolla config""" - return utils.get_config_value(cfg, 'DEFAULT', param) - def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) @@ -211,15 +206,14 @@ class BuildImage(command.Command): if parsed_args.use_buildah: deps = json.loads(result) - cfg = kolla_config_files - bb = buildah.BuildahBuilder(kolla_tmp_dir, deps, - BuildImage.kolla_cfg(cfg, 'base'), - BuildImage.kolla_cfg(cfg, 'type'), - BuildImage.kolla_cfg(cfg, 'tag'), - BuildImage.kolla_cfg(cfg, - 'namespace'), - BuildImage.kolla_cfg(cfg, - 'registry')) + kolla_cfg = utils.get_read_config(kolla_config_files) + bb = buildah.BuildahBuilder( + kolla_tmp_dir, deps, + utils.get_from_cfg(kolla_cfg, "base"), + utils.get_from_cfg(kolla_cfg, "type"), + utils.get_from_cfg(kolla_cfg, "tag"), + utils.get_from_cfg(kolla_cfg, "namespace"), + utils.get_from_cfg(kolla_cfg, "registry")) bb.build_all() elif parsed_args.list_dependencies: deps = json.loads(result)