Stop running register_configs at load time

Stop running register_configs so it is not run unnecessarily and
simplifies unit tests. As part of this make sure that CONFIGS is
refreshed after an upgrade as config files and contexts may
have changed.

Change-Id: I08a847abe7db22a11860f359442e1e8945585466
Closes-Bug: #1844325

hooks/horizon_hooks.py

@@ -105,7 +105,25 @@ from hooks.horizon_utils import (
 )
 
 hooks = Hooks()
-CONFIGS = register_configs()
+# Note that CONFIGS is now set up via resolve_CONFIGS so that it is not a
+# module load time constraint.
+CONFIGS = None
+
+
+def resolve_CONFIGS(force_update=False):
+    """lazy function to resolve the CONFIGS so that it doesn't have to evaluate
+    at module load time.  Note that it also returns the CONFIGS so that it can
+    be used in other, module loadtime, functions.
+
+    :param force_update: Force a refresh of CONFIGS
+    :type force_update: bool
+    :returns: CONFIGS variable
+    :rtype: `:class:templating.OSConfigRenderer`
+    """
+    global CONFIGS
+    if CONFIGS is None or not force_update:
+        CONFIGS = register_configs()
+    return CONFIGS
 
 
 @hooks.hook('install.real')
@@ -132,6 +150,7 @@ def install():
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 @harden()
 def upgrade_charm():
+    resolve_CONFIGS()
     execd_preinstall()
     apt_install(filter_installed_packages(determine_packages()), fatal=True)
     packages_removed = remove_old_packages()
@@ -148,6 +167,7 @@ def upgrade_charm():
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 @harden()
 def config_changed():
+    resolve_CONFIGS()
     if config('prefer-ipv6'):
         setup_ipv6()
         localhost = 'ip6-localhost'
@@ -168,6 +188,7 @@ def config_changed():
         if openstack_upgrade_available('openstack-dashboard'):
             status_set('maintenance', 'Upgrading to new OpenStack release')
             do_openstack_upgrade(configs=CONFIGS)
+            resolve_CONFIGS(force_update=True)
 
     env_vars = {
         'OPENSTACK_URL_HORIZON':
@@ -208,6 +229,7 @@ def keystone_joined(rel_id=None):
 @hooks.hook('identity-service-relation-changed')
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 def keystone_changed():
+    resolve_CONFIGS()
     CONFIGS.write_all()
     if relation_get('ca_cert'):
         install_ca_cert(b64decode(relation_get('ca_cert')))
@@ -224,6 +246,7 @@ def cluster_joined(relation_id=None):
             'cluster-relation-changed')
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 def cluster_relation():
+    resolve_CONFIGS()
     CONFIGS.write(HAPROXY_CONF)
 
 
@@ -273,6 +296,7 @@ def plugin_relation_joined(rel_id=None):
 @hooks.hook('dashboard-plugin-relation-changed')
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 def update_plugin_config():
+    resolve_CONFIGS()
     CONFIGS.write(LOCAL_SETTINGS)
 
 
@@ -305,6 +329,7 @@ def db_joined():
 @hooks.hook('shared-db-relation-changed')
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 def db_changed():
+    resolve_CONFIGS()
     if 'shared-db' not in CONFIGS.complete_contexts():
         log('shared-db relation incomplete. Peer not ready?')
         return
@@ -326,6 +351,7 @@ def db_changed():
             'websso-fid-service-provider-relation-departed')
 @restart_on_change(restart_map(), stopstart=True, sleep=3)
 def websso_sp_changed():
+    resolve_CONFIGS()
     CONFIGS.write_all()
 
 
@@ -367,6 +393,7 @@ def main():
         hooks.execute(sys.argv)
     except UnregisteredHookError as e:
         log('Unknown hook {} - skipping.'.format(e))
+    resolve_CONFIGS()
     assess_status(CONFIGS)
 
 
@@ -379,6 +406,7 @@ def certs_joined(relation_id=None):
 
 @hooks.hook('certificates-relation-changed')
 def certs_changed(relation_id=None, unit=None):
+    resolve_CONFIGS()
     process_certificates('horizon', relation_id, unit)
     CONFIGS.write_all()
     service_reload('apache2')
@@ -388,6 +416,7 @@ def certs_changed(relation_id=None, unit=None):
 @hooks.hook('pre-series-upgrade')
 def pre_series_upgrade():
     log("Running prepare series upgrade hook", "INFO")
+    resolve_CONFIGS()
     series_upgrade_prepare(
         pause_unit_helper, CONFIGS)
 
@@ -395,6 +424,7 @@ def pre_series_upgrade():
 @hooks.hook('post-series-upgrade')
 def post_series_upgrade():
     log("Running complete series upgrade hook", "INFO")
+    resolve_CONFIGS()
     series_upgrade_complete(
         resume_unit_helper, CONFIGS)
 

unit_tests/test_horizon_hooks.py

