Merge lp:~stub/charms/precise/postgresql/manual-replication into lp:charms/postgresql

[Stuart Bishop]

Allow replication to be handled manually, rather than Juju controlling it.

The main use of this is to allow replication to be setup between non-Juju PostgreSQL databases and charmed databases, such as the pending migration of the Launchpad database from its legacy servers to new MaaS and Juju controlled servers.

I also have a work in progress for cascading replication, where a PostgreSQL service can be configured to replicate to/from another PostgreSQL service. This too could be used to integrate non-Juju and charmed databases (or cross Juju environment), but it is a much more complex setup and lacks the flexibility of just allowing an experienced admin to dive in and handle it themselves.

[Adam Israel (aisrael)]

Hey Stuart,

I had the opportunity to review this MP. The changes here look to be solid. Nice work, once again.

There is one minor lint error preventing me from marking this as approved, though. Once that is fixed, the test passes cleanly.

hooks/hooks.py:1148:9: E128 continuation line under-indented for visual indent

Thanks again for submitting this!

review: Needs Fixing

[lp:~stub/charms/precise/postgresql/manual-replication]

108. By Stuart Bishop on 2014-12-04

Merge trunk

109. By Stuart Bishop on 2014-12-04

delint and typo

[Stuart Bishop (stub)]

Thanks. The lint leaked in from trunk. I've fixed it and corrected a minor spelling error in a comment.

[Adam Israel (aisrael)]

Thanks for the quick fix! I've tested and confirmed that the tests now pass cleanly. +1 from me.

review: Approve

[Charles Butler (lazypower)]

+1

Thanks for the merge and diligence on getting this landed Stub. I'm approving and merging this - I left a note on IRC, and re-posting here for clarity.

<lazyPower> Looking over your merge against precise postgres charm - does this warrant additional action against the trusty charm as well when accepted for precise? https://code.launchpad.net/~stub/charms/precise/postgresql/manual-replication/+merge/240556
<lazyPower> or is this scoped strictly to precise

review: Approve

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10922-results

review: Needs Fixing (automated testing)

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-10927-results

review: Needs Fixing (automated testing)

=== modified file 'config.yaml'
--- config.yaml	2014-11-11 06:02:03 +0000
+++ config.yaml	2014-12-04 17:50:16 +0000
@@ -331,7 +331,22 @@
         This will be written to the pg_hba.conf file, one line per rule.
         Note that this should not be needed as db relations already create
         those rules the right way. Only use this if you really need too
-        (e.g. on a development environment)
+        (e.g. on a development environment), or are connecting juju managed
+        databases to external managed systems, or configuring replication
+        between unrelated PostgreSQL services using the manual_replication
+        option.
+  manual_replication:
+    type: boolean
+    default: False
+    description: |
+        Enable or disable charm managed replication. When manual_replication
+        is True, the operator is responsible for maintaining recovery.conf
+        and performing any necessary database mirroring. The charm will
+        still advertise the unit as standalone, master or hot standby to
+        relations based on whether the system is in recovery mode or not.
+        Note that this option makes it possible to create a PostgreSQL
+        service with multiple master units, which is probably a very silly
+        thing to do.
   backup_dir:
     default: "/var/lib/postgresql/backups"
     type: string

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-11-11 06:02:03 +0000
+++ hooks/hooks.py	2014-12-04 17:50:16 +0000
@@ -646,6 +646,10 @@
 
 
 def create_recovery_conf(master_host, master_port, restart_on_change=False):
+    if hookenv.config('manual_replication'):
+        log('manual_replication; should not be here', CRITICAL)
+        raise RuntimeError('manual_replication; should not be here')
+
     version = pg_version()
     cluster_name = hookenv.config('cluster_name')
     postgresql_cluster_dir = os.path.join(
@@ -1087,6 +1091,21 @@
         return False
 
 
+def reset_manual_replication_state():
+    '''In manual replication mode, the state of the local database cluster
+    is outside of Juju's control. We need to detect and update the charm
+    state to match reality.
+    '''
+    if hookenv.config('manual_replication'):
+        if os.path.exists('recovery.conf'):
+            local_state['state'] = 'hot standby'
+        elif slave_count():
+            local_state['state'] = 'master'
+        else:
+            local_state['state'] = 'standalone'
+        local_state.publish()
+
+
 @hooks.hook()
 def config_changed(force_restart=False, mount_point=None):
     validate_config()
@@ -1109,6 +1128,8 @@
                 "(config_changed_volume_apply failure)", ERROR)
             sys.exit(1)
 
+    reset_manual_replication_state()
+
     postgresql_config_dir = _get_postgresql_config_dir(config_data)
     postgresql_config = os.path.join(postgresql_config_dir, "postgresql.conf")
     postgresql_hba = os.path.join(postgresql_config_dir, "pg_hba.conf")
