Merge lp:~bac/charms/oneiric/buildbot-master/bbm-pst into lp:~yellow/charms/oneiric/buildbot-master/trunk

[Brad Crittenden]

Move generic, non-juju-related methods out of helpers and into a new package python-shell-toolbox in the ~yellow PPA.

The install hook must now add the apt repository for the PPA, install the package, and ensure local and helpers are not imported until that is done since they depend on shelltoolbox. The other hooks can just import and use shelltoolbox normally since we can assume the install hook did its job.

[Graham Binns (gmb)]

Approved based on conversation with Brad on G+.

review: Approve (code)

=== modified file 'hooks/buildbot-relation-changed'
--- hooks/buildbot-relation-changed	2012-02-13 12:26:29 +0000
+++ hooks/buildbot-relation-changed	2012-02-29 22:49:25 +0000
@@ -4,6 +4,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import os
+from shelltoolbox import su
 
 from helpers import (
     log,
@@ -11,7 +12,6 @@
     log_exit,
     relation_get,
     relation_set,
-    su,
     )
 from local import (
     buildbot_reconfig,

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-02-23 21:50:41 +0000
+++ hooks/config-changed	2012-02-29 22:49:25 +0000
@@ -7,23 +7,24 @@
 import json
 import os
 import os.path
+from shelltoolbox import (
+    apt_get_install,
+    command,
+    DictDiffer,
+    get_user_ids,
+    install_extra_repositories,
+    run,
+    su,
+    )
 import shutil
 import subprocess
 import sys
 
 from helpers import (
-    apt_get_install,
-    cd,
-    command,
-    DictDiffer,
     get_config,
-    get_user_ids,
-    install_extra_repository,
     log,
     log_entry,
     log_exit,
-    run,
-    su,
     )
 from local import (
     buildbot_create,
@@ -31,8 +32,6 @@
     config_json,
     fetch_history,
     generate_string,
-    get_bucket,
-    get_key,
     get_wrapper_cfg_path,
     put_history,
     slave_json,
@@ -75,7 +74,12 @@
     added_or_changed = diff.added_or_changed
 
     if extra_repo and 'extra-repository' in added_or_changed:
-        install_extra_repository(extra_repo)
+        try:
+            install_extra_repositories(extra_repo)
+        except subprocess.CalledProcessError as e:
+            log('Error adding repository: ' + extra_repo)
+            log(e)
+            raise
         restart_required = True
     if extra_pkgs and 'extra-packages' in added_or_changed:
         apt_get_install(

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-14 17:00:53 +0000
+++ hooks/helpers.py	2012-02-29 22:49:25 +0000
@@ -5,37 +5,24 @@
 
 __metaclass__ = type
 __all__ = [
-    'apt_get_install',
-    'cd',
-    'command',
-    'DictDiffer',
     'get_config',
-    'get_user_ids',
-    'get_user_home',
-    'get_value_from_line',
-    'grep',
-    'install_extra_repository',
     'log',
     'log_entry',
     'log_exit',
-    'run',
     'relation_get',
     'relation_set',
-    'su',
     'unit_info',
     ]
 
 from collections import namedtuple
-from contextlib import contextmanager
 import json
 import operator
-import os
-import pwd
-import re
-import subprocess
-import sys
+from shelltoolbox import (
+    command,
+    run,
+    script_name,
+    )
 import tempfile
-from textwrap import dedent
 import time
 import urllib2
 import yaml
@@ -43,49 +30,15 @@
 
 Env = namedtuple('Env', 'uid gid home')
 
-
-def run(*args):
-    """Run the command with the given arguments.
-
-    The first argument is the path to the command to run, subsequent arguments
-    are command-line arguments to be passed.
-    """
-    process = subprocess.Popen(args, stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE, close_fds=True)
-    stdout, stderr = process.communicate()
-    if process.returncode:
-        raise subprocess.CalledProcessError(
-            process.returncode, repr(args), output=stdout+stderr)
-    return stdout
-
-
-def command(*base_args):
-    """Return a callable that will run the given command with any arguments.
-
-    The first argument is the path to the command to run, subsequent arguments
-    are command-line arguments to "bake into" the returned callable.
-
-    The callable runs the given executable and also takes arguments that will
-    be appeneded to the "baked in" arguments.
-
-    For example, this code will list a file named "foo" (if it exists):
-
-        ls_foo = command('/bin/ls', 'foo')
-        ls_foo()
-
-    While this invocation will list "foo" and "bar" (assuming they exist):
-
-        ls_foo('bar')
-    """
-    def callable_command(*args):
-        all_args = base_args + args
-        return run(*all_args)
-
-    return callable_command
-
-
 log = command('juju-log')
-apt_get_install = command('apt-get', 'install', '-y', '--force-yes')
+
+
+def log_entry():
+    log("--> Entering {}".format(script_name()))
+
+
+def log_exit():
+    log("<-- Exiting {}".format(script_name()))
 
 
 def get_config():
@@ -93,16 +46,6 @@
     return json.loads(config_get())
 
 
-def install_extra_repository(extra_repository):
-    try:
-        run('apt-add-repository', extra_repository)
-        run('apt-get', 'update')
-    except subprocess.CalledProcessError as e:
-        log('Error adding repository: ' + extra_repository)
-        log(e)
-        raise
-
-
 def relation_get(*args):
     cmd = command('relation-get')
     return cmd(*args).strip()
@@ -114,96 +57,6 @@
     return cmd(*args)
 
 
-def grep(content, filename):
-    with open(filename) as f:
-        for line in f:
-            if re.match(content, line):
-                return line.strip()
-
-
-def get_value_from_line(line):
-    return line.split('=')[1].strip('"\' ')
-
-
-def script_name():
-    return os.path.basename(sys.argv[0])
-
-
-def log_entry():
-    log("<-- Entering {}".format(script_name()))
-
-
-def log_exit():
-    log("--> Exiting {}".format(script_name()))
-
-
-class DictDiffer:
-    """
-    Calculate the difference between two dictionaries as:
-    (1) items added
-    (2) items removed
-    (3) keys same in both but changed values
-    (4) keys same in both and unchanged values
-    """
-
-    # Based on answer by hughdbrown at:
-    # http://stackoverflow.com/questions/1165352
-
-    def __init__(self, current_dict, past_dict):
-        self.current_dict = current_dict
-        self.past_dict = past_dict
-        self.set_current = set(current_dict)
-        self.set_past = set(past_dict)
-        self.intersect = self.set_current.intersection(self.set_past)
-
-    @property
-    def added(self):
-        return self.set_current - self.intersect
-
-    @property
-    def removed(self):
-        return self.set_past - self.intersect
-
-    @property
-    def changed(self):
-        return set(key for key in self.intersect
-                   if self.past_dict[key] != self.current_dict[key])
-    @property
-    def unchanged(self):
-        return set(key for key in self.intersect
-                   if self.past_dict[key] == self.current_dict[key])
-    @property
-    def modified(self):
-        return self.current_dict != self.past_dict
-
-    @property
-    def added_or_changed(self):
-        return self.added.union(self.changed)
-
-    def _changes(self, keys):
-        new = {}
-        old = {}
-        for k in keys:
-            new[k] = self.current_dict.get(k)
-            old[k] = self.past_dict.get(k)
-        return "%s -> %s" % (old, new)
-
-    def __str__(self):
-        if self.modified:
-            s = dedent("""\
-            added: %s
-            removed: %s
-            changed: %s
-            unchanged: %s""") % (
-                self._changes(self.added),
-                self._changes(self.removed),
-                self._changes(self.changed),
-                list(self.unchanged))
-        else:
-            s = "no changes"
-        return s
-
-
 def make_charm_config_file(charm_config):
     charm_config_file = tempfile.NamedTemporaryFile()
     charm_config_file.write(yaml.dump(charm_config))
@@ -312,80 +165,3 @@
         if time.time() - start_time >= timeout:
             raise RuntimeError('timeout waiting for contents of ' + url)
         time.sleep(0.1)
-
-
-class Serializer:
-    """Handle JSON (de)serialization."""
-
-    def __init__(self, path, default=None, serialize=None, deserialize=None):
-        self.path = path
-        self.default = default or {}
-        self.serialize = serialize or json.dump
-        self.deserialize = deserialize or json.load
-
-    def exists(self):
-        return os.path.exists(self.path)
-
-    def get(self):
-        if self.exists():
-            with open(self.path) as f:
-                return self.deserialize(f)
-        return self.default
-
-    def set(self, data):
-        with open(self.path, 'w') as f:
-            self.serialize(data, f)
-
-
-@contextmanager
-def cd(directory):
-    """A context manager to temporary change current working dir, e.g.::
-
-        >>> import os
-        >>> os.chdir('/tmp')
-        >>> with cd('/bin'): print os.getcwd()
-        /bin
-        >>> os.getcwd()
-        '/tmp'
-    """
-    cwd = os.getcwd()
-    os.chdir(directory)
-    yield
-    os.chdir(cwd)
-
-
-def get_user_ids(user):
-    """Return the uid and gid of given `user`, e.g.::
-
-        >>> get_user_ids('root')
-        (0, 0)
-    """
-    userdata = pwd.getpwnam(user)
-    return userdata.pw_uid, userdata.pw_gid
-
-
-def get_user_home(user):
-    """Return the home directory of the given `user`.
-
-        >>> get_user_home('root')
-        '/root'
-    """
-    return pwd.getpwnam(user).pw_dir
-
-
-@contextmanager
-def su(user):
-    """A context manager to temporary run the script as a different user."""
-    uid, gid = get_user_ids(user)
-    os.setegid(gid)
-    os.seteuid(uid)
-    current_home = os.getenv('HOME')
-    home = get_user_home(user)
-    os.environ['HOME'] = home
-    try:
-        yield Env(uid, gid, home)
-    finally:
-        os.setegid(os.getgid())
-        os.seteuid(os.getuid())
-        if current_home is not None:
-            os.environ['HOME'] = current_home

=== modified file 'hooks/install'
--- hooks/install	2012-02-22 19:42:50 +0000
+++ hooks/install	2012-02-29 22:49:25 +0000
@@ -5,15 +5,75 @@
 
 import os
 import shutil
-from subprocess import CalledProcessError
+import subprocess
+
+
+def run(*args):
+    """Run the command with the given arguments.
+
+    The first argument is the path to the command to run, subsequent arguments
+    are command-line arguments to be passed.
+    """
+    process = subprocess.Popen(args, stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE, close_fds=True)
+    stdout, stderr = process.communicate()
+    if process.returncode:
+        raise subprocess.CalledProcessError(
+            process.returncode, repr(args), output=stdout+stderr)
+    return stdout
+
+
+def command(*base_args):
+    """Return a callable that will run the given command with any arguments.
+
+    The first argument is the path to the command to run, subsequent arguments
+    are command-line arguments to "bake into" the returned callable.
+
+    The callable runs the given executable and also takes arguments that will
+    be appeneded to the "baked in" arguments.
+
+    For example, this code will list a file named "foo" (if it exists):
+
+        ls_foo = command('/bin/ls', 'foo')
+        ls_foo()
+
+    While this invocation will list "foo" and "bar" (assuming they exist):
+
+        ls_foo('bar')
+    """
+    def callable_command(*args):
+        all_args = base_args + args
+        return run(*all_args)
+
+    return callable_command
+
+
+log = command('juju-log')
+
+
+def install_extra_repository(extra_repository):
+    try:
+        run('apt-add-repository', extra_repository)
+        run('apt-get', 'update')
+    except subprocess.CalledProcessError as e:
+        log('Error adding repository: ' + extra_repository)
+        log(e)
+        raise
+
+
+def install_packages():
+    apt_get_install = command('apt-get', 'install', '-y', '--force-yes')
+    apt_get_install('bzr', 'python-boto')
+    install_extra_repository('ppa:yellow/ppa')
+    apt_get_install('python-shell-toolbox')
+
+
+install_packages()
 
 from helpers import (
-    apt_get_install,
     get_config,
-    log,
     log_entry,
     log_exit,
-    run,
     )
 from local import (
     config_json,
@@ -22,13 +82,12 @@
 
 
 def cleanup(buildbot_dir):
-    apt_get_install('bzr', 'python-boto')
     # Since we may be installing into a pre-existing service, ensure the
     # buildbot directory is removed.
     if os.path.exists(buildbot_dir):
         try:
             run('buildbot', 'stop', buildbot_dir)
-        except (CalledProcessError, OSError):
+        except (subprocess.CalledProcessError, OSError):
             # This usually happens because buildbot hasn't been
             # installed yet, or that it wasn't running; just
             # ignore the error.
@@ -52,6 +111,7 @@
 
 
 if __name__ == '__main__':
+
     log_entry()
     try:
         main()

=== modified file 'hooks/local.py'
--- hooks/local.py	2012-02-23 21:50:41 +0000
+++ hooks/local.py	2012-02-29 22:49:25 +0000
@@ -26,14 +26,16 @@
 import subprocess
 import uuid
 
-from helpers import (
+from shelltoolbox import (
     cd,
-    get_config,
-    log,
     run,
     Serializer,
     su,
     )
+from helpers import (
+    get_config,
+    log,
+    )
 
 
 HTTP_PORT_PROTOCOL = '8010/TCP'

=== modified file 'hooks/start'
--- hooks/start	2012-02-22 19:42:50 +0000
+++ hooks/start	2012-02-29 22:49:25 +0000
@@ -3,10 +3,11 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from shelltoolbox import run
+
 from helpers import (
     log_entry,
     log_exit,
-    run,
     )
 from local import HTTP_PORT_PROTOCOL
 

=== modified file 'hooks/stop'
--- hooks/stop	2012-02-22 19:42:50 +0000
+++ hooks/stop	2012-02-29 22:49:25 +0000
@@ -3,12 +3,13 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from shelltoolbox import run
+
 from helpers import (
-    get_config,
+    get_config
     log,
     log_entry,
     log_exit,
-    run,
     )
 from local import (
     HTTP_PORT_PROTOCOL,

=== modified file 'hooks/tests.py'
--- hooks/tests.py	2012-02-23 16:45:54 +0000
+++ hooks/tests.py	2012-02-29 22:49:25 +0000
@@ -12,14 +12,14 @@
 import tempfile
 import unittest
 
-from helpers import (
+from shelltoolbox import (
     cd,
     command,
     DictDiffer,
     run,
     su,
-    unit_info,
     )
+from helpers import unit_info
 from local import (
     get_bucket,
     get_key,
@@ -28,95 +28,6 @@
     )
 
 
-class TestRun(unittest.TestCase):
-
-    def testSimpleCommand(self):
-        # Running a simple command (ls) works and running the command
-        # produces a string.
-        self.assertIsInstance(run('/bin/ls'), str)
-
-    def testStdoutReturned(self):
-        # Running a simple command (ls) works and running the command
-        # produces a string.
-        self.assertIn('Usage:', run('/bin/ls', '--help'))
-
-    def testCalledProcessErrorRaised(self):
-        # If an error occurs a CalledProcessError is raised with the return
-        # code, command executed, and the output of the command.
-        with self.assertRaises(CalledProcessError) as info:
-            run('ls', '--not a valid switch')
-        exception = info.exception
-        self.assertEqual(2, exception.returncode)
-        self.assertEqual("('ls', '--not a valid switch')", exception.cmd)
-        self.assertIn('unrecognized option', exception.output)
-
-
-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 TestDictDiffer(unittest.TestCase):
-
-    def testStr(self):
-        a = dict(cow='moo', pig='oink')
-        b = dict(cow='moo', pig='oinkoink', horse='nay')
-        diff = DictDiffer(b, a)
-        s = str(diff)
-        self.assertIn("added: {'horse': None} -> {'horse': 'nay'}", s)
-        self.assertIn("removed: {} -> {}", s)
-        self.assertIn("changed: {'pig': 'oink'} -> {'pig': 'oinkoink'}", s)
-        self.assertIn("unchanged: ['cow']", s)
-
-    def testStrUnmodified(self):
-        a = dict(cow='moo', pig='oink')
-        diff = DictDiffer(a, a)
-        s = str(diff)
-        self.assertEquals('no changes', s)
-
-    def testAddedOrChanged(self):
-        a = dict(cow='moo', pig='oink')
-        b = dict(cow='moo', pig='oinkoink', horse='nay')
-        diff = DictDiffer(b, a)
-        expected = set(['horse', 'pig'])
-        self.assertEquals(expected, diff.added_or_changed)
-
-
 class TestUnit_info(unittest.TestCase):
 
     def make_data(self, state='started'):
@@ -153,83 +64,6 @@
         self.assertNotEqual('started', state)
 
 
-current_euid = os.geteuid()
-current_egid = os.getegid()
-current_home = os.environ['HOME']
-example_euid = current_euid + 1
-example_egid = current_egid + 1
-example_home = '/var/lib/example'
-userinfo = {'example_user': dict(
-        ids=(example_euid, example_egid), home=example_home)}
-effective_values = dict(uid=current_euid, gid=current_egid)
-
-
-def stub_os_seteuid(value):
-    effective_values['uid'] = value
-
-
-def stub_os_setegid(value):
-    effective_values['gid'] = value
-
-
-class TestSuContextManager(unittest.TestCase):
-
-    def setUp(self):
-        import helpers
-        self.os_seteuid = os.seteuid
-        self.os_setegid = os.setegid
-        self.helpers_get_user_ids = helpers.get_user_ids
-        self.helpers_get_user_home = helpers.get_user_home
-        os.seteuid = stub_os_seteuid
-        os.setegid = stub_os_setegid
-        helpers.get_user_ids = lambda user: userinfo[user]['ids']
-        helpers.get_user_home = lambda user: userinfo[user]['home']
-
-    def tearDown(self):
-        import helpers
-        os.seteuid = self.os_seteuid
-        os.setegid = self.os_setegid
-        helpers.get_user_ids = self.helpers_get_user_ids
-        helpers.get_user_home = self.helpers_get_user_home
-
-    def testChange(self):
-        with su('example_user'):
-            self.assertEqual(example_euid, effective_values['uid'])
-            self.assertEqual(example_egid, effective_values['gid'])
-            self.assertEqual(example_home, os.environ['HOME'])
-
-    def testEnvironment(self):
-        with su('example_user') as e:
-            self.assertEqual(example_euid, e.uid)
-            self.assertEqual(example_egid, e.gid)
-            self.assertEqual(example_home, e.home)
-
-    def testRevert(self):
-        with su('example_user'):
-            pass
-        self.assertEqual(current_euid, effective_values['uid'])
-        self.assertEqual(current_egid, effective_values['gid'])
-        self.assertEqual(current_home, os.environ['HOME'])
-
-    def testRevertAfterFailure(self):
-        try:
-            with su('example_user'):
-                raise RuntimeError()
-        except RuntimeError:
-            self.assertEqual(current_euid, effective_values['uid'])
-            self.assertEqual(current_egid, effective_values['gid'])
-            self.assertEqual(current_home, os.environ['HOME'])
-
-
-class TestCdContextManager(unittest.TestCase):
-    def test_cd(self):
-        curdir = os.getcwd()
-        self.assertNotEqual('/var', curdir)
-        with cd('/var'):
-            self.assertEqual('/var', os.getcwd())
-        self.assertEqual(curdir, os.getcwd())
-
-
 class StubBucket:
     """Stub S3 Bucket."""
     def __init__(self, name):