From 4666b8024ca5330d1990281b19fa30bdb53007d1 Mon Sep 17 00:00:00 2001
From: David Ames <david.ames@canonical.com>
Date: Fri, 16 Feb 2018 23:36:10 +0000
Subject: [PATCH] Do not set gnocchi URL until clustering complete

Gnocchi was providing its URL to client charms before its VIP was
completely setup. This change checks that an hacluster relation
exists and if so waits to provide its URL until the hacluster setup
is complete.

Depends-On: I23eb5e70537a62d5b9e5e24d09f37519b63a1717
Change-Id: I3a6991ecb4eca8659c08d5c5d00d35b8d22bf79e
Closes-Bug: #1749280
---
 .gitignore                          |  2 ++
 src/reactive/gnocchi_handlers.py    | 18 ++++++++++++++++++
 unit_tests/test_gnocchi_handlers.py | 29 ++++++++++++++++++++++++++++-
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 36567f0..158771a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,3 +8,5 @@ test-charm/
 .project
 .pydevproject
 **/__pycache__
+interfaces
+layers
diff --git a/src/reactive/gnocchi_handlers.py b/src/reactive/gnocchi_handlers.py
index ffe915d..76df707 100644
--- a/src/reactive/gnocchi_handlers.py
+++ b/src/reactive/gnocchi_handlers.py
@@ -54,17 +54,22 @@ def render_config(*args):
         charm_class.render_with_interfaces(args)
         charm_class.enable_webserver_site()
         charm_class.assess_status()
+    hookenv.log("Configuration rendered", hookenv.DEBUG)
     reactive.set_state('config.rendered')
 
 
 # db_sync checks if sync has been done so rerunning is a noop
 @reactive.when('config.rendered')
+@reactive.when_not('db.synced')
 def init_db():
     with charm.provide_charm_instance() as charm_class:
         charm_class.db_sync()
+    hookenv.log("Database synced", hookenv.DEBUG)
+    reactive.set_state('db.synced')
 
 
 @reactive.when('ha.connected')
+@reactive.when_not('ha.available')
 def cluster_connected(hacluster):
     """Configure HA resources in corosync"""
     with charm.provide_charm_instance() as charm_class:
@@ -99,6 +104,19 @@ def storage_ceph_disconnected():
 
 @reactive.when('metric-service.connected')
 @reactive.when('config.rendered')
+@reactive.when('db.synced')
 def provide_gnocchi_url(metric_service):
+    # Reactive endpoints are only fully functional in their respective relation
+    # hooks. So we cannot rely on ha.is_clustered() which would return
+    # False when not in a relation hook. Use flags instead.
+    # Check if the optional relation, ha is connected. If it is,
+    # check if it is available.
+    if (reactive.flags.is_flag_set('ha.connected') and
+            not reactive.flags.is_flag_set('ha.available')):
+        hookenv.log("Hacluster is related but not yet clustered",
+                    hookenv.DEBUG)
+        return
     with charm.provide_charm_instance() as charm_class:
+        hookenv.log("Providing gnocchi URL: {}"
+                    .format(charm_class.public_url), hookenv.DEBUG)
         metric_service.set_gnocchi_url(charm_class.public_url)
diff --git a/unit_tests/test_gnocchi_handlers.py b/unit_tests/test_gnocchi_handlers.py
index 77d8a02..a071137 100644
--- a/unit_tests/test_gnocchi_handlers.py
+++ b/unit_tests/test_gnocchi_handlers.py
@@ -28,6 +28,7 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
         defaults = [
             'charm.installed',
             'shared-db.connected',
+            'ha.connected',
             'identity-service.connected',
             'identity-service.available',
             'config.changed',
@@ -50,6 +51,7 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
                 'provide_gnocchi_url': (
                     'metric-service.connected',
                     'config.rendered',
+                    'db.synced',
                 ),
                 'configure_ceph': (
                     'storage-ceph.available',
@@ -65,6 +67,12 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
                 'disable_services': (
                     'config.rendered',
                 ),
+                'cluster_connected': (
+                    'ha.available',
+                ),
+                'init_db': (
+                    'db.synced',
+                ),
             },
         }
         # test that the hooks were registered via the
@@ -129,7 +137,26 @@ class TestHandlers(test_utils.PatchHelper):
         handlers.storage_ceph_disconnected()
         mock_ceph_helper.delete_keyring.assert_called_once_with('gnocchi')
 
-    def test_provide_gnocchi_url(self):
+    @mock.patch.object(handlers.reactive.flags, 'is_flag_set')
+    def test_provide_gnocchi_url(self, mock_is_flag_set):
+        mock_is_flag_set.return_value = False
+        mock_gnocchi = mock.MagicMock()
+        self.gnocchi_charm.public_url = "http://gnocchi:8041"
+        handlers.provide_gnocchi_url(mock_gnocchi)
+        mock_gnocchi.set_gnocchi_url.assert_called_once_with(
+            "http://gnocchi:8041"
+        )
+
+    @mock.patch.object(handlers.reactive.flags, 'is_flag_set')
+    def test_provide_gnocchi_url_ha_connected(self, mock_is_flag_set):
+        mock_is_flag_set.side_effect = [True, False]
+        mock_gnocchi = mock.MagicMock()
+        handlers.provide_gnocchi_url(mock_gnocchi)
+        mock_gnocchi.set_gnocchi_url.assert_not_called()
+
+    @mock.patch.object(handlers.reactive.flags, 'is_flag_set')
+    def test_provide_gnocchi_url_ha_available(self, mock_is_flag_set):
+        mock_is_flag_set.side_effect = [True, True]
         mock_gnocchi = mock.MagicMock()
         self.gnocchi_charm.public_url = "http://gnocchi:8041"
         handlers.provide_gnocchi_url(mock_gnocchi)