Merge lp:~stub/charms/precise/postgresql/cleanups into lp:charms/postgresql

[Stuart Bishop]

Update documentation and code cleanups.

[lp:~stub/charms/precise/postgresql/cleanups]

72. By Stuart Bishop on 2013-08-23

Add missing template

[Mark Mims (mark-mims)]

review: Approve

=== modified file 'README.md'
--- README.md	2013-08-07 13:50:02 +0000
+++ README.md	2013-08-23 09:40:09 +0000
@@ -41,15 +41,6 @@
    maintains replication for you, using standard PostgreSQL streaming
    replication.
 
- - Multiple services linked using 'master'/'slave' relationships. A
-   single service can be the 'master', and multiple services connected
-   to this master in a 'slave' role. Each service can contain multiple
-   units; the 'master' service will contain a single 'master' unit and
-   remaining units all 'hot standby'. The 'slave' services will only
-   contain 'hot standby' units. 'Cascading replication is not
-   supported', so do not attempt to relate an existing 'slave' service
-   as a 'master' to another service.
-
 
 To setup a single 'standalone' service::
 
@@ -61,41 +52,34 @@
 
     juju add-unit pg-a
 
-To deploy a new service containing a 'master' and a 'hot standby'::
-
-    juju deploy -n 2 postgresql pg-b
-
-
-To relate a PostgreSQL service as a 'slave' of another PostgreSQL service.
-**Caution** - this destroys the existing databases in the pg-b service::
-
-    juju add-relation pg-a:master pg-b:slave
-
-
-To setup a client using a PostgreSQL database, in this case OpenERP and
-its web front end. Note that OpenERP requires an administrative level
-connection::
+To deploy a new service containing a 'master' and two 'hot standbys'::
+
+    juju deploy -n 3 postgresql pg-b
+
+You can remove units as normal. If the master unit is removed, failover
+occurs and the most up to date 'hot standby' is promoted to 'master'.
+The 'db-relation-changed' and 'db-admin-relation-changed' hooks are
+fired, letting clients adjust::
+
+    juju remove-unit pg-b/0
+
+
+To setup a client using a PostgreSQL database, in this case a vanilla
+Django installation listening on port 8080::
 
     juju deploy postgresql
-    juju deploy postgresql pg-standby
-    juju deploy openerp-web
-    juju deploy openerp-server
-
-    juju add-relation postgresql:master pg-standby:slave
-    juju add-relation openerp-server:db postgresql:db-admin
-    juju add-relation openerp-web openerp-server
-
-    juju expose openerp-web
-    juju expose openerp-server
+    juju deploy python-django
+    juju deploy gunicorn
+    juju add-relation python-django postgresql:db
+    juju add-relation python-django gunicorn
+    juju expose python-django
 
 
 ## Restrictions
 
 - Do not attempt to relate client charms to a PostgreSQL service
   containing multiple units unless you know the charm supports
-  a replicated service. You can use a 'master'/'slave' relationship
-  to create a redundant copy of your database until the client charms
-  are updated.
+  a replicated service.
 
 - You cannot host multiple units in a single juju container. This is
   problematic as some PostgreSQL features, such as tablespaces, use
@@ -103,10 +87,23 @@
 
 # Interacting with the Postgresql Service
 
-Typically, you just need to join a the `db` relation, and a user and database
-will be created for you.  For more advanced uses, you can join the `db-admin`
-relation, and a super user will be created.  Using this account, you can
-manipulate all other aspects of the database.
+At a minimum, you just need to join a the `db` relation, and a user and
+database will be created for you.  For more complex environments, 
+you can provide the `database` name allowing multiple services to share
+the same database. A client may also wish to defer its setup until the
+unit name is listed in `allowed-units`, to avoid attempting to connect
+to a database before it has been authorized.
+
+The `db-admin` relation may be used similarly to the `db` relation.
+The automatically generated user for `db-admin` relations is a
+PostgreSQL superuser.
+
+## During db-relation-joined
+
+### the client service provides:
+
+- `database`: Optional. The name of the database to use. The postgresql
+              service will create it if necessary.
 
 ## During db-relation-changed
 

=== modified file 'charm-helpers.yaml'
--- charm-helpers.yaml	2013-08-23 09:40:09 +0000
+++ charm-helpers.yaml	2013-08-23 09:40:09 +0000
@@ -1,4 +1,4 @@
 destination: hooks/charmhelpers
