Merge lp:~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot into lp:~yellow/charms/oneiric/buildbot-master/trunk

[Benji York]

This branch translates the hooks from bash into Python, adds some hook/test helpers (with tests) and adds the first charm test.

[Brad Crittenden (bac)]

Hi Benji,

The Pythonization is great and overdue! And thanks for setting up a testing structure.

typo: s/about to being/about to begin/ (Several occurrences.)

line 119 is missing a comma, as I found out the hard way.

In my branch I've moved the definition of 'log' into the helpers.py. DRY.

The sleep at 431 seems awful aggressive given the long time it takes to deploy.

A lot of install and config-changed hooks I've already modified, so they were not reviewed too closely.

review: Approve

[lp:~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot]

17. By Benji York on 2012-02-02

ignore revision

[Benji York (benji)]

> typo: s/about to being/about to begin/ (Several occurrences.)

Thanks. Fixed.

> line 119 is missing a comma, as I found out the hard way.

Ooh! So it is. Automatic string concatenation isn't one of my favorite
Python features. Fixed.

> In my branch I've moved the definition of 'log' into the helpers.py.
> DRY.

Sounds good. I'll leave that for your branch so as not to increase the
already great chance of merge conflicts.

> The sleep at 431 seems awful aggressive given the long time it takes
> to deploy.

My reasoning was more focused on getting out of the loop faster than
worrying too much about how many times we go through the loop. I
believe that doing 10 "juju status" commands a second is so cheap as to
be negligible.

Thanks for the good review.

[lp:~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot]

18. By Benji York on 2012-02-02

s/being/begin/g

19. By Benji York on 2012-02-02

add missing comma

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2012-02-02 20:27:18 +0000
@@ -0,0 +1,5 @@
+.emacs*
+Session.vim
+tags
+TAGS
+revision

=== added file 'HACKING.txt'
--- HACKING.txt	1970-01-01 00:00:00 +0000
+++ HACKING.txt	2012-02-02 20:27:18 +0000
@@ -0,0 +1,23 @@
+Running the charm tests
+=======================
+
+1) Establish a charm repository if you do not already have one.  A charm
+   repository is a directory with subdirectories for each Ubuntu version being
+   used.  Inside those per-Ubuntu-version directories are the charm
+   directories.  For example, to make a charm repository for this charm under
+   Oneiric follow these steps:
+
+    a) mkdir -p ~/juju-charms/oneiric
+    b) ln -s `pwd` ~/juju-charms/oneiric/buildbot-master
+
+2) Copy the juju_wrapper file into some place earlier in your PATH than the
+   real juju executable, naming it "juju".  Edit the CHARM_TEST_REPO variable
+   therein to reflect the location of the charm repo from step 1.
+
+3) Run the tests: RESOLVE_TEST_CHARMS=1 tests/buildbot-master.test
+
+
+Running the charm helper tests
+==============================
+
+Just run "python hooks/tests.py".

=== modified file 'hooks/config-changed' (properties changed: +x to -x)
--- hooks/config-changed	2012-01-30 14:43:09 +0000
+++ hooks/config-changed	2012-02-02 20:27:18 +0000
@@ -1,51 +1,70 @@
-#!/bin/bash
-# Hook for handling config changes.
-set -eux # -x for verbose logging to juju debug-log
+#!/usr/bin/python
+
+from helpers import command, Config, run
+from subprocess import CalledProcessError
+import base64
+import os
+import os.path
+
 
 # config_file is changed via juju like:
 # juju set buildbot-master config-file=`uuencode master.cfg`
 
