1468812 - Consolidate use-artifacts and fetches

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Andrew Halberstadt [:ahal]]

There are two mechanisms to download artifacts from dependencies in taskgraph at the moment, use-artifacts and fetches.

The latter is more generic, but can only download artifacts from dependent tasks with the 'fetch' kind. The former can download artifacts from any kind of dependency, but only works with run-task based tasks.

I think we could make the 'use_fetches' transform work with artifacts of any dependency, then refactor the use-artifacts mechanism to use that instead.

Comment 1

[Mike Hommey [:glandium]]

There's also toolchains.

Comment 2

[Gregory Szorc [:gps]]

... yeah. I think ahal and I both developed use-artifacts and fetches at the same time.

There are a few parts to fetches:

1) fetch tasks that fetch something and "vendor" it as an artifact
2) dependencies between tasks that use the artifact of a fetch task
3) the `fetch-content` script for fetching artifacts from another task into the runtime environment of the current task

#2 and #3 are the things we can consolidate.

I'd love to consolidate the code for "fetch an artifact." While this may ultimately be performed by the worker itself, we do have a need for developers to fetch task artifacts on their local machine. I'm not sure if it is better to obtain and execute a Go binary for this or just to redundantly implement things in Python. We already have at least 3 copies of that Python code. So maybe a first step is consolidating the Python code?

Comment 3

[Andrew Halberstadt [:ahal]]

I want to start using fetch in 'test' tasks. I think doing this consolidation before extending fetch further makes sense.

The only thing use-artifacts can do that 'fetches' can't is grabbing artifacts from non-fetch dependencies. To implement this, we could redefine an entry in 'fetches' such that it's either a string or a dict. If it's a string, then we assume we are looking for a fetch artifact (and we add the corresponding fetch task as a dependency implicitly). If it's a dict, we assume we are looking for an artifact from an explicitly defined dependency (and we just add it to the MOZ_FETCHES variable).

The only other thing to do would be to add a way of calling 'fetch-content' to the run-task script. In order to add 'fetch' support to 'test' tasks, I'll also need to call fetch-content from mozharness somewhere, but that can be done in a follow-up.

Aside: if we have generic 'fetches' support in mozharness, we can remove all the "download-and-extract" steps from the various mozharness scripts. This would go a long way towards your "single place to fetch artifacts" vision.

Comment 4

[Andrew Halberstadt [:ahal]]

Also I haven't looked into the 'use_toolchain' transform at all yet so not sure how it compares to fetches or use-artifacts. But this is complicated enough that I'd prefer to leave consolidating that to a follow-up (assuming consolidating it is something we should be doing).

Comment 5

[Gregory Szorc [:gps]]

Something else I've been mulling around is how to make the useful bits of `run-task` and `fetch-content` available outside the context of those processes. e.g. how do we implement things as a Python module so we can share and test code more easily.

Another thing I've been contemplating is how to get `run-task` and `fetch-content` integrated into Windows tasks. We have a chicken-and-egg problem where we want tasks to use in-repo files but we don't have a good way of getting access to the repo data from the task initially. We could potentially use the "mounts" worker feature to pull in URLs from hg.mo or a task artifact. Perhaps we should create a task that creates an archive with the "task runtime" (run-task, fetch-content, etc), have a "mounts" pull it in, and then execute `run-task` from it. I dunno.

Comment 6

[Andrew Halberstadt [:ahal]]

Yeah, I was also thinking we'd need to use "mount". As long as mozharness still exists there are cases where we'll want fetch-content but not run-task. For now probably easiest to just mount them separately. But I can definitely see run-task evolving from a script to a package in the future, at which point we'll want a bundle.

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [taskgraph] Move 'use_fetches' transform to the job section

The 'use_fetches' transform is currently only being used by toolchain tasks,
but we'd like to expand this to more kinds (like 'test' and 'source_test').

The problem is that 'use_fetches' doesn't have a schema, and assumes things
about the kinds of keys that will be set in the job. For example, it assumes
that job['worker']['env'] is going to be forwarded up to the jobdesc properly.

By moving this transform into the set applied to all 'job' tasks, we:

