[go: up one dir, main page]

trigger_scripts: update trigger script for infra change

This doesn't change build output, but necessary for infra change.

This is synced to
https://chromium.googlesource.com/chromium/src/+/e361e8ebea8ddfa24870af1f64bea8ded7ddaf4e/testing/trigger_scripts

Bug: 1170924
Change-Id: I7aeb89c4658c8b321b4d930b195161360e51e4c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2654549
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/branch-heads/4324@{#2066}
Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
diff --git a/testing/trigger_scripts/OWNERS b/testing/trigger_scripts/OWNERS
index b6159823..a2a1d51 100644
--- a/testing/trigger_scripts/OWNERS
+++ b/testing/trigger_scripts/OWNERS
@@ -1,3 +1,5 @@
+bsheedy@chromium.org
 eyaich@chromium.org
 kbr@chromium.org
 wenbinzhang@google.com
+ynovikov@chromium.org
diff --git a/testing/trigger_scripts/PRESUBMIT.py b/testing/trigger_scripts/PRESUBMIT.py
index a4db812..377de4e 100644
--- a/testing/trigger_scripts/PRESUBMIT.py
+++ b/testing/trigger_scripts/PRESUBMIT.py
@@ -10,7 +10,7 @@
 
 def CommonChecks(input_api, output_api):
   return input_api.canned_checks.RunUnitTestsInDirectory(
-      input_api, output_api, '.', whitelist=['.*test.py'])
+      input_api, output_api, '.', files_to_check=['.*test.py'])
 
 def CheckChangeOnUpload(input_api, output_api):
   return CommonChecks(input_api, output_api)
diff --git a/testing/trigger_scripts/base_test_triggerer.py b/testing/trigger_scripts/base_test_triggerer.py
index 2239941..0728e5e 100755
--- a/testing/trigger_scripts/base_test_triggerer.py
+++ b/testing/trigger_scripts/base_test_triggerer.py
@@ -37,12 +37,13 @@
 def _convert_to_go_swarming_args(args):
   go_args = []
   map_flags = {'--dimension', '--env', '--env-prefix', '--named-cache'}
-  has_opt_dim = False
   i = 0
   while i < len(args):
     current_arg = args[i]
     if current_arg == '--swarming':
       current_arg = '--server'
+    elif current_arg == '--resultdb':
+      current_arg = '--enable-resultdb'
     go_args.append(current_arg)
     i += 1
     if current_arg in map_flags:
@@ -54,16 +55,6 @@
       # https://source.chromium.org/chromium/infra/infra/+/master:go/src/go.chromium.org/luci/client/cmd/swarming/lib/trigger.go;l=458-465;drc=ef40d3f3503c2cc7bb0f3f6807b14a39bafb6ce4
       go_args.append('{}:{}={}'.format(path, name, version))
       i += 1
-    elif current_arg == '--optional-dimension':
-      assert not has_opt_dim, ('Go swarming only supports one '
-                               '--optional-dimension, got: %s (index=%d)') % (
-                                   args[i:i + 3], i)
-      has_opt_dim = True
-
-      k, v, exp = args[i:i + 3]
-      # https://source.chromium.org/chromium/infra/infra/+/master:go/src/go.chromium.org/luci/client/cmd/swarming/lib/trigger.go;l=243;drc=ef40d3f3503c2cc7bb0f3f6807b14a39bafb6ce4
-      go_args.append('{}={}:{}'.format(k, v, exp))
-      i += 3
   return go_args
 
 
@@ -155,8 +146,7 @@
   # swarming to its own object to make trigger logic more clear.
   def query_swarming(self, api, query_args, verbose,
                      limit='0',
-                     server='chromium-swarm.appspot.com',
-                     service_account=None):
+                     server='chromium-swarm.appspot.com'):
     try:
       temp_file = self.make_temp_file(prefix='base_trigger_dimensions',
                                       suffix='.json')
