Hooks around mutate_config_files
This patch adds register_mutate_hook to ConfigOpts. When mutate_config_files is otherwise finished, it calls all the registered hooks. Each hook is passed the complete, mutated conf object, plus a dict of just the mutated keys with old and new values. Currently there's no way to change or remove a hook once registered. There's no obvious usecase for this (to me at least) and the direct implementation where we accept a unique slug alongside each hook has a good back-compat story (generate slugs if one is not provided) so I've plumped for the absolute simplest system. Hooks are called in a non-deterministic order. This is intentional, it makes it easy to avoid duplicates in the collection of hooks and enforces independence of hook functions. It also means we don't need a complex API for inspecting and reordering this collection. Initially we wanted to avoid hooks in oslo.config entirely. This leads to two problems. One, Nova delegates calling mutate_config_files to oslo.service. How is oslo.service to know which other oslo libs Nova uses and hence which need to be notified? We could solve this by passing a list of hooks to oslo.service from Nova. Ugly but workable. However, then if Nova is using oslo.foo and oslo.foo adds support for mutable config, this leads to problem two. We must patch Nova to pass the new hook to oslo.service. With 30+ OpenStack applications, this would be tedious. The collection is held on the ConfigOpts object because some projects use multiple configs. I could offer a decorator which takes the conf to register with as an argument but this would only be useful to projects which use a global conf and generally seems like extra code for minimal benefit. Doug suggested offering a hook per option or group. The former is inefficient when multiple options relate to a single resource. The latter falls down on the default group, which multiple things need to be notified about changes to. Change-Id: Ied006631a6edbeeffae485d28eff700b13a626c1
This commit is contained in:
@@ -2025,6 +2025,7 @@ class ConfigOpts(collections.Mapping):
|
||||
self._oparser = None
|
||||
self._namespace = None
|
||||
self._mutable_ns = None
|
||||
self._mutate_hooks = set([])
|
||||
self.__cache = {}
|
||||
self._config_opts = []
|
||||
self._cli_opts = collections.deque()
|
||||
@@ -2197,6 +2198,7 @@ class ConfigOpts(collections.Mapping):
|
||||
self._oparser = None
|
||||
self._namespace = None
|
||||
self._mutable_ns = None
|
||||
# Keep _mutate_hooks
|
||||
self._validate_default_values = False
|
||||
self.unregister_opts(self._config_opts)
|
||||
for group in self._groups.values():
|
||||
@@ -2797,12 +2799,25 @@ class ConfigOpts(collections.Mapping):
|
||||
self._namespace = namespace
|
||||
return True
|
||||
|
||||
def register_mutate_hook(self, hook):
|
||||
"""Registers a hook to be called by mutate_config_files.
|
||||
|
||||
:param hook: a function accepting this ConfigOpts object and a dict of
|
||||
config mutations, as returned by mutate_config_files.
|
||||
:return None
|
||||
"""
|
||||
self._mutate_hooks.add(hook)
|
||||
|
||||
@__clear_cache
|
||||
def mutate_config_files(self):
|
||||
"""Reload configure files and parse all options.
|
||||
|
||||
Only options marked as 'mutable' will appear to change.
|
||||
|
||||
Hooks are called in a NON-DETERMINISTIC ORDER. Do not expect hooks to
|
||||
be called in the same order as they were added.
|
||||
|
||||
:return {(None or 'group', 'optname'): (old_value, new_value), ... }
|
||||
:raises Error if reloading fails
|
||||
"""
|
||||
|
||||
@@ -2821,6 +2836,8 @@ class ConfigOpts(collections.Mapping):
|
||||
groupname = groupname if groupname else 'DEFAULT'
|
||||
LOG.info("Option %s.%s changed from [%s] to [%s]",
|
||||
groupname, optname, old, new)
|
||||
for hook in self._mutate_hooks:
|
||||
hook(self, fresh)
|
||||
return fresh
|
||||
|
||||
def _warn_immutability(self):
|
||||
|
||||
@@ -1727,6 +1727,37 @@ class ConfigFileMutateTestCase(BaseTestCase):
|
||||
"Option group.boo changed from [old_boo] to [new_boo]\n")
|
||||
self.assertEqual(expected, self.log_fixture.output)
|
||||
|
||||
def test_hooks(self):
|
||||
fresh = {}
|
||||
result = [0]
|
||||
|
||||
def foo(conf, foo_fresh):
|
||||
self.assertEqual(self.conf, conf)
|
||||
self.assertEqual(fresh, foo_fresh)
|
||||
result[0] += 1
|
||||
|
||||
self.conf.register_mutate_hook(foo)
|
||||
self.conf.register_mutate_hook(foo)
|
||||
self._test_conf_files_mutate()
|
||||
self.assertEqual(1, result[0])
|
||||
|
||||
def test_clear(self):
|
||||
"""Show that #clear doesn't undeclare opts.
|
||||
|
||||
This justifies not clearing mutate_hooks either. ResetAndClearTestCase
|
||||
shows that values are cleared.
|
||||
"""
|
||||
self.conf.register_cli_opt(cfg.StrOpt('cli'))
|
||||
self.conf.register_opt(cfg.StrOpt('foo'))
|
||||
dests = [info['opt'].dest for info, _ in self.conf._all_opt_infos()]
|
||||
self.assertIn('cli', dests)
|
||||
self.assertIn('foo', dests)
|
||||
|
||||
self.conf.clear()
|
||||
dests = [info['opt'].dest for info, _ in self.conf._all_opt_infos()]
|
||||
self.assertIn('cli', dests)
|
||||
self.assertIn('foo', dests)
|
||||
|
||||
|
||||
class OptGroupsTestCase(BaseTestCase):
|
||||
|
||||
|
||||
Reference in New Issue
Block a user