Change _ArchiveTestResults to upload to Google Storage

bin/cbuildbot.py

Index: bin/cbuildbot.py
diff --git a/bin/cbuildbot.py b/bin/cbuildbot.py
index 372ddcaf5ff646d2a966b897cffe00c95d9beea7..ce1224e7cd9fea7aad592fdad8df916bd7742a05 100755
--- a/bin/cbuildbot.py
+++ b/bin/cbuildbot.py
@@ -401,48 +401,61 @@ def _UprevPush(buildroot, tracking_branch, board, overlays):
              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 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('Some files could not be copied')
+    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):
@@ -488,6 +501,11 @@ def main():
   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()
 
@@ -535,8 +553,11 @@ def main():
         _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.