From 7c065062d292b3f87254deb4333623a5b967e276 Mon Sep 17 00:00:00 2001
From: David Ames <david.ames@canonical.com>
Date: Wed, 8 Nov 2017 18:40:12 +0000
Subject: [PATCH] Ensure HTTPS configuration completes

There was a race where the https apache2 site,
openstack_https_frontend.conf, would be rendered in one hook, then
subsequently the config-changed hook would run and enable that site.
However, the subsequent config-changed hook would see the template as
having not changed and therefore it would fail to restart apache2.
This lead to apache2 failing to listen on the correct ports.

This was due to CONFIGS.write_all() being called but a2ensite not
being called. This change fixes this race and adds a call to
configure_https() to ensure the configuration completes and apache2
is restarted.

Change-Id: I229d25c707a0630c9d609fd20a962a0de2e42c77
Closes-Bug: #1723892
---
 hooks/keystone_hooks.py           |  3 ++-
 unit_tests/test_keystone_hooks.py | 19 ++++++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py
index f43c0259..f2bd35d5 100755
--- a/hooks/keystone_hooks.py
+++ b/hooks/keystone_hooks.py
@@ -372,9 +372,10 @@ def pgsql_db_joined():
 
 
 def update_all_identity_relation_units(check_db_ready=True):
-    CONFIGS.write_all()
     if is_unit_paused_set():
         return
+    CONFIGS.write_all()
+    configure_https()
     if check_db_ready and not is_db_ready():
         log('Allowed_units list provided and this unit not present',
             level=INFO)
diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py
index a19b94bb..f90af736 100644
--- a/unit_tests/test_keystone_hooks.py
+++ b/unit_tests/test_keystone_hooks.py
@@ -1137,6 +1137,7 @@ class KeystoneRelationTests(CharmTestCase):
         self.assertFalse(self.migrate_database.called)
         self.assertFalse(update.called)
 
+    @patch.object(hooks, 'configure_https')
     @patch.object(hooks, 'admin_relation_changed')
     @patch.object(hooks, 'identity_credentials_changed')
     @patch.object(hooks, 'identity_changed')
@@ -1146,7 +1147,8 @@ class KeystoneRelationTests(CharmTestCase):
                                                 is_db_initialized,
                                                 identity_changed,
                                                 identity_credentials_changed,
-                                                admin_relation_changed):
+                                                admin_relation_changed,
+                                                configure_https):
         """ Verify all identity relations are updated """
         is_db_initialized.return_value = True
         self.relation_ids.return_value = ['identity-relation:0']
@@ -1168,8 +1170,9 @@ class KeystoneRelationTests(CharmTestCase):
         admin_relation_changed.assert_called_with('identity-relation:0')
         self.log.assert_has_calls(log_calls, any_order=True)
 
+    @patch.object(hooks, 'configure_https')
     @patch.object(hooks, 'CONFIGS')
-    def test_update_all_db_not_ready(self, configs):
+    def test_update_all_db_not_ready(self, configs, configure_https):
         """ Verify update identity relations when DB is not ready """
         self.is_db_ready.return_value = False
         hooks.update_all_identity_relation_units(check_db_ready=True)
@@ -1179,9 +1182,11 @@ class KeystoneRelationTests(CharmTestCase):
                                     'unit not present', level='INFO')
         self.assertFalse(self.relation_ids.called)
 
+    @patch.object(hooks, 'configure_https')
     @patch.object(hooks, 'is_db_initialised')
     @patch.object(hooks, 'CONFIGS')
-    def test_update_all_db_not_initializd(self, configs, is_db_initialized):
+    def test_update_all_db_not_initializd(self, configs, is_db_initialized,
+                                          configure_https):
         """ Verify update identity relations when DB is not initialized """
         is_db_initialized.return_value = False
         hooks.update_all_identity_relation_units(check_db_ready=False)
@@ -1192,9 +1197,11 @@ class KeystoneRelationTests(CharmTestCase):
                                     level='INFO')
         self.assertFalse(self.relation_ids.called)
 
+    @patch.object(hooks, 'configure_https')
     @patch.object(hooks, 'is_db_initialised')
     @patch.object(hooks, 'CONFIGS')
-    def test_update_all_leader(self, configs, is_db_initialized):
+    def test_update_all_leader(self, configs, is_db_initialized,
+                               configure_https):
         """ Verify update identity relations when the leader"""
         self.is_elected_leader.return_value = True
         is_db_initialized.return_value = True
@@ -1204,9 +1211,11 @@ class KeystoneRelationTests(CharmTestCase):
         # Still updates relations
         self.assertTrue(self.relation_ids.called)
 
+    @patch.object(hooks, 'configure_https')
     @patch.object(hooks, 'is_db_initialised')
     @patch.object(hooks, 'CONFIGS')
-    def test_update_all_not_leader(self, configs, is_db_initialized):
+    def test_update_all_not_leader(self, configs, is_db_initialized,
+                                   configure_https):
         """ Verify update identity relations when not the leader"""
         self.is_elected_leader.return_value = False
         is_db_initialized.return_value = True