Restart apache when keystone policy changes

Keystone does not reliably pick up policy changes (observed on
Queens) unless apache is restarted. This change triggers a restart
when  policy is changed.

Change-Id: Ia29312baa9c1b8113649fc2826f0464588c3ce11

hooks/keystone_hooks.py

@@ -208,7 +208,10 @@ def install():
             disable_unused_apache_sites()
             service_pause('keystone')
     # call the policy overrides handler which will install any policy overrides
-    maybe_do_policyd_overrides(os_release('keystone'), 'keystone')
+    maybe_do_policyd_overrides(
+        os_release('keystone'),
+        'keystone',
+        restart_handler=lambda: service_restart('apache2'))
 
 
 @hooks.hook('config-changed')
@@ -236,8 +239,10 @@ def config_changed():
         cluster_joined(rid=r_id)
 
     # call the policy overrides handler which will install any policy overrides
-    maybe_do_policyd_overrides_on_config_changed(os_release('keystone'),
-                                                 'keystone')
+    maybe_do_policyd_overrides_on_config_changed(
+        os_release('keystone'),
+        'keystone',
+        restart_handler=lambda: service_restart('apache2'))
 
     config_changed_postupgrade()
 
@@ -735,7 +740,10 @@ def upgrade_charm():
         ensure_all_service_accounts_protected_for_pci_dss_options()
 
     # call the policy overrides handler which will install any policy overrides
-    maybe_do_policyd_overrides(os_release('keystone'), 'keystone')
+    maybe_do_policyd_overrides(
+        os_release('keystone'),
+        'keystone',
+        restart_handler=lambda: service_restart('apache2'))
 
 
 @hooks.hook('update-status')

unit_tests/test_keystone_hooks.py

@@ -137,7 +137,7 @@ class KeystoneRelationTests(CharmTestCase):
              'python3-six', 'uuid'], fatal=True)
         self.disable_unused_apache_sites.assert_not_called()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
 
     @patch.object(hooks, 'maybe_do_policyd_overrides')
     @patch.object(utils, 'os_release')
@@ -160,7 +160,7 @@ class KeystoneRelationTests(CharmTestCase):
              'python3-six', 'uuid'], fatal=True)
         self.disable_unused_apache_sites.assert_called_with()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
 
     mod_ch_openstack_utils = 'charmhelpers.contrib.openstack.utils'
 
@@ -279,7 +279,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertTrue(mock_notify_middleware.called_once)
 
         (mock_maybe_do_policyd_overrides_on_config_changed
-         .assert_called_once_with(ANY, "keystone"))
+         .assert_called_once_with(ANY, "keystone", restart_handler=ANY))
 
     @patch.object(hooks, 'maybe_do_policyd_overrides_on_config_changed')
     @patch.object(hooks, 'is_db_initialised')
@@ -328,7 +328,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertTrue(mock_update_domains.called)
 
         (mock_maybe_do_policyd_overrides_on_config_changed
-         .assert_called_once_with(ANY, "keystone"))
+         .assert_called_once_with(ANY, "keystone", restart_handler=ANY))
 
     @patch.object(hooks, 'maybe_do_policyd_overrides_on_config_changed')
     @patch.object(hooks, 'update_all_domain_backends')
@@ -381,7 +381,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertTrue(mock_update_domains.called)
 
         (mock_maybe_do_policyd_overrides_on_config_changed
-         .assert_called_once_with(ANY, "keystone"))
+         .assert_called_once_with(ANY, "keystone", restart_handler=ANY))
 
     @patch.object(hooks, 'maybe_do_policyd_overrides_on_config_changed')
     @patch.object(hooks, 'is_expected_scale')
@@ -411,7 +411,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertFalse(self.do_openstack_upgrade_reexec.called)
 
         (mock_maybe_do_policyd_overrides_on_config_changed
-         .assert_called_once_with(ANY, "keystone"))
+         .assert_called_once_with(ANY, "keystone", restart_handler=ANY))
 
     @patch.object(hooks, 'is_db_initialised')
     @patch('keystone_utils.log')
@@ -620,7 +620,7 @@ class KeystoneRelationTests(CharmTestCase):
         mock_bootstrap_keystone.assert_called_once_with(configs=ANY)
         mock_stop_manager_instance.assert_called_once_with()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
         mock_protect_service_accounts.assert_called_once_with()
 
     @patch.object(hooks, 'bootstrap_keystone')
@@ -661,7 +661,7 @@ class KeystoneRelationTests(CharmTestCase):
         mock_bootstrap_keystone.assert_called_once_with(configs=ANY)
         mock_stop_manager_instance.assert_called_once_with()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
         mock_protect_service_accounts.assert_called_once_with()
 
     @patch.object(hooks, 'bootstrap_keystone')
@@ -836,7 +836,7 @@ class KeystoneRelationTests(CharmTestCase):
         mock_bootstrap_keystone.assert_called_once_with(configs=ANY)
         mock_stop_manager_instance.assert_called_once()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
 
     @patch.object(hooks, 'bootstrap_keystone')
     @patch.object(hooks, 'maybe_do_policyd_overrides')
@@ -866,7 +866,7 @@ class KeystoneRelationTests(CharmTestCase):
         mock_bootstrap_keystone.assert_called_once_with(configs=ANY)
         mock_stop_manager_instance.assert_called_once()
         mock_maybe_do_policyd_overrides.assert_called_once_with(
-            ANY, "keystone")
+            ANY, "keystone", restart_handler=ANY)
 
     def test_domain_backend_changed_v2(self):
         self.get_api_version.return_value = 2