@@ -168,10 +158,6 @@
              limit,
              '--json',
              temp_file]
-      # Add in service account auth if present
-      if service_account:
-        args.append('--auth-service-account-json')
-        args.append(service_account)
       # Append the query at the end
       args.append(('%s?%s' % (api, encoded_args)))
       ret = self.run_swarming(args, verbose)
@@ -239,10 +225,29 @@
       logging.info('Running Swarming with args: %s', args)
     return subprocess.call([sys.executable, SWARMING_PY] + args)
 
-  def run_swarming_go(self, args, verbose):
+  def run_swarming_go(self, args, verbose, json_path, shard_index, shards,
+                      merged_json=None):
     if verbose:
       logging.info('Running Go `swarming` with args: %s', args)
-    return subprocess.call([SWARMING_GO] + _convert_to_go_swarming_args(args))
+
+    if merged_json is None:
+      merged_json = {}
+
+    if 'tasks' not in merged_json:
+      merged_json['tasks'] = {}
+
+    ret = subprocess.call([SWARMING_GO] + _convert_to_go_swarming_args(args))
+    result_json = self.read_json_from_temp_file(json_path)
+
+    tasks = {
+      task['request']['task_id']: task['request']
+      for task  in result_json['tasks']
+    }
+    for k, v in tasks.items():
+      v['shard_index'] = shard_index
+      merged_json['tasks'][k + ':%d:%d' % (shard_index, shards)] = v
+    self.write_json_to_file(merged_json, json_path)
+    return ret
 
   def prune_test_specific_configs(self, args, verbose):
     # Ability for base class to further prune configs to
@@ -306,26 +311,12 @@
                                         suffix='.json')
         args_to_pass = self.modify_args(filtered_remaining_args, bot_index,
                                         shard_index, args.shards, json_temp)
-        if args.use_swarming_go:
-          ret = self.run_swarming_go(args_to_pass, verbose)
-        else:
-          ret = self.run_swarming(args_to_pass, verbose)
+        ret = self.run_swarming_go(
+          args_to_pass, verbose, json_temp, shard_index, args.shards,
+          merged_json)
         if ret:
           sys.stderr.write('Failed to trigger a task, aborting\n')
           return ret
-        result_json = self.read_json_from_temp_file(json_temp)
-        if not merged_json:
-          # Copy the entire JSON -- in particular, the "request"
-          # dictionary -- from the first shard. "swarming.py collect" uses
-          # some keys from this dictionary, in particular related to
-          # expiration. It also contains useful debugging information.
-          merged_json = copy.deepcopy(result_json)
-          # However, reset the "tasks" entry to an empty dictionary,
-          # which will be handled specially.
-          merged_json['tasks'] = {}
-        for k, v in result_json['tasks'].items():
-          v['shard_index'] = shard_index
-          merged_json['tasks'][k + ':%d:%d' % (shard_index, args.shards)] = v
       finally:
         self.delete_temp_file(json_temp)
     self.write_json_to_file(merged_json, args.dump_json)
@@ -350,14 +341,4 @@
     parser.add_argument('--shard-index', type=int, default=None,
                         help='Which shard to trigger. Duplicated from the '
                              '`swarming.py trigger` command.')
-    BaseTestTriggerer.add_use_swarming_go_arg(parser)
     return parser
-
-  @staticmethod
-  def add_use_swarming_go_arg(parser):
-    parser.add_argument(
-        '--use-swarming-go',
-        default=False,
-        action='store_true',
-        help='Uses swarming Go CLI to trigger tasks.')
-
diff --git a/testing/trigger_scripts/base_test_triggerer_unittest.py b/testing/trigger_scripts/base_test_triggerer_unittest.py
old mode 100644
new mode 100755
index 0e5fdec..a93eabd1
--- a/testing/trigger_scripts/base_test_triggerer_unittest.py
+++ b/testing/trigger_scripts/base_test_triggerer_unittest.py
@@ -1,3 +1,4 @@
+#!/usr/bin/env vpython
 # Copyright 2020 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.