@@ -22,8 +22,6 @@ from unit_tests.test_utils import CharmTestCase
 sys.modules['apt'] = MagicMock()
 
 import hooks.horizon_utils as utils
-_register_configs = utils.register_configs
-utils.register_configs = MagicMock()
 
 with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec:
     mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f:
@@ -32,7 +30,6 @@ with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec:
     import hooks.horizon_hooks as hooks
 
 RESTART_MAP = utils.restart_map()
-utils.register_configs = _register_configs
 
 TO_PATCH = [
     'config',
@@ -63,6 +60,7 @@ TO_PATCH = [
     'remove_old_packages',
     'generate_ha_relation_data',
     'resolve_address',
+    'register_configs',
 ]
 
 
@@ -149,7 +147,7 @@ class TestHorizonHooks(CharmTestCase):
         self.filter_installed_packages.return_value = ['foo']
         self._call_hook('upgrade-charm')
         self.apt_install.assert_called_with(['foo'], fatal=True)
-        self.assertTrue(self.CONFIGS.write_all.called)
+        self.assertTrue(self.register_configs().write_all.called)
         ex = [
             call('stop', 'apache2'),
             call('stop', 'memcached'),
@@ -229,18 +227,28 @@ class TestHorizonHooks(CharmTestCase):
         self.assertTrue(self.enable_ssl.called)
         self.do_openstack_upgrade.assert_not_called()
         self.assertTrue(self.save_script_rc.called)
-        self.assertTrue(self.CONFIGS.write_all.called)
+        self.assertTrue(self.register_configs().write_all.called)
         self.open_port.assert_has_calls([call(80), call(443)])
         self.assertTrue(_custom_theme.called)
 
     @patch('hooks.horizon_hooks.check_custom_theme')
     def test_config_changed_do_upgrade(self, _custom_theme):
+        config_mock1 = MagicMock()
+        config_mock2 = MagicMock()
+        config_mocks = [config_mock1, config_mock2]
+
+        def _register_configs():
+            return config_mocks.pop()
+        self.register_configs.side_effect = _register_configs
         self.relation_ids.return_value = []
         self.test_config.set('openstack-origin', 'cloud:precise-grizzly')
         self.openstack_upgrade_available.return_value = True
         self._call_hook('config-changed')
         self.assertTrue(self.do_openstack_upgrade.called)
         self.assertTrue(_custom_theme.called)
+        # Assert that the second mock is used for writing config as
+        # that shows that CONFIGS was refreshed post-upgrade.
+        config_mock2.write_all.assert_called_once_with()
 
     def test_keystone_joined_in_relation(self):
         self._call_hook('identity-service-relation-joined')
@@ -261,22 +269,24 @@ class TestHorizonHooks(CharmTestCase):
     def test_keystone_changed_no_cert(self):
         self.relation_get.return_value = None
         self._call_hook('identity-service-relation-changed')
-        self.CONFIGS.write_all.assert_called_with()
+        self.register_configs().write_all.assert_called_with()
         self.install_ca_cert.assert_not_called()
 
     def test_keystone_changed_cert(self):
         self.relation_get.return_value = 'certificate'
         self._call_hook('identity-service-relation-changed')
-        self.CONFIGS.write_all.assert_called_with()
+        self.register_configs().write_all.assert_called_with()
         self.install_ca_cert.assert_called_with('certificate')
 
     def test_cluster_departed(self):
         self._call_hook('cluster-relation-departed')
-        self.CONFIGS.write.assert_called_with('/etc/haproxy/haproxy.cfg')
+        self.register_configs().write.assert_called_with(
+            '/etc/haproxy/haproxy.cfg')
 
     def test_cluster_changed(self):
         self._call_hook('cluster-relation-changed')
-        self.CONFIGS.write.assert_called_with('/etc/haproxy/haproxy.cfg')
+        self.register_configs().write.assert_called_with(
+            '/etc/haproxy/haproxy.cfg')
 
     def test_website_joined(self):
         self.unit_get.return_value = '192.168.1.1'
@@ -296,7 +306,7 @@ class TestHorizonHooks(CharmTestCase):
 
     def test_websso_fid_service_provider_changed(self):
         self._call_hook('websso-fid-service-provider-relation-changed')
-        self.CONFIGS.write_all.assert_called_with()
+        self.register_configs().write_all.assert_called_with()
 
     def test_websso_trusted_dashboard_changed_no_tls(self):
         def relation_ids_side_effect(rname):
@@ -418,6 +428,6 @@ class TestHorizonHooks(CharmTestCase):
         self._call_hook('certificates-relation-changed')
         _process_certificates.assert_called_with(
             'horizon', None, None)
-        self.CONFIGS.write_all.assert_called_with()
+        self.register_configs().write_all.assert_called_with()
         _service_reload.assert_called_with('apache2')
         self.enable_ssl.assert_called_with()