Use cfg's new global CONF object

Implements blueprint cfg-global-object

Change-Id: Ic53b41dafa8666ce21f33697f7e8697f1e5cb0fd

11 files changed, 193 insertions, 139 deletions

diff --git a/bin/keystone-all b/bin/keystone-all
index 4767c258..fc499967 100755
--- a/bin/keystone-all
+++ b/bin/keystone-all
@@ -73,7 +73,7 @@ if __name__ == '__main__':
     if os.path.exists(dev_conf):
         config_files = [dev_conf]
 
-    CONF(config_files=config_files, args=sys.argv)
+    CONF(project='keystone', default_config_files=config_files)
 
     config.setup_logging(CONF)
 
diff --git a/keystone/cli.py b/keystone/cli.py
index 439ffb9e..add94d52 100644
--- a/keystone/cli.py
+++ b/keystone/cli.py
@@ -23,9 +23,7 @@ import textwrap
 from keystone import config
 from keystone.openstack.common import importutils
 
-
 CONF = config.CONF
-CONF.set_usage('%prog COMMAND')
 
 
 class BaseApp(object):
@@ -136,7 +134,10 @@ def run(cmd, args):
 
 def main(argv=None, config_files=None):
     CONF.reset()
-    args = CONF(config_files=config_files, args=argv)
+    args = CONF(args=argv,
+                project='keystone',
+                usage='%prog COMMAND',
+                default_config_files=config_files)
 
     if len(args) < 2:
         CONF.print_help()
diff --git a/keystone/config.py b/keystone/config.py
index 3a22fbdf..23762ccd 100644
--- a/keystone/config.py
+++ b/keystone/config.py
@@ -25,24 +25,7 @@ from keystone.openstack.common import cfg
 gettext.install('keystone', unicode=1)
 
 
-class ConfigMixin(object):
-    def __call__(self, config_files=None, *args, **kw):
-        if config_files is not None:
-            self._opts['config_file']['opt'].default = config_files
-        kw.setdefault('args', [])
-        return super(ConfigMixin, self).__call__(*args, **kw)
-
-    def set_usage(self, usage):
-        self.usage = usage
-        self._oparser.usage = usage
-
-
-class Config(ConfigMixin, cfg.ConfigOpts):
-    pass
-
-
-class CommonConfig(ConfigMixin, cfg.CommonConfigOpts):
-    pass
+CONF = cfg.CONF
 
 
 def setup_logging(conf):
@@ -128,9 +111,6 @@ def register_cli_int(*args, **kw):
     return conf.register_cli_opt(cfg.IntOpt(*args, **kw), group=group)
 
 
-CONF = CommonConfig(project='keystone')
-
-
 register_str('admin_token', default='ADMIN')
 register_str('bind_host', default='0.0.0.0')
 register_str('compute_port', default=8774)
diff --git a/keystone/openstack/common/cfg.py b/keystone/openstack/common/cfg.py
index 03c9b703..f272b2a1 100644
--- a/keystone/openstack/common/cfg.py
+++ b/keystone/openstack/common/cfg.py
@@ -95,7 +95,7 @@ and --config-dir::
 
     class ConfigOpts(object):
 