-branch: lp:charm-helpers
+branch: lp:~stub/charm-helpers/bug-1214793-service-wrappers
 include:
     - core

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2013-08-23 09:40:09 +0000
+++ hooks/charmhelpers/core/host.py	2013-08-23 09:40:09 +0000
@@ -20,20 +20,22 @@
 
 
 def service_start(service_name):
-    service('start', service_name)
+    return service('start', service_name)
 
 
 def service_stop(service_name):
-    service('stop', service_name)
+    return service('stop', service_name)
 
 
 def service_restart(service_name):
-    service('restart', service_name)
+    return service('restart', service_name)
 
 
 def service_reload(service_name, restart_on_failure=False):
-    if not service('reload', service_name) and restart_on_failure:
-        service('restart', service_name)
+    service_result = service('reload', service_name)
+    if not service_result and restart_on_failure:
+        service_result = service('restart', service_name)
+    return service_result
 
 
 def service(action, service_name):

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2013-08-23 09:40:09 +0000
+++ hooks/hooks.py	2013-08-23 09:40:09 +0000
@@ -32,20 +32,13 @@
     return Template(*args, **kw)
 
 
-def write_file(path, contents, owner='root', group='root', perms=0o444):
-    '''Temporary alternative to charm-helpers write_file().
-
-    charm-helpers' write_file() magic makes it useless for any file
-    containing curly brackets, so work around for now until the feature
-    can be discussed.
-    '''
-    log("Writing file {} {}:{} {:o}".format(path, owner, group, perms))
-    uid = getpwnam(owner).pw_uid
-    gid = getgrnam(group).gr_gid
-    dest_fd = os.open(path, os.O_WRONLY | os.O_TRUNC | os.O_CREAT, perms)
-    os.fchown(dest_fd, uid, gid)
-    with os.fdopen(dest_fd, 'w') as destfile:
-        destfile.write(str(contents))
+def log(msg, lvl=INFO):
+    # Per Bug #1208787, log messages sent via juju-log are being lost.
+    # Spit messages out to a log file to work around the problem.
+    myname = hookenv.local_unit().replace('/', '-')
+    with open('/tmp/{}-debug.log'.format(myname), 'a') as f:
+        f.write('{}: {}\n'.format(lvl, msg))
+    hookenv.log(msg, lvl)
 
 
 class State(dict):
@@ -87,8 +80,6 @@
 
         replication_state = dict(client_state)
 
-        add(replication_state, 'public_ssh_key')
-        add(replication_state, 'ssh_host_key')
         add(replication_state, 'replication_password')
         add(replication_state, 'wal_received_offset')
         add(replication_state, 'following')
@@ -189,30 +180,18 @@
     return output
 
 
-#------------------------------------------------------------------------------
-# Enable/disable service start by manipulating policy-rc.d
-#------------------------------------------------------------------------------
-def enable_service_start(service):
-    ### NOTE: doesn't implement per-service, this can be an issue
-    ###       for colocated charms (subordinates)
-    log("enabling {} start by policy-rc.d".format(service))
-    if os.path.exists('/usr/sbin/policy-rc.d'):
-        os.unlink('/usr/sbin/policy-rc.d')
-        return True
-    return False
-
-
-def disable_service_start(service):
-    log("disabling {} start by policy-rc.d".format(service))
-    policy_rc = '/usr/sbin/policy-rc.d'
-    policy_rc_tmp = "{}.tmp".format(policy_rc)
-    open(policy_rc_tmp, 'w').write("""#!/bin/bash
-[[ "$1"-"$2" == %s-start ]] && exit 101
-exit 0
-EOF
-""" % service)
-    os.chmod(policy_rc_tmp, 0755)
-    os.rename(policy_rc_tmp, policy_rc)
+def postgresql_autostart(enabled):
+    if enabled:
+        log("Enabling PostgreSQL startup in {}".format(startup_file))
+        mode = 'auto'
+    else:
+        log("Disabling PostgreSQL startup in {}".format(startup_file))
+        mode = 'manual'
+    startup_file = os.path.join(postgresql_config_dir, 'start.conf')
+    contents = Template(open("templates/start_conf.tmpl").read()).render(
+        {'mode': mode})
+    host.write_file(
+        startup_file, contents, 'postgres', 'postgres', perms=0o644)
 
 
 def run(command, exit_on_error=True):
