From 713217322f7c82d3e1b2faf7ce6f294e580d8de2 Mon Sep 17 00:00:00 2001
From: Alexis Lee <lxsli@hpe.com>
Date: Tue, 16 Feb 2016 16:56:13 +0000
Subject: [PATCH] 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
---
 oslo_config/cfg.py            | 17 +++++++++++++++++
 oslo_config/tests/test_cfg.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py
index 756a5947..8795902a 100644
--- a/oslo_config/cfg.py
+++ b/oslo_config/cfg.py
@@ -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):
diff --git a/oslo_config/tests/test_cfg.py b/oslo_config/tests/test_cfg.py
index b9f58efb..e8ffbe5b 100644
--- a/oslo_config/tests/test_cfg.py
+++ b/oslo_config/tests/test_cfg.py
@@ -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):