@@ -1124,7 +1145,7 @@
     update_service_port()
     update_nrpe_checks()
     write_metrics_cronjob('/usr/local/bin/postgres_to_statsd.py',
-        '/etc/cron.d/postgres_metrics')
+                          '/etc/cron.d/postgres_metrics')
 
     # If an external mountpoint has caused an old, existing DB to be
     # mounted, we need to ensure that all the users, databases, roles
@@ -1578,6 +1599,7 @@
 
 @hooks.hook('db-relation-joined', 'db-relation-changed')
 def db_relation_joined_changed():
+    reset_manual_replication_state()
     if local_state['state'] == 'hot standby':
         publish_hot_standby_credentials()
         return
@@ -1630,6 +1652,7 @@
 
 @hooks.hook('db-admin-relation-joined', 'db-admin-relation-changed')
 def db_admin_relation_joined_changed():
+    reset_manual_replication_state()
     if local_state['state'] == 'hot standby':
         publish_hot_standby_credentials()
         return
@@ -2001,6 +2024,10 @@
         authorized_units.add(unit)
     local_state['authorized'] = authorized_units
 
+    if hookenv.config('manual_replication'):
+        log('manual_replication, nothing to do')
+        return
+
     master = elected_master()
 
     # Handle state changes:
@@ -2150,18 +2177,21 @@
 
         # Block until users and database has replicated, so we know the
         # connection details we publish are actually valid. This will
-        # normally be pretty much instantaneous.
-        timeout = 60
-        start = time.time()
-        while time.time() < start + timeout:
-            cur = db_cursor(autocommit=True)
-            cur.execute('select datname from pg_database')
-            if cur.fetchone() is not None:
-                break
-            del cur
-            log('Waiting for database {} to be replicated'.format(
-                connection_settings['database']))
-            time.sleep(10)
+        # normally be pretty much instantaneous. Do not block if we are
+        # running in manual replication mode, as it is outside of juju's
+        # control when replication is actually setup and running.
+        if not hookenv.config('manual_replication'):
+            timeout = 60
+            start = time.time()
+            while time.time() < start + timeout:
+                cur = db_cursor(autocommit=True)
+                cur.execute('select datname from pg_database')
+                if cur.fetchone() is not None:
+                    break
+                del cur
+                log('Waiting for database {} to be replicated'.format(
+                    connection_settings['database']))
+                time.sleep(10)
 
         log("Relation {} connection settings {!r}".format(
             client_relation, connection_settings), DEBUG)

=== modified file 'test.py'
--- test.py	2014-10-14 09:12:29 +0000
+++ test.py	2014-12-04 17:50:16 +0000
@@ -209,10 +209,11 @@
                             '{} not yet authorized by {} ({})'.format(
                                 psql_unit, unit, allowed_units))
 
+                # Alas, this is no longer true with manual replication.
                 # We must not have multiple masters in this relation.
-                if len(rel_pg_units) > 1 and num_masters != 1:
-                    raise NotReady(
-                        '{} masters'.format(num_masters))
+                # if len(rel_pg_units) > 1 and num_masters != 1:
+                #     raise NotReady(
+                #         '{} masters'.format(num_masters))
 
         if pg_units != all_rel_pg_units:
             raise NotReady(
@@ -271,8 +272,9 @@
             elif peer_state != 'hot standby':
                 raise NotReady('Peer {} in state {}'.format(peer, peer_state))
 
-        if num_masters != 1:
-            raise NotReady('No masters seen from {}'.format(pg_unit))
+        # No longer true with manual replication.
+        # if num_masters < 1:
+        #     raise NotReady('No masters seen from {}'.format(pg_unit))
 
     def sql(self, sql, postgres_unit=None, psql_unit=None, dbname=None):
         '''Run some SQL on postgres_unit from psql_unit.
@@ -896,6 +898,29 @@
         standby_data = self.sql('SELECT * FROM Foo', 'hot standby')[0][0]
         self.assertTrue(standby_data)
 
+    def test_manual_replication(self):
+        self.pg_config['manual_replication'] = True
+        self.juju.deploy(
+            TEST_CHARM, 'postgresql', num_units=2, config=self.pg_config)
+        self.juju.deploy(PSQL_CHARM, 'psql')
+        self.juju.do(['add-relation', 'postgresql:db', 'psql:db'])
+        pg_units = ['postgresql/0', 'postgresql/1']
+        self.wait_until_ready(pg_units)
+
+        # Because we have enabled manual_replication mode, and not
+        # actually manually configured replication, we will have
+        # two masters.
+        self.assertTrue(self.is_master('postgresql/0'))
+        self.assertTrue(self.is_master('postgresql/1'))
+
+        # Both units are advertised as master on the relation.
+        rel_info = self.juju.relation_info('psql/0')
+        for relname in rel_info:
+            for relid in rel_info[relname]:
+                for unit in pg_units:
+                    self.assertEqual(rel_info[relname][relid][unit]['state'],
+                                     'master')
+
 
 class PG91Tests(
         PostgreSQLCharmBaseTestCase,