[gnuoy, r=james-page] Rexec config-changed hook after upgrade to avoid issues with packages changing under the feet of the running hook

actions/openstack_upgrade.py

@@ -1,4 +1,5 @@
 #!/usr/bin/python
+import os
 import sys
 
 sys.path.append('hooks/')
@@ -8,7 +9,6 @@ from charmhelpers.contrib.openstack.utils import (
 )
 
 from keystone_hooks import (
-    config_changed,
     CONFIGS,
 )
 
@@ -31,7 +31,7 @@ def openstack_upgrade():
     if (do_action_openstack_upgrade('keystone',
                                     do_openstack_upgrade,
                                     CONFIGS)):
-        config_changed()
+        os.execl('./hooks/config-changed-postupgrade', '')
 
 if __name__ == '__main__':
     openstack_upgrade()

hooks/config-changed-postupgrade

@@ -0,0 +1 @@
+keystone_hooks.py

hooks/keystone_hooks.py

@@ -53,7 +53,7 @@ from charmhelpers.contrib.openstack.utils import (
 from keystone_utils import (
     add_service_to_keystone,
     determine_packages,
-    do_openstack_upgrade,
+    do_openstack_upgrade_reexec,
     ensure_initial_admin,
     get_admin_passwd,
     git_install,
@@ -152,8 +152,15 @@ def config_changed():
     elif not config('action-managed-upgrade'):
         if openstack_upgrade_available('keystone'):
             status_set('maintenance', 'Running openstack upgrade')
-            do_openstack_upgrade(configs=CONFIGS)
+            do_openstack_upgrade_reexec(configs=CONFIGS)
 
+    config_changed_postupgrade()
+
+
+@hooks.hook('config-changed-postupgrade')
+@restart_on_change(restart_map())
+@synchronize_ca_if_changed(fatal=True)
+def config_changed_postupgrade():
     # Ensure ssl dir exists and is unison-accessible
     ensure_ssl_dir()
 

hooks/keystone_utils.py

@@ -370,6 +370,12 @@ def save_script_rc():
     _save_script_rc(**env_vars)
 
 
+def do_openstack_upgrade_reexec(configs):
+    do_openstack_upgrade(configs)
+    log("Re-execing hook to pickup upgraded packages",  level=INFO)
+    os.execl('./hooks/config-changed-postupgrade', '')
+
+
 def do_openstack_upgrade(configs):
     new_src = config('openstack-origin')
     new_os_rel = get_os_codename_install_source(new_src)
@@ -395,7 +401,6 @@ def do_openstack_upgrade(configs):
         else:
             log("Database not ready - deferring to shared-db relation",
                 level=INFO)
-            return
 
 
 def is_db_initialised():

unit_tests/test_actions_openstack_upgrade.py

@@ -12,16 +12,16 @@ from test_utils import (
 )
 
 TO_PATCH = [
-    'config_changed',
     'do_openstack_upgrade',
+    'os',
 ]
 
 
-class TestCinderUpgradeActions(CharmTestCase):
+class TestKeystoneUpgradeActions(CharmTestCase):
 
     def setUp(self):
-        super(TestCinderUpgradeActions, self).setUp(openstack_upgrade,
-                                                    TO_PATCH)
+        super(TestKeystoneUpgradeActions, self).setUp(openstack_upgrade,
+                                                      TO_PATCH)
 
     @patch.object(hooks, 'register_configs')
     @patch('charmhelpers.contrib.openstack.utils.config')
@@ -37,7 +37,8 @@ class TestCinderUpgradeActions(CharmTestCase):
         openstack_upgrade.openstack_upgrade()
 
         self.assertTrue(self.do_openstack_upgrade.called)
-        self.assertTrue(self.config_changed.called)
+        self.os.execl.assert_called_with('./hooks/config-changed-postupgrade',
+                                         '')
 
     @patch.object(hooks, 'register_configs')
     @patch('charmhelpers.contrib.openstack.utils.config')
@@ -53,4 +54,4 @@ class TestCinderUpgradeActions(CharmTestCase):
         openstack_upgrade.openstack_upgrade()
 
         self.assertFalse(self.do_openstack_upgrade.called)
-        self.assertFalse(self.config_changed.called)
+        self.assertFalse(self.os.execl.called)

unit_tests/test_keystone_hooks.py

@@ -51,7 +51,7 @@ TO_PATCH = [
     # keystone_utils
     'restart_map',
     'register_configs',
-    'do_openstack_upgrade',
+    'do_openstack_upgrade_reexec',
     'openstack_upgrade_available',
     'save_script_rc',
     'migrate_database',
@@ -467,7 +467,7 @@ class KeystoneRelationTests(CharmTestCase):
         ensure_user.assert_called_with(user=self.ssh_user, group='keystone')
         get_homedir.assert_called_with(self.ssh_user)
 
-        self.assertTrue(self.do_openstack_upgrade.called)
+        self.assertTrue(self.do_openstack_upgrade_reexec.called)
 
         self.save_script_rc.assert_called_with()
         configure_https.assert_called_with()
@@ -538,7 +538,7 @@ class KeystoneRelationTests(CharmTestCase):
         hooks.config_changed()
         self.git_install.assert_called_with(projects_yaml)
         self.assertFalse(self.openstack_upgrade_available.called)
-        self.assertFalse(self.do_openstack_upgrade.called)
+        self.assertFalse(self.do_openstack_upgrade_reexec.called)
 
     @patch.object(hooks, 'git_install_requested')
     @patch.object(hooks, 'config_value_changed')
@@ -569,7 +569,7 @@ class KeystoneRelationTests(CharmTestCase):
 
         hooks.config_changed()
 
-        self.assertFalse(self.do_openstack_upgrade.called)
+        self.assertFalse(self.do_openstack_upgrade_reexec.called)
 
     @patch('keystone_utils.log')
     @patch('keystone_utils.ensure_ssl_cert_master')