Bug 1496768 - [taskgraph] Move remaining try configs from the morph to full_task_graph generation, r=tomprince

Handles 'env' and 'chemspill-prio' configs in the transforms. The 'rebuild'
task config is purposefully excluded from the full_task_graph and instead
handled at the target_tasks stage. Otherwise if a user ran '--rebuild 20' then
retriggered one of those tasks, they'd instead get another 20 which is almost
certainly not what we want.

Differential Revision: https://phabricator.services.mozilla.com/D51417

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Andrew Halberstadt 2019-11-22 23:28:57 +00:00
parent e9d9093e44
commit c1a671d9c9
21 changed files with 136 additions and 355 deletions

View File

@ -88,45 +88,25 @@ obtained with:
Modifying Tasks in a Try Push
,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
It's possible to alter the definition of a task with templates. Templates are
`JSON-e`_ files that live in the `taskgraph module`_. Templates can be specified
from the ``try_task_config.json`` like this:
It's possible to alter the definition of a task by defining additional
configuration in ``try_task_config.json``. For example, to set an environment
variable in all tasks, you can add:
.. parsed-literal::
{
"version": 1,
"tasks": [...],
"templates": {
env: {"FOO": "bar"}
"env": {
"FOO": "bar"
}
}
Each key in the templates object denotes a new template to apply, and the value
denotes extra context to use while rendering. When specified, a template will
be applied to every task no matter what. If the template should only be applied
to certain kinds of tasks, this needs to be specified in the template itself
using JSON-e `condition statements`_.
The context available to the JSON-e render contains attributes from the
:py:class:`taskgraph.task.Task` class. It looks like this:
.. parsed-literal::
{
"attributes": task.attributes,
"kind": task.kind,
"label": task.label,
"target_tasks": [<tasks from try_task_config.json>],
"task": task.task,
"taskId": task.task_id,
"input": ...
}
The ``input`` context can be any arbitrary value or object. What it contains
depends on each specific template. Templates must return objects that have have
either ``attributes`` or ``task`` as a top level key. All other top level keys
will be ignored. See the `existing templates`_ for examples.
The allowed configs are defined in :py:obj:`taskgraph.decision.try_task_config_schema`.
The values will be available to all transforms, so how each config applies will
vary wildly from one context to the next. Some (such as ``env``) will affect
all tasks in the graph. Others might only affect certain kinds of task. The
``use-artifact-builds`` config only applies to build tasks for instance.
Empty Try
:::::::::
@ -170,10 +150,4 @@ config.
}
}
.. _JSON-e: https://taskcluster.github.io/json-e/
.. _taskgraph module: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/templates
.. _condition statements: https://taskcluster.github.io/json-e/#%60$if%60%20-%20%60then%60%20-%20%60else%60
.. _existing templates: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/templates
.. _SCM Level: https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/

View File

