Closed Bug 1468812 Opened 2 years ago Closed 2 years ago

Consolidate use-artifacts and fetches

Categories

(Firefox Build System :: Task Configuration, task)

3 Branch
task
Not set
normal

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(5 files, 1 obsolete file)

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.
There's also toolchains.
... 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?
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.
Assignee: nobody → ahal
Status: NEW → ASSIGNED
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).
Blocks: 1472800
See Also: → 1473541
See Also: → 1472236
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.
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.
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.
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 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
Attachment #8990716 - Flags: review+
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
Attachment #8991712 - Flags: review+
Going to try out the "snake eating its tail" workflow.
Whiteboard: [leave-open]
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b6d9dab31392
[taskgraph] Move 'use_fetches' transform to the job section r=gps
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.
Attachment #8992419 - Attachment is obsolete: true
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.
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.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f68b31440829
[taskgraph] Support artifacts from any dependency via fetches r=gps
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
Flags: needinfo?(ahal)
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
Attachment #8992436 - Flags: review+
(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.
Flags: needinfo?(ahal)
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/485ecb3a2ccb
[taskgraph] Support artifacts from any dependency via fetches r=gps
Blocks: 1445952
Blocks: 1476372
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b7ee061674e9
[fetch-content] Implement ability to specify a per-fetch subdirectory to extract into r=gps
Whiteboard: [leave-open]
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
Attachment #8992439 - Flags: review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/61f33f8c8750
[ci] Support MOZ_FETCHES and fetch-content in run-task r=gps
https://hg.mozilla.org/mozilla-central/rev/61f33f8c8750
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1478050
Depends on: 1478211
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.
Flags: needinfo?(ahal)
Duplicate of this bug: 1478211
Have this fixed on try:
https://treeherder.mozilla.org/#/jobs?repo=try&author=ahalberstadt@mozilla.com&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.
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 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
Attachment #8994930 - Flags: review+
Whiteboard: [leave-open]
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a1307ae81453
[ci] Support MOZ_FETCHES and fetch-content in run-task r=gps
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.
Pushed by ahalberstadt@mozilla.com:
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
Whiteboard: [leave-open]
Blocks: 1473541
See Also: 1473541
https://hg.mozilla.org/mozilla-central/rev/bc10207e894d
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Blocks: 1486995
Depends on: 1490247
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.