From cd15766fd6d35e62d90ce0f90df176c83df179cf Mon Sep 17 00:00:00 2001 From: Wander Lairson Costa Date: Tue, 31 Jan 2017 15:03:10 -0200 Subject: [PATCH] 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 : retrigger talos tests N times. --setenv =: add extra environments variables. --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: EQlE6q5E8z7 --HG-- extra : rebase_source : 4b7323cd915e8ef9820816015b4b45524811eaf1 --- taskcluster/ci/build/kind.yml | 2 + taskcluster/ci/test/kind.yml | 2 + taskcluster/docs/loading.rst | 9 ++ taskcluster/taskgraph/generator.py | 11 +- taskcluster/taskgraph/target_tasks.py | 32 +++- .../taskgraph/test/test_target_tasks.py | 5 + .../taskgraph/test/test_try_option_syntax.py | 37 +++++ taskcluster/taskgraph/transforms/build.py | 12 ++ taskcluster/taskgraph/transforms/tests.py | 19 +++ taskcluster/taskgraph/try_option_syntax.py | 146 +++++++++++------- 10 files changed, 211 insertions(+), 64 deletions(-) diff --git a/taskcluster/ci/build/kind.yml b/taskcluster/ci/build/kind.yml index 83d3a73d71c3..232d09003f59 100644 --- a/taskcluster/ci/build/kind.yml +++ b/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 diff --git a/taskcluster/ci/test/kind.yml b/taskcluster/ci/test/kind.yml index 31348b994d50..1bc3a4366d6c 100644 --- a/taskcluster/ci/test/kind.yml +++ b/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 diff --git a/taskcluster/docs/loading.rst b/taskcluster/docs/loading.rst index 1fa3c50f1ecb..9c92f1182a51 100644 --- a/taskcluster/docs/loading.rst +++ b/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. diff --git a/taskcluster/taskgraph/generator.py b/taskcluster/taskgraph/generator.py index 7dd7596d93be..5748115e2474 100644 --- a/taskcluster/taskgraph/generator.py +++ b/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) diff --git a/taskcluster/taskgraph/target_tasks.py b/taskcluster/taskgraph/target_tasks.py index 40401c854674..55a5328b3d95 100644 --- a/taskcluster/taskgraph/target_tasks.py +++ b/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: diff --git a/taskcluster/taskgraph/test/test_target_tasks.py b/taskcluster/taskgraph/test/test_target_tasks.py index 035ccefd83f1..b112fb38abfa 100644 --- a/taskcluster/taskgraph/test/test_target_tasks.py +++ b/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 diff --git a/taskcluster/taskgraph/test/test_try_option_syntax.py b/taskcluster/taskgraph/test/test_try_option_syntax.py index 59134619f850..7ee985a7863a 100644 --- a/taskcluster/taskgraph/test/test_try_option_syntax.py +++ b/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() diff --git a/taskcluster/taskgraph/transforms/build.py b/taskcluster/taskgraph/transforms/build.py index 0cf79003e61b..9ff9c5dc6838 100644 --- a/taskcluster/taskgraph/transforms/build.py +++ b/taskcluster/taskgraph/transforms/build.py @@ -29,4 +29,16 @@ def set_defaults(config, jobs): job['extra']['chainOfTrust']['inputs']['docker-image'] = { "task-reference": "" } + 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 diff --git a/taskcluster/taskgraph/transforms/tests.py b/taskcluster/taskgraph/transforms/tests.py index c97975076b29..c211f5bb57e8 100644 --- a/taskcluster/taskgraph/transforms/tests.py +++ b/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.""" diff --git a/taskcluster/taskgraph/try_option_syntax.py b/taskcluster/taskgraph/try_option_syntax.py index 3e3f0020171c..1d50745fbe2f 100644 --- a/taskcluster/taskgraph/try_option_syntax.py +++ b/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), ])