A) Have a task schema we can reliably depend on
B) Can automatically use it from any 'job' task without kind specific
modifications

Since the toolchain tasks apply the 'job' transforms (almost) right after
the 'use_fetches' transform, this change just works.

Comment 8

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [taskgraph] Support artifacts from any dependency via fetches

Fetches no longer need to be artifacts exposed via a 'fetch' task, they can
also be artifacts from a task's dependencies. The new format is:

fetches:
    fetch:
        - fetch-artifact-1.zip
        - fetch-artifact-2.zip
    build:
        - build-artifact-1.zip
        ...

Specifying 'build' artifacts to fetch will error out if the task doesn't have
any build dependencies.

The 'fetch' key works the same as before, but it is now a special case. Unlike
'build' (or other dependencies), adding a fetch task's artifact here will
implicitly make our task depend on the corresponding fetch task. It will not
be an error.

Depends on D2028.

Comment 9

[Phabricator Automation]

Comment on attachment 8990716 [details]
Bug 1468812 - [taskgraph] Move 'use_fetches' transform to the job section

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D2028

Comment 10

[Phabricator Automation]

Comment on attachment 8991712 [details]
Bug 1468812 - [taskgraph] Support artifacts from any dependency via fetches

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D2102

Comment 11

[Andrew Halberstadt [:ahal]]

Going to try out the "snake eating its tail" workflow.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6d9dab31392
[taskgraph] Move 'use_fetches' transform to the job section r=gps

Comment 13

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [taskgraph] Support artifacts from any dependency via fetches

Fetches no longer need to be artifacts exposed via a 'fetch' task, they can
also be artifacts from a task's dependencies. The new format is:

fetches:
    fetch:
        - fetch-artifact-1.zip
        - fetch-artifact-2.zip
    build:
        - build-artifact-1.zip
        ...

Specifying 'build' artifacts to fetch will error out if the task doesn't have
any build dependencies.

The 'fetch' key works the same as before, but it is now a special case. Unlike
'build' (or other dependencies), adding a fetch task's artifact here will
implicitly make our task depend on the corresponding fetch task. It will not
be an error.

Comment 14

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [fetch-content] Implement ability to specify a per-fetch subdirectory to extract into

Currently 'fetch' artifacts are all extracted in the same directory, this could
make the extdir messy, or in the worst case, cause file name collisions.

Some artifacts are ok to extract into the same directory as they're already
bundled within the archive. But other artifacts are not. This patch keeps the
default behaviour (extracting everything into the same directory), but allows
task authors to specify per-artifact directories to extract into.

The syntax is:
path[>dest]@<task>

The 'dest' value will be a subdirectory of the MOZ_FETCHES_DIR environment
variable.

Depends on D2102.

Comment 15

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [ci] Support MOZ_FETCHES and fetch-content in run-task

This removes the 'use-artifacts' mechanism in favour of fetches. There are a
few pieces here that need to land atomically:

1. Remove use-artifact related code
2. Call 'fetch-content' from the run-task script
3. Convert existing tasks on top of fetches (jsshell, python unittest)
4. Stop calling 'fetch-content' from toolchain setup tasks (as this now gets handled in run-task)

Depends on D2166.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f68b31440829
[taskgraph] Support artifacts from any dependency via fetches r=gps

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for Gecko decision task failures.

Backout: https://hg.mozilla.org/integration/autoland/rev/35e15e6009cdb8d8ca63db47c349c03c62b5b13e

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f68b31440829ed7a587f3933fbd27d6dacb82ea9&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=188447598&repo=autoland&lineNumber=369

