Keep track of hashes triggered instead of builds.

scripts/slave/gatekeeper_ng.py

 #!/usr/bin/env python
 # Copyright 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.
 
 """Closes tree if configured masters have failed tree-closing steps.
 
 Given a list of masters, gatekeeper_ng will get a list of the latest builds from
 the specified masters. It then checks if any tree-closing steps have failed, and
 if so closes the tree and emails appropriate parties. Configuration for which
@@ skipping 20 matching lines @@
 from common import chromium_utils
 from slave import gatekeeper_ng_config
 
 SCRIPTS_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                            '..', '..')
 
 # Buildbot status enum.
 SUCCESS, WARNINGS, FAILURE, SKIPPED, EXCEPTION, RETRY = range(6)
 
 
+# Bump each time there is an incompatible change in build_db.
+BUILD_DB_VERSION = 1
+
+
 def get_pwd(password_file):
   if os.path.isfile(password_file):
     return open(password_file, 'r').read().strip()
   return getpass.getpass()
 
 
 def update_status(tree_message, tree_status_url, username, password):
   """Connects to chromium-status and closes the tree."""
   #TODO(xusydoc): append status if status is already closed.
   params = urllib.urlencode({
       'message': tree_message,
       'username': username,
       'password': password
   })
 
   # Standard urllib doesn't raise an exception on 403, urllib2 does.
   f = urllib2.urlopen(tree_status_url, params)
   f.close()
   logging.info('success')
 
 
 def get_root_json(master_url):
   """Pull down root JSON which contains builder and build info."""
   logging.info('opening %s' % (master_url + '/json'))
   with closing(urllib2.urlopen(master_url + '/json')) as f:
     return json.load(f)
 
 
-def find_new_builds(master_url, root_json, build_db):
+def find_new_builds(master_url, root_json, build_db, options):

[iannucci, 2014/02/20 03:30:55]

GREAT EVIL!!!

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

   """Given a dict of previously-seen builds, find new builds on each builder.
 