@@ -5,25 +6,32 @@
 """Tests for base_device_trigger.py."""
 
 import argparse
+import json
 import unittest
 
+import mock
+
+from pyfakefs import fake_filesystem_unittest
+
 import base_test_triggerer
 
 
-class UnitTest(unittest.TestCase):
+class UnitTest(fake_filesystem_unittest.TestCase):
+  def setUp(self):
+    self.setUpPyfakefs()
+
   def test_convert_to_go_swarming_args(self):
     args = [
         '--swarming', 'x.apphost.com', '--dimension', 'pool', 'ci',
         '--dimension', 'os', 'linux', '--env', 'FOO', 'foo', '--hello',
         '--cipd-package', 'path:name:123', '--scalar', '42',
-        '--optional-dimension', 'os', 'ubuntu', '60'
+        '--resultdb'
     ]
     go_args = base_test_triggerer._convert_to_go_swarming_args(args)
     expected = [
         '--server', 'x.apphost.com', '--dimension', 'pool=ci', '--dimension',
         'os=linux', '--env', 'FOO=foo', '--hello', '--cipd-package',
-        'path:name=123', '--scalar', '42', '--optional-dimension',
-        'os=ubuntu:60'
+        'path:name=123', '--scalar', '42', '--enable-resultdb'
     ]
     self.assertEquals(go_args, expected)
 
@@ -41,34 +49,54 @@
         ], IndexError),
         # expected format: --cipd-package path:name:version
         (['--cipd-package', 'path:name'], ValueError),
-        # expected format: --optional-dimension key value expiry
-        (['--optional-dimension', 'key', 'value'], ValueError),
-        # can only specify one optional dimension
-        ([
-            '--optional-dimension',
-            'k1',
-            'v1',
-            '123',
-            '--optional-dimension',
-            'k2',
-            'v2',
-            '456',
-        ], AssertionError),
     ]
     for args, ex in invalid_args:
       self.assertRaises(ex, base_test_triggerer._convert_to_go_swarming_args,
                         args)
 
-  def test_arg_parser(self):
-    # Added for https://crbug.com/1143224
-    parser = argparse.ArgumentParser()
-    base_test_triggerer.BaseTestTriggerer.add_use_swarming_go_arg(parser)
-    swarming_args = ['--server', 'x.apphost.com', '--dimension', 'os', 'Linux']
-    args, _ = parser.parse_known_args(swarming_args)
-    self.assertFalse(args.use_swarming_go)
+  def test_trigger_tasks(self):
+    parser = base_test_triggerer.BaseTestTriggerer.setup_parser_contract(
+      argparse.ArgumentParser())
+    dump_json = 'dump.json'
+    args, remaining = parser.parse_known_args([
+      '--multiple-dimension-script-verbose', 'True',
+      'trigger', '--shards', '1', '--dump-json', dump_json
+    ])
+    triggerer = base_test_triggerer.BaseTestTriggerer()
 
-    args, _ = parser.parse_known_args(swarming_args + ['--use-swarming-go'])
-    self.assertTrue(args.use_swarming_go)
+    def mock_subprocess_call(args):
+      # write json file generated by 'swarming trigger' command.
+      json_path = args[args.index('--dump-json') + 1]
+      with open(json_path, 'w') as f:
+        f.write(
+          json.dumps({
+            'tasks': [{
+              'request': {
+                'task_id': 'f0',
+              },
+            }],
+          }))
+
+    # make some not important functions nop.
+    with mock.patch.object(
+        triggerer, 'parse_bot_configs'), mock.patch.object(
+          triggerer, '_bot_configs',
+          []), mock.patch.object(
+            triggerer, 'select_config_indices',
+            return_value=[(0, 0)]), mock.patch(
+              'subprocess.call', side_effect=mock_subprocess_call):
+      triggerer.trigger_tasks(args, remaining)
+
+    with open(dump_json) as f:
+      self.assertEqual(json.load(f), {
+        u'tasks': {
+          u'f0:0:1': {
+            u'shard_index': 0,
+            u'task_id': u'f0'
+          }
+        }
+      })
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/testing/trigger_scripts/chromeos_device_trigger.py b/testing/trigger_scripts/chromeos_device_trigger.py
index 49c22528..a3b5fe4 100755
--- a/testing/trigger_scripts/chromeos_device_trigger.py
+++ b/testing/trigger_scripts/chromeos_device_trigger.py
@@ -65,18 +65,10 @@
       dest='optional_dimensions',
       help='Optional dimensions which will result in additional task slices. '
            'Duplicated from the `swarming.py trigger` command.')
