Merge lp:~clint-fewbar/charms/precise/mysql/fix-db-relation-idempotency into lp:charms/mysql

[Clint Byrum]

Often when one removes/adds the db relations from mysql, the add fails to work because the joined hook considers an existing database a reason to not do grants/relation-set. This is a big problem though, as it makes using another database service with the same data in it fail. This fix corrects that.

[Clint Byrum (clint-fewbar)]

Note that this was retracted the first time a bit while I figured out a better scheme.

=== modified file 'hooks/common.py'
--- hooks/common.py	2012-11-02 06:43:18 +0000
+++ hooks/common.py	2012-11-02 06:43:18 +0000
@@ -7,21 +7,22 @@
 import uuid
 
 def get_service_user_file(service):
-    return '/var/lib/juju/%s.service_user' % service
+    return '/var/lib/juju/%s.service_user2' % service
 
 
 def get_service_user(service):
     if service == '':
-        return ''
-    sfile = '/var/lib/juju/%s.service_user' % service
+        return (None, None)
+    sfile = get_service_user_file(service)
     if os.path.exists(sfile):
         with open(sfile, 'r') as f:
-            return f.readline().strip()
-    suser = subprocess.check_output(['pwgen', '-N 1', '15']).strip().split("\n")[0]
+            return (f.readline().strip(), f.readline().strip())
+    (suser, service_password) = subprocess.check_output(['pwgen', '-N 2', '15']).strip().split("\n")
     with open(sfile, 'w') as f:
         f.write("%s\n" % suser)
+        f.write("%s\n" % service_password)
         f.flush()
-    return suser
+    return (suser, service_password)
 
 
 def cleanup_service_user(service):
@@ -37,13 +38,16 @@
 database_name = ''
 if change_unit:
     database_name, _ = change_unit.split("/")
+    with open(database_name_file, 'w') as dbnf:
+        dbnf.write("%s\n" % database_name)
+        dbnf.flush()
 elif os.path.exists(database_name_file):
     with open(database_name_file, 'r') as dbname:
-        database_name = dbname.readline()
+        database_name = dbname.readline().strip()
 else:
-    print 'No established database and no REMOTE_UNIT. Exitting gracefully.'
+    print 'No established database and no REMOTE_UNIT.'
 # A user per service unit so we can deny access quickly
-user = get_service_user(database_name)
+user, service_password = get_service_user(database_name)
 connection = None
 lastrun_path = '/var/lib/juju/%s.%s.lastrun' % (database_name,user)
 slave_configured_path = '/var/lib/juju.slave.configured.for.%s' % database_name
 
=== modified file 'hooks/db-relation-joined'
--- hooks/db-relation-joined	2012-04-25 08:23:44 +0000
+++ hooks/db-relation-joined	2012-11-02 06:43:18 +0000
@@ -18,24 +18,6 @@
 	print "[%s]" % sql
 	cursor.execute(sql)
 
-# Determine if we need to create a new database
-if slave and slave_configured and not broken:
-  print "%s exists, assuming configuration done" % slave_configured_path
-  sys.exit(0)
-
-if not slave and not broken and not admin:
-  # Find existing databases
-  cursor.execute("show databases")
-  databases = [i[0] for i in cursor.fetchall()]
-  if database_name in databases:
-    print "database exists, assuming configuration has happened already"
-    sys.exit(0)
-
-# Database is created just before relation-set in case other steps fail
-
-# Create database user and grant access
-service_password = "".join(random.sample(string.letters, 10))
-
 runsql(
     "grant replication client on *.* to `%s` identified by '%s'"  % (
     user,
@@ -78,11 +60,17 @@
 # Create new database or touch slave.configured file
 if slave:
     open(slave_configured_path,'w').close()
-else:
-    if not broken and not admin:
-        runsql("create database `%s` character set utf8" % database_name)
-        with open(database_name_file, 'w') as dbname:
-            dbname.write(database_name)
+elif not broken and not admin:
+  # Find existing databases
+  cursor.execute("show databases")
+  databases = [i[0] for i in cursor.fetchall()]
+  if database_name in databases:
+      print "database exists already"
+  else:
+      if not broken and not admin:
+          runsql("create database `%s` character set utf8" % database_name)
+          with open(database_name_file, 'w') as dbname:
+              dbname.write(database_name)
 
 if broken:
     os.unlink(broken_path)
@@ -97,4 +85,3 @@
      "password=%s" % service_password,
      'host=%s' % hostname,
      'slave=%s' % slave,])
-
 
=== modified file 'hooks/master-relation-changed'
--- hooks/master-relation-changed	2012-11-02 06:43:18 +0000
+++ hooks/master-relation-changed	2012-11-02 06:43:18 +0000
@@ -21,7 +21,7 @@
         fi
         if [ "$runit" = "$JUJU_REMOTE_UNIT" ] ; then
             remote_ip=$rhost
-            if ch_is_ip $remote_ip ; else
+            if ! ch_is_ip $remote_ip ; then
                 remote_ip=$(ch_get_ip $remote_ip)
             fi
         fi
@@ -66,7 +66,8 @@
 chown -v -R www-data.www-data /var/www
 
 user=${JUJU_REMOTE_UNIT%%/*}
-action=${0##master-relation-}
+action=$(basename $0)
+action=${action##master-relation-}
 case "$action" in
 changed)
     mysql $ROOTARGS -e "GRANT REPLICATION SLAVE, REPLICATION CLIENT ON *.* TO \`$user\`@\`$remote_ip\` IDENTIFIED BY '$pass'"
 
=== modified file 'hooks/upgrade-charm'
--- hooks/upgrade-charm	2011-12-06 21:17:17 +0000
+++ hooks/upgrade-charm	2012-11-02 06:43:18 +0000
@@ -1,4 +1,11 @@
 #!/bin/sh
 home=`dirname $0`
+# Remove any existing .service_user files, which will cause
+# new users/pw's to be generated, which is a good thing
+old_service_user_files=$(ls /var/lib/juju/$.service_user)
+if [ -n "$old_service_user_files" ] ; then
+    juju-log -l WARNING "Stale users left around, should be revoked: $(cat $old_service_user_files)"
+    rm -f $old_service_user_files
+fi
 $home/install
 exec $home/config-changed
 
=== modified file 'revision'
--- revision	2012-11-02 06:43:18 +0000
+++ revision	2012-11-02 06:43:18 +0000
@@ -1,1 +1,1 @@
-156
+164