-  Note that we use the 'cachedBuilds here since it should be faster, and this
+  Note that we use the 'cachedBuilds' here since it should be faster, and this
   script is meant to be run frequently enough that it shouldn't skip any builds.
 
   'Frequently enough' means 1 minute in the case of Buildbot or cron, so the
   only way for gatekeeper_ng to be overwhelmed is if > cachedBuilds builds
   complete within 1 minute. As cachedBuilds is scaled per number of slaves per
   builder, the only way for this to really happen is if a build consistently
   takes < 1 minute to complete.
   """
   new_builds = {}
   build_db[master_url] = build_db.get(master_url, {})
+
+  last_finished_build = {}
+  for builder, builds in build_db[master_url].iteritems():
+    if any(b.get('finished') for b in builds):
+      last_finished_build[builder] = max(
+          b['build'] for b in builds if b.get('finished'))

[iannucci, 2014/02/20 03:30:55]

lame to iterate over builds twice, but still O(N) :P

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

+
   for buildername, builder in root_json['builders'].iteritems():
     candidate_builds = set(builder['cachedBuilds'] + builder['currentBuilds'])

[iannucci, 2014/02/20 03:30:55]

comment: cachedBuilds == finishedBuilds

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

-    if buildername in build_db[master_url]:
-      new_builds[buildername] = [x for x in candidate_builds
-                                 if x > build_db[master_url][buildername]]
+    if buildername in last_finished_build:
+      new_builds[buildername] = [{'build': x} for x in candidate_builds
+                                 if x > last_finished_build[buildername]]
     else:
-      new_builds[buildername] = candidate_builds
+      if (buildername in build_db[master_url] or
+          options.process_finished_builds_on_new_builder):
+        # Scan finished builds as well as unfinished.
+        new_builds[buildername] = [{'build': x} for x in candidate_builds]

[iannucci, 2014/02/20 03:30:55]

should have structs or namedtuples or classes or something for building up this
db

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

+      else:
+        # New builder or master, ignore past builds.
+        new_builds[buildername] = [
+            {'build': x} for x in builder['currentBuilds']]
 
-    # This is a heuristic, as currentBuilds may become completed by the time we
-    # scan them. The buildDB is fixed up later to account for this.
-    completed = set(builder['cachedBuilds']) - set(builder['currentBuilds'])
-    if completed:
-      build_db[master_url][buildername] = max(completed)
+    # Update build_db but don't duplicate builds already in there.
+    for build in new_builds.get(buildername, []):
+      build_db_builds = build_db[master_url].setdefault(buildername, [])
+      if not any(x['build'] == build['build'] for x in build_db_builds):
+        build_db_builds.append(build)
+
+    # Pull old + new unfinished builds from build_db.
+    new_builds[buildername] = [
+        b for b in build_db[master_url].setdefault(buildername, [])
+        if not b.get('finished')]
 
   return new_builds
 
 
-def find_new_builds_per_master(masters, build_db):
+def find_new_builds_per_master(masters, build_db, options):
   """Given a list of masters, find new builds and collect them under a dict."""
   builds = {}
   master_jsons = {}
   for master in masters:
     root_json = get_root_json(master)
     master_jsons[master] = root_json
-    builds[master] = find_new_builds(master, root_json, build_db)
+    builds[master] = find_new_builds(master, root_json, build_db, options)
   return builds, master_jsons
 
 
-def get_build_json(url_pair):
-  url, master = url_pair
+def get_build_json(url_tuple):
+  url, master, builder, build = url_tuple
   logging.debug('opening %s...' % url)
   with closing(urllib2.urlopen(url)) as f:
-    return json.load(f), master
+    return json.load(f), master, builder, build
 
 
 def get_build_jsons(master_builds, build_db, processes):
   """Get all new builds on specified masters.
 
   This takes a dict in the form of [master][builder][build], formats that URL
   and appends that to url_list. Then, it forks out and queries each build_url
   for build information.
   """
   url_list = []
   for master, builder_dict in master_builds.iteritems():
     for builder, new_builds in builder_dict.iteritems():
       for build in new_builds:
         safe_builder = urllib.quote(builder)
-        url = master + '/json/builders/%s/builds/%s' % (safe_builder, build)
-        url_list.append((url, master))
-  # The async/get is so that ctrl-c can interrupt the scans.
-  # See http://stackoverflow.com/questions/1408356/
-  # keyboard-interrupts-with-pythons-multiprocessing-pool
-  with chromium_utils.MultiPool(processes) as pool:
-    builds = filter(bool, pool.map_async(get_build_json, url_list).get(9999999))
+        url = master + '/json/builders/%s/builds/%s' % (safe_builder,
+                                                        build['build'])
+        url_list.append((url, master, builder, build))
+  # Prevent map from hanging, see http://bugs.python.org/issue12157.
+  if url_list:
+    # The async/get is so that ctrl-c can interrupt the scans.
+    # See http://stackoverflow.com/questions/1408356/
+    # keyboard-interrupts-with-pythons-multiprocessing-pool
+    with chromium_utils.MultiPool(processes) as pool:
+      builds = filter(bool, pool.map_async(get_build_json, url_list).get(
+          9999999))
+  else:
+    builds = []
 
-  for build_json, master in builds:
+  # Pools pickle and unpickle, which means the build object we use isn't the
+  # real build object. We recover it here so we can modify build_db later in the
+  # program.
+  def find_original_build(master, builder, build):
+    return next(b for b in build_db[master][builder]
+                if b['build'] == build['build'])
+  builds = [(u, m, find_original_build(m, bd, bl))
+            for u, m, bd, bl in builds]

[iannucci, 2014/02/20 03:30:55]

zip url_list with the rest of the data so that you don't need this fixup loop...
or just use the build number as the key of the hash.

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

+
+  # This is needed for the --sync-db option.
+  for build_json, master, build in builds:
     if build_json.get('results', None) is not None:
-      build_db[master][build_json['builderName']] = max(
-          build_json['number'],
-          build_db[master][build_json['builderName']])
+      build['finished'] = True
   return builds
 
 
-def check_builds(master_builds, master_jsons, build_db, gatekeeper_config):
+def check_builds(master_builds, master_jsons, gatekeeper_config):
   """Given a gatekeeper configuration, see which builds have failed."""
   failed_builds = []
-  for build_json, master_url in master_builds:
+  for build_json, master_url, build in master_builds:
     gatekeeper_sections = gatekeeper_config.get(master_url, [])
     for gatekeeper_section in gatekeeper_sections:
+      section_hash = gatekeeper_ng_config.gatekeeper_section_hash(
+          gatekeeper_section)
+
       if build_json['builderName'] in gatekeeper_section:
         gatekeeper = gatekeeper_section[build_json['builderName']]
       elif '*' in gatekeeper_section:
         gatekeeper = gatekeeper_section['*']
       else:
         gatekeeper = {}
+
       steps = build_json['steps']
       forgiving = set(gatekeeper.get('forgiving_steps', []))
       forgiving_optional = set(gatekeeper.get('forgiving_optional', []))
       closing_steps = set(gatekeeper.get('closing_steps', [])) | forgiving
       closing_optional = set(
           gatekeeper.get('closing_optional', [])) | forgiving_optional
       tree_notify = set(gatekeeper.get('tree_notify', []))
       sheriff_classes = set(gatekeeper.get('sheriff_classes', []))
       subject_template = gatekeeper.get('subject_template',
                                         gatekeeper_ng_config.DEFAULTS[
@@ skipping 25 matching lines @@
       # If the entire build failed.
       if (not unsatisfied_steps and 'results' in build_json and
           build_json['results'] != SUCCESS and respect_build_status):
         unsatisfied_steps.add('[overall build status]')
 
       buildbot_url = master_jsons[master_url]['project']['buildbotURL']
       project_name = master_jsons[master_url]['project']['title']
 
       logging.debug('%sbuilders/%s/builds/%d ----', buildbot_url,
                     build_json['builderName'], build_json['number'])
+      logging.debug('  section hash: %s', section_hash)
       logging.debug('  build steps: %s', ', '.join(s['name'] for s in steps))
       logging.debug('  closing steps: %s', ', '.join(closing_steps))
       logging.debug('  closing optional steps: %s', ', '.join(closing_optional))
       logging.debug('  finished steps: %s', ', '.join(finished_steps))
       logging.debug('  successful: %s', ', '.join(successful_steps))
       logging.debug('  build complete: %s', bool(
           build_json.get('results', None) is not None))
       logging.debug('  unsatisfied steps: %s', ', '.join(unsatisfied_steps))
       logging.debug('  set to close tree: %s', close_tree)
       logging.debug('  build failed: %s', bool(unsatisfied_steps))
-      logging.debug('----')
 
 
       if unsatisfied_steps:
-        build_db[master_url][build_json['builderName']] = max(
-            build_json['number'],
-            build_db[master_url][build_json['builderName']])
+        if section_hash in build.get('triggered', []):
+          logging.debug('  section has already been triggered for this build, '
+                        'skipping...')
+        else:
+          build.setdefault('triggered', []).append(section_hash)
 
-        failed_builds.append({'base_url': buildbot_url,
-                              'build': build_json,
-                              'close_tree': close_tree,
-                              'forgiving_steps': forgiving | forgiving_optional,
-                              'project_name': project_name,
-                              'sheriff_classes': sheriff_classes,
-                              'subject_template': subject_template,
-                              'tree_notify': tree_notify,
-                              'unsatisfied': unsatisfied_steps,
-                             })
+          failed_builds.append({'base_url': buildbot_url,
+                                'build': build_json,
+                                'close_tree': close_tree,
+                                'forgiving_steps': (
+                                    forgiving | forgiving_optional),
+                                'project_name': project_name,
+                                'sheriff_classes': sheriff_classes,
+                                'subject_template': subject_template,
+                                'tree_notify': tree_notify,
+                                'unsatisfied': unsatisfied_steps,
+                               })
+      logging.debug('----')
 
   return failed_builds
 
 
 def parse_sheriff_file(url):
   """Given a sheriff url, download and parse the appropirate sheriff list."""
   with closing(urllib2.urlopen(url)) as f:
     line = f.readline()
   usernames_matcher_ = re.compile(r'document.write\(\'([\w, ]+)\'\)')
   usernames_match = usernames_matcher_.match(line)
@@ skipping 158 matching lines @@
   """Open the build_db file.
 
   filename: the filename of the build db.
   """
   build_db = None
   if os.path.isfile(filename):
     print 'loading build_db from', filename
     with open(filename) as f:
       build_db = json.load(f)
 
-  return build_db or {}
+  if build_db and build_db.get('build_db_version', 0) != BUILD_DB_VERSION:
+    new_fn = '%s.old' % filename
+    logging.warn('%s is an older db version: %d (expecting %d). moving to '
+                 '%s' % (filename, build_db.get('build_db_version', 0),
+                         BUILD_DB_VERSION, new_fn))
+    chromium_utils.MoveFile(filename, new_fn)
+    build_db = None
+
+  if build_db and 'masters' in build_db:
+    return build_db['masters']
+  return {}
 
 
-def save_build_db(build_db, filename):
+def save_build_db(build_db_data, gatekeeper_config, filename):
   """Save the build_db file.
 
   build_db: dictionary to jsonize and store as build_db.
   filename: the filename of the build db.
