Bug 1333167: Add extra try options to taskcluster. r=dustin a=jmaher

We add the following command line options to Taskcluster try syntax:

--spsProfile - enable profile mode.
--rebuild-talos <N> - retrigger talos tests N times.
--setenv <VAR>=<val> - add extra environments variables.
--tag <TAG> - run tests only the tag TAG.
--no-retry - doesn't retry failed jobs.

We have a chicken-egg problem, as we first generate the full task graph
and then parse the try message. But the graph generation step needs to
know the try message to process the aforementioned options. The
solution is to parse the message before graph generation and then
pass the command line options to the transforms. Then, each transform
can look at the option that interests it and process it accordingly.

The message parse function is configured in kind.yml, which gives some
flexibility for future implementations of alternative syntaxes.

MozReview-Commit-ID: GPFdi0FD6Vn

--HG--
extra : rebase_source : b992786158851f1099aedfce8669a163228edc51

taskcluster/ci/build/kind.yml

@@ -15,3 +15,5 @@ jobs-from:
     - linux.yml
     - macosx.yml
     - windows.yml
+
+parse-commit: taskgraph.try_option_syntax:parse_message

taskcluster/ci/test/kind.yml

@@ -6,3 +6,5 @@ kind-dependencies:
 transforms:
    - taskgraph.transforms.tests:transforms
    - taskgraph.transforms.task:transforms
+
+parse-commit: taskgraph.try_option_syntax:parse_message

taskcluster/docs/loading.rst

@@ -29,3 +29,12 @@ on tasks that already exist.  You can see a nice example of this behavior in
 
 For more information on how all of this works, consult the docstrings and
 comments in the source code itself.
+
+Try option syntax
+-----------------
+
+The ``parse-commit`` optional field specified in ``kind.yml`` links to a
+function to parse the command line options in the ``--message`` mach parameter.
+Currently, the only valid value is ``taskgraph.try_option_syntax:parse_message``.
+The parsed arguments are stored in ``config.config['args']``, it corresponds
+to the same object returned by ``parse_args`` from ``argparse`` Python module.

taskcluster/taskgraph/generator.py

@@ -6,6 +6,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 import logging
 import os
 import yaml
+import copy
 
 from . import filter_tasks
 from .graph import Graph
@@ -38,7 +39,15 @@ class Kind(object):
 
     def load_tasks(self, parameters, loaded_tasks):
         impl_class = self._get_impl_class()