-BUILDBOT_DIR=`config-get installdir`
-juju-log "--> config-changed"
-
-juju-log "Updating buildbot configuration."
-CONFIG_FILE=`config-get config-file`
-CONFIG_TRANSPORT=`config-get config-transport`
-CONFIG_URL=`config-get config-url`
-
-#
-if [[ -n $CONFIG_FILE ]]; then
-    echo "$CONFIG_FILE" | uudecode -o "$BUILDBOT_DIR"/master.cfg
-    juju-log "Config decoded and written."
-elif [ "$CONFIG_TRANSPORT" == "bzr" ] && [[ -n $CONFIG_URL ]]; then
+log = command('juju-log')
+bzr = command('bzr')
+
+# Log the fact that we're about to begin the install step.
+log('--> config-changed')
+
+config = Config()
+buildbot_dir =  config['installdir']
+config_file = config['config-file']
+config_transport = config['config-transport']
+config_url = config['config-url']
+
+log('Updating buildbot configuration.')
+# Write the buildbot config to disk (fetching it if necessary).
+if config_file:
+    with open(os.path.join(buildbot_dir, 'master.cfg', 'w')) as f:
+        f.write(base64.decode(config_file))
+    log('Config decoded and written.')
+elif config_transport == 'bzr' and config_url:
     # If the branch is private then more setup needs to be done.  The
     # gpg-agent needs to send the key over and the bzr launchpad-login
     # needs to be set. 
-    LP_ID=`config-get config-user`
-    if [[ -n $LP_ID ]]; then
-	bzr launchpad-login $LP_ID
-    fi
-    PKEY=`config-get config-private-key`
-    if [[ -n $PKEY ]]; then
+    lp_id = config['config-user']
+    if lp_id:
+        bzr('launchpad-login', lp_id)
+
+    private_key = config['config-private-key']
+    if private_key:
         # Set up the .ssh directory.
-        mkdir ~/.ssh
-        chmod 700 ~/.ssh
-        echo "$PKEY" | uudecode -o ~/.ssh/lp_key
-    fi
-    bzr branch --use-existing-dir $CONFIG_URL "$BUILDBOT_DIR"
-    chown -R ubuntu:ubuntu "$BUILDBOT_DIR"
-fi
+        ssh_dir = os.expanduser('~/.ssh')
+        os.mkdir(ssh_dir)
+        os.chmod(ssh_dir, 0700)
+        with open(os.path.join(ssh_dir, 'lp_key', w)) as f:
+            f.write(base64.decode(private_key))
+
+    bzr('branch', '--use-existing-dir', config_url, buildbot_dir)
+    run('chown', '-R', 'ubuntu:ubuntu', buildbot_dir)
+else:
+    # XXX Is it an error to get to this point?
+    pass
 
 # Restart buildbot if it is running.
-PIDFILE="$BUILDBOT_DIR"/twistd.pid
-if [ -f $PIDFILE ]; then
-  BUILDBOT_PID=`cat $PIDFILE`
-  if kill -0 $BUILDBOT_PID; then
-    # Buildbot is running, reconfigure it.
-    juju-log "Reconfiguring buildbot"
-    buildbot reconfig "$BUILDBOT_DIR"
-    juju-log "Buildbot reconfigured"
-  fi
-fi
+pidfile = os.path.join(buildbot_dir, 'twistd.pid')
+if os.path.exists(pidfile):
+    buildbot_pid = open(pidfile).read().strip()
+    try:
+        # Is buildbot running?
+        run('kill', '-0', buildbot_pid)
+    except CalledProcessError:
+        # Buildbot isn't running, so no need to reconfigure it.
+        pass
+    else:
+        # Buildbot is running, reconfigure it.
+        log('Reconfiguring buildbot')
+        run('buildbot', 'reconfig', buildbot_dir)
+        log('Buildbot reconfigured')
 
-juju-log "<-- config-changed"
+log('<-- config-changed')

=== added file 'hooks/helpers.py'
--- hooks/helpers.py	1970-01-01 00:00:00 +0000
+++ hooks/helpers.py	2012-02-02 20:27:18 +0000
@@ -0,0 +1,35 @@
+from collections import defaultdict
+import subprocess
+import yaml
+
+def command(*base_args):
+    def callable_command(*args):
+        return subprocess.check_output(base_args+args, shell=False)
+
+    return callable_command
+
+
+def run(*args):
+    return subprocess.check_output(args, shell=False)
+
+
+def unit_info(service_name, item_name, data=None):
+    if data is None:
+        data = yaml.safe_load(run('juju', 'status'))
+    services = data['services'][service_name]
+    units = services['units']
+    item = units.items()[0][1][item_name]
+    return item
+
+
+class Config(defaultdict):
+
+    def __init__(self):
+        super(defaultdict, self).__init__()
+        self.config_get = command('config-get')
+
+    def __missing__(self, key):
+        return self.config_get(key).strip()
+
+    def __setitem__(self, key, value):
+        raise RuntimeError('configuration is read-only')