@@ -229,10 +208,6 @@
             raise
 
 
-#------------------------------------------------------------------------------
-# postgresql_stop, postgresql_start, postgresql_is_running:
-# wrappers over invoke-rc.d, with extra check for postgresql_is_running()
-#------------------------------------------------------------------------------
 def postgresql_is_running():
     # init script always return true (9.1), add extra check to make it useful
     status, output = commands.getstatusoutput("invoke-rc.d postgresql status")
@@ -244,17 +219,12 @@
 
 
 def postgresql_stop():
-    status, output = commands.getstatusoutput("invoke-rc.d postgresql stop")
-    if status != 0:
-        return False
+    host.service_stop('postgresql')
     return not postgresql_is_running()
 
 
 def postgresql_start():
-    status, output = commands.getstatusoutput("invoke-rc.d postgresql start")
-    if status != 0:
-        log(output, CRITICAL)
-        return False
+    host.service_start('postgresql')
     return postgresql_is_running()
 
 
@@ -275,12 +245,9 @@
                 last_warning = time.time()
             time.sleep(5)
 
-        status, output = \
-            commands.getstatusoutput("invoke-rc.d postgresql restart")
-        if status != 0:
-            return False
+        return host.service_restart('postgresql')
     else:
-        postgresql_start()
+        return host.service_start('postgresql')
 
     # Store a copy of our known live configuration so
     # postgresql_reload_or_restart() can make good choices.
@@ -406,7 +373,7 @@
     # Return it as pg_config
     pg_config = Template(
         open("templates/postgresql.conf.tmpl").read()).render(config_data)
-    write_file(
+    host.write_file(
         postgresql_config, pg_config,
         owner="postgres",  group="postgres", perms=0600)
 
@@ -419,7 +386,7 @@
     ident_data = {}
     pg_ident_template = Template(
         open("templates/pg_ident.conf.tmpl").read())
-    write_file(
+    host.write_file(
         postgresql_ident, pg_ident_template.render(ident_data),
         owner="postgres", group="postgres", perms=0600)
 
@@ -520,7 +487,7 @@
         relation_data.append(local_replication)
 
     pg_hba_template = Template(open("templates/pg_hba.conf.tmpl").read())
-    write_file(
+    host.write_file(
         postgresql_hba, pg_hba_template.render(access_list=relation_data),
         owner="postgres", group="postgres", perms=0600)
     postgresql_reload()
@@ -542,7 +509,7 @@
     }
     crontab_template = Template(
         open("templates/postgres.cron.tmpl").read()).render(crontab_data)
-    write_file('/etc/cron.d/postgres', crontab_template, perms=0600)
+    host.write_file('/etc/cron.d/postgres', crontab_template, perms=0600)
 
 
 def create_recovery_conf(master_host, restart_on_change=False):
@@ -557,7 +524,7 @@
             'host': master_host,
             'password': local_state['replication_password']})
     log(recovery_conf, DEBUG)
-    write_file(
+    host.write_file(
         os.path.join(postgresql_cluster_dir, 'recovery.conf'),
         recovery_conf, owner="postgres", group="postgres", perms=0o600)
 
@@ -782,7 +749,7 @@
     volid = volume_get_volume_id()
     if not volid:
         ## Invalid configuration (whether ephemeral, or permanent)
-        disable_service_start("postgresql")
+        postgresql_autostart(False)
         postgresql_stop()
         mounts = volume_get_all_mounted()
         if mounts:
@@ -797,9 +764,9 @@
         ## config_changed_volume_apply will stop the service if it founds
         ## it necessary, ie: new volume setup
         if config_changed_volume_apply():
-            enable_service_start("postgresql")
+            postgresql_autostart(True)
         else:
-            disable_service_start("postgresql")
+            postgresql_autostart(False)
             postgresql_stop()
             mounts = volume_get_all_mounted()
             if mounts:
@@ -863,10 +830,10 @@
         open("templates/dump-pg-db.tmpl").read()).render(paths)
     backup_job = Template(
         open("templates/pg_backup_job.tmpl").read()).render(paths)
-    write_file(
+    host.write_file(
         '{}/dump-pg-db'.format(postgresql_scripts_dir),
         dump_script, perms=0755)
-    write_file(
+    host.write_file(
         '{}/pg_backup_job'.format(postgresql_scripts_dir),
         backup_job, perms=0755)
     install_postgresql_crontab(postgresql_crontab)
@@ -1256,56 +1223,6 @@
             local_state.save()
 
 
-def ensure_local_ssh():
-    """Generate SSH keys for postgres user.
-
-    The public key is stored in public_ssh_key on the relation.
-
-    Bidirectional SSH access is required by repmgr.
-    """
-    comment = 'repmgr key for {}'.format(os.environ['JUJU_UNIT_NAME'])
-    if not os.path.isdir(postgres_ssh_dir):
-        host.mkdir(postgres_ssh_dir, "postgres", "postgres", 0o700)
-    if not os.path.exists(postgres_ssh_private_key):
-        run("sudo -u postgres -H ssh-keygen -q -t rsa -C '{}' -N '' "
-            "-f '{}'".format(comment, postgres_ssh_private_key))
-    public_key = open(postgres_ssh_public_key, 'r').read().strip()
-    host_key = open('/etc/ssh/ssh_host_ecdsa_key.pub').read().strip()
-    local_state['public_ssh_key'] = public_key
-    local_state['ssh_host_key'] = host_key
-    local_state.publish()
-
-
-def authorize_remote_ssh():
-    """Generate the SSH authorized_keys file."""
-    authorized_units = set()
-    authorized_keys = set()
-    known_hosts = set()
-    for relid in hookenv.relation_ids('replication'):
-        for unit in hookenv.related_units(relid):
-            relation = hookenv.relation_get(unit=unit, rid=relid)
-            public_key = relation.get('public_ssh_key', None)
-            if public_key:
-                authorized_units.add(unit)
-                authorized_keys.add(public_key)
-                known_hosts.add('{} {}'.format(
-                    relation['private-address'], relation['ssh_host_key']))
-
-    # Generate known_hosts
-    write_file(
-        postgres_ssh_known_hosts, '\n'.join(known_hosts),
-        owner="postgres", group="postgres", perms=0o644)
-
-    # Generate authorized_keys
-    write_file(
-        postgres_ssh_authorized_keys, '\n'.join(authorized_keys),
-        owner="postgres", group="postgres", perms=0o400)
-
-    # Publish details, so relation knows they have been granted access.
-    local_state['authorized'] = authorized_units
-    local_state.publish()
-
-
 @contextmanager
 def pgpass():
     passwords = {}
@@ -1665,6 +1582,16 @@
     config_changed()
 
 
+@contextmanager
+def switch_cwd(new_working_directory):
+    org_dir = os.getcwd()
+    os.chdir(new_working_directory)
+    try:
+        yield new_working_directory
+    finally:
+        os.chdir(org_dir)
+
+
 def clone_database(master_unit, master_host):
     with pgpass():
         postgresql_stop()
@@ -1680,7 +1607,10 @@
             shutil.rmtree(postgresql_cluster_dir)
 
         try:
-            output = subprocess.check_output(cmd)
+            # Change directory the postgres user can read.
+            with switch_cwd('/tmp'):
+                # Run the sudo command.
+                output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
             log(output, DEBUG)
             # Debian by default expects SSL certificates in the datadir.
             os.symlink(
@@ -1690,7 +1620,7 @@
                 '/etc/ssl/private/ssl-cert-snakeoil.key',
                 os.path.join(postgresql_cluster_dir, 'server.key'))
             create_recovery_conf(master_host)
-        except subprocess.CalledProcessError, x:
+        except subprocess.CalledProcessError as x:
             # We failed, and this cluster is broken. Rebuild a
             # working cluster so start/stop etc. works and we
             # can retry hooks again. Even assuming the charm is
@@ -1817,7 +1747,7 @@
 check_file_age -w {} -c {} -f {}".format(warn_age, crit_age, backup_log))
 
     if os.path.isfile('/etc/init.d/nagios-nrpe-server'):
-        subprocess.call(['service', 'nagios-nrpe-server', 'reload'])
+        host.service_reload('nagios-nrpe-server')
 
 
 ###############################################################################
@@ -1841,12 +1771,6 @@
     config_data['backup_dir'].strip() or
     os.path.join(postgresql_data_dir, 'backups'))
 postgresql_logs_dir = os.path.join(postgresql_data_dir, 'logs')
-postgres_ssh_dir = os.path.expanduser('~postgres/.ssh')
-postgres_ssh_public_key = os.path.join(postgres_ssh_dir, 'id_rsa.pub')
-postgres_ssh_private_key = os.path.join(postgres_ssh_dir, 'id_rsa')
-postgres_ssh_authorized_keys = os.path.join(postgres_ssh_dir,
-                                            'authorized_keys')
-postgres_ssh_known_hosts = os.path.join(postgres_ssh_dir, 'known_hosts')
 hook_name = os.path.basename(sys.argv[0])
 replication_relation_types = ['master', 'slave', 'replication']
 local_state = State('local_state.pickle')
@@ -1859,5 +1783,4 @@
     if hookenv.relation_id():
         log("Relation {} with {}".format(
             hookenv.relation_id(), hookenv.remote_unit()))
-
     hooks.execute(sys.argv)

=== modified file 'metadata.yaml'
--- metadata.yaml	2013-06-25 11:29:13 +0000
+++ metadata.yaml	2013-08-23 09:40:09 +0000
@@ -1,13 +1,17 @@
 name: postgresql
-summary: "object-relational SQL database (supported version)"
+summary: "PostgreSQL object-relational SQL database (supported version)"
 description: |
-  PostgreSQL is a fully featured object-relational database management
-  system.  It supports a large part of the SQL standard and is designed
-  to be extensible by users in many aspects.  Some of the features are:
-  ACID transactions, foreign keys, views, sequences, subqueries,
-  triggers, user-defined types and functions, outer joins, multiversion
-  concurrency control.  Graphical user interfaces and bindings for many
-  programming languages are available as well.
+  PostgreSQL is a powerful, open source object-relational database system.
+  It has more than 15 years of active development and a proven
+  architecture that has earned it a strong reputation for reliability,
+  data integrity, and correctness. It is fully ACID compliant, has full
+  support for foreign keys, joins, views, triggers, and stored procedures
+  (in multiple languages). It includes most SQL:2008 data types, including
+  INTEGER, NUMERIC, BOOLEAN, CHAR, VARCHAR, DATE, INTERVAL, and TIMESTAMP.
+  It also supports storage of binary large objects, including pictures,
+  sounds, or video. It has native programming interfaces for C/C++, Java,
+  .Net, Perl, Python, Ruby, Tcl, ODBC, among others, and exceptional
+  documentation (http://www.postgresql.org/docs/manuals/).
 maintainer: Stuart Bishop <[e-mail]>
 categories:
     - databases

=== added file 'templates/start_conf.tmpl'
--- templates/start_conf.tmpl	1970-01-01 00:00:00 +0000
+++ templates/start_conf.tmpl	2013-08-23 09:40:09 +0000
@@ -0,0 +1,13 @@
+#
+# This file is managed by Juju.
+#
+# Automatic startup configuration
+# auto: automatically start/stop the cluster in the init script
+# manual: do not start/stop in init scripts, but allow manual startup with
+#         pg_ctlcluster
+# disabled: do not allow manual startup with pg_ctlcluster (this can be easily
+#           circumvented and is only meant to be a small protection for
+#           accidents).
+
+{{mode}}
+

=== modified file 'test.py'
--- test.py	2013-08-23 09:40:09 +0000
+++ test.py	2013-08-23 09:40:09 +0000
@@ -128,13 +128,19 @@
         # enough that our system is probably stable. This means we have
         # extremely slow and flaky tests, but that is possibly better
         # than no tests.
-        time.sleep(30)
+        time.sleep(45)
 
     def setUp(self):
         DEBUG("JujuFixture.setUp()")
         super(JujuFixture, self).setUp()
         self.reset()
-        self.addCleanup(self.reset)
+        # Optionally, don't teardown services and machines after running
+        # a test. If a subsequent test is run, they will be torn down at
+        # that point. This option is only useful when running a single
+        # test, or when the test harness is set to abort after the first
+        # failed test.
+        if not os.environ.get('TEST_DONT_TEARDOWN_JUJU', False):
+            self.addCleanup(self.reset)
 
     def reset(self):
         DEBUG("JujuFixture.reset()")
@@ -286,7 +292,6 @@
         result = self.sql('SELECT TRUE', dbname='postgres')
         self.assertEqual(result, [['t']])
 
-
     def is_master(self, postgres_unit, dbname=None):
         is_master = self.sql(
             'SELECT NOT pg_is_in_recovery()',

=== added directory 'tests'
=== added symlink 'tests/01_pg_testsuite.test'
=== target is u'../test.py'