Modify annotated_run to read factory configs from the slave checkout.

scripts/slave/annotated_run.py

 #!/usr/bin/env python
 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # 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
 import sys
 
 BUILD_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(
     os.path.abspath(__file__))))
 sys.path.append(os.path.join(BUILD_ROOT, 'scripts'))
 sys.path.append(os.path.join(BUILD_ROOT, 'third_party'))
 
 from common import annotator
 from common import chromium_utils
 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()
+  mastername = factory_properties['mastername']
+  buildername = factory_properties['buildername']
+
+  properties = {}
+
+  if mastername and buildername:
+    properties = get_on_disk_factory_properties(mastername, buildername)
+
+    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
 
-  # 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]
+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
 
-      # 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']))
-  return properties
+  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."""
 
   parser = optparse.OptionParser(
       description='Entry point for annotated builds.')
   parser.add_option('--build-properties',
                     action='callback', callback=chromium_utils.convert_json,
                     type='string', default={},
                     help='build properties in JSON format')
   parser.add_option('--factory-properties',
                     action='callback', callback=chromium_utils.convert_json,
                     type='string', default={},
                     help='factory properties in JSON format')
   parser.add_option('--build-properties-gz',
                     action='callback', callback=chromium_utils.convert_gz_json,
                     type='string', default={}, dest='build_properties',
                     help='build properties in b64 gz JSON format')
   parser.add_option('--factory-properties-gz',
                     action='callback', callback=chromium_utils.convert_gz_json,
                     type='string', default={}, dest='factory_properties',
                     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)
 
 
 def update_scripts():
   if os.environ.get('RUN_SLAVE_UPDATED_SCRIPTS'):
     os.environ.pop('RUN_SLAVE_UPDATED_SCRIPTS')
     return False
 
   stream = annotator.StructuredAnnotationStream()
 
@@ skipping 17 matching lines @@
     # After running update scripts, set PYTHONIOENCODING=UTF-8 for the real
     # annotated_run.
     os.environ['PYTHONIOENCODING'] = 'UTF-8'
 
     return True
 
 
 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())
   return ret.status_code
 
 
 def shell_main(argv):
   if update_scripts():
     return subprocess.call([sys.executable] + argv)
   else:
     return main(argv)
 
 if __name__ == '__main__':
   sys.exit(shell_main(sys.argv))