Change _ArchiveTestResults to upload to Google Storage

bin/cbuildbot.py

 #!/usr/bin/python
 
 # Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """CBuildbot is wrapper around the build process used by the pre-flight queue"""
 
 import errno
 import heapq
@@ skipping 383 matching lines @@
   else:
     overlays = [public_overlay, private_overlay]
   RunCommand(['./cros_mark_as_stable', '--srcroot=..',
               '--board=%s' % board,
               '--overlays=%s' % " ".join(overlays),
               '--tracking_branch=%s' % tracking_branch,
               '--push_options=--bypass-hooks -f', 'push'],
              cwd=cwd)
 
 
-def _ArchiveTestResults(buildroot, board, archive_dir, test_results_dir):
+def _gsutil_with_retries(command, num_retries):

[sosa, 2010/11/12 18:49:09]

Name style is wrong

[scottz-goog, 2010/11/12 19:15:02]

This will is just a RunCommandWithRetries and you are calling it with the gsutil
binary

[scottz-goog, 2010/11/12 19:15:02]

While I know this file doesn't follow it exactly we should start convering on
what we've started using as the "Standard" for docstrings

"""One line describing the function.

More indepth description if needed.

Arguments:
  command: a list of commands/args to execute
  num_retries: the number of retries to perform before dying

Returns:
  Only use this is we are returning something otherwise drop it.

Raises:
   Only use this if something is raised 

etc etc. 

+  """Run the gsutil command and retry in case of failure.
+
+  This is helpful when attempting to upload a large file to Google Storage.
+  gsutil will resume any broken upload when it is retried.
+
+  command: Command to run

[sosa, 2010/11/12 18:49:09]

Command should be an array

+  num_retries: Number of times to retry the command
+  """
+  for i in range(num_retries):
+    try:
+      RunCommand(command)
+      return

[sosa, 2010/11/12 18:49:09]

No explicit return

+    except:
+      Warning('Retrying command')

[sosa, 2010/11/12 18:49:09]

Print out command you are retrying.

+
+
+def _ArchiveTestResults(buildroot, board, test_results_dir,
+                        gsutil, archive_dir, acl):
   """Archives the test results into the www dir for later use.
 
-  Takes the results from the test_results_dir and dumps them into the archive
-  dir specified.  This also archives the last qemu image.
+  Takes the results from the test_results_dir and the last qemu image and
+  uploads them to Google Storage.
 
   board:  Board to find the qemu image.
-  archive_dir:  Path from ARCHIVE_BASE to store image.
   test_results_dir: Path from buildroot/chroot to find test results.  This must
     a subdir of /tmp.
