Merge lp:~gary/charms/oneiric/buildbot-slave/run-as-buildbot into lp:~yellow/charms/oneiric/buildbot-slave/trunk

[Gary Poster]

These changes, along with those in https://code.launchpad.net/~gary/charms/oneiric/buildbot-master/run-as-buildbot/+merge/92399 , make the buildbot slave run as the buildbot user, and generally matches the Debian setup better.

[Gary Poster (gary)]

(As from the buildbot-master branch...)

The su contextmanager has an issue: the euid and egid are not set back properly when there is an error in the with block, for some reason. This can cause some problems: for some reason the juju log command hangs when the buildbot user tries to use it. Benji and I took a quick look at it and did not see a reason why they would do this. Perhaps it takes a moment to propagate?

I have not yet run the existing tests. I will do this before landing, resolving any issues. That said, this will bring up a master and slave for lpbuildbot, and at least check out the branch. Subsequently there's an error within Twisted/Buildbot itself, but it's on the right track.

[Francesco Banconi (frankban)]

Thank you Gary for this nice branch.

100 + # This is naive; we can make it more sophisticated (e.g., allowing
101 + # escaping dollar signs) if we discover we need it. For now,
102 + # simplicity wins.
103 + replaced_args = args.replace('$INSTALLDIR', buildbot_dir)

We could also do something like::

    formatted_args = args.format(**config)

And then, in the config file::

    script-args: "-u buildbot -e <email address hidden> -f 'Launchpad PQM' {installdir}"

It's as naive as your version, but maybe a little more generic?

60 - command('hg', 'clone', source, target)
61 + run('hg', 'clone', source, target)

Thank you for fixing all the script retrieval functions.
I have to make a note to myself: command helper returns a function!

Also thanks for your change on how the su context manager retrieves the user home directory:
please see the comment on the master MP for a suggestion on how to fix the exceptions problem too.

I've seen now buildslave is created in the install hook (great IMHO), but the same code is still present in config-changed: this could generate errors (buildslave created twice?), and requires further investigation.

review: Approve

=== modified file 'config.setuplxc.yaml'
--- config.setuplxc.yaml	2012-02-06 21:11:38 +0000
+++ config.setuplxc.yaml	2012-02-10 01:16:25 +0000
@@ -1,8 +1,10 @@
 buildbot-slave:
   builders: lucid_lp,lucid_db_lp
   script-retrieval-method: bzr_cat
-  script-url: "http://bazaar.launchpad.net/~gmb/launchpad/neuter-setuplxc/utilities/setuplxc.py"
+  script-url: "http://bazaar.launchpad.net/~gary/launchpad/setuplxc/utilities/setuplxc.py"
   script-path: setuplxc.py
-  script-args: "-u launchpad-pqm -e [e-mail] -f 'Launchpad PQM' /home/launchpad-pqm/launchpad/"
+  # The buildbot user's home directory is /var/lib/buildout.
+  # The LP dependencies will be in /var/lib/buildout/dependencies.
+  script-args: "-u buildbot -e [e-mail] -f 'Launchpad PQM' $INSTALLDIR"
   extra-repository: deb http://us.archive.ubuntu.com/ubuntu/ lucid main universe
   buildbot-pkg: buildbot/lucid

=== modified file 'config.yaml'
--- config.yaml	2012-02-06 21:11:38 +0000
+++ config.yaml	2012-02-10 01:16:25 +0000
@@ -40,4 +40,4 @@
     description: |
       The directory where the Buildbot slave will be installed.
     type: string
-    default: /tmp/buildslave
+    default: /var/lib/buildbot/slaves/slave

=== modified file 'hooks/install'
--- hooks/install	2012-02-09 10:44:48 +0000
+++ hooks/install	2012-02-10 01:16:25 +0000
@@ -7,11 +7,16 @@
     apt_get_install,
     command,
     get_config,
+    install_extra_repository,
     log,
     log_entry,
     log_exit,
     run,
     )
+from local import (
+    config_json,
+    create_slave,
+    )
 import os
 import shlex
 import subprocess
@@ -40,21 +45,21 @@
 
 def wget(source, path):
     target = os.path.join('/tmp', path)
-    command('wget', '-O', target, source)
+    run('wget', '-O', target, source)
     return target
 
 
 def hg_fetch(source, path):
     apt_get_install('mercurial')
     target = tempfile.mktemp()
-    command('hg', 'clone', source, target)
+    run('hg', 'clone', source, target)
     return os.path.join(target, path)
 
 
 def git_fetch(source, path):
     apt_get_install('git')
     target = tempfile.mktemp()
-    command('git', 'clone', source, target)
+    run('git', 'clone', source, target)
     return os.path.join(target, path)
 
 
@@ -82,9 +87,33 @@
     url = config.get('script-url')
     path = config.get('script-path')
     args = config.get('script-args')
+    buildbot_pkg = config.get('buildbot-pkg')
+    extra_repo = config.get('extra-repository')
+    buildbot_dir = config.get('installdir')
+
+    if extra_repo:
+        install_extra_repository(extra_repo)
+
+    if buildbot_pkg:
+        log('Installing ' + buildbot_pkg)
+        apt_get_install(buildbot_pkg)
+        log('Creating initial buildbot slave in ' + buildbot_dir)
+        create_slave('temporary', 'temporary', buildbot_dir=buildbot_dir)
+
+    config_json.set(config)
+
     retrieve = METHODS.get(method)
     if retrieve and url and path:
-        sys.exit(handle_script(retrieve, url, path, args))
+        # Make buildbot user have a shell by editing /etc/passwd.
+        # Otherwise you cannot ssh as this user, which some scripts
+        # need (e.g. those that create lxc containers).  We choose sh as
+        # a standard and basic "system" shell.
+        run('usermod', '-s', '/bin/sh', 'buildbot')
+        # This is naive; we can make it more sophisticated (e.g., allowing
+        # escaping dollar signs) if we discover we need it.  For now,
+        # simplicity wins.
+        replaced_args = args.replace('$INSTALLDIR', buildbot_dir)
+        sys.exit(handle_script(retrieve, url, path, replaced_args))
 
 
 if __name__ == '__main__':