yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/charms/oneiric/buildbot-master/bbm-cleanup into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow): code

For more details, see:
https://code.launchpad.net/~bac/charms/oneiric/buildbot-master/bbm-cleanup/+merge/94444

This branch fixes a problem with the way the original master.cfg was handled.  The function initialize_buildbot moved the master.cfg file to master_original.cfg without regard for whether it had been called before, and substituted in a wrapper config file as master.cfg.  So if initialize_buildbot were to be called a second time, then the original config file would be overwritten and we'd have two copies of the wrapper.

The change here is to rename hooks/master.cfg to hooks/wrapper.cfg.  The original master.cfg file is never touched.  Instead wrapper.cfg includes master.cfg and buildbot.tac is modified to use wrapper.cfg as the configfile to use.  
-- 
https://code.launchpad.net/~bac/charms/oneiric/buildbot-master/bbm-cleanup/+merge/94444
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~bac/charms/oneiric/buildbot-master/bbm-cleanup into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-02-14 15:51:49 +0000
+++ hooks/config-changed	2012-02-23 20:20:12 +0000
@@ -27,7 +27,10 @@
     buildbot_reconfig,
     config_json,
     generate_string,
+    get_tac_filename,
+    get_wrapper_cfg_path,
     slave_json,
+    WRAPPER_FN,
     )
 
 REQUIRED_KEYS = [
@@ -95,8 +98,6 @@
     log("ADDED OR CHANGED: {}".format(added_or_changed))
     if config_file and 'config-file' in added_or_changed:
         buildbot_create(buildbot_dir)
-        # This file will be moved to master.cfg.original in
-        # initialize_buildbot().
         with su('buildbot'), open(master_cfg_path, 'w') as f:
             f.write(config_file)
         log('config_file written.')
@@ -138,13 +139,18 @@
     # Initialize the buildbot directory and (re)start buildbot.
     log("Initializing buildbot")
     buildbot_dir =  config.get('installdir')
-    master_cfg_path = os.path.join(buildbot_dir, 'master.cfg')
-    shutil.move(master_cfg_path, master_cfg_path + '.original')
+    tac_filename = get_tac_filename(buildbot_dir)
+    with open(tac_filename, 'r') as f:
+        tac = f.read()
+    mod_tac = tac.replace("master.cfg", "wrapper.cfg")
+    with open(tac_filename, 'buildbot.tac'), 'w') as f:
+        f.write(mod_tac)
+    wrapper_cfg_path = get_wrapper_cfg_path()
     shutil.copy(
-        os.path.join(os.path.dirname(__file__), 'master.cfg'),
-        master_cfg_path)
+        os.path.join(os.path.dirname(__file__), WRAPPER_FN),
+        wrapper_cfg_path)
     uid, gid = get_user_ids('buildbot')
-    os.chown(master_cfg_path, uid, gid)
+    os.chown(wrapper_cfg_path, uid, gid)
     with su('buildbot'):
         # Initialise the slave JSON if it doesn't exist.
         if not slave_json.exists():

=== modified file 'hooks/local.py'
--- hooks/local.py	2012-02-10 01:05:09 +0000
+++ hooks/local.py	2012-02-23 20:20:12 +0000
@@ -11,8 +11,11 @@
     'config_json',
     'create_slave',
     'generate_string',
+    'get_tac_filename',
+    'get_wrapper_cfg_path',
     'HTTP_PORT_PROTOCOL',
     'slave_json',
+    'WRAPPER_FN',
     ]
 
 import os
@@ -29,6 +32,7 @@
 
 
 HTTP_PORT_PROTOCOL = '8010/TCP'
+WRAPPER_FN = 'wrapper.cfg'
 
 
 def _get_buildbot_dir():
@@ -36,6 +40,15 @@
     return config.get('installdir')
 
 
+def get_tac_filename(buildbot_dir):
+    return os.path.join(buildbot_dir, 'buildbot.tac')
+
+
+def get_wrapper_cfg_path():
+    return os.path.join(
+        _get_buildbot_dir(), WRAPPER_FN)
+
+
 def generate_string(prefix=""):
     """Generate a unique string and return it."""
     return prefix + uuid.uuid4().hex
@@ -43,7 +56,7 @@
 
 def buildbot_create(buildbot_dir):
     """Create a buildbot instance in `buildbot_dir`."""
-    if not os.path.exists(os.path.join(buildbot_dir, 'buildbot.tac')):
+    if not os.path.exists(get_tac_filename(buildbot_dir)):
         with su('buildbot'):
             return run('buildbot', 'create-master', buildbot_dir)
 
@@ -102,10 +115,6 @@
             log('<-- Starting buildbot')
 
 
-def _get_tac_filename(buildbot_dir):
-    return os.path.join(buildbot_dir, 'buildbot.tac')
-
-
 def buildslave_stop(buildbot_dir=None):
     if buildbot_dir is None:
         buildbot_dir = _get_buildbot_dir()
@@ -117,7 +126,7 @@
             exit_code = subprocess.call(['buildslave', 'stop', buildbot_dir])
         except OSError:
             exit_code = subprocess.call(['buildbot', 'stop', buildbot_dir])
-    tac_file = _get_tac_filename(buildbot_dir)
+    tac_file = get_tac_filename(buildbot_dir)
     if os.path.exists(tac_file):
         os.remove(tac_file)
     return exit_code

=== renamed file 'hooks/master.cfg' => 'hooks/wrapper.cfg'
--- hooks/master.cfg	2012-02-07 13:29:08 +0000
+++ hooks/wrapper.cfg	2012-02-23 20:20:12 +0000
@@ -1,7 +1,7 @@
 # -*- python -*-
 # ex: set syntax=python:
 
-with open('master.cfg.original') as f:
+with open('master.cfg') as f:
     exec f
 
 import uuid