yellow team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot/+merge/91323

This branch translates the hooks from bash into Python, adds some hook/test helpers (with tests) and adds the first charm test.
-- 
https://code.launchpad.net/~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot/+merge/91323
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~benji/charms/oneiric/buildbot-master/buildbot-master-lpbuildbot into lp:~yellow/charms/oneiric/buildbot-master/trunk.

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

=== added file 'HACKING.txt'
--- HACKING.txt	1970-01-01 00:00:00 +0000
+++ HACKING.txt	2012-02-02 18:42: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'
--- hooks/config-changed	2012-01-30 14:43:09 +0000
+++ hooks/config-changed	2012-02-02 18:42: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 being 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 18:42: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'
--- hooks/install	2012-01-30 14:43:09 +0000
+++ hooks/install	2012-02-02 18:42: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 being 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'
--- hooks/start	2012-01-30 17:03:24 +0000
+++ hooks/start	2012-02-02 18:42: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 being 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 18:42: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 18:42: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

=== modified file 'revision'
--- revision	2012-01-30 16:43:59 +0000
+++ revision	2012-02-02 18:42:18 +0000
@@ -1,1 +1,1 @@
-48
+79

=== 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 18:42: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'