-        def __init__(self, ...):
+        def __call__(self, ...):
 
             opts = [
                 MultiStrOpt('config-file',
@@ -233,12 +233,28 @@ log files:
         ...
      ]
 
+This module also contains a global instance of the CommonConfigOpts class
+in order to support a common usage pattern in OpenStack:
+
+  from openstack.common import cfg
+
+  opts = [
+    cfg.StrOpt('bind_host' default='0.0.0.0'),
+    cfg.IntOpt('bind_port', default=9292),
+  ]
+
+  CONF = cfg.CONF
+  CONF.register_opts(opts)
+
+  def start(server, app):
+      server.start(app, CONF.bind_port, CONF.bind_host)
+
 """
 
 import collections
 import copy
-import glob
 import functools
+import glob
 import optparse
 import os
 import string
@@ -768,6 +784,14 @@ class OptGroup(object):
 
         return True
 
+    def _unregister_opt(self, opt):
+        """Remove an opt from this group.
+
+        :param opt: an Opt object
+        """
+        if opt.dest in self._opts:
+            del self._opts[opt.dest]
+
     def _get_optparse_group(self, parser):
         """Build an optparse.OptionGroup for this group."""
         if self._optparse_group is None:
@@ -775,6 +799,10 @@ class OptGroup(object):
                                                         self.help)
         return self._optparse_group
 
+    def _clear(self):
+        """Clear this group's option parsing state."""
+        self._optparse_group = None
+
 
 class ParseError(iniparser.ParseError):
     def __init__(self, msg, lineno, line, filename):
@@ -849,57 +877,41 @@ class ConfigOpts(collections.Mapping):
     the values of options.
     """
 
-    def __init__(self,
-                 project=None,
-                 prog=None,
-                 version=None,
-                 usage=None,
-                 default_config_files=None):
-        """Construct a ConfigOpts object.
+    def __init__(self):
+        """Construct a ConfigOpts object."""
+        self._opts = {}  # dict of dicts of (opt:, override:, default:)
+        self._groups = {}
 
-        Automatically registers the --config-file option with either a supplied
-        list of default config files, or a list from find_config_files().
+        self._args = None
+        self._oparser = None
+        self._cparser = None
+        self._cli_values = {}
+        self.__cache = {}
+        self._config_opts = []
+        self._disable_interspersed_args = False
 
-        :param project: the toplevel project name, used to locate config files
-        :param prog: the name of the program (defaults to sys.argv[0] basename)
-        :param version: the program version (for --version)
-        :param usage: a usage string (%prog will be expanded)
-        :param default_config_files: config files to use by default
-        """
+    def _setup(self, project, prog, version, usage, default_config_files):
+        """Initialize a ConfigOpts object for option parsing."""
         if prog is None:
             prog = os.path.basename(sys.argv[0])
 
         if default_config_files is None:
             default_config_files = find_config_files(project, prog)
 
-        self.project = project
-        self.prog = prog
-        self.version = version
-        self.usage = usage
-        self.default_config_files = default_config_files
+        self._oparser = optparse.OptionParser(prog=prog,
+                                              version=version,
+                                              usage=usage)
+        if self._disable_interspersed_args:
+            self._oparser.disable_interspersed_args()
 
-        self._opts = {}  # dict of dicts of (opt:, override:, default:)
-        self._groups = {}
-
-        self._args = None
-        self._cli_values = {}
-
-        self._oparser = optparse.OptionParser(prog=self.prog,
-                                              version=self.version,
-                                              usage=self.usage)
-        self._cparser = None
-
-        self.__cache = {}
-
-        opts = [
+        self._config_opts = [
              MultiStrOpt('config-file',
-                         default=self.default_config_files,
+                         default=default_config_files,
                          metavar='PATH',
                          help='Path to a config file to use. Multiple config '
                               'files can be specified, with values in later '
                               'files taking precedence. The default files '
-                              ' used are: %s' %
-                              (self.default_config_files, )),
+                              ' used are: %s' % (default_config_files, )),
             StrOpt('config-dir',
                    metavar='DIR',
                    help='Path to a config directory to pull *.conf '
@@ -910,7 +922,13 @@ class ConfigOpts(collections.Mapping):
                         'hence over-ridden options in the directory take '
                         'precedence.'),
             ]
-        self.register_cli_opts(opts)
+        self.register_cli_opts(self._config_opts)
+
+        self.project = project
+        self.prog = prog
+        self.version = version
+        self.usage = usage
+        self.default_config_files = default_config_files
 
     def __clear_cache(f):
         @functools.wraps(f)
@@ -921,7 +939,13 @@ class ConfigOpts(collections.Mapping):
 
         return __inner
 
-    def __call__(self, args=None):
+    def __call__(self,
+                 args=None,
+                 project=None,
+                 prog=None,
+                 version=None,
+                 usage=None,
+                 default_config_files=None):
         """Parse command line arguments and config files.
 
         Calling a ConfigOpts object causes the supplied command line arguments
@@ -931,35 +955,34 @@ class ConfigOpts(collections.Mapping):
         The object may be called multiple times, each time causing the previous
         set of values to be overwritten.
 
+        Automatically registers the --config-file option with either a supplied
+        list of default config files, or a list from find_config_files().
+
         If the --config-dir option is set, any *.conf files from this
         directory are pulled in, after all the file(s) specified by the
         --config-file option.
 
-        :params args: command line arguments (defaults to sys.argv[1:])
+        :param args: command line arguments (defaults to sys.argv[1:])
+        :param project: the toplevel project name, used to locate config files
+        :param prog: the name of the program (defaults to sys.argv[0] basename)
+        :param version: the program version (for --version)
+        :param usage: a usage string (%prog will be expanded)
+        :param default_config_files: config files to use by default
         :returns: the list of arguments left over after parsing options
         :raises: SystemExit, ConfigFilesNotFoundError, ConfigFileParseError,
-                 RequiredOptError
+                 RequiredOptError, DuplicateOptError
         """
         self.clear()
 
-        self._args = args
-
-        (values, args) = self._oparser.parse_args(self._args)
-
-        self._cli_values = vars(values)
-
-        def _list_config_dir():
-            return sorted(glob.glob(os.path.join(self.config_dir, '*.conf')))
+        self._setup(project, prog, version, usage, default_config_files)
 
-        from_file = list(self.config_file)
+        self._cli_values, leftovers = self._parse_cli_opts(args)
 
-        from_dir = _list_config_dir() if self.config_dir else []
-
-        self._parse_config_files(from_file + from_dir)
+        self._parse_config_files()
 
         self._check_required_opts()
 
-        return args
+        return leftovers
 
     def __getattr__(self, name):
         """Look up an option value and perform string substitution.
@@ -996,8 +1019,12 @@ class ConfigOpts(collections.Mapping):
     def clear(self):
         """Clear the state of the object to before it was called."""
         self._args = None
-        self._cli_values = {}
+        self._cli_values.clear()
+        self._oparser = None
         self._cparser = None
+        self.unregister_opts(self._config_opts)
+        for group in self._groups.values():
+            group._clear()
 
     @__clear_cache
     def register_opt(self, opt, group=None):
@@ -1044,15 +1071,7 @@ class ConfigOpts(collections.Mapping):
         if self._args is not None:
             raise ArgsAlreadyParsedError("cannot register CLI option")
 
-        if not self.register_opt(opt, group, clear_cache=False):
-            return False
-
-        if group is not None:
-            group = self._get_group(group, autocreate=True)
-
-        opt._add_to_cli(self._oparser, group)
-
-        return True
+        return self.register_opt(opt, group, clear_cache=False)
 
     @__clear_cache
     def register_cli_opts(self, opts, group=None):
@@ -1074,6 +1093,28 @@ class ConfigOpts(collections.Mapping):
         self._groups[group.name] = copy.copy(group)
 
     @__clear_cache
+    def unregister_opt(self, opt, group=None):
+        """Unregister an option.
+
+        :param opt: an Opt object
+        :param group: an optional OptGroup object or group name
+        :raises: ArgsAlreadyParsedError, NoSuchGroupError
+        """
+        if self._args is not None:
+            raise ArgsAlreadyParsedError("reset before unregistering options")
+
+        if group is not None:
+            self._get_group(group)._unregister_opt(opt)
+        elif opt.dest in self._opts:
+            del self._opts[opt.dest]
+
+    @__clear_cache
+    def unregister_opts(self, opts, group=None):
+        """Unregister multiple CLI option schemas at once."""
+        for opt in opts:
+            self.unregister_opt(opt, group, clear_cache=False)
+
+    @__clear_cache
     def set_override(self, name, override, group=None):
         """Override an opt value.
 
@@ -1103,16 +1144,24 @@ class ConfigOpts(collections.Mapping):
         opt_info = self._get_opt_info(name, group)
         opt_info['default'] = default
 
+    def _all_opt_infos(self):
+        """A generator function for iteration opt infos."""
+        for info in self._opts.values():
+            yield info, None
+        for group in self._groups.values():
+            for info in group._opts.values():
+                yield info, group
+
+    def _all_opts(self):
+        """A generator function for iteration opts."""
+        for info, group in self._all_opt_infos():
+            yield info['opt'], group
+
     def _unset_defaults_and_overrides(self):
         """Unset any default or override on all options."""
-        def unset(opts):
-            for info in opts.values():
-                info['default'] = None
-                info['override'] = None
-
-        unset(self._opts)
-        for group in self._groups.values():
-            unset(group._opts)
+        for info, group in self._all_opt_infos():
+            info['default'] = None
+            info['override'] = None
 
     def disable_interspersed_args(self):
         """Set parsing to stop on the first non-option.
@@ -1131,13 +1180,13 @@ class ConfigOpts(collections.Mapping):
 
         i.e. argument parsing is stopped at the first non-option argument.
         """
-        self._oparser.disable_interspersed_args()
+        self._disable_interspersed_args = True
 
     def enable_interspersed_args(self):
         """Set parsing to not stop on the first non-option.
 
         This it the default behaviour."""
-        self._oparser.enable_interspersed_args()
+        self._disable_interspersed_args = False
 
     def find_file(self, name):
         """Locate a file located alongside the config files.
@@ -1331,11 +1380,17 @@ class ConfigOpts(collections.Mapping):
 
         return opts[opt_name]
 
-    def _parse_config_files(self, config_files):
-        """Parse the supplied configuration files.
+    def _parse_config_files(self):
+        """Parse the config files from --config-file and --config-dir.
 
         :raises: ConfigFilesNotFoundError, ConfigFileParseError
         """
+        config_files = list(self.config_file)
+
+        if self.config_dir:
+            config_dir_glob = os.path.join(self.config_dir, '*.conf')
+            config_files += sorted(glob.glob(config_dir_glob))
+
         self._cparser = MultiConfigParser()
 
         try:
@@ -1347,8 +1402,12 @@ class ConfigOpts(collections.Mapping):
             not_read_ok = filter(lambda f: f not in read_ok, config_files)
             raise ConfigFilesNotFoundError(not_read_ok)
 
-    def _do_check_required_opts(self, opts, group=None):
-        for info in opts.values():
+    def _check_required_opts(self):
+        """Check that all opts marked as required have values specified.
+
+        :raises: RequiredOptError
+        """
+        for info, group in self._all_opt_infos():
             default, opt, override = [info[k] for k in sorted(info.keys())]
 
             if opt.required:
@@ -1359,15 +1418,25 @@ class ConfigOpts(collections.Mapping):
                 if self._get(opt.name, group) is None:
                     raise RequiredOptError(opt.name, group)
 
-    def _check_required_opts(self):
-        """Check that all opts marked as required have values specified.
+    def _parse_cli_opts(self, args):
+        """Parse command line options.
+
+        Initializes the command line option parser and parses the supplied
+        command line arguments.
+
+        :param args: the command line arguments
+        :returns: a dict of parsed option values
+        :raises: SystemExit, DuplicateOptError
 
-        :raises: RequiredOptError
         """
-        self._do_check_required_opts(self._opts)
+        self._args = args
 
-        for group in self._groups.values():
-            self._do_check_required_opts(group._opts, group)
+        for opt, group in self._all_opts():
+            opt._add_to_cli(self._oparser, group)
+
+        values, leftovers = self._oparser.parse_args(args)
+
+        return vars(values), leftovers
 
     class GroupAttr(collections.Mapping):
 
@@ -1483,7 +1552,10 @@ class CommonConfigOpts(ConfigOpts):
                help='syslog facility to receive log lines')
         ]
 
