Merge lp:~gnuoy/charms/trusty/mysql/lp-1353135 into lp:charms/trusty/mysql

[Liam Young]

Set a list of allowed_units which the client units can query to see if their permissions have been setup yet. The reason for this is to allow the client to work around a race condition which occurs when a service has multiple units:

1) A service consisting of multiple units joins mysql and triggers mysql to run shared-db-changed for each unit in turn. But as soon as shared-db-changed has been run for the *first* time mysql runs relation set publishing the password.

2) All units in the client service then run their shared-db-changed hook in response to the relation set that mysql issued when it did the *first* run of shared-db-changed. At this point permissions have only been granted to one unit and the rest will fail if they try and access the db.

This is the same behaviour as the postgres charm which was added in response to Bug #1187508

[lp:~gnuoy/charms/trusty/mysql/lp-1353135]

129. By Liam Young on 2014-08-27

Fixed bug causing slave-relation-changed to fail when constructing the allowed list when multiple dbs were specified

[Cory Johns (johnsca)]

Liam,

Thank you for this fix. It looks good and works as expected, though the affected charms will need to be updated to make use of the new field, of course (the charm I was testing this with errored and blocked the list from being fully updated, but one unit did get the full list and another unit got a partial list before being blocked by the other units failing).

review: Approve

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://reports.vapour.ws/charm-tests/charm-bundle-test-962-results

review: Approve (cbt)

=== modified file 'hooks/common.py'
--- hooks/common.py	2014-03-04 17:28:10 +0000
+++ hooks/common.py	2014-08-27 09:57:20 +0000
@@ -84,6 +84,8 @@
 
 def grant_exists(db_name, db_user, remote_ip):
     cursor = get_db_cursor()
+    priv_string = "GRANT ALL PRIVILEGES ON `{}`.* " \
+                  "TO '{}'@'{}'".format(db_name, db_user, remote_ip)
     try:
         cursor.execute("SHOW GRANTS for '{}'@'{}'".format(db_user,
                                                           remote_ip))
@@ -93,7 +95,7 @@
         return False
     finally:
         cursor.close()
-    return "GRANT ALL PRIVILEGES ON `{}`".format(db_name) in grants
+    return priv_string in grants
 
 
 def create_grant(db_name, db_user,

=== modified file 'hooks/shared_db_relations.py'
--- hooks/shared_db_relations.py	2014-06-02 19:52:50 +0000
+++ hooks/shared_db_relations.py	2014-08-27 09:57:20 +0000
@@ -19,6 +19,7 @@
 import os
 import lib.utils as utils
 import lib.cluster_utils as cluster
+from charmhelpers.core import hookenv
 
 LEADER_RES = 'res_mysql_vip'
 
@@ -33,8 +34,30 @@
                                                'json']))
 
 
+def unit_sorted(units):
+    """Return a sorted list of unit names."""
+    return sorted(
+        units, lambda a, b: cmp(int(a.split('/')[-1]), int(b.split('/')[-1])))
+
+
+def get_unit_addr(relid, unitid):
+    return hookenv.relation_get(attribute='private-address',
+                                unit=unitid,
+                                rid=relid)
+
+
 def shared_db_changed():
 
+    def get_allowed_units(database, username):
+        allowed_units = set()
+        for relid in hookenv.relation_ids('shared-db'):
+            for unit in hookenv.related_units(relid):
+                if grant_exists(database,
+                                username,
+                                get_unit_addr(relid, unit)):
+                    allowed_units.add(unit)
+        return allowed_units
+
     def configure_db(hostname,
                      database,
                      username):
@@ -81,12 +104,17 @@
         password = configure_db(settings['hostname'],
                                 settings['database'],
                                 settings['username'])
+        allowed_units = " ".join(unit_sorted(get_allowed_units(
+            settings['database'],
+            settings['username'])))
         if not cluster.is_clustered():
             utils.relation_set(db_host=local_hostname,
-                               password=password)
+                               password=password,
+                               allowed_units=allowed_units)
         else:
             utils.relation_set(db_host=utils.config_get("vip"),
-                               password=password)
+                               password=password,
+                               allowed_units=allowed_units)
 
     else:
         # Process multiple database setup requests.
@@ -115,12 +143,17 @@
                 databases[db] = {}
             databases[db][x] = v
         return_data = {}
+        allowed_units = []
         for db in databases:
             if singleset.issubset(databases[db]):
                 return_data['_'.join([db, 'password'])] = \
                     configure_db(databases[db]['hostname'],
                                  databases[db]['database'],
                                  databases[db]['username'])
+                return_data['_'.join([db, 'allowed_units'])] = \
+                    " ".join(unit_sorted(get_allowed_units(
+                        databases[db]['database'],
+                        databases[db]['username'])))
         if len(return_data) > 0:
             utils.relation_set(**return_data)
         if not cluster.is_clustered():