[task 2018-07-16T20:48:36.936Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 119, in rewrite_when_to_optimization
[task 2018-07-16T20:48:36.936Z]     for job in jobs:
[task 2018-07-16T20:48:36.936Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/transforms/job/__init__.py", line 113, in validate
[task 2018-07-16T20:48:36.936Z]     "In job {!r}:".format(job.get('name', job.get('label'))))
[task 2018-07-16T20:48:36.936Z]   File "/builds/worker/checkouts/gecko/taskcluster/taskgraph/util/schema.py", line 31, in validate_schema
[task 2018-07-16T20:48:36.936Z]     raise Exception('\n'.join(msg) + '\n' + pprint.pformat(obj))
[task 2018-07-16T20:48:36.937Z] Exception: In job 'android-test-ccov/opt':
[task 2018-07-16T20:48:36.937Z] expected a dictionary for dictionary value @ data['fetches']
[task 2018-07-16T20:48:36.937Z] {u'attributes': {u'build_platform': u'android-test-ccov',
[task 2018-07-16T20:48:36.937Z]                  u'build_type': u'opt'},
[task 2018-07-16T20:48:36.937Z]  u'dependencies': {u'toolchain-linux64-android-gradle-dependencies': u'toolchain-linux64-android-gradle-dependencies',
[task 2018-07-16T20:48:36.937Z]                    u'toolchain-linux64-android-sdk-linux-repack': u'toolchain-linux64-android-sdk-linux-repack'},
[task 2018-07-16T20:48:36.937Z]  'description': 'Android armv7 unit test coverage report',
[task 2018-07-16T20:48:36.937Z]  'fetches': ['grcov-linux-x86_64'],
[task 2018-07-16T20:48:36.937Z]  'index': {'job-name': 'android-test-ccov', 'product': 'mobile'},
[task 2018-07-16T20:48:36.937Z]  u'job-from': 'android-stuff.yml',
[task 2018-07-16T20:48:36.937Z]  u'name': 'android-test-ccov/opt',
[task 2018-07-16T20:48:36.937Z]  u'optimization': {u'skip-unless-schedules': [u'android']},
[task 2018-07-16T20:48:36.937Z]  'run': {'actions': ['get-secrets build'],
[task 2018-07-16T20:48:36.937Z]          'config': ['builds/releng_base_android_64_builds.py'],
[task 2018-07-16T20:48:36.937Z]          'custom-build-variant-cfg': 'android-test-ccov',
[task 2018-07-16T20:48:36.938Z]          'script': 'mozharness/scripts/fx_desktop_build.py',
[task 2018-07-16T20:48:36.938Z]          'secrets': True,
[task 2018-07-16T20:48:36.938Z]          'tooltool-downloads': 'internal',
[task 2018-07-16T20:48:36.938Z]          'using': 'mozharness'},
[task 2018-07-16T20:48:36.938Z]  'run-on-projects': ['mozilla-central'],
[task 2018-07-16T20:48:36.938Z]  u'scopes': [u'queue:get-artifact:project/gecko/android-sdk/*'],
[task 2018-07-16T20:48:36.938Z]  'treeherder': {'kind': 'build',
[task 2018-07-16T20:48:36.938Z]                 'platform': 'android-4-0-armv7-api16/opt',
[task 2018-07-16T20:48:36.938Z]                 'symbol': 'A(test-ccov)',
[task 2018-07-16T20:48:36.938Z]                 'tier': 1},
[task 2018-07-16T20:48:36.938Z]  'worker': {'artifacts': [{'name': 'public/code-coverage-grcov.zip',
[task 2018-07-16T20:48:36.938Z]                            'path': '/builds/worker/workspace/build/src/obj-firefox/code-coverage-grcov.zip',
[task 2018-07-16T20:48:36.938Z]                            'type': 'file'}],
[task 2018-07-16T20:48:36.938Z]             u'chain-of-trust': True,
[task 2018-07-16T20:48:36.938Z]             'docker-image': {'in-tree': 'android-build'},
[task 2018-07-16T20:48:36.938Z]             'env': {'GRADLE_USER_HOME': '/builds/worker/workspace/build/src/mobile/android/gradle/dotgradle-offline',
[task 2018-07-16T20:48:36.939Z]                     u'MOZ_TOOLCHAINS': {u'task-reference': u'public/build/android-gradle-dependencies.tar.xz@<toolchain-linux64-android-gradle-dependencies> project/gecko/android-sdk/android-sdk-linux.tar.xz@<toolchain-linux64-android-sdk-linux-repack>'},
[task 2018-07-16T20:48:36.939Z]                     'PERFHERDER_EXTRA_OPTIONS': 'android-test-ccov'},
[task 2018-07-16T20:48:36.939Z]             'max-run-time': 7200,
[task 2018-07-16T20:48:36.939Z]             u'taskcluster-proxy': True},
[task 2018-07-16T20:48:36.939Z]  'worker-type': 'aws-provisioner-v1/gecko-{level}-b-android'}
[taskcluster 2018-07-16 20:48:37.197Z] === Task Finished ===
[taskcluster 2018-07-16 20:48:38.456Z] Unsuccessful task run with exit code: 1 completed in 14.624 seconds

Comment 18

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/b6d9dab31392

Comment 19

[Phabricator Automation]

Comment on attachment 8992436 [details]
Bug 1468812 - [fetch-content] Implement ability to specify a per-fetch subdirectory to extract into

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D2166

Comment 20

[Andrew Halberstadt [:ahal]]

(In reply to Bogdan Tara[:bogdan_tara] from comment #18)
> https://hg.mozilla.org/mozilla-central/rev/b6d9dab31392

Apologies, I had that fixed locally but when I ran `arc diff` to update the revision it created a new revision instead (I have no idea why). Then I left and forgot to amend it to the original revision.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/485ecb3a2ccb
[taskgraph] Support artifacts from any dependency via fetches r=gps

Comment 22

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/485ecb3a2ccb

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7ee061674e9
[fetch-content] Implement ability to specify a per-fetch subdirectory to extract into r=gps

Comment 24

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b7ee061674e9

Comment 25

[Phabricator Automation]

Comment on attachment 8992439 [details]
Bug 1468812 - [ci] Support MOZ_FETCHES and fetch-content in run-task

Gregory Szorc [:gps] has approved the revision.

https://phabricator.services.mozilla.com/D2167

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61f33f8c8750
[ci] Support MOZ_FETCHES and fetch-content in run-task r=gps

Comment 27

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/61f33f8c8750

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for Linux ccov mass failures (bug 1478211):

https://hg.mozilla.org/mozilla-central/rev/4c4abe35d80851590954b52eabf828c763104696

merge to central with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=dd386b5b9fa7f5cd6dc4bbbfa0503b3eb2969af5&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Comment 29

[Andrew Halberstadt [:ahal]]

These only run on central, so that's why it didn't get backed out earlier.

I think fetch-content just needs to be added to their docker image. The android ccov tasks might need something extra.

Comment 31

[Andrew Halberstadt [:ahal]]

Have this fixed on try:
https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&fromchange=7d38a98e1044662056d9f7ec0f6e29fb2c982d49&tochange=e52b96252c5cc3270c64f2ce3f97021527ecaba8

I'm also adding a new patch to the series to get the code coverage infrastructure to stop downloading MOZ_FETCHES a second time.

Comment 32

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1468812 - [ccov] Don't re-download MOZ_FETCHES if it's already been setup by run-task

The 'codecoverage' mozharness script and a code coverage mach command for Android
did their own custom bootstrapping of MOZ_FETCHES. Now that run-task does this
automatically, we should make sure we aren't downloading MOZ_FETCHES twice.

Depends on D2167.

Comment 33

[Phabricator Automation]

Comment on attachment 8994930 [details]
Bug 1468812 - [ccov] Don't re-download MOZ_FETCHES if it's already been setup by run-task

Marco Castelluccio [:marco] has approved the revision.

https://phabricator.services.mozilla.com/D2368

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a1307ae81453
[ci] Support MOZ_FETCHES and fetch-content in run-task r=gps

Comment 35

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/a1307ae81453

Comment 36

[Andrew Halberstadt [:ahal]]

The last commit in this series gets mozharness' codecoverage.py to look for grcov in MOZ_FETCHES_DIR first, and then if not found download it manually. Just fyi, the Linux ccov tasks will have the MOZ_FETCHES downloaded automatically by run-task, and the windows ccov tasks will use the manual fallback. When/if we get these using run-task, we can remove the fallback code.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bc10207e894d
[ccov] Don't re-download MOZ_FETCHES if it's already been setup by run-task r=marco

Comment 38

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/bc10207e894d