client.skia: separate builder and buildslave configs

masters/master.client.skia/master.cfg

Index: masters/master.client.skia/master.cfg
diff --git a/masters/master.client.skia/master.cfg b/masters/master.client.skia/master.cfg
index 19b3e2e9f469fe0aee19383fadfa92e05f6a4133..e77745f386573ec41a03adb43798685ef66e4097 100644
--- a/masters/master.client.skia/master.cfg
+++ b/masters/master.client.skia/master.cfg
@@ -3,12 +3,15 @@
 # found in the LICENSE file.
 # vim: set ft=python:
 
-from buildbot.changes.gitpoller import GitPoller
+import collections
 
+from buildbot.changes.gitpoller import GitPoller
+from buildbot.scheduler import Scheduler
+from buildbot.scheduler import Triggerable
+from common import chromium_utils
 from master import build_utils
 from master import master_config
 from master import master_utils
-from master import recipe_master_helper
 from master import slaves_list
 from master.factory import annotator_factory
 
@@ -16,6 +19,82 @@ import config
 import master_site_config
 
 
+def AddSchedulersAndTriggers(buildmaster_config=None,
+                             builders=None,
+                             scheduler_name=None,
+                             branch=None):

[iannucci, 2014/07/21 17:31:03]

buildmaster_config and builders shouldn't default to None. This function would
break if that happened.

[borenet, 2014/07/21 18:04:31]

Yeah, and I don't know what happens when the scheduler name is None either, so
removed the default.

+  """Clone of recipe_master_helper.AddSchedulersAndTriggers which separates
+  builders and buildslaves more cleanly.
+  """
+  c = buildmaster_config
+  polling_builders = []
+  # Maps the parent builder to a set of the names of the builders it triggers.
+  trigger_map = collections.defaultdict(list)
+  # Maps the name of the parent builder to the (synthesized) name of its
+  # trigger, wrapped in a list.
+  trigger_name_map = {}
+  next_group_id = 0
+  for builder in builders:
+    builder_name = builder['name']
+    parent_builder = builder.get('triggered_by')
+    if parent_builder is not None:
+      for maybe_parent in builders:

[iannucci, 2014/07/21 17:31:03]

instead of looping a second time, I would add all of the
builders-which-will-be-parents to a set, and then check that potential_parents
>= set(builders.iterkeys()) at the end of the outer loop.

[borenet, 2014/07/21 18:04:31]

Did something similar but used difference so I could provide an interesting
error message.

+        if maybe_parent['name'] == parent_builder:
+          break
+      else:
+        raise Exception('Could not find parent builder %s for builder %s' %
+                        (parent_builder, builder_name))
+      trigger_map[parent_builder].append(builder_name)
+      if parent_builder not in trigger_name_map:
+        trigger_name_map[parent_builder] = 'trigger_group_%d' % next_group_id
+        next_group_id += 1
+    else:
+      polling_builders.append(builder_name)
+  s = Scheduler(name=scheduler_name,
+                branch=branch,
+                treeStableTimer=60,
+                builderNames=polling_builders)
+  c['schedulers'] = [s]
+  for name, builders in trigger_map.iteritems():
+    c['schedulers'].append(Triggerable(name=trigger_name_map[name],
+                                       builderNames=builders))
+  return trigger_name_map
+
+
+def AddRecipeBasedBuilders(buildmaster_config=None,
+                           builders=None,
+                           annotator=None,
+                           trigger_name_map=None,
+                           default_auto_reboot=False):
+  """Clone of recipe_master_helper.AddRecipeBasedBuilders which separates
+  builders and buildslaves more cleanly.
+  """
+  builders_list = []
+  for builder in builders:
+    if 'recipe' in builder:

[iannucci, 2014/07/21 17:31:03]

if it's not, that should be an error, right?

[borenet, 2014/07/21 18:04:31]

In the next CL, I'll remove the 'if' and just default to the "skia" recipe.

+      name = builder['name']
+      build_config = 'Release' if 'Release' in name else 'Debug'

[iannucci, 2014/07/21 17:31:03]

hm, sneaky. If we were making the names have semantic value, I'd prefer that the
builder names be /generated/ from a spec, instead of having the spec parsed from
the name. But as you like.

[borenet, 2014/07/21 18:04:31]

Yeah, this is bad. Skia's bots currently do exactly that: the names are derived
from their configurations.  However, in this new world where all the interesting
stuff happens on the buildslave, I think the way to go is to have the list of
builder names on the master and parse the name to get the configs on the slave
side (and remove this build_config property here).  This is another thing I was
saving for the next CL; should I just lump the two together?

+      factory_properties = {
+        'test_results_server': 'test-results.appspot.com',
+        'generate_gtest_json': True,
+        'build_config': build_config,
+      }
+      if 'perf_id' in builder:
+        factory_properties['show_perf_results'] = True
+        factory_properties['perf_id'] = builder['perf_id']
+      builder_dict = {
+        'name': name,
+        'factory': annotator.BaseFactory(
+          builder['recipe'],
+          factory_properties,
+          [trigger_name_map[name]] if name in trigger_name_map else None),
+        'gatekeeper': builder.get('gatekeeper_categories', ''),
+      }
+      builder_dict['auto_reboot'] = builder.get('auto_reboot', False)
+      builders_list.append(builder_dict)
+  buildmaster_config['builders'] = builders_list
+
+
 ActiveMaster = master_site_config.Skia
 
 c = BuildmasterConfig = {}
@@ -31,37 +110,31 @@ poller = GitPoller(
     pollinterval=10,
     revlinktmpl='https://skia.googlesource.com/skia/+/%s')
 
-
 c['change_source'] = [poller]
 
-
 ####### SLAVES
 
 # Load the slave list. We need some information from it in order to
 # produce the builders.
-slaves = slaves_list.SlavesList('slaves.cfg', 'Skia')
-
-####### SCHEDULERS
-
-## configure the Schedulers
+slaves = slaves_list.SlavesList('slaves.cfg', ActiveMaster.project_name)
 
-# Main scheduler for all changes in trunk.
+####### BUILDERS
 
-trigger_name_map = recipe_master_helper.AddSchedulersAndTriggers(
-    buildmaster_config=c, slave_list=slaves, scheduler_name='skia',
-    branch='master')
+# Load the builders list.
+builders = chromium_utils.ParsePythonCfg('builders.cfg')['builders']
 
-####### BUILDERS
+####### SCHEDULERS
 
-builders = []
+# Configure the Schedulers.
+trigger_name_map = AddSchedulersAndTriggers(buildmaster_config=c,
+                                            builders=builders,
+                                            scheduler_name='skia',
+                                            branch='master')
 
-# ----------------------------------------------------------------------------
-# FACTORIES
+####### FACTORIES
 
 m_annotator = annotator_factory.AnnotatorFactory()
-
-recipe_master_helper.AddRecipeBasedBuilders(
-    c, slaves, m_annotator, trigger_name_map)
+AddRecipeBasedBuilders(c, builders, m_annotator, trigger_name_map)
 
 # Associate the slaves to the manual builders. The configuration is in
 # slaves.cfg.
@@ -74,7 +147,6 @@ for builder in c['builders']:
 # slaves registered to a builder. Remove dupes.
 c['slaves'] = master_utils.AutoSetupSlaves(c['builders'],
                                            config.Master.GetBotPassword())
-
 master_utils.VerifySetup(c, slaves)
 
 ####### STATUS TARGETS