run_isolated: support non-isolated commands

client/run_isolated.py

 #!/usr/bin/env python
 # Copyright 2012 The LUCI Authors. All rights reserved.
 # Use of this source code is governed by the Apache v2.0 license that can be
 # found in the LICENSE file.
 
-"""Reads a .isolated, creates a tree of hardlinks and runs the test.
+"""Runs a command with optional isolated input/output.
 
-To improve performance, it keeps a local cache. The local cache can safely be
-deleted.
+Despite name "run_isolated", can run a generic non-isolated command specified as
+args.
+
+If input isolated hash is provided, fetches it, creates a tree of hard links,
+appends args to the command in the fetched isolated and runs it.
+To improve performance, keeps a local cache.
+The local cache can safely be deleted.
 
 Any ${ISOLATED_OUTDIR} on the command line will be replaced by the location of a
 temporary directory upon execution of the command specified in the .isolated
 file. All content written to this directory will be uploaded upon termination
 and the .isolated file describing this directory will be printed to stdout.
 """
 
-__version__ = '0.6.1'
+__version__ = '0.7.0'
 
 import base64
 import logging
 import optparse
 import os
 import sys
 import tempfile
 import time
 
 from third_party.depot_tools import fix_encoding
 
 from utils import file_path
 from utils import fs
 from utils import large
 from utils import logging_utils
 from utils import on_error
 from utils import subprocess42
 from utils import tools
 from utils import zip_package
 
 import auth
-import isolated_format
 import isolateserver
 
 
+ISOLATED_OUTDIR_PARAMETER = '${ISOLATED_OUTDIR}'
+
 # Absolute path to this file (can be None if running from zip on Mac).
 THIS_FILE_PATH = os.path.abspath(__file__) if __file__ else None
 
 # Directory that contains this file (might be inside zip package).
 BASE_DIR = os.path.dirname(THIS_FILE_PATH) if __file__ else None
 
 # Directory that contains currently running script file.
 if zip_package.get_main_script_path():
   MAIN_DIR = os.path.dirname(
       os.path.abspath(zip_package.get_main_script_path()))
@@ skipping 76 matching lines @@
     # deleted.
     file_path.make_tree_writeable(rootdir)
   else:
     raise ValueError(
         'change_tree_read_only(%s, %s): Unknown flag %s' %
         (rootdir, read_only, read_only))
 
 
 def process_command(command, out_dir):
   """Replaces isolated specific variables in a command line."""
-  def fix(arg):
-    if '${ISOLATED_OUTDIR}' in arg:
-      return arg.replace('${ISOLATED_OUTDIR}', out_dir).replace('/', os.sep)
-    return arg
-
-  return [fix(arg) for arg in command]
+  out_dir = out_dir.replace('/', os.sep)
+  return [arg.replace(ISOLATED_OUTDIR_PARAMETER, out_dir) for arg in command]

[M-A Ruel, 2016/05/02 14:18:14]

The reason for the previous code was things like:

"${ISOLATED_OUTDIR}/foo/bar"

[nodir, 2016/05/02 17:43:22]

got it, reverted, added comment

 
 
 def run_command(command, cwd, tmp_dir, hard_timeout, grace_period):
   """Runs the command.
 
   Returns:
     tuple(process exit code, bool if had a hard timeout)
   """
   logging.info('run_command(%s, %s)' % (command, cwd))
 
@@ skipping 140 matching lines @@
     logging.exception('Had difficulties removing out_dir %s: %s', out_dir, e)
   stats = {
     'duration': time.time() - start,
     'items_cold': base64.b64encode(large.pack(cold)),
     'items_hot': base64.b64encode(large.pack(hot)),
   }
   return outputs_ref, success, stats
 
 
 def map_and_run(
-    isolated_hash, storage, cache, leak_temp_dir, root_dir, hard_timeout,
-    grace_period, extra_args):
-  """Maps and run the command. Returns metadata about the result."""
+    isolated_hash, args, storage, cache, leak_temp_dir, root_dir, hard_timeout,
+    grace_period):
+  """Runs a command with optional isolated input/output.
+
+  See run_tha_test for argument documentation.
+
+  Returns metadata about the result.
+  """
   result = {
     'duration': None,
     'exit_code': None,
     'had_hard_timeout': False,
     'internal_failure': None,
     'stats': {
     #  'download': {
     #    'duration': 0.,
     #    'initial_number_items': 0,
     #    'initial_size': 0,
@@ skipping 11 matching lines @@
   }
   if root_dir:
     file_path.ensure_tree(root_dir, 0700)
     prefix = u''
   else:
     root_dir = os.path.dirname(cache.cache_dir) if cache.cache_dir else None
     prefix = u'isolated_'
   run_dir = make_temp_dir(prefix + u'run', root_dir)
   out_dir = make_temp_dir(prefix + u'out', root_dir)
   tmp_dir = make_temp_dir(prefix + u'tmp', root_dir)
+  cwd = run_dir
+
   try:
-    bundle, result['stats']['download'] = fetch_and_measure(
-        isolated_hash=isolated_hash,
-        storage=storage,
-        cache=cache,
-        outdir=run_dir)
-    if not bundle.command:
-      # Handle this as a task failure, not an internal failure.
-      sys.stderr.write(
-          '<The .isolated doesn\'t declare any command to run!>\n'
-          '<Check your .isolate for missing \'command\' variable>\n')
-      if os.environ.get('SWARMING_TASK_ID'):
-        # Give an additional hint when running as a swarming task.
-        sys.stderr.write('<This occurs at the \'isolate\' step>\n')
-      result['exit_code'] = 1
-      return result
+    command = args
+    if isolated_hash:
+      bundle, result['stats']['download'] = fetch_and_measure(
+          isolated_hash=isolated_hash,
+          storage=storage,
+          cache=cache,
+          outdir=run_dir)
+      if not bundle.command:
+        # Handle this as a task failure, not an internal failure.
+        sys.stderr.write(
+            '<The .isolated doesn\'t declare any command to run!>\n'
+            '<Check your .isolate for missing \'command\' variable>\n')
+        if os.environ.get('SWARMING_TASK_ID'):
+          # Give an additional hint when running as a swarming task.
+          sys.stderr.write('<This occurs at the \'isolate\' step>\n')
+        result['exit_code'] = 1
+        return result
 
-    change_tree_read_only(run_dir, bundle.read_only)
-    cwd = os.path.normpath(os.path.join(run_dir, bundle.relative_cwd))
-    command = bundle.command + extra_args
+      change_tree_read_only(run_dir, bundle.read_only)
+      cwd = os.path.normpath(os.path.join(cwd, bundle.relative_cwd))
+      command = bundle.command + args
     file_path.ensure_command_has_abs_path(command, cwd)
     sys.stdout.flush()
     start = time.time()
     try:
       result['exit_code'], result['had_hard_timeout'] = run_command(
           process_command(command, out_dir), cwd, tmp_dir, hard_timeout,
           grace_period)
     finally:
       result['duration'] = max(time.time() - start, 0)
   except Exception as e:
@@ skipping 48 matching lines @@
       if not success and result['exit_code'] == 0:
         result['exit_code'] = 1
     except Exception as e:
       # Swallow any exception in the main finally clause.
       logging.exception('Leaking out_dir %s: %s', out_dir, e)
       result['internal_failure'] = str(e)
   return result
 
 
 def run_tha_test(
-    isolated_hash, storage, cache, leak_temp_dir, result_json, root_dir,
-    hard_timeout, grace_period, extra_args):
-  """Downloads the dependencies in the cache, hardlinks them into a temporary
-  directory and runs the executable from there.
+    isolated_hash, args, storage, cache, leak_temp_dir, result_json, root_dir,
+    hard_timeout, grace_period):
+  """Runs an executable and records execution metadata.
+
+  If isolated_hash is specified, downloads the dependencies in the cache,
+  hardlinks them into a temporary directory and runs the command specified in
+  the .isolated.
 
   A temporary directory is created to hold the output files. The content inside
   this directory will be uploaded back to |storage| packaged as a .isolated
   file.
 
   Arguments:
-    isolated_hash: the SHA-1 of the .isolated file that must be retrieved to
-                   recreate the tree of files to run the target executable.
+    isolated_hash: if not empty, the SHA-1 of the .isolated file that must be
+                   retrieved to recreate the tree of files to run the target
+                   executable. The command specified in the .isolated is
+                   executed.
+    args: if isolated_hash is not empty, arguments to append to the command in
+          the .isolated file. Otherwise, the command line to execute.

[M-A Ruel, 2016/05/02 14:51:17]

I'm not a fan of this ambivalence.

[nodir, 2016/05/02 17:43:22]

fixed

     storage: an isolateserver.Storage object to retrieve remote objects. This
              object has a reference to an isolateserver.StorageApi, which does
              the actual I/O.
     cache: an isolateserver.LocalCache to keep from retrieving the same objects
            constantly by caching the objects retrieved. Can be on-disk or
            in-memory.
     leak_temp_dir: if true, the temporary directory will be deliberately leaked
                    for later examination.
     result_json: file path to dump result metadata into. If set, the process
                  exit code is always 0 unless an internal error occured.
     root_dir: directory to the path to use to create the temporary directory. If
               not specified, a random temporary directory is created.
     hard_timeout: kills the process if it lasts more than this amount of
                   seconds.
     grace_period: number of seconds to wait between SIGTERM and SIGKILL.
-    extra_args: optional arguments to add to the command stated in the .isolate
-                file.
 
   Returns:
     Process exit code that should be used.
   """