-  # BaseTestTriggerer's setup_parser_contract() takes care of adding needed
-  # swarming.py args if they're not already present. But only do this if
-  # '--shard-index' is passed in. (The exact usage of trigger scripts are
-  # currently changing. See crbug.com/926987 for more info.)
-  if '--shard-index' in sys.argv:
-    base_test_triggerer.BaseTestTriggerer.setup_parser_contract(parser)
-    args, additional_args = parser.parse_known_args()
-    additional_args = triggerer.modify_args(
-        additional_args, 0, args.shard_index, args.shards, args.dump_json)
-  else:
-    base_test_triggerer.BaseTestTriggerer.add_use_swarming_go_arg(parser)
-    args, additional_args = parser.parse_known_args()
+  base_test_triggerer.BaseTestTriggerer.setup_parser_contract(parser)
+  args, additional_args = parser.parse_known_args()
+  additional_args = triggerer.modify_args(
+      additional_args, 0, args.shard_index, args.shards, args.dump_json)
 
   if additional_args[0] != 'trigger':
     parser.error(
@@ -122,16 +114,13 @@
     new_args.extend(['--dimension', 'device_status', 'available'])
 
   new_args.extend([
-      '--optional-dimension',
-      'device_os',
-      current_lkgm,
-      str(PRIMARY_SLICE_EXPIRATION_S),
+      '-optional-dimension',
+      'device_os=%s:%d' % (current_lkgm, PRIMARY_SLICE_EXPIRATION_S),
   ])
   new_args += additional_args[1:]
 
-  if args.use_swarming_go:
-    return triggerer.run_swarming_go(new_args, True)
-  return triggerer.run_swarming(new_args, True)
+  return triggerer.run_swarming_go(
+    new_args, True, args.dump_json, args.shard_index or 0, args.shards)
 
 
 if __name__ == '__main__':
diff --git a/testing/trigger_scripts/perf_device_trigger.py b/testing/trigger_scripts/perf_device_trigger.py
index 748dcf9..64b75222 100755
--- a/testing/trigger_scripts/perf_device_trigger.py
+++ b/testing/trigger_scripts/perf_device_trigger.py
@@ -84,7 +84,6 @@
       # Store what swarming server we need and whether or not we need
       # to send down authentication with it
       self._swarming_server = self._get_swarming_server(swarming_args)
-      self._service_account = self._get_service_account(swarming_args)
 
       # Map of all existing bots in swarming that satisfy the current
       # set of dimensions indexed by bot id.
@@ -234,8 +233,7 @@
       values.append(('dimensions', '%s:%s' % (key, value)))
 
     query_result = self.query_swarming(
-        'bots/list', values, True, server=self._swarming_server,
-        service_account=self._service_account)
+        'bots/list', values, True, server=self._swarming_server)
     if 'items' not in query_result:
       return {}
     perf_bots = {}
@@ -269,8 +267,7 @@
 
     # Query for the last task that ran with these dimensions and this shard
     query_result = self.query_swarming(
-          'tasks/list', values, True, limit='1', server=self._swarming_server,
-         service_account=self._service_account)
+          'tasks/list', values, True, limit='1', server=self._swarming_server)
     tasks = query_result.get('items')
     if tasks:
       # We queried with a limit of 1 so we could only get back
