From 7f31ccb7bbe0f78a34d704c59d0562ea10029893 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Tue, 24 May 2016 10:42:10 +0200 Subject: [PATCH] Adopt to config_dir option being a list and not a string Since oslo.config 3.8.0 (that included Ibd0566f11df62da031afb128c9687c5e8c7b27ae), config_dir option is a list, not a string. While our custom provider configuration parser for multistring options assumes the latter. It makes all installations that 1) pass at least one --config-dir option in CLI and 2) enable any service plugin that relies on provider definitions, to fail to start neutron-server. For example, this affects any RDO Mitaka installation with *aas service plugins enabled. Since Newton requires >=3.9.0, we are fine to switch to the list type without any code to support backwards compatibility with older option type. For Mitaka backport, we will need to handle both cases. Change-Id: I10e399a852d9fba0fd1aea79a10e2e7c906e4b3c Closes-Bug: #1585102 --- neutron/services/provider_configuration.py | 35 +++++++---- neutron/tests/etc/neutron_test2.conf.example | 2 + .../services/test_provider_configuration.py | 58 +++++++++++++++++++ 3 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 neutron/tests/etc/neutron_test2.conf.example diff --git a/neutron/services/provider_configuration.py b/neutron/services/provider_configuration.py index 9420660d611..6fafb74cab4 100644 --- a/neutron/services/provider_configuration.py +++ b/neutron/services/provider_configuration.py @@ -14,6 +14,7 @@ # under the License. import importlib +import itertools import os from neutron_lib import exceptions as n_exc @@ -64,18 +65,32 @@ class NeutronModule(object): # Return an INI parser for the child module def ini(self, neutron_dir=None): if self.repo['ini'] is None: - try: - neutron_dir = neutron_dir or cfg.CONF.config_dir - except cfg.NoSuchOptError: - pass - if neutron_dir is None: - neutron_dir = '/etc/neutron' - ini_file = cfg.ConfigOpts() ini_file.register_opts(serviceprovider_opts, 'service_providers') - ini_path = os.path.join(neutron_dir, '%s.conf' % self.module_name) - if os.path.exists(ini_path): - ini_file(['--config-file', ini_path]) + + if neutron_dir is not None: + neutron_dirs = [neutron_dir] + else: + neutron_dirs = cfg.CONF.config_dirs or ['/etc/neutron'] + + # load configuration from all matching files to reflect oslo.config + # behaviour + config_files = [] + for neutron_dir in neutron_dirs: + ini_path = os.path.join(neutron_dir, + '%s.conf' % self.module_name) + if os.path.exists(ini_path): + config_files.append(ini_path) + + # NOTE(ihrachys): we could pass project=self.module_name instead to + # rely on oslo.config to find configuration files for us, but: + # 1. that would render neutron_dir argument ineffective; + # 2. that would break loading configuration file from under + # /etc/neutron in case no --config-dir is passed. + # That's why we need to explicitly construct CLI here. + ini_file(args=list(itertools.chain.from_iterable( + ['--config-file', file_] for file_ in config_files + ))) self.repo['ini'] = ini_file return self.repo['ini'] diff --git a/neutron/tests/etc/neutron_test2.conf.example b/neutron/tests/etc/neutron_test2.conf.example new file mode 100644 index 00000000000..a0470e1e31f --- /dev/null +++ b/neutron/tests/etc/neutron_test2.conf.example @@ -0,0 +1,2 @@ +[service_providers] +service_provider=zzz diff --git a/neutron/tests/unit/services/test_provider_configuration.py b/neutron/tests/unit/services/test_provider_configuration.py index bb076fcbef2..e1d35905c22 100644 --- a/neutron/tests/unit/services/test_provider_configuration.py +++ b/neutron/tests/unit/services/test_provider_configuration.py @@ -12,6 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. +import os +import shutil + import mock from neutron_lib import exceptions as n_exc from oslo_config import cfg @@ -211,3 +214,58 @@ class NeutronModuleTestCase(base.BaseTestCase): mod.ini(base.ETCDIR) self.assertEqual(['foo', 'bar'], mod.service_providers(), 'Expected two providers, only one read') + + +class NeutronModuleConfigDirTestCase(base.BaseTestCase): + + def setup_config(self): + self.config_parse(args=['--config-dir', base.ETCDIR]) + + def test_can_parse_multi_opt_service_provider_from_conf_dir(self): + mod = provconf.NeutronModule('neutron_test') + mod.ini() + self.assertEqual(['foo', 'bar'], mod.service_providers()) + + +class NeutronModuleMultiConfigDirTestCase(base.BaseTestCase): + + def setUp(self): + self.tmpdir = self.get_default_temp_dir().path + shutil.copyfile( + os.path.join(base.ETCDIR, 'neutron_test2.conf.example'), + os.path.join(self.tmpdir, 'neutron_test.conf')) + super(NeutronModuleMultiConfigDirTestCase, self).setUp() + + def setup_config(self): + self.config_parse(args=[ + # NOTE(ihrachys): we expect the second directory to be checked + '--config-dir', self.tmpdir, '--config-dir', base.ETCDIR + ]) + + def test_read_configuration_from_all_matching_files(self): + mod = provconf.NeutronModule('neutron_test') + mod.ini() + self.assertEqual(['zzz', 'foo', 'bar'], mod.service_providers()) + + +class NeutronModuleMultiConfigFileTestCase(base.BaseTestCase): + + def setUp(self): + self.tmpdir = self.get_default_temp_dir().path + self.filepath1 = os.path.join(self.tmpdir, 'neutron_test.conf') + self.filepath2 = os.path.join(base.ETCDIR, 'neutron_test.conf') + shutil.copyfile( + os.path.join(base.ETCDIR, 'neutron_test2.conf.example'), + self.filepath1) + super(NeutronModuleMultiConfigFileTestCase, self).setUp() + + def setup_config(self): + self.config_parse(args=[ + # NOTE(ihrachys): we expect both directories to be checked + '--config-file', self.filepath1, '--config-file', self.filepath2 + ]) + + def test_read_configuration_from_all_matching_files(self): + mod = provconf.NeutronModule('neutron_test') + mod.ini() + self.assertEqual(['zzz', 'foo', 'bar'], mod.service_providers())