Modify annotated_run to read factory configs from the slave checkout.

scripts/slave/annotated_run.py

Index: scripts/slave/annotated_run.py
diff --git a/scripts/slave/annotated_run.py b/scripts/slave/annotated_run.py
index 136e84bde1fd9c6e37f5af71d4a00c1f94b58d0c..440e2e22fe2980dd9c2a71a3a3c9c5977474c8c5 100755
--- a/scripts/slave/annotated_run.py
+++ b/scripts/slave/annotated_run.py
@@ -3,6 +3,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import json
 import optparse
 import os
 import subprocess
@@ -20,39 +21,76 @@ from slave import recipe_universe
 from recipe_engine import main as recipe_main
 
 
-def get_recipe_properties(factory_properties, build_properties):
+def get_recipe_properties(factory_properties, build_properties,
+                          override_lookup):
   """Constructs the recipe's properties from buildbot's properties.
 
-  This merges factory_properties and build_properties.  Furthermore, it
-  tries to reconstruct the 'recipe' property from builders.pyl if it isn't
-  already there, and in that case merges in properties form builders.pyl.
+  This retrieves the current factory properties from the master_config
+  in the slave's checkout (the factory properties handed to us from the
+  master might be out of date), and merges in the build properties.
+
+  Using the values from the checkout allows us to do things like change
+  the recipe and other factory properties for a builder without needing
+  a master restart.
   """
-  properties = factory_properties.copy()
-  properties.update(build_properties)
+  mastername = factory_properties['mastername']
+  buildername = factory_properties['buildername']
+
+  properties = {}
+
+  if mastername and buildername:
+    properties = get_on_disk_factory_properties(mastername, buildername)
 
-  # Try to reconstruct the recipe from builders.pyl if not given.
-  if 'recipe' not in properties:
-    mastername = properties['mastername']
-    buildername = properties['buildername']
-
-    master_path = chromium_utils.MasterPath(mastername)
-    builders_file = os.path.join(master_path, 'builders.pyl')
-    if os.path.isfile(builders_file):
-      builders = chromium_utils.ReadBuildersFile(builders_file)
-      assert buildername in builders['builders'], (
-        'buildername %s is not listed in %s' % (buildername, builders_file))
-      builder = builders['builders'][buildername]
-
-      # Update properties with builders.pyl data.
-      properties['recipe'] = builder['recipe']
-      properties.update(builder.get('properties', {}))
-    else:
-      raise LookupError('Cannot find recipe for %s on %s' %
-                        (build_properties['buildername'],
-                        build_properties['mastername']))
+    if override_lookup:
+      if properties['recipe'] != factory_properties['recipe']:
+        print >> sys.stderr, (
+            'WARNING: Using recipe "%s" given on the command line '
+            'command line, rather than "%s" as listed in the master.cfg' %
+            (properties['recipe'], factory_properties['recipe']))

[Dirk Pranke, 2015/06/11 22:40:21]

I don't warn about other factory properties also overriding, but I'm a little
worried that people might see this and not realize that the
other properties will *also* override.

I know that it's worth enumerating all of the mismatches, but I could do that. 

We could also just print a generic message and not any of the details.

Thoughts?

[iannucci, 2015/06/11 23:01:49]

I'd err on the side of enumerating the mismatches just to avoid headscratchers.
Should be able to do

for key in (set(properties) | set(factory_properties)):
  etc.

[Dirk Pranke, 2015/06/11 23:22:47]

Ok. It's easy enough to do.

+      properties.update(factory_properties)
+  else:
+    properties = factory_properties.copy()
+
+  properties.update(build_properties)
   return properties
 
 
+def get_on_disk_factory_properties(mastername, buildername):
+  script_path = os.path.join(BUILD_ROOT, 'scripts', 'tools',
+                             'dump_master_cfg.py')
+  dump_cmd = [sys.executable,
+              script_path,
+              mastername]
+  proc = subprocess.Popen(dump_cmd, cwd=BUILD_ROOT, stdout=subprocess.PIPE)
+  out, _ = proc.communicate()
+  exit_code = proc.returncode
+
+  if exit_code:
+    raise LookupError('Failed to get the master config; dump_master_cfg %s'
+                      'returned %d):\n%s\n'% (
+                      mastername, exit_code, out))
+
+  config = json.loads(out)
+
+  # Now extract just the factory properties for the requested builder
+  # from the master config.
+  props = None
+  for builder_dict in config['builders']:
+    if builder_dict['name'] == buildername:
+      props = dict((k, v) for k, v, _ in
+                  builder_dict['factory'].properties.asList())

[iannucci, 2015/06/11 23:01:49]

hm... isn't config json though? I don't think there's going to be any asList

[Dirk Pranke, 2015/06/11 23:22:47]

Yeah, that's probably true. I need to run this, because I'm not sure whether the
properties will contain the third, 'source' parameter or not as part of the
serialized info. Hopefully it doesn't, and we can use the dict as-is.

(will verify).

+
+  if not props:
+    raise LookupError('builder "%s" not found on in master "%s"' %
+                      (buildername, mastername))
+
+  if 'recipe' not in props:
+    raise LookupError('Cannot find recipe for %s on %s' %
+                      (buildername, mastername))
+
+  return props
+
+
 def get_args(argv):
   """Process command-line arguments."""
 
@@ -76,6 +114,10 @@ def get_args(argv):
                     help='factory properties in b64 gz JSON format')
   parser.add_option('--keep-stdin', action='store_true', default=False,
                     help='don\'t close stdin when running recipe steps')
+  parser.add_option('--override-lookup', action='store_true',
+                    help='use the recipe and other factory properties '
+                         'given on the command line rather than the '
+                         'properties in the master.cfg')

[iannucci, 2015/06/11 23:01:49]

IIRC, run_recipe passes all args as build properties, right?

[Dirk Pranke, 2015/06/11 23:22:47]

looks like run_recipe passes all args as both, just to be confusing :).

   return parser.parse_args(argv)
 
 
@@ -113,7 +155,7 @@ def update_scripts():
 def main(argv):
   opts, _ = get_args(argv)
   properties = get_recipe_properties(
-      opts.factory_properties, opts.build_properties)
+      opts.factory_properties, opts.build_properties, opts.override_lookup)
   stream = annotator.StructuredAnnotationStream()
   ret = recipe_main.run_steps(properties, stream,
                               universe=recipe_universe.get_universe())