yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/oneiric/buildbot-master/02-13 into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/02-13/+merge/92812

*Changes*:

- Relation broken: the master correctly reconfigures buildbot when the relationship with a slave is broken (e.g. when you call remove-relation or destroy-service).

- The JSON file containing slaves info is now initialized in the `install` hook: cleaning up things in `install` is useful since we do not have a destroy hook.

- The helper function `wait_for_page_contents` now accepts a callable as a validator to break the inner loop (this is used in buildbot-slave tests).

-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/02-13/+merge/92812
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/charms/oneiric/buildbot-master/02-13 into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== added file 'hooks/buildbot-relation-broken'
--- hooks/buildbot-relation-broken	1970-01-01 00:00:00 +0000
+++ hooks/buildbot-relation-broken	2012-02-13 17:58:02 +0000
@@ -0,0 +1,56 @@
+#!/usr/bin/env python
+
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from helpers import (
+    log,
+    log_entry,
+    log_exit,
+    run,
+    su,
+    )
+from local import (
+    buildbot_reconfig,
+    slave_json,
+    )
+
+
+def get_remote_unit():
+    """Return the name of the remote unit for which the relation is broken."""
+    # XXX 2012-02-13 frankban:
+    #    This is a workaround for https://bugs.launchpad.net/juju/+bug/791042.
+    #    Normally we should use `os.getenv('JUJU_REMOTE_UNIT')` to get
+    #    the remote unit name.
+    current_relations = set(slave_json.get().keys())
+    relations = run('relation-list').split()
+    return current_relations.difference(relations).pop()
+
+
+def update_slave_json(name):
+    with su('buildbot'):
+        slave_info = slave_json.get()
+        if name in slave_info:
+            del slave_info[name]
+            slave_json.set(slave_info)
+            return True
+    return False
+
+
+def main():
+    name = get_remote_unit()
+    log('Departed unit: {}.'.format(name))
+    if update_slave_json(name):
+        log('Removed unit {}.'.format(name))
+        log("Reconfiguring buildbot.")
+        buildbot_reconfig()
+    else:
+        log("Slave not used. Returning.")
+
+
+if __name__ == '__main__':
+    log_entry()
+    try:
+        main()
+    finally:
+        log_exit()

=== modified file 'hooks/buildbot-relation-changed'
--- hooks/buildbot-relation-changed	2012-02-10 00:22:21 +0000
+++ hooks/buildbot-relation-changed	2012-02-13 17:58:02 +0000
@@ -20,21 +20,16 @@
     )
 
 
-def get_or_create(key, prefix=''):
-    log("Retrieving {}.".format(key))
-    value = relation_get(key)
-    if not value:
-        log("Generating {}.".format(key))
-        value = generate_string(prefix)
-    log("{}: {}".format(key, value))
-    return value
-
-
-def update_slave_json(name, passwd, builders):
+def update_slave_json(builders):
+    name = os.getenv('JUJU_REMOTE_UNIT')
     with su('buildbot'):
         slave_info = slave_json.get()
+        passwd, _ = slave_info.get(name, (None, None))
+        if passwd is None:
+            passwd = generate_string()
         slave_info[name] = (passwd, builders)
         slave_json.set(slave_info)
+    return name, passwd
 
 
 def main():
@@ -42,20 +37,15 @@
     builders = filter(
         None,
         (b.strip() for b in relation_get('builders').split(',')))
-    if not builders:
-        log("No builders set.  Returning.")
-        return
-    log("builders: {}".format(builders))
-    name = os.environ.get('JUJU_REMOTE_UNIT')
-    slave_info = slave_json.get()
-    passwd, dummy = slave_info.get(name, (None, None))
-    if passwd is None:
-        passwd = generate_string()
-    update_slave_json(name, passwd, builders)
-    log("Reconfiguring buildbot.")
-    buildbot_reconfig()
-    log("Sending name and password to the slave.")
-    relation_set(name=name, passwd=passwd)
+    if builders:
+        log("builders: {}".format(builders))
+        name, passwd = update_slave_json(builders)
+        log("Reconfiguring buildbot.")
+        buildbot_reconfig()
+        log("Sending name and password to the slave.")
+        relation_set(name=name, passwd=passwd)
+    else:
+        log("No builders set. Returning.")
 
 
 if __name__ == '__main__':

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-02-10 21:19:58 +0000
+++ hooks/config-changed	2012-02-13 17:58:02 +0000
@@ -28,7 +28,6 @@
     buildbot_reconfig,
     config_json,
     generate_string,
-    slave_json,
     )
 
 REQUIRED_KEYS = [
@@ -147,9 +146,6 @@
     uid, gid = get_user_ids('buildbot')
     os.chown(master_cfg_path, uid, gid)
     with su('buildbot'):
-        # Initialise the slave JSON if it doesn't exist.
-        if not slave_json.exists():
-            slave_json.set({})
         placeholder_path = os.path.join(buildbot_dir, 'placeholder.json')
         if not os.path.exists(placeholder_path):
             with open(placeholder_path, 'w') as f:

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-10 21:29:42 +0000
+++ hooks/helpers.py	2012-02-13 17:58:02 +0000
@@ -28,6 +28,7 @@
 from collections import namedtuple
 from contextlib import contextmanager
 import json
+import operator
 import os
 import pwd
 import re
@@ -253,7 +254,9 @@
         time.sleep(0.1)
 
 
-def wait_for_page_contents(url, s, timeout=120):
+def wait_for_page_contents(url, s, timeout=120, validate=None):
+    if validate is None:
+        validate = operator.contains
     start_time = time.time()
     while True:
         try:
@@ -262,7 +265,7 @@
             pass
         else:
             page = stream.read()
-            if s in page:
+            if validate(page, s):
                 return page
         if time.time() - start_time >= timeout:
             raise RuntimeError('timeout waiting for contents of ' + url)

=== modified file 'hooks/install'
--- hooks/install	2012-02-08 14:26:58 +0000
+++ hooks/install	2012-02-13 17:58:02 +0000
@@ -14,9 +14,11 @@
     log_entry,
     log_exit,
     run,
+    su,
     )
 from local import (
     config_json,
+    slave_json,
     )
 
 
@@ -36,6 +38,9 @@
     # Initialize the cached config so that old configs don't hang around
     # after the service is torn down.
     config_json.set({})
+    # # Initialise the slave JSON.
+    with su('buildbot'):
+        slave_json.set({})
 
 
 def main():