@ -128,10 +128,12 @@ visual_metrics_jobs_schema = Schema({
try_task_config_schema = Schema({
Required('tasks'): [basestring],
Optional('templates'): {basestring: object},
Optional('disable-pgo'): bool,
Optional('browsertime'): bool,
Optional('chemspill-prio'): bool,
Optional('disable-pgo'): bool,
Optional('env'): {basestring: basestring},
Optional('gecko-profile'): bool,
Optional('rebuild'): int,
Optional('use-artifact-builds'): bool,
# Keep in sync with JOB_SCHEMA in taskcluster/docker/visual-metrics/run-visual-metrics.py.
Optional('visual-metrics-jobs'): visual_metrics_jobs_schema,
@ -144,7 +146,9 @@ try_task_config_schema = Schema({
description="Run linux desktop tests on Ubuntu 18.04 (bionic)."
): bool,
})
"""
Schema for try_task_config.json files.
"""
try_task_config_schema_v2 = Schema({
Optional('parameters'): {basestring: object},

View File

@ -23,12 +23,10 @@ import logging
import os
import re
import jsone
from slugid import nice as slugid
from .task import Task
from .graph import Graph
from .taskgraph import TaskGraph
from .util.yaml import load_yaml
here = os.path.abspath(os.path.dirname(__file__))
logger = logging.getLogger(__name__)
@ -164,42 +162,17 @@ def add_index_tasks(taskgraph, label_to_taskid):
return taskgraph, label_to_taskid
class apply_jsone_templates(object):
"""Apply a set of JSON-e templates to each task's `task` attribute.
class add_try_task_duplicates():
:param templates: A dict with the template name as the key, and extra context
to use (in addition to task.to_json()) as the value.
"""
template_dir = os.path.join(here, 'templates')
def __init__(self, try_task_config):
self.templates = try_task_config.get('templates')
self.target_tasks = try_task_config.get('tasks')
def __init__(self, try_config):
self.try_config = try_config
def __call__(self, taskgraph, label_to_taskid):
if not self.templates:
return taskgraph, label_to_taskid
for task in taskgraph.tasks.itervalues():
for template in sorted(self.templates):
context = {
'task': task.task,
'taskGroup': None,
'taskId': task.task_id,
'kind': task.kind,
'input': self.templates[template],
# The following context differs from action tasks
'attributes': task.attributes,
'label': task.label,
'target_tasks': self.target_tasks,
}
template = load_yaml(self.template_dir, template + '.yml')
result = jsone.render(template, context) or {}
for attr in ('task', 'attributes'):
if attr in result:
setattr(task, attr, result[attr])
rebuild = self.try_config.get('rebuild')
if rebuild:
for task in taskgraph.tasks.itervalues():
if task.label in self.try_config.get('tasks', []):
task.attributes['task_duplicates'] = rebuild
return taskgraph, label_to_taskid
@ -207,9 +180,8 @@ def morph(taskgraph, label_to_taskid, parameters):
"""Apply all morphs"""
morphs = [
add_index_tasks,
add_try_task_duplicates(parameters['try_task_config']),
]
if parameters['try_mode'] == 'try_task_config':
morphs.append(apply_jsone_templates(parameters['try_task_config']))
for m in morphs:
taskgraph, label_to_taskid = m(taskgraph, label_to_taskid)

View File

@ -1,9 +0,0 @@
---
task:
$mergeDeep:
- $eval: task
# increase the priority from lowest and very-low -> low, but leave others unchanged
- priority:
$if: task.priority == 'lowest' || task.priority == 'very-low'
then: low
else: {$eval: task.priority}

View File

@ -1,7 +0,0 @@
---
task:
$mergeDeep:
- $eval: task
- payload:
env:
$eval: input

View File

@ -1,8 +0,0 @@
---
$if: label in target_tasks
then:
attributes:
$merge:
- $eval: attributes
- task_duplicates:
$eval: input

View File

@ -105,7 +105,7 @@ class TestGetDecisionParameters(unittest.TestCase):
def test_try_task_config(self, mock_get_hg_commit_message, mock_get_hg_revision_branch):
mock_get_hg_commit_message.return_value = 'Fuzzy query=foo'
mock_get_hg_revision_branch.return_value = 'default'
ttc = {'tasks': ['a', 'b'], 'templates': {}}
ttc = {'tasks': ['a', 'b']}
self.options['project'] = 'try'
with MockedOpen({self.ttc_file: json.dumps(ttc)}):
params = decision.get_decision_parameters(FAKE_GRAPH_CONFIG, self.options)

View File

@ -106,6 +106,7 @@ class TestGenerator(unittest.TestCase):
'_kinds': kinds,
'target_tasks_method': 'test_method',
'try_mode': None,
'try_task_config': {},
})
parameters.update(params)

View File

@ -89,115 +89,5 @@ def test_make_index_tasks(make_taskgraph):
assert index_task.task['scopes'] == ['index:insert-task:gecko.v2.mozilla-central.*']
TASKS = [
{
'kind': 'build',
'label': 'a',
'attributes': {},
'task': {
'extra': {
'treeherder': {
'group': 'tc',
'symbol': 'B'
}
},
'payload': {
'env': {
'FOO': 'BAR'
}
},
'tags': {
'kind': 'build'
}
}
},
{
'kind': 'test',
'label': 'b',
'attributes': {},
'task': {
'extra': {
'suite': 'talos',
'treeherder': {
'group': 'tc',
'symbol': 't'
}
},
'payload': {
'env': {
'FOO': 'BAR'
}
},
'tags': {
'kind': 'test'
}
}
},
]
@pytest.fixture
def get_morphed(make_taskgraph):
def inner(try_task_config, tasks=None):
tasks = tasks or TASKS
taskgraph = make_taskgraph({
t['label']: Task(**t) for t in tasks[:]
})
fn = morph.apply_jsone_templates(try_task_config)
return fn(*taskgraph)[0]
return inner
def test_template_env(get_morphed):
morphed = get_morphed({
'templates': {
'env': {
'ENABLED': 1,
'FOO': 'BAZ',
}
},
})
assert len(morphed.tasks) == 2
for t in morphed.tasks.values():
assert len(t.task['payload']['env']) == 2
assert t.task['payload']['env']['ENABLED'] == 1
assert t.task['payload']['env']['FOO'] == 'BAZ'
morphed = get_morphed({
'templates': {
'env': {
'ENABLED': 0,
'FOO': 'BAZ',
}
},
})
assert len(morphed.tasks) == 2
for t in morphed.tasks.values():
assert len(t.task['payload']['env']) == 2
assert t.task['payload']['env']['ENABLED'] == 0
assert t.task['payload']['env']['FOO'] == 'BAZ'
def test_template_rebuild(get_morphed):
morphed = get_morphed({
'tasks': ['b'],
'templates': {
'rebuild': 4,
},
})
tasks = morphed.tasks.values()
assert len(tasks) == 2
for t in tasks:
if t.label == 'a':
assert 'task_duplicates' not in t.attributes
elif t.label == 'b':
assert 'task_duplicates' in t.attributes
assert t.attributes['task_duplicates'] == 4
if __name__ == '__main__':
main()

View File

@ -1749,6 +1749,29 @@ def add_index_routes(config, tasks):
yield task
@transforms.add
def try_task_config_env(config, tasks):
"""Set environment variables in the task."""
env = config.params['try_task_config'].get('env')
# Find all implementations that have an 'env' key.
implementations = {name for name, builder in payload_builders.items()
if 'env' in builder.schema.schema}
for task in tasks:
if env and task['worker']['implementation'] in implementations:
task['worker']['env'].update(env)
yield task
@transforms.add
def try_task_config_chemspill_prio(config, tasks):
"""Increase the priority from lowest and very-low -> low, but leave others unchanged."""
chemspill_prio = config.params['try_task_config'].get('chemspill-prio')
for task in tasks:
if chemspill_prio and task['priority'] in ('lowest', 'very-low'):
task['priority'] = 'low'
yield task
@transforms.add
def build_task(config, tasks):
for task in tasks:

View File

@ -319,10 +319,8 @@ Would produce the following ``try_task_config.json``:
.. code-block:: json
{
"templates":{
"env":{
"MOZHARNESS_TEST_PATHS":"{\"reftest\":\"layout/reftests/reftest-sanity\"}"
}
"env":{
"MOZHARNESS_TEST_PATHS":"{\"reftest\":\"layout/reftests/reftest-sanity\"}"
},
"tasks":[
"test-linux64-qr/debug-reftest-e10s-1",

View File

@ -81,10 +81,7 @@ def check_working_directory(push=True):
def generate_try_task_config(method, labels, try_config=None):
try_task_config = try_config or {}
templates = try_task_config.setdefault('templates', {})
templates.setdefault('env', {}).update({'TRY_SELECTOR': method})
try_task_config.setdefault('env', {})['TRY_SELECTOR'] = method
try_task_config.update({
'version': 1,
'tasks': sorted(labels),

View File

@ -373,7 +373,7 @@ def run(try_config={}, full=False, parameters=None, push=True, message='{msg}',
# Set the test paths to be run by setting MOZHARNESS_TEST_PATHS.
path_env = {'MOZHARNESS_TEST_PATHS': json.dumps(resolve_tests_by_suite(test_files))}
try_config.setdefault('templates', {}).setdefault('env', {}).update(path_env)
try_config.setdefault('env', {}).update(path_env)
# Build commit message.
msg = 'try coverage - ' + test_count_message

View File

@ -3,8 +3,8 @@
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
"""
Templates provide a way of modifying the task definition of selected
tasks. They live under taskcluster/taskgraph/templates.
Templates provide a way of modifying the task definition of selected tasks.
They are added to 'try_task_config.json' and processed by the transforms.
"""
from __future__ import absolute_import, print_function, unicode_literals
@ -47,17 +47,6 @@ class TryConfig(object):
pass
class Template(TryConfig):
def try_config(self, **kwargs):
context = self.context(**kwargs)
if context:
return {'templates': context}
@abstractmethod
def context(self, **kwargs):
pass
class Artifact(TryConfig):
arguments = [
@ -98,7 +87,7 @@ class Artifact(TryConfig):
}
class Pernosco(Template):
class Pernosco(TryConfig):
arguments = [
[['--pernosco'],
{'action': 'store_true',
@ -117,7 +106,7 @@ class Pernosco(Template):
group = parser.add_mutually_exclusive_group()
return super(Pernosco, self).add_arguments(group)
def context(self, pernosco, **kwargs):
def try_config(self, pernosco, **kwargs):
if pernosco is None:
return
@ -160,7 +149,7 @@ class Pernosco(Template):
}
class Path(Template):
class Path(TryConfig):
arguments = [
[['paths'],
@ -170,7 +159,7 @@ class Path(Template):
}],
]
def context(self, paths, **kwargs):
def try_config(self, paths, **kwargs):
if not paths:
return
@ -187,7 +176,7 @@ class Path(Template):
}
class Environment(Template):
class Environment(TryConfig):
arguments = [
[['--env'],
@ -198,7 +187,7 @@ class Environment(Template):
}],
]
def context(self, env, **kwargs):
def try_config(self, env, **kwargs):
if not env:
return
return {
@ -222,7 +211,7 @@ class RangeAction(Action):
setattr(namespace, self.dest, values)
class Rebuild(Template):
class Rebuild(TryConfig):
arguments = [
[['--rebuild'],
@ -235,7 +224,7 @@ class Rebuild(Template):
}],
]
def context(self, rebuild, **kwargs):
def try_config(self, rebuild, **kwargs):
if not rebuild:
return
@ -244,7 +233,7 @@ class Rebuild(Template):
}
class ChemspillPrio(Template):
class ChemspillPrio(TryConfig):
arguments = [
[['--chemspill-prio'],
@ -253,7 +242,7 @@ class ChemspillPrio(Template):
}],
]
def context(self, chemspill_prio, **kwargs):
def try_config(self, chemspill_prio, **kwargs):
if chemspill_prio:
return {
'chemspill-prio': {}

View File

@ -45,9 +45,7 @@ def test_try_again(monkeypatch):
try_task_config = kwargs.pop("try_task_config")
assert sorted(try_task_config.get("tasks")) == sorted(["foo", "bar"])
assert try_task_config.get("templates") == {
"env": {"TRY_SELECTOR": "fuzzy"},
}
assert try_task_config.get("env") == {"TRY_SELECTOR": "fuzzy"}
assert try_task_config.get('use-artifact-builds')
with open(push.history_path, "r") as fh:

View File

@ -10,12 +10,10 @@ Test empty selector
Pushed via `mach try empty`
Calculated try_task_config.json:
{
"tasks": [],
"templates": {
"env": {
"TRY_SELECTOR": "empty"
}
"env": {
"TRY_SELECTOR": "empty"
},
"tasks": [],
"version": 1
}
@ -26,12 +24,10 @@ Test empty selector
Pushed via `mach try empty`
Calculated try_task_config.json:
{
"tasks": [],
"templates": {
"env": {
"TRY_SELECTOR": "empty"
}
"env": {
"TRY_SELECTOR": "empty"
},
"tasks": [],
"version": 1
}
@ -42,12 +38,10 @@ Test empty selector
Pushed via `mach try empty`
Calculated try_task_config.json:
{
"tasks": [],
"templates": {
"env": {
"TRY_SELECTOR": "empty"
}
"env": {
"TRY_SELECTOR": "empty"
},
"tasks": [],
"version": 1
}

View File

@ -10,15 +10,13 @@ Test fuzzy selector
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -31,15 +29,13 @@ Test fuzzy selector
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/bar-debug",
"test/bar-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -53,17 +49,15 @@ Test multiple selectors
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/bar-debug",
"test/bar-opt",
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -77,14 +71,12 @@ Test query intersection
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -101,15 +93,13 @@ Test intersection with preset containing multiple queries
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -120,15 +110,13 @@ Test intersection with preset containing multiple queries
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -142,15 +130,13 @@ Test exact match
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -161,15 +147,13 @@ Test exact match
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/bar-debug",
"test/bar-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -184,15 +168,13 @@ Test templates
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"use-artifact-builds": true,
"version": 1
}
@ -204,17 +186,15 @@ Test templates
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"BAR": "baz",
"FOO": "1",
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"BAR": "baz",
"FOO": "1",
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}

View File

@ -12,15 +12,13 @@ Test custom commit messages with fuzzy selector
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}
@ -31,15 +29,13 @@ Test custom commit messages with fuzzy selector
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}

View File

@ -131,16 +131,14 @@ Test preset with fuzzy subcommand
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5,
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5
},
"version": 1
}
@ -152,16 +150,14 @@ Test preset with fuzzy subcommand
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5,
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5
},
"version": 1
}
@ -175,17 +171,15 @@ Queries can be appended to presets
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5,
"tasks": [
"build-baz",
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5
},
"version": 1
}
@ -197,15 +191,13 @@ Queries can be appended to presets
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5,
"tasks": [
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
},
"rebuild": 5
},
"version": 1
}

View File

@ -52,15 +52,13 @@ Test migration of syntax preset
Pushed via `mach try fuzzy`
Calculated try_task_config.json:
{
"env": {
"TRY_SELECTOR": "fuzzy"
},
"tasks": [
"test/foo-debug",
"test/foo-opt"
],
"templates": {
"env": {
"TRY_SELECTOR": "fuzzy"
}
},
"version": 1
}

View File

@ -56,19 +56,18 @@ def test_templates(template_patch_resolver, template, args, expected):
t = all_templates[template]()
t.add_arguments(parser)
fn = getattr(t, 'context', t.try_config)
if inspect.isclass(expected) and issubclass(expected, BaseException):
with pytest.raises(expected):
args = parser.parse_args(args)
if template == 'path':
template_patch_resolver(**vars(args))
fn(**vars(args))
t.try_config(**vars(args))
else:
args = parser.parse_args(args)
if template == 'path':
template_patch_resolver(**vars(args))
assert fn(**vars(args)) == expected
assert t.try_config(**vars(args)) == expected
if __name__ == '__main__':