-    def __init__(self, **kwargs):
-        super(CommonConfigOpts, self).__init__(**kwargs)
+    def __init__(self):
+        super(CommonConfigOpts, self).__init__()
         self.register_cli_opts(self.common_cli_opts)
         self.register_cli_opts(self.logging_cli_opts)
+
+
+CONF = CommonConfigOpts()
diff --git a/keystone/test.py b/keystone/test.py
index 54dc01b0..03488703 100644
--- a/keystone/test.py
+++ b/keystone/test.py
@@ -165,13 +165,13 @@ class TestCase(NoModule, unittest.TestCase):
 
     def setUp(self):
         super(TestCase, self).setUp()
-        self.config()
+        self.config([etcdir('keystone.conf.sample'),
+                     testsdir('test_overrides.conf')])
         self.mox = mox.Mox()
         self.stubs = stubout.StubOutForTesting()
 
-    def config(self):
-        CONF(config_files=[etcdir('keystone.conf.sample'),
-                           testsdir('test_overrides.conf')])
+    def config(self, config_files):
+        CONF(args=[], project='keystone', default_config_files=config_files)
 
     def tearDown(self):
         try:
diff --git a/tests/_ldap_livetest.py b/tests/_ldap_livetest.py
index 6a4420b8..7afd2ec0 100644
--- a/tests/_ldap_livetest.py
+++ b/tests/_ldap_livetest.py
@@ -60,9 +60,9 @@ def clear_live_database():
 class LDAPIdentity(test.TestCase, test_backend.IdentityTests):
     def setUp(self):
         super(LDAPIdentity, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_liveldap.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_liveldap.conf')])
         clear_live_database()
         self.identity_api = identity_ldap.Identity()
         self.load_fixtures(default_fixtures)
diff --git a/tests/test_backend_ldap.py b/tests/test_backend_ldap.py
index 0c882b67..56b2f5cf 100644
--- a/tests/test_backend_ldap.py
+++ b/tests/test_backend_ldap.py
@@ -34,9 +34,9 @@ def clear_database():
 class LDAPIdentity(test.TestCase, test_backend.IdentityTests):
     def setUp(self):
         super(LDAPIdentity, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_ldap.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_ldap.conf')])
         clear_database()
         self.identity_api = identity_ldap.Identity()
         self.load_fixtures(default_fixtures)
diff --git a/tests/test_backend_sql.py b/tests/test_backend_sql.py
index d600e772..3f7ea098 100644
--- a/tests/test_backend_sql.py
+++ b/tests/test_backend_sql.py
@@ -33,9 +33,9 @@ CONF = config.CONF
 class SqlIdentity(test.TestCase, test_backend.IdentityTests):
     def setUp(self):
         super(SqlIdentity, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_sql.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_sql.conf')])
         sql_util.setup_test_database()
         self.identity_api = identity_sql.Identity()
         self.load_fixtures(default_fixtures)
@@ -135,9 +135,9 @@ class SqlIdentity(test.TestCase, test_backend.IdentityTests):
 class SqlToken(test.TestCase, test_backend.TokenTests):
     def setUp(self):
         super(SqlToken, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_sql.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_sql.conf')])
         sql_util.setup_test_database()
         self.token_api = token_sql.Token()
 
diff --git a/tests/test_import_legacy.py b/tests/test_import_legacy.py
index 5b1412b1..5c6ee61d 100644
--- a/tests/test_import_legacy.py
+++ b/tests/test_import_legacy.py
@@ -33,9 +33,9 @@ CONF = config.CONF
 class ImportLegacy(test.TestCase):
     def setUp(self):
         super(ImportLegacy, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_sql.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_sql.conf')])
         sql_util.setup_test_database()
         self.identity_api = identity_sql.Identity()
 
diff --git a/tests/test_keystoneclient_sql.py b/tests/test_keystoneclient_sql.py
index 8002d88b..a70f7603 100644
--- a/tests/test_keystoneclient_sql.py
+++ b/tests/test_keystoneclient_sql.py
@@ -27,10 +27,11 @@ CONF = config.CONF
 
 
 class KcMasterSqlTestCase(test_keystoneclient.KcMasterTestCase):
-    def config(self):
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_sql.conf')])
+    def config(self, config_files):
+        super(KcMasterSqlTestCase, self).config([
+                test.etcdir('keystone.conf.sample'),
+                test.testsdir('test_overrides.conf'),
+                test.testsdir('backend_sql.conf')])
         sql_util.setup_test_database()
 
     def test_endpoint_crud(self):
diff --git a/tests/test_migrate_nova_auth.py b/tests/test_migrate_nova_auth.py
index 78c42a30..fdd40ff1 100644
--- a/tests/test_migrate_nova_auth.py
+++ b/tests/test_migrate_nova_auth.py
@@ -68,9 +68,9 @@ FIXTURE = {
 class MigrateNovaAuth(test.TestCase):
     def setUp(self):
         super(MigrateNovaAuth, self).setUp()
-        CONF(config_files=[test.etcdir('keystone.conf.sample'),
-                           test.testsdir('test_overrides.conf'),
-                           test.testsdir('backend_sql.conf')])
+        self.config([test.etcdir('keystone.conf.sample'),
+                     test.testsdir('test_overrides.conf'),
+                     test.testsdir('backend_sql.conf')])
         sql_util.setup_test_database()
         self.identity_api = identity_sql.Identity()
         self.ec2_api = ec2_sql.Ec2()