+  if any(ISOLATED_OUTDIR_PARAMETER in a for a in args):
+    assert storage is not None, 'storage is None although outdir is specified'
+
   if result_json:
     # Write a json output file right away in case we get killed.
     result = {
       'exit_code': None,
       'had_hard_timeout': False,
       'internal_failure': 'Was terminated before completion',
       'outputs_ref': None,
       'version': 2,
     }
     tools.write_json(result_json, result, dense=True)
 
   # run_isolated exit code. Depends on if result_json is used or not.
   result = map_and_run(
-      isolated_hash, storage, cache, leak_temp_dir, root_dir, hard_timeout,
-      grace_period, extra_args)
+      isolated_hash, args, storage, cache, leak_temp_dir, root_dir,
+      hard_timeout, grace_period)
   logging.info('Result:\n%s', tools.format_json(result, dense=True))
   if result_json:
     # We've found tests to delete 'work' when quitting, causing an exception
     # here. Try to recreate the directory if necessary.
     file_path.ensure_tree(os.path.dirname(result_json))
     tools.write_json(result_json, result, dense=True)
     # Only return 1 if there was an internal error.
     return int(bool(result['internal_failure']))
 
   # Marshall into old-style inline output.
   if result['outputs_ref']:
     data = {
       'hash': result['outputs_ref']['isolated'],
       'namespace': result['outputs_ref']['namespace'],
       'storage': result['outputs_ref']['isolatedserver'],
     }
     sys.stdout.flush()
     print(
         '[run_isolated_out_hack]%s[/run_isolated_out_hack]' %
         tools.format_json(data, dense=True))
     sys.stdout.flush()
   return result['exit_code'] or int(bool(result['internal_failure']))
 
 
 def main(args):
   parser = logging_utils.OptionParserWithLogging(
-      usage='%prog <options>',
+      usage='%prog <options> [args]',

[M-A Ruel, 2016/05/02 14:51:17]

[command to run or extra args]

[nodir, 2016/05/02 17:43:22]

Done.

       version=__version__,
       log_file=RUN_ISOLATED_LOG_FILE)
   parser.add_option(
       '--clean', action='store_true',
       help='Cleans the cache, trimming it necessary and remove corrupted items '
            'and returns without executing anything; use with -v to know what '
            'was done')
   parser.add_option(
       '--json',
       help='dump output metadata to json file. When used, run_isolated returns '
            'non-zero only on internal failure')
   parser.add_option(
       '--hard-timeout', type='float', help='Enforce hard timeout in execution')
   parser.add_option(
       '--grace-period', type='float',
       help='Grace period between SIGTERM and SIGKILL')
   data_group = optparse.OptionGroup(parser, 'Data source')
   data_group.add_option(
       '-s', '--isolated',
-      help='Hash of the .isolated to grab from the isolate server')
+      help='Hash of the .isolated to grab from the isolate server.')
   isolateserver.add_isolate_server_options(data_group)
   parser.add_option_group(data_group)
 
   isolateserver.add_cache_options(parser)
   parser.set_defaults(cache='cache')
 
   debug_group = optparse.OptionGroup(parser, 'Debugging')
   debug_group.add_option(
       '--leak-temp-dir',
       action='store_true',
@@ skipping 10 matching lines @@
   if options.clean:
     if options.isolated:
       parser.error('Can\'t use --isolated with --clean.')
     if options.isolate_server:
       parser.error('Can\'t use --isolate-server with --clean.')
     if options.json:
       parser.error('Can\'t use --json with --clean.')
     cache.cleanup()
     return 0
 
+  if not options.isolated and not args:
+    parser.error('--isolated or args is required.')

[M-A Ruel, 2016/05/02 14:51:17]

s/args/command to run/ ?

[nodir, 2016/05/02 17:43:22]

Done.

+
   auth.process_auth_options(parser, options)
-  isolateserver.process_isolate_server_options(parser, options, True)
+
+  isolateserver.process_isolate_server_options(
+      parser, options, True, required=False)
+  if not options.isolate_server:
+    if options.isolated:
+      parser.error('--isolated requires --isolate-server')
+    if ISOLATED_OUTDIR_PARAMETER in args:
+      parser.error(
+          '%s in args requires --isolate-server' % ISOLATED_OUTDIR_PARAMETER)
 
   if options.root_dir:
     options.root_dir = unicode(os.path.abspath(options.root_dir))
   if options.json:
     options.json = unicode(os.path.abspath(options.json))
-  if not options.isolated:
-    parser.error('--isolated is required.')
-  with isolateserver.get_storage(
-      options.isolate_server, options.namespace) as storage:
+
+  storage = None
+  if options.isolate_server:
+    storage = isolateserver.get_storage(
+        options.isolate_server, options.namespace)
+    assert storage.hash_algo == cache.hash_algo
+
+  with storage or tools.noop_context():

[M-A Ruel, 2016/05/02 14:51:17]

while I understand the goal to not copy paste the function call twice, I'm not
sure this is worth, I think it's fine to just copy paste lines 598-601.

[nodir, 2016/05/02 17:43:22]

Done.

     # Hashing schemes used by |storage| and |cache| MUST match.
-    assert storage.hash_algo == cache.hash_algo
     return run_tha_test(
-        options.isolated, storage, cache, options.leak_temp_dir, options.json,
-        options.root_dir, options.hard_timeout, options.grace_period, args)
+        options.isolated, args, storage, cache, options.leak_temp_dir,
+        options.json, options.root_dir, options.hard_timeout,
+        options.grace_period)
 
 
 if __name__ == '__main__':
   # Ensure that we are always running with the correct encoding.
   fix_encoding.fix_encoding()
   sys.exit(main(sys.argv[1:]))