@@ -299,10 +296,6 @@
         # Strip out the protocol
         return server[slashes_index:]
 
-  def _get_service_account(self, args):
-    for i in xrange(len(args) - 1):
-      if '--auth-service-account-json' in args[i]:
-        return args[i+1]
 
 def main():
   logging.basicConfig(
@@ -319,4 +312,3 @@
 
 if __name__ == '__main__':
   sys.exit(main())
-
diff --git a/testing/trigger_scripts/perf_device_trigger_unittest.py b/testing/trigger_scripts/perf_device_trigger_unittest.py
index 62980df9..629214df 100755
--- a/testing/trigger_scripts/perf_device_trigger_unittest.py
+++ b/testing/trigger_scripts/perf_device_trigger_unittest.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env vpython
 # Copyright 2018 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.
@@ -16,7 +16,6 @@
     self.dump_json = ''
     self.multiple_trigger_configs = None
     self.multiple_dimension_script_verbose = False
-    self.use_swarming_go = False
 
 
 class FakeTriggerer(perf_device_trigger.PerfDeviceTriggerer):
@@ -53,7 +52,8 @@
     del verbose #unused
     self._swarming_runs.append(args)
 
-  def run_swarming_go(self, args, verbose):
+  def run_swarming_go(self, args, verbose, _json_path, _shard_index, _shard,
+                      _merged_json=None):
     self._triggered_with_swarming_go += 1
     self.run_swarming(args, verbose)
 
@@ -61,19 +61,15 @@
   def setup_and_trigger(self,
                         previous_task_assignment_map,
                         alive_bots,
-                        dead_bots,
-                        use_swarming_go=False):
+                        dead_bots):
     args = Args()
     args.shards = len(previous_task_assignment_map)
     args.dump_json = 'output.json'
     args.multiple_dimension_script_verbose = True
-    args.use_swarming_go = use_swarming_go
     swarming_args = [
         'trigger',
         '--swarming',
         'http://foo_server',
-        '--auth-service-account-json',
-        '/creds/test_service_account',
         '--dimension',
         'pool',
         'chrome-perf-fyi',
@@ -114,18 +110,11 @@
       file_index = file_index + 1
     for i in xrange(num_shards):
       task = {
-        'base_task_name': 'webgl_conformance_tests',
-        'request': {
-          'expiration_secs': 3600,
-          'properties': {
-            'execution_timeout_secs': 3600,
-          },
-        },
-        'tasks': {
-          'webgl_conformance_tests on NVIDIA GPU on Windows': {
+        'tasks': [{
+          'request': {
             'task_id': 'f%d' % i,
           },
-        },
+        }],
       }
       files['base_trigger_dimensions%d.json' % file_index] = task
       file_index = file_index + 1
@@ -173,9 +162,6 @@
       self.assertTrue('query' in triggerer._swarming_runs[i])
       self.assertTrue(self.list_contains_sublist(
         triggerer._swarming_runs[i], ['-S', 'foo_server']))
-      self.assertTrue(self.list_contains_sublist(
-        triggerer._swarming_runs[i], ['--auth-service-account-json',
-                                      '/creds/test_service_account']))
 
   def get_triggered_shard_to_bot(self, triggerer, num_shards):
     self.assert_query_swarming_args(triggerer, num_shards)
@@ -300,15 +286,6 @@
     self.assertEquals(set(expected_task_assignment.values()),
         {'build3', 'build4', 'build5', 'build7'})
 
-  def test_use_swarming_go_to_trigger(self):
-    triggerer = self.setup_and_trigger(
-        previous_task_assignment_map={0: 'build1'},
-        alive_bots=['build1'],
-        dead_bots=[],
-        use_swarming_go=True)
-
-    self.assertEquals(triggerer._triggered_with_swarming_go, 1)
-
 
 if __name__ == '__main__':
   unittest.main()