-        return impl_class.load_tasks(self.name, self.path, self.config,
+        config = copy.deepcopy(self.config)
+
+        if 'parse-commit' in self.config:
+            parse_commit = find_object(config['parse-commit'])
+            config['args'] = parse_commit(parameters['message'])
+        else:
+            config['args'] = None
+
+        return impl_class.load_tasks(self.name, self.path, config,
                                      parameters, loaded_tasks)
 
 

taskcluster/taskgraph/target_tasks.py

@@ -42,12 +42,32 @@ def target_tasks_try_option_syntax(full_task_graph, parameters):
     target_tasks_labels = [t.label for t in full_task_graph.tasks.itervalues()
                            if options.task_matches(t.attributes)]
 
-    # If the developer wants test jobs to be rebuilt N times we add that value here
-    if int(options.trigger_tests) > 1:
-        for l in target_tasks_labels:
-            task = full_task_graph[l]
-            if 'unittest_suite' in task.attributes:
-                task.attributes['task_duplicates'] = options.trigger_tests
+    attributes = {
+        k: getattr(options, k) for k in [
+            'env',
+            'no_retry',
+            'tag',
+        ]
+    }
+
+    for l in target_tasks_labels:
+        task = full_task_graph[l]
+        if 'unittest_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.trigger_tests
+
+    for l in target_tasks_labels:
+        task = full_task_graph[l]
+        # If the developer wants test jobs to be rebuilt N times we add that value here
+        if options.trigger_tests > 1 and 'unittest_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.trigger_tests
+            task.attributes['profile'] = False
+
+        # If the developer wants test talos jobs to be rebuilt N times we add that value here
+        if options.talos_trigger_tests > 1 and 'talos_suite' in task.attributes:
+            task.attributes['task_duplicates'] = options.talos_trigger_tests
+            task.attributes['profile'] = options.profile
+
+        task.attributes.update(attributes)
 
     # Add notifications here as well
     if options.notifications:

taskcluster/taskgraph/test/test_target_tasks.py

@@ -18,7 +18,12 @@ class FakeTryOptionSyntax(object):
 
     def __init__(self, message, task_graph):
         self.trigger_tests = 0
+        self.talos_trigger_tests = 0
         self.notifications = None
+        self.env = []
+        self.profile = False
+        self.tag = None
+        self.no_retry = False
 
     def task_matches(self, attributes):
         return 'at-at' in attributes

taskcluster/taskgraph/test/test_try_option_syntax.py

@@ -56,6 +56,12 @@ class TestTryOptionSyntax(unittest.TestCase):
         self.assertEqual(tos.unittests, [])
         self.assertEqual(tos.talos, [])
         self.assertEqual(tos.platforms, [])
+        self.assertEqual(tos.trigger_tests, 0)
+        self.assertEqual(tos.talos_trigger_tests, 0)
+        self.assertEqual(tos.env, [])
+        self.assertFalse(tos.profile)
+        self.assertIsNone(tos.tag)
+        self.assertFalse(tos.no_retry)
 
     def test_message_without_try(self):
         "Given a non-try message, it should return an empty value"
@@ -65,6 +71,12 @@ class TestTryOptionSyntax(unittest.TestCase):
         self.assertEqual(tos.unittests, [])
         self.assertEqual(tos.talos, [])
         self.assertEqual(tos.platforms, [])
+        self.assertEqual(tos.trigger_tests, 0)
+        self.assertEqual(tos.talos_trigger_tests, 0)
+        self.assertEqual(tos.env, [])
+        self.assertFalse(tos.profile)
+        self.assertIsNone(tos.tag)
+        self.assertFalse(tos.no_retry)
 
     def test_unknown_args(self):
         "unknown arguments are ignored"
@@ -246,6 +258,11 @@ class TestTryOptionSyntax(unittest.TestCase):
         tos = TryOptionSyntax('try: --rebuild 10', empty_graph)
         self.assertEqual(tos.trigger_tests, 10)
 
+    def test_talos_trigger_tests(self):
+        "--rebuild-talos 10 sets talos_trigger_tests"
+        tos = TryOptionSyntax('try: --rebuild-talos 10', empty_graph)
+        self.assertEqual(tos.talos_trigger_tests, 10)
+
     def test_interactive(self):
         "--interactive sets interactive"
         tos = TryOptionSyntax('try: --interactive', empty_graph)
@@ -266,5 +283,25 @@ class TestTryOptionSyntax(unittest.TestCase):
         tos = TryOptionSyntax('try:', empty_graph)
         self.assertEqual(tos.notifications, None)
 
+    def test_setenv(self):
+        "--setenv VAR=value adds a environment variables setting to env"
+        tos = TryOptionSyntax('try: --setenv VAR1=value1 --setenv VAR2=value2', empty_graph)
+        self.assertEqual(tos.env, ['VAR1=value1', 'VAR2=value2'])
+
+    def test_profile(self):
+        "--spsProfile sets profile to true"
+        tos = TryOptionSyntax('try: --spsProfile', empty_graph)
+        self.assertTrue(tos.profile)
+
+    def test_tag(self):
+        "--tag TAG sets tag to TAG value"
+        tos = TryOptionSyntax('try: --tag tagName', empty_graph)
+        self.assertEqual(tos.tag, 'tagName')
+
+    def test_no_retry(self):
+        "--no-retry sets no_retry to true"
+        tos = TryOptionSyntax('try: --no-retry', empty_graph)
+        self.assertTrue(tos.no_retry)
+
 if __name__ == '__main__':
     main()

taskcluster/taskgraph/transforms/build.py

@@ -29,4 +29,16 @@ def set_defaults(config, jobs):
             job['extra']['chainOfTrust']['inputs']['docker-image'] = {
                 "task-reference": "<docker-image>"
             }
+        job['worker'].setdefault('env', {})
+        yield job
+
+
+@transforms.add
+def set_env(config, jobs):
+    """Set extra environment variables from try command line."""
+    for job in jobs:
+        env = config.config['args'].env
+        if env:
+            job_env = job['worker']['env']
+            job_env.update(dict(x.split('=') for x in env))
         yield job

taskcluster/taskgraph/transforms/tests.py

@@ -567,6 +567,25 @@ def set_retry_exit_status(config, tests):
         yield test
 
 
+@transforms.add
+def set_profile(config, tests):
+    """Set profiling mode for tests."""
+    for test in tests:
+        if config.config['args'].profile and test['suite'] == 'talos':
+            test['mozharness']['extra-options'].append('--spsProfile')
+        yield test
+
+
+@transforms.add
+def set_tag(config, tests):
+    """Set test for a specific tag."""
+    for test in tests:
+        tag = config.config['args'].tag
+        if tag:
+            test['mozharness']['extra-options'].extend(['--tag', tag])
+        yield test
+
+
 @transforms.add
 def remove_linux_pgo_try_talos(config, tests):
     """linux64-pgo talos tests don't run on try."""

taskcluster/taskgraph/try_option_syntax.py

@@ -172,6 +172,73 @@ RIDEALONG_BUILDS = {
 TEST_CHUNK_SUFFIX = re.compile('(.*)-([0-9]+)$')
 
 
+def escape_whitespace_in_brackets(input_str):
+    '''
+    In tests you may restrict them by platform [] inside of the brackets
+    whitespace may occur this is typically invalid shell syntax so we escape it
+    with backslash sequences    .
+    '''
+    result = ""
+    in_brackets = False
+    for char in input_str:
+        if char == '[':
+            in_brackets = True
+            result += char
+            continue
+
+        if char == ']':
+            in_brackets = False
+            result += char
+            continue
+
+        if char == ' ' and in_brackets:
+            result += '\ '
+            continue
+
+        result += char
+
+    return result
+
+
+def parse_message(message):
+    # shlex used to ensure we split correctly when giving values to argparse.
+    parts = shlex.split(escape_whitespace_in_brackets(message))
+    try_idx = None
+    for idx, part in enumerate(parts):
+        if part == TRY_DELIMITER:
+            try_idx = idx
+            break
+
+    if try_idx is None:
+        return None
+
+    # Argument parser based on try flag flags
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-b', '--build', dest='build_types')
+    parser.add_argument('-p', '--platform', nargs='?',
+                        dest='platforms', const='all', default='all')
+    parser.add_argument('-u', '--unittests', nargs='?',
+                        dest='unittests', const='all', default='all')
+    parser.add_argument('-t', '--talos', nargs='?', dest='talos', const='all', default='none')
+    parser.add_argument('-i', '--interactive',
+                        dest='interactive', action='store_true', default=False)
+    parser.add_argument('-e', '--all-emails',
+                        dest='notifications', action='store_const', const='all')
+    parser.add_argument('-f', '--failure-emails',
+                        dest='notifications', action='store_const', const='failure')
+    parser.add_argument('-j', '--job', dest='jobs', action='append')
+    parser.add_argument('--rebuild-talos', dest='talos_trigger_tests', action='store',
+                        type=int, default=1)
+    parser.add_argument('--setenv', dest='env', action='append')
+    parser.add_argument('--spsProfile', dest='profile', action='store_true')
+    parser.add_argument('--tag', dest='tag', action='store', default=None)
+    parser.add_argument('--no-retry', dest='no_retry', action='store_true')
+    # In order to run test jobs multiple times
+    parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1)
+    args, _ = parser.parse_known_args(parts[try_idx:])
+    return args
+
+
 class TryOptionSyntax(object):
 
     def __init__(self, message, full_task_graph):
@@ -189,8 +256,11 @@ class TryOptionSyntax(object):
         - trigger_tests: the number of times tests should be triggered (--rebuild)
         - interactive: true if --interactive
         - notifications: either None if no notifications or one of 'all' or 'failure'
-
-        Note that -t is currently completely ignored.
+        - talos_trigger_tests: the number of time talos tests should be triggered (--rebuild-talos)
+        - env: additional environment variables (ENV=value)
+        - profile: run talos in profile mode
+        - tag: restrict tests to the specified tag
+        - no_retry: do not retry failed jobs
 
         The unittests and talos lists contain dictionaries of the form:
 
@@ -208,37 +278,16 @@ class TryOptionSyntax(object):
         self.trigger_tests = 0
         self.interactive = False
         self.notifications = None
+        self.talos_trigger_tests = 0
+        self.env = []
+        self.profile = False
+        self.tag = None
+        self.no_retry = False
 
-        # shlex used to ensure we split correctly when giving values to argparse.
-        parts = shlex.split(self.escape_whitespace_in_brackets(message))
-        try_idx = None
-        for idx, part in enumerate(parts):
-            if part == TRY_DELIMITER:
-                try_idx = idx
-                break
-
-        if try_idx is None:
+        args = parse_message(message)
+        if args is None:
             return
 
-        # Argument parser based on try flag flags
-        parser = argparse.ArgumentParser()
-        parser.add_argument('-b', '--build', dest='build_types')
-        parser.add_argument('-p', '--platform', nargs='?',
-                            dest='platforms', const='all', default='all')
-        parser.add_argument('-u', '--unittests', nargs='?',
-                            dest='unittests', const='all', default='all')
-        parser.add_argument('-t', '--talos', nargs='?', dest='talos', default='none')
-        parser.add_argument('-i', '--interactive',
-                            dest='interactive', action='store_true', default=False)
-        parser.add_argument('-e', '--all-emails',
-                            dest='notifications', action='store_const', const='all')
-        parser.add_argument('-f', '--failure-emails',
-                            dest='notifications', action='store_const', const='failure')
-        parser.add_argument('-j', '--job', dest='jobs', action='append')
-        # In order to run test jobs multiple times
-        parser.add_argument('--rebuild', dest='trigger_tests', type=int, default=1)
-        args, _ = parser.parse_known_args(parts[try_idx:])
-
         self.jobs = self.parse_jobs(args.jobs)
         self.build_types = self.parse_build_types(args.build_types)
         self.platforms = self.parse_platforms(args.platforms)
@@ -248,6 +297,11 @@ class TryOptionSyntax(object):
         self.trigger_tests = args.trigger_tests
         self.interactive = args.interactive
         self.notifications = args.notifications
+        self.talos_trigger_tests = args.talos_trigger_tests
+        self.env = args.env
+        self.profile = args.profile
+        self.tag = args.tag
+        self.no_retry = args.no_retry
 
     def parse_jobs(self, jobs_arg):
         if not jobs_arg or jobs_arg == ['all']:
@@ -467,33 +521,6 @@ class TryOptionSyntax(object):
                     rv.add(a[len(prefix):])
         return sorted(rv)
 
-    def escape_whitespace_in_brackets(self, input_str):
-        '''
-        In tests you may restrict them by platform [] inside of the brackets
-        whitespace may occur this is typically invalid shell syntax so we escape it
-        with backslash sequences    .
-        '''
-        result = ""
-        in_brackets = False
-        for char in input_str:
-            if char == '[':
-                in_brackets = True
-                result += char
-                continue
-
-            if char == ']':
-                in_brackets = False
-                result += char
-                continue
-
-            if char == ' ' and in_brackets:
-                result += '\ '
-                continue
-
-            result += char
-
-        return result
-
     def task_matches(self, attributes):
         attr = attributes.get
 
@@ -560,4 +587,9 @@ class TryOptionSyntax(object):
             "trigger_tests: " + str(self.trigger_tests),
             "interactive: " + str(self.interactive),
             "notifications: " + str(self.notifications),
+            "talos_trigger_tests: " + str(self.talos_trigger_tests),
+            "env: " + str(self.env),
+            "profile: " + str(self.profile),
+            "tag: " + str(self.tag),
+            "no_retry: " + str(self.no_retry),
         ])