Give "source-check" tasks the ability to run on more than one platform

RESOLVED FIXED in mozilla53

Status

task
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

unspecified
mozilla53

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Currently source-check tasks can only specify a single platform under task['treeherder']['platform']. I think the rationale was that because no build is required, it doesn't matter which platform is used.

But some source-check tasks do need to run on multiple platforms. For example the mozbase tests (especially mozprocess) have very different code paths on windows vs linux vs mac.
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

Hey Dustin, this isn't ready for review yet.. but I was wondering what you thought of the approach, is something like this more or less ok?

Some open questions:
- Should this just get implemented in TransformTask?
- Is "get_inputs" the right place to be doing this?
- Is the 'build_attrs' transform necessary for 'source-check' tasks? It doesn't seem to be needed

Thanks!
Attachment #8811877 - Flags: feedback?(dustin)
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

https://reviewboard.mozilla.org/r/93786/#review93942

::: taskcluster/taskgraph/task/source.py:15
(Diff revision 1)
> +    def get_inputs(cls, *args):
> +        jobs = super(SourceTask, cls).get_inputs(*args)
> +        for job in jobs:
> +            if 'treeherder' in job and 'platforms' in job['treeherder']:
> +                for platform in job['treeherder']['platforms']:
> +                    pjob = copy.deepcopy(job)
> +                    pjob['treeherder']['platform'] = platform
> +                    del pjob['treeherder']['platforms']
> +                    yield pjob
> +            else:
> +                yield job

This could be implemented as a transform -- no need to subclas TransformTask :)

::: taskcluster/taskgraph/task/source.py:18
(Diff revision 1)
> +
> +    @classmethod
> +    def get_inputs(cls, *args):
> +        jobs = super(SourceTask, cls).get_inputs(*args)
> +        for job in jobs:
> +            if 'treeherder' in job and 'platforms' in job['treeherder']:

I'm not sure where, but this should be documented somewhere that people wondering "what the heck is treeherder.platforms?" will find it..
Hah, midair, more or less :)

The build_attrs is required because source-check objects are considered builds by try syntax.
Thanks for the quick reply! I figured that because 'source-check' jobs are now scheduled regardless of try syntax anyway, maybe it wasn't important.. but I guess it's still needed for when '-j' is passed in.

Also would it make more sense to put 'platforms' in the top-level (i.e outside of the treeherder sub-config), since I guess 'treeherder' is forwarded wholesale to some API?
(In reply to Andrew Halberstadt [:ahal] from comment #5)
> Also would it make more sense to put 'platforms' in the top-level (i.e
> outside of the treeherder sub-config), since I guess 'treeherder' is
> forwarded wholesale to some API?

This makes sense -- it would introduce something like a "source check description" which is very close to a job, but among other things has a top-level `platforms` attribute.
Attachment #8811877 - Flags: feedback?(dustin)
Here's a try run showing this in action:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1b40fed2f3b0d67e13c2cc8e5d14974dd85a263

The third commit isn't up for review or anything, but shows how you could e.g make the tg task run on multiple platforms. (And without try syntax either ;))
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

https://reviewboard.mozilla.org/r/93786/#review94600

So, I think the biggest issue here is, this just changes the treeherder platform strings -- it doesn't actually run the tasks on those other platforms.  For example, here's a "win10" tg that ran in a linux docker image, taken from your try push:
  https://tools.taskcluster.net/task-inspector/#UiYk9A9HQA2_vGQON3WSeg/

So the "platforms" bit is going to need to do more than just set the platforms :)

I'd recommend that you distinguish and design "source check descriptions", similar to test descriptions.  Source checks currently start life as job descriptions, are transformed into task descriptions, and those become task definitions.  You would add "source check descriptions" to the beginning of that list, with transforms to create job descriptions.  So there's no need to change the implementation of job descriptions, nor task descriptions: rather, the transform from source check description to job description would take care of all of the multi-platform stuff.  Source check descriptions can also take care of the platform stuff, separately from `build_attrs.py`.

Here's an idea of how a source-check description might look.  This resembles test definitions in that it uses a `by-..` prefix to parameterize certain fields on the platform.

    taskgraph-tests:
        description: taskcluster/taskgraph unit tests
        platforms:
            - linux64/opt
            - win10/opt
        treeherder:
            symbol: tg
            kind: test
            tier: 2
        worker-type:
            by-platform:
                linux64: aws-provisioner-v1/b2gtest
                win10: aws-provisioner-v1/gecko-t-win10-64-gpu
        worker:
            by-platform:
                linux64:
                    implementation: docker-worker
                    docker-image: {in-tree: "lint"}
                    max-run-time: 1800
                win10:
                    implementation: generic-worker
                    # .. ?
        run:
            using: mach
            mach: taskgraph python-tests
        run-on-projects:
            - integration
            - release
        when:
            files-changed:
                - 'taskcluster/**/*.py'
                - 'config/mozunit.py'
                - 'python/mach/**/*.py'

The `kind.yml` would look like

    implementation: taskgraph.task.transform:TransformTask

    transforms:
       - taskgraph.transforms.source_check:transforms
       - taskgraph.transforms.job:transforms
       - taskgraph.transforms.task:transforms

    jobs-from:
        - python-tests.yml
        - mozlint.yml
        - doc.yml

where `taskcluster/taskgraph/transforms/source_check.py` would have transforms to make a copy of the task for each platform, setting appropriate fields (`label`, `treeherder.platform`) and attributes and delete `sc_desc['platform']`.  A second transform would then apply `by-platform` to the necessary fields.  The result would be a series of job descriptions, one per platform.
Attachment #8811877 - Flags: review?(dustin) → review-
Heh oops, the fact that it seemed too easy should have been a giveaway..
Attachment #8812315 - Flags: review?(dustin)
Attachment #8812315 - Attachment is obsolete: true
I think the above patch is working, though when I went to try it out I realized that all of the "job" related stuff is very much tied to "docker-worker":
https://treeherder.mozilla.org/logviewer.html#?job_id=31894575&repo=try#L258

So I guess there's still a lot more work to do before we can run source-check on windows. That's another bug though, and I think we can still land this patch in the meantime.
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

https://reviewboard.mozilla.org/r/93786/#review96114

I like this!

It's a little magical, though.  If you see this growing and being easily used by others, it may be worth defining a schema for a "source-check description" including this `platforms` field, and describing it in `taskcluster/docs/transforms.rst`.
Attachment #8811877 - Flags: review?(dustin) → review+
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

Not sure how to reset the review flag in mozreview, but you'll probably want to have a second look at this.
Attachment #8811877 - Flags: review+ → review?(dustin)
Comment on attachment 8811877 [details]
Bug 1318438 - [taskcluster] "job" tasks should have ability to run on multiple platforms,

https://reviewboard.mozilla.org/r/93786/#review96882

I like it!  I can see this beuing useful in unifying test jobs, too.  Thanks!
Attachment #8811877 - Flags: review?(dustin) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6461f9c75e80
[taskcluster] "job" tasks should have ability to run on multiple platforms, r=dustin
https://hg.mozilla.org/mozilla-central/rev/6461f9c75e80
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.