+  gatekeeper_config: the gatekeeper config used for this pass.
   """
   print 'saving build_db to', filename
+
+  # Remove all but the last finished build.
+  for builders in build_db_data.values():
+    for builder in builders:
+      if any(b.get('finished') for b in builders[builder]):
+        last_finished_build = max(
+            b['build'] for b in builders[builder] if b.get('finished'))
+        builders[builder] = [
+            b for b in builders[builder] if (b['build'] == last_finished_build
+                                             or not b.get('finished'))]
+
+  build_db = {
+      'build_db_version': BUILD_DB_VERSION,
+      'masters': build_db_data,
+      'sections': {},
+  }
+
+  # Output the gatekeeper sections we're operating with, so a human reading the
+  # file can debug issues. This is discarded by the parser in get_build_db.
+  used_sections = set([])
+  for builders in build_db_data.values():
+    for builds in builders.values():
+      used_sections |= reduce(
+          lambda x, y: x | set(y.get('triggered', [])), builds, set([]))

[iannucci, 2014/02/20 03:30:55]

{ t 
  for b in builds
  for t in b.get('triggered', []) 
}

[ghost stip (do not use), 2014/02/22 10:03:07]

Done.

+  for master in gatekeeper_config.values():
+    for section in master:
+      section_hash = gatekeeper_ng_config.gatekeeper_section_hash(section)
+      if (section_hash in used_sections and
+          section_hash not in build_db['sections']):

[iannucci, 2014/02/20 03:30:55]

Use the sets, luke.

[ghost stip (do not use), 2014/02/22 10:03:07]

dude you just unlocked an entire set of lgtm sagas for luqui@ (see
https://chromiumcodereview.appspot.com/71553003/#msg11)

+        build_db['sections'][section_hash] = section
+
   with open(filename, 'wb') as f:
-    json.dump(build_db, f)
+    gatekeeper_ng_config.flatten_to_json(build_db, f)
 
 
 def get_options():
   prog_desc = 'Closes the tree if annotated builds fail.'
   usage = '%prog [options] <one or more master urls>'
   parser = optparse.OptionParser(usage=(usage + '\n\n' + prog_desc))
   parser.add_option('--build-db', default='build_db.json',
                     help='records the last-seen build for each builder')
   parser.add_option('--clear-build-db', action='store_true',
                     help='reset build_db to be empty')
@@ skipping 35 matching lines @@
   parser.add_option('--no-email-app', action='store_true',
                     help='don\'t send emails')
   parser.add_option('--json', default='gatekeeper.json',
                     help='location of gatekeeper configuration file')
   parser.add_option('--verify', action='store_true',
                     help='verify that the gatekeeper config file is correct')
   parser.add_option('--flatten-json', action='store_true',
                     help='display flattened gatekeeper.json for debugging')
   parser.add_option('--no-hashes', action='store_true',
                     help='don\'t insert gatekeeper section hashes')
+  parser.add_option('--process-finished-builds-on-new-builder',
+                    action='store_true',
+                    help='when processing a new builder, process finished '
+                         'builds')
   parser.add_option('-v', '--verbose', action='store_true',
                     help='turn on extra debugging information')
 
   options, args = parser.parse_args()
 
   options.email_app_secret = None
   options.password = None
 
   if options.no_hashes and not options.flatten_json:
     parser.error('specifying --no-hashes doesn\'t make sense without '
@@ skipping 42 matching lines @@
 
   masters = set(args)
   if not masters <= set(gatekeeper_config):
     print 'The following masters are not present in the gatekeeper config:'
     for m in masters - set(gatekeeper_config):
       print '  ' + m
     return 1
 
   if options.clear_build_db:
     build_db = {}
-    save_build_db(build_db, options.build_db)
+    save_build_db(build_db, gatekeeper_config, options.build_db)
   else:
     build_db = get_build_db(options.build_db)
 
-  new_builds, master_jsons = find_new_builds_per_master(masters, build_db)
+  new_builds, master_jsons = find_new_builds_per_master(masters, build_db,
+                                                        options)
+  build_jsons = get_build_jsons(new_builds, build_db, options.parallelism)
+
   if options.sync_build_db:
-    save_build_db(build_db, options.build_db)
+    save_build_db(build_db, gatekeeper_config, options.build_db)
     return 0
-  build_jsons = get_build_jsons(new_builds, build_db, options.parallelism)
-  failed_builds = check_builds(build_jsons, master_jsons, build_db,
-                               gatekeeper_config)
+
+  failed_builds = check_builds(build_jsons, master_jsons, gatekeeper_config)
   if options.set_status:
     options.password = get_pwd(options.password_file)
 
   close_tree_if_failure(failed_builds, options.status_user, options.password,
                         options.status_url, options.set_status,
                         options.sheriff_url, options.default_from_email,
                         options.email_app_url, options.email_app_secret,
                         options.email_domain, options.filter_domain,
                         options.disable_domain_filter)
 
   if not options.skip_build_db_update:
-    save_build_db(build_db, options.build_db)
+    save_build_db(build_db, gatekeeper_config, options.build_db)
 
   return 0
 
 
 if __name__ == '__main__':
   sys.exit(main())