Get rid of ConfigParser code in ProviderConfiguration

We may be unable to get rid of this code, but at least we can
make it slimmer and more oslo_config friendly.

This patch also fixes an issue where plain ConfigParser cannot
handle config options with the same name under same section.

Related-bug: #1492069

Change-Id: I596613eb7b2fc3e894a3c1cc7b7cb24d7137cbc5

diff --git a/neutron/services/provider_configuration.py b/neutron/services/provider_configuration.py
index 9bbf824d6a97579eb2e1846b5ba430ad212b5b2e..fb19552d07f037f8978bf32126ecde6c2eb3f3e5 100644 (file)
--- a/neutron/services/provider_configuration.py
+++ b/neutron/services/provider_configuration.py
@@ -18,7 +18,6 @@ import os
 
 from oslo_config import cfg
 from oslo_log import log as logging
-from six.moves import configparser as ConfigParser
 import stevedore
 
 from neutron.common import exceptions as n_exc
@@ -62,23 +61,21 @@ class NeutronModule(object):
         return self.repo['mod']
 
     # Return an INI parser for the child module
-    def ini(self):
+    def ini(self, neutron_dir=None):
         if self.repo['ini'] is None:
-            neutron_dir = None
             try:
-                neutron_dir = cfg.CONF.config_dir
+                neutron_dir = neutron_dir or cfg.CONF.config_dir
             except cfg.NoSuchOptError:
                 pass
-
             if neutron_dir is None:
                 neutron_dir = '/etc/neutron'
 
-            ini = ConfigParser.SafeConfigParser()
+            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.read(ini_path)
-
-            self.repo['ini'] = ini
+                ini_file(['--config-file', ini_path])
+            self.repo['ini'] = ini_file
 
         return self.repo['ini']
 
@@ -102,13 +99,7 @@ class NeutronModule(object):
         # this may be necessary, if modules are loaded on the fly
         # (DevStack may be an example)
         if not providers:
-            ini = self.ini()
-            try:
-                for name, value in ini.items('service_providers'):
-                    if name == 'service_provider':
-                        providers.append(value)
-            except ConfigParser.NoSectionError:
-                pass
+            providers = self.ini().service_providers.service_provider
 
         return providers
 

diff --git a/neutron/tests/etc/neutron_test.conf b/neutron/tests/etc/neutron_test.conf
new file mode 100644 (file)
index 0000000..cd4110d
--- /dev/null
+++ b/neutron/tests/etc/neutron_test.conf
@@ -0,0 +1,3 @@
+[service_providers]
+service_provider=foo
+service_provider=bar

diff --git a/neutron/tests/unit/services/test_provider_configuration.py b/neutron/tests/unit/services/test_provider_configuration.py
index 1930c278beffd8c2a7bc29c3d85fe04824630f95..21531a41a90d5856d1df87d48f127197e65cb707 100644 (file)
--- a/neutron/tests/unit/services/test_provider_configuration.py
+++ b/neutron/tests/unit/services/test_provider_configuration.py
@@ -203,3 +203,12 @@ class GetProviderDriverClassTestCase(base.BaseTestCase):
     def test_get_provider_driver_class_miss(self):
         retval = provconf.get_provider_driver_class('foo')
         self.assertEqual('foo', retval)
+
+
+class NeutronModuleTestCase(base.BaseTestCase):
+
+    def test_can_parse_multi_opt_service_provider_from_conf_file(self):
+        mod = provconf.NeutronModule('neutron_test')
+        mod.ini(base.ETCDIR)
+        self.assertEqual(['foo', 'bar'], mod.service_providers(),
+                         'Expected two providers, only one read')