+  gsutil: Location of gsutil
+  archive_dir: Google Storage path to store the archive
+  acl: ACL to set on archive in Google Storage
   """
+  num_gsutil_retries = 5
   test_results_dir = test_results_dir.lstrip('/')
-  if not os.path.exists(ARCHIVE_BASE):
-    os.makedirs(ARCHIVE_BASE)
-  else:
-    dir_entries = os.listdir(ARCHIVE_BASE)
-    if len(dir_entries) >= ARCHIVE_COUNT:
-      oldest_dirs = heapq.nsmallest((len(dir_entries) - ARCHIVE_COUNT) + 1,
-          [os.path.join(ARCHIVE_BASE, filename) for filename in dir_entries],
-          key=lambda fn: os.stat(fn).st_mtime)
-      Info('Removing archive dirs %s' % oldest_dirs)
-      for oldest_dir in oldest_dirs:
-        shutil.rmtree(os.path.join(ARCHIVE_BASE, oldest_dir))
-
-  archive_target = os.path.join(ARCHIVE_BASE, str(archive_dir))
-  if os.path.exists(archive_target):
-    shutil.rmtree(archive_target)
-
   results_path = os.path.join(buildroot, 'chroot', test_results_dir)
   RunCommand(['sudo', 'chmod', '-R', '+r', results_path])
   try:
-    shutil.copytree(results_path, archive_target)
+    _gsutil_with_retries([gsutil, 'cp', '-R', results_path, archive_dir],
+                         num_gsutil_retries)
   except:

[scottz-goog, 2010/11/12 19:15:02]

We should never be catching blank exceptions. You are calling RunCommand in both
instances so we should be catching the exception it throws. When you catch
blanket exceptions you run the risk of masking problems. I've definitely created
a few "mystery" problems by doing this. 

-    Warning('Some files could not be copied')
+    Warning('Some files could not be uploaded')
+
+  try:
+    RunCommand([gsutil, 'setacl', acl, archive_dir])

[sosa, 2010/11/12 18:49:09]

All these try's should be in the same try clause probably.  If gsutil is being
flaky in one try it'll prob be flaky again right after.

+  except:

[scottz-goog, 2010/11/12 19:15:02]

see above

+    Warning('Could not set archive ACL')
 
   image_name = 'chromiumos_qemu_image.bin'
   image_path = os.path.join(buildroot, 'src', 'build', 'images', board,
                             'latest', image_name)
-  RunCommand(['gzip', '-f', '--fast', image_path])
-  shutil.copyfile(image_path + '.gz', os.path.join(archive_target,
-                                                   image_name + '.gz'))
-
+  try:
+    RunCommand(['gzip', '-f', '--fast', image_path])
+    _gsutil_with_retries([gsutil, 'cp', image_path + '.gz', archive_dir],
+                         num_gsutil_retries)
+  except:

[scottz-goog, 2010/11/12 19:15:02]

See above.

+    Warning('Could not gzip/upload QEMU image')
 
 
 def _GetConfig(config_name):
   """Gets the configuration for the build"""
   default = config['default']
   buildconfig = {}
   if not config.has_key(config_name):
     Warning('Non-existent configuration specified.')
     Warning('Please specify one of:')
     config_names = config.keys()
@@ skipping 25 matching lines @@
                     default=False,
                     help='Clobbers an old checkout before syncing')
   parser.add_option('--debug', action='store_true', dest='debug',
                     default=False,
                     help='Override some options to run as a developer.')
   parser.add_option('-t', '--tracking-branch', dest='tracking_branch',
                     default='cros/master', help='Run the buildbot on a branch')
   parser.add_option('-u', '--url', dest='url',
                     default='http://git.chromium.org/git/manifest',
                     help='Run the buildbot on internal manifest')
+  parser.add_option('-g', '--gsutil', default='', help='Location of gsutil')
+  parser.add_option('-c', '--gsutil_archive', default='',
+                    help='Datastore archive location')
+  parser.add_option('-a', '--acl', default='',

[sosa, 2010/11/12 18:49:09]

Why should this be user specified?  SHouldn't we have a default value?

+                    help='ACL to set on GSD archives')
 
   (options, args) = parser.parse_args()
 
   buildroot = options.buildroot
   revisionfile = options.revisionfile
   tracking_branch = options.tracking_branch
 
   if len(args) >= 1:
     buildconfig = _GetConfig(args[-1])
   else:
@@ skipping 27 matching lines @@
 
     _BuildImage(buildroot)
 
     if buildconfig['smoke_bvt']:
       _BuildVMImageForTesting(buildroot)
       test_results_dir = '/tmp/run_remote_tests.%s' % options.buildnumber
       try:
         _RunSmokeSuite(buildroot, test_results_dir)
       finally:
         _ArchiveTestResults(buildroot, buildconfig['board'],
-                            archive_dir=options.buildnumber,
-                            test_results_dir=test_results_dir)
+                            test_results_dir=test_results_dir,
+                            gsutil=options.gsutil,
+                            archive_dir=options.gsutil_archive + '/' +

[sosa, 2010/11/12 18:49:09]

use os.path.join here

+                                        str(options.buildnumber),

[sosa, 2010/11/12 18:49:09]

is str() needed here?

+                            acl=options.acl)
 
     if buildconfig['uprev']:
       # Don't push changes for developers.
       if not options.debug:
         if buildconfig['master']:
           # Master bot needs to check if the other slaves completed.
           if cbuildbot_comm.HaveSlavesCompleted(config):
             _UprevPush(buildroot, tracking_branch, buildconfig['board'],
                        buildconfig['overlays'])
           else:
             Die('CBUILDBOT - One of the slaves has failed!!!')
 
         else:
           # Publish my status to the master if its expecting it.
           if buildconfig['important']:
             cbuildbot_comm.PublishStatus(cbuildbot_comm.STATUS_BUILD_COMPLETE)
 
   except:
     # Send failure to master bot.
     if not buildconfig['master'] and buildconfig['important']:
       cbuildbot_comm.PublishStatus(cbuildbot_comm.STATUS_BUILD_FAILED)
 
     raise
 
 
 if __name__ == '__main__':
     main()