Merge lp:~stub/charms/trusty/postgresql/fix-bsb into lp:charms/trusty/postgresql

[Stuart Bishop]

Fix the regressions in block storage broker external mounts.

Per Bug #1533502 and Bug #1533503, the request for disk was not being sent to the storage subordinate, and if it was, it would not do anything if an existing database was already in place. With this fixed, it again becomes possible to migrate PostgreSQL databases into fresh environments by remounting them on the new unit.

[lp:~stub/charms/trusty/postgresql/fix-bsb]

142. By Stuart Bishop on 2016-01-13

Fix tests

[Björn Tillenius (bjornt)]

I can't give an official +1, but the code looks good to me, and I've confirmed that it works with one postgresql unit having persistent storage. I haven't gotten it to work when two postgresql units are deployed, but it doesn't look like the postgresql charm is the blame for that.

review: Approve

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/2173/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/2154/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/2188/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/2169/

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/2204/

review: Approve (automated testing)

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/2186/

review: Approve (automated testing)

=== modified file 'hooks/storage.py'
--- hooks/storage.py	2015-08-10 16:23:33 +0000
+++ hooks/storage.py	2016-01-13 11:00:29 +0000
@@ -19,7 +19,7 @@
 import time
 
 from charmhelpers import context
-from charmhelpers.core import hookenv
+from charmhelpers.core import hookenv, host
 from charmhelpers.core.hookenv import DEBUG
 
 from coordinator import coordinator
@@ -42,7 +42,7 @@
         raise SystemExit(0)
     elif data_rels:
         relid, rel = list(data_rels.items())[0]
-        rel['mountpoint'] = external_volume_mount
+        rel.local['mountpoint'] = external_volume_mount
 
     if needs_remount():
         # Migrate any data when we can restart.
@@ -55,12 +55,29 @@
     return mounted and not linked
 
 
+def fix_perms(data_dir):
+    # The path to data_dir must be world readable, so the postgres user
+    # can traverse to it.
+    p = data_dir
+    while p != '/':
+        p = os.path.dirname(p)
+        subprocess.check_call(['chmod', 'a+rX', p], universal_newlines=True)
+
+    # data_dir and all of its contents should be owned by the postgres
+    # user and group.
+    host.chownr(data_dir, 'postgres', 'postgres', follow_links=False)
+
+    # data_dir should not be world readable.
+    os.chmod(data_dir, 0o700)
+
+
 @data_ready_action
 def remount():
     if not needs_remount():
         return
 
     if postgresql.is_running():
+        # Attempting this while PostgreSQL is live would be really, really bad.
         postgresql.stop()
 
     old_data_dir = postgresql.data_dir()
@@ -69,9 +86,11 @@
     backup_data_dir = '{}-{}'.format(old_data_dir, int(time.time()))
 
     if not os.path.isdir(new_data_dir):
+        # If there is not an existing db on the mount, migrate the current
+        # db there.
         hookenv.log('Migrating data from {} to {}'.format(old_data_dir,
                                                           new_data_dir))
-        helpers.makedirs(new_data_dir, mode=0o770,
+        helpers.makedirs(new_data_dir, mode=0o700,
                          user='postgres', group='postgres')
         try:
             rsync_cmd = ['rsync', '-av',
@@ -79,10 +98,12 @@
                          new_data_dir + '/']
             hookenv.log('Running {}'.format(' '.join(rsync_cmd)), DEBUG)
             subprocess.check_call(rsync_cmd)
-            os.replace(old_data_dir, backup_data_dir)
-            os.symlink(new_data_dir, old_data_dir)
         except subprocess.CalledProcessError:
             helpers.status_set('blocked',
                                'Failed to sync data from {} to {}'
                                ''.format(old_data_dir, new_data_dir))
             raise SystemExit(0)
+
+    os.replace(old_data_dir, backup_data_dir)
+    os.symlink(new_data_dir, old_data_dir)
+    fix_perms(new_data_dir)

=== modified file 'tests/test_integration.py'
--- tests/test_integration.py	2015-12-19 08:25:47 +0000
+++ tests/test_integration.py	2016-01-13 11:00:29 +0000
@@ -538,6 +538,29 @@
     test_config = dict(version=(None if SERIES == 'trusty' else '9.3'),
                        pgdg=(False if SERIES == 'trusty' else True))
 
+    def test_mount(self):
+        client_unit = self.deployment.sentry['postgresql'][0].info['unit_name']
+        details = subprocess.check_output(['juju', 'run',
+                                           '--unit', client_unit,
+                                           'stat --format "%A %U %G %N" '
+                                           '/var/lib/postgresql/9.3/main'],
+                                          stderr=subprocess.DEVNULL,
+                                          universal_newlines=True).strip()
+        self.assertEqual(details,
+                         "lrwxrwxrwx root root "
+                         "'/var/lib/postgresql/9.3/main' -> "
+                         "'/srv/data/postgresql/9.3/main'")
+
+        details = subprocess.check_output(['juju', 'run',
+                                           '--unit', client_unit,
+                                           'stat --format "%A %U %G %N" '
+                                           '/srv/data/postgresql/9.3/main'],
+                                          stderr=subprocess.DEVNULL,
+                                          universal_newlines=True).strip()
+        self.assertEqual(details,
+                         "drwx------ postgres postgres "
+                         "'/srv/data/postgresql/9.3/main'")
+
 
 class PG94Tests(PGBaseTestCase, unittest.TestCase):
     test_config = dict(version=(None if SERIES == 'wily' else '9.4'),