=== modified file 'hooks/install' (properties changed: +x to -x)
--- hooks/install	2012-01-30 14:43:09 +0000
+++ hooks/install	2012-02-02 20:27:18 +0000
@@ -1,22 +1,39 @@
-#!/bin/bash
-# Here do anything needed to install the service
-# i.e. apt-get install -y foo  or  bzr branch http://myserver/mycode /srv/webroot
-set -eux # -x for verbose logging to juju debug-log
-
-juju-log "--> install"
-BUILDBOT_DIR=`config-get installdir`
-apt-get install -y buildbot sharutils
-# Needed only for the demo site.
-apt-get install -y git
-
-juju-log "Creating master in '$BUILDBOT_DIR'."
+#!/usr/bin/python
+
+from helpers import command, Config, run
+from subprocess import CalledProcessError
+import os
+import os.path
+import shutil
+
+log = command('juju-log')
+
+# Log the fact that we're about to begin the install step.
+log('--> install')
+
+config = Config()
+
+buildbot_dir =  config['installdir']
+run('apt-get', 'install', '-y', 'sharutils', 'bzr')
+
+# Get the lucid version of buildbot.
+run('apt-add-repository',
+    'deb http://us.archive.ubuntu.com/ubuntu/ lucid main universe')
+run('apt-get', 'update')
+run('apt-get', 'install', '-y', 'buildbot/lucid')
+
+log('Creating master in {}.'.format(buildbot_dir))
 # Since we may be installing into a pre-existing service, ensure the
 # buildbot directory is removed.
-if [ -e "$BUILDBOT_DIR" ]; then
-  buildbot stop "$BUILDBOT_DIR"
-  rm -rf "$BUILDBOT_DIR"
-fi
-mkdir -p "$BUILDBOT_DIR"
-buildbot create-master "$BUILDBOT_DIR"
-mv "$BUILDBOT_DIR"/master.cfg.sample "$BUILDBOT_DIR"/master.cfg
-juju-log "<-- install"
+if os.path.exists(buildbot_dir):
+    try:
+        run('buildbot', 'stop', buildbot_dir)
+    except CalledProcessError:
+        # It probably wasn't running; just ignore the error.
+        pass
+    shutil.rmtree(buildbot_dir)
+lpbuildbot_checkout = os.path.join(os.environ['CHARM_DIR'], 'lpbuildbot')
+shutil.copytree(lpbuildbot_checkout, buildbot_dir)
+
+# Log the fact that the install step is done.
+log('<-- install')

=== modified file 'hooks/start' (properties changed: +x to -x)
--- hooks/start	2012-01-30 17:03:24 +0000
+++ hooks/start	2012-02-02 20:27:18 +0000
@@ -1,12 +1,16 @@
-#!/bin/bash
-# Here put anything that is needed to start the service.
-# Note that currently this is run directly after install
-# i.e. 'service apache2 start'
-set -eux # -x for verbose logging to juju debug-log
-
-BUILDBOT_DIR=`config-get installdir`
-
-juju-log "--> start"
-buildbot start "$BUILDBOT_DIR"
-open-port 8010/TCP
-juju-log "<-- start"
+#!/usr/bin/python
+
+from helpers import command, Config, run
+
+log = command('juju-log')
+
+# Log the fact that we're about to begin the start step.
+log('--> start')
+
+config = Config()
+buildbot_dir =  config['installdir']
+run('buildbot', 'start', buildbot_dir)
+run('open-port', '8010/TCP')
+
+# Log the fact that the start step is done.
+log('<-- start')

=== added file 'hooks/tests.py'
--- hooks/tests.py	1970-01-01 00:00:00 +0000
+++ hooks/tests.py	2012-02-02 20:27:18 +0000
@@ -0,0 +1,83 @@
+import unittest
+from subprocess import CalledProcessError
+from helpers import command, unit_info
+
+
+class testCommand(unittest.TestCase):
+
+    def testSimpleCommand(self):
+        # Creating a simple command (ls) works and running the command
+        # produces a string.
+        ls = command('/bin/ls')
+        self.assertIsInstance(ls(), str)
+
+    def testArguments(self):
+        # Arguments can be passed to commands.
+        ls = command('/bin/ls')
+        self.assertIn('Usage:', ls('--help'))
+
+    def testMissingExecutable(self):
+        # If the command does not exist, an OSError (No such file or
+        # directory) is raised.
+        bad = command('this command does not exist')
+        with self.assertRaises(OSError) as info:
+            bad()
+        self.assertEqual(2, info.exception.errno)
+
+    def testError(self):
+        # If the command returns a non-zero exit code, an exception is raised.
+        ls = command('/bin/ls')
+        with self.assertRaises(CalledProcessError):
+            ls('--not a valid switch')
+
+    def testBakedInArguments(self):
+        # Arguments can be passed when creating the command as well as when
+        # executing it.
+        ll = command('/bin/ls', '-l')
+        self.assertIn('rw', ll()) # Assumes a file is r/w in the pwd.
+        self.assertIn('Usage:', ll('--help'))
+
+    def testQuoting(self):
+        # There is no need to quote special shell characters in commands.
+        ls = command('/bin/ls')
+        ls('--help', '>')
+
+
+class testUnit_info(unittest.TestCase):
+
+    def make_data(self, state='started'):
+        return {
+            'machines': {0: {
+                'dns-name': 'localhost',
+                'instance-id': 'local',
+                'instance-state': 'running',
+                'state': 'running'}},
+            'services': {'foo-service': {
+                'charm': 'local:oneiric/foo-service-77',
+                'relations': {},
+                'units': {'foo-unit/29': {
+                    'machine': 0,
+                    'public-address': '192.168.122.160',
+                    'relations': {},
+                    'state': state}}}}}
+
+    def testDataParameter(self):
+        # The unit_info function can take a data parameter (primarily for
+        # testing) that provides the juju service information to be queried.
+        # If not provided the "juju status" command is run and its results
+        # parsed.
+        unit_info('foo-service', 'state', data=self.make_data())
+
+    def testStateFetching(self):
+        # The most common data to fetch about a unit is its state.
+        state = unit_info('foo-service', 'state', data=self.make_data())
+        self.assertEqual('started', state)
+
+    def testFailedState(self):
+        state = unit_info(
+            'foo-service', 'state', data=self.make_data(state='bad'))
+        self.assertNotEqual('started', state)
+
+
+if __name__ == '__main__':
+    unittest.main()

=== added file 'juju_wrapper'
--- juju_wrapper	1970-01-01 00:00:00 +0000
+++ juju_wrapper	2012-02-02 20:27:18 +0000
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+[ -n "$RESOLVE_TEST_CHARMS" ] || exec /usr/bin/juju $@
+#set -x
+
+CHARM_TEST_REPO=~/juju-charms # <---- Change this.
+
+cmd=$1
+case $cmd in
+deploy)
+    shift
+    charm=$1
+    shift
+    exec /usr/bin/juju deploy --repository $CHARM_TEST_REPO local:$charm $@
+    ;;
+*)
+    exec /usr/bin/juju $@
+    ;;
+esac

=== removed file 'revision'
--- revision	2012-01-30 16:43:59 +0000
+++ revision	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-48

=== added directory 'tests'
=== added file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	1970-01-01 00:00:00 +0000
+++ tests/buildbot-master.test	2012-02-02 20:27:18 +0000
@@ -0,0 +1,25 @@
+#!/usr/bin/python
+
+from helpers import command, run, unit_info
+import time
+import unittest
+
+juju = command('juju')
+
+class testCharm(unittest.TestCase):
+
+    def testDeploy(self):
+        try:
+            juju('deploy', 'buildbot-master')
+            while True:
+                status = unit_info('buildbot-master', 'state')
+                if 'error' in status or status == 'started':
+                    break
+                time.sleep(0.1)
+            self.assertEqual(unit_info('buildbot-master', 'state'), 'started')
+        finally:
+            juju('destroy-service', 'buildbot-master')
+
+
+if __name__ == '__main__':
+    unittest.main()

=== added symlink 'tests/helpers.py'
=== target is u'../hooks/helpers.py'