From a92cfd6a8a78016fac887c789b950bd08b572195 Mon Sep 17 00:00:00 2001
From: Liam Young <liam.young@canonical.com>
Date: Mon, 16 Oct 2017 16:38:01 +0000
Subject: [PATCH] Non-leaders set allowed_units down shared-db rel

The allowed_units key needs to be set by all units rather than
just the leader incase the leader dies and the setting is lost.

Change-Id: I0ed687e5615ee1de5ea34c9169e2728fe557886b
Closes-Bug: #1712383
---
 hooks/percona_hooks.py           | 13 +++---
 unit_tests/test_percona_hooks.py | 73 +++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/hooks/percona_hooks.py b/hooks/percona_hooks.py
index d66e646..fe80ef9 100755
--- a/hooks/percona_hooks.py
+++ b/hooks/percona_hooks.py
@@ -528,13 +528,13 @@ def shared_db_changed(relation_id=None, unit=None):
         # with the info dies the settings die with it Bug# 1355848
         if is_relation_made('cluster'):
             for rel_id in relation_ids('shared-db'):
-                peerdb_settings = \
+                client_settings = \
                     peer_retrieve_by_prefix(rel_id, exc_list=['hostname'])
 
-                passwords = [key for key in peerdb_settings.keys()
+                passwords = [key for key in client_settings.keys()
                              if 'password' in key.lower()]
                 if len(passwords) > 0:
-                    relation_set(relation_id=rel_id, **peerdb_settings)
+                    relation_set(relation_id=rel_id, **client_settings)
         return
 
     # Bail if leader is not ready
@@ -579,7 +579,8 @@ def shared_db_changed(relation_id=None, unit=None):
         db_host = get_db_host(hostname)
         peer_store_and_set(relation_id=relation_id,
                            db_host=db_host,
-                           password=password)
+                           password=password,
+                           allowed_units=allowed_units)
     else:
         # Process multiple database setup requests.
         # from incoming relation data:
@@ -632,9 +633,11 @@ def shared_db_changed(relation_id=None, unit=None):
                 a_units = db_helper.get_allowed_units(database, username,
                                                       relation_id=relation_id)
                 a_units = ' '.join(unit_sorted(a_units))
-                allowed_units['%s_allowed_units' % (db)] = a_units
+                allowed_units_key = '%s_allowed_units' % (db)
+                allowed_units[allowed_units_key] = a_units
 
                 return_data['%s_password' % (db)] = password
+                return_data[allowed_units_key] = a_units
                 db_host = get_db_host(hostname)
 
         if allowed_units:
diff --git a/unit_tests/test_percona_hooks.py b/unit_tests/test_percona_hooks.py
index b904888..8e88e73 100644
--- a/unit_tests/test_percona_hooks.py
+++ b/unit_tests/test_percona_hooks.py
@@ -33,7 +33,78 @@ TO_PATCH = ['log', 'config',
             'get_ipv6_addr',
             'get_hacluster_config',
             'update_dns_ha_resource_params',
-            'sst_password']
+            'sst_password',
+            'seeded',
+            'is_leader',
+            'leader_node_is_ready',
+            'get_db_helper',
+            'peer_store_and_set',
+            'leader_get',
+            'relation_clear',
+            'is_relation_made',
+            'peer_retrieve_by_prefix',
+            'client_node_is_ready',
+            'relation_set',
+            'relation_get']
+
+
+class TestSharedDBRelation(CharmTestCase):
+
+    def setUp(self):
+        CharmTestCase.setUp(self, hooks, TO_PATCH)
+        self.network_get_primary_address.side_effect = NotImplementedError
+        self.sst_password.return_value = 'ubuntu'
+
+    def test_allowed_units_non_leader(self):
+        self.seeded.return_value = True
+        self.is_leader.return_value = False
+        self.client_node_is_ready.return_value = True
+        self.is_relation_made.return_value = True
+        self.relation_ids.return_value = ['shared-db:3']
+        self.peer_retrieve_by_prefix.return_value = {
+            'password': 'pass123',
+            'allowed_units': 'keystone/1 keystone/2'}
+        hooks.shared_db_changed()
+        self.relation_set.assert_called_once_with(
+            allowed_units='keystone/1 keystone/2',
+            password='pass123',
+            relation_id='shared-db:3')
+
+    @mock.patch.object(hooks, 'get_db_host')
+    @mock.patch.object(hooks, 'configure_db_for_hosts')
+    def test_allowed_units_leader(self, configure_db_for_hosts, get_db_host):
+        self.config.return_value = None
+        allowed_unit_mock = mock.MagicMock()
+        allowed_unit_mock.get_allowed_units.return_value = [
+            'keystone/1',
+            'keystone/2']
+        self.get_db_helper.return_value = allowed_unit_mock
+        self.test_config.set('access-network', None)
+        self.seeded.return_value = True
+        self.is_leader.return_value = True
+        self.resolve_hostname_to_ip.return_value = '10.0.0.10'
+        self.relation_get.return_value = {
+            'hostname': 'keystone-0',
+            'database': 'keystone',
+            'username': 'keyuser',
+        }
+        get_db_host.return_value = 'dbhost1'
+        configure_db_for_hosts.return_value = 'password'
+        hooks.shared_db_changed()
+        self.relation_set.assert_called_once_with(
+            allowed_units='keystone/1 keystone/2',
+            relation_id=None)
+        calls = [
+            mock.call(
+                relation_id=None,
+                relation_settings={'access-network': None}),
+            mock.call(
+                relation_id=None,
+                db_host='dbhost1',
+                password='password',
+                allowed_units='keystone/1 keystone/2')
+        ]
+        self.peer_store_and_set.assert_has_calls(calls)
 
 
 class TestHARelation(CharmTestCase):