Closed Bug 1465181 Opened 5 years ago Closed 5 years ago

Support native-engine in run-task

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Version:
3 Branch
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(firefox62 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox62 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(4 files)

Bug 1465181 - [taskgraph] Support use-artifacts with native-engine in run-task,
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1465181 - [taskgraph] Stop hardcoding the workdir to /builds/worker in 'job' tasks,
59 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1465181 - [run-task] Remove requirement to run as root on POSIX systems,
59 bytes, text/x-review-board-request
gps
: review+
Details
Bug 1465181 - Use releng-hardware/gecko-t-linux-talos workerType for js-bench tasks,
59 bytes, text/x-review-board-request
jmaher
: review+
Details 
Native engine is nominally already supported:
https://searchfox.org/mozilla-central/source/taskcluster/taskgraph/transforms/job/run_task.py#111

But when I created a task that tried to use it, I ran into a bunch of issues. Mostly revolving around the fact that the task isn't being run as root.

Fixing this will allow us to run the new js-bench tasks on hardware as opposed to AWS.
See Also: → 1436037
Assignee: nobody → ahal
Status: NEW → ASSIGNED
Comment on attachment 8981625 [details]
Bug 1465181 - [taskgraph] Stop hardcoding the workdir to /builds/worker in 'job' tasks,

https://reviewboard.mozilla.org/r/247746/#review253772

The bulk of the changes seem fine. I'm just a bit unnerved by the dynamic schema modification.

::: taskcluster/taskgraph/transforms/job/__init__.py:183
(Diff revision 1)
>  
>      The decorated function should have the signature `using_foo(config, job,
>      taskdesc) and should modify the task description in-place.  The skeleton of
>      the task description is already set up, but without a payload."""
> +    if schema:
> +        schema = schema.extend({Optional('workdir'): basestring})

Modifying the schema here doesn't seem correct. Shouldn't the schema be defined in a constant somewhere?

If we do dynamic schema modification elsewhere, I'm inclined to turn a blind eye. But this feels not right.
Attachment #8981625 - Flags: review?(gps) → review-
Comment on attachment 8981626 [details]
Bug 1465181 - [run-task] Remove requirement to run as root on POSIX systems,

https://reviewboard.mozilla.org/r/247748/#review253774

Yes, the check is superficial.
Attachment #8981626 - Flags: review?(gps) → review+
Comment on attachment 8981625 [details]
Bug 1465181 - [taskgraph] Stop hardcoding the workdir to /builds/worker in 'job' tasks,

https://reviewboard.mozilla.org/r/247746/#review253772

> Modifying the schema here doesn't seem correct. Shouldn't the schema be defined in a constant somewhere?
> 
> If we do dynamic schema modification elsewhere, I'm inclined to turn a blind eye. But this feels not right.

Good point. The fact that schemas are duplicated all over the place is one of the things that annoy me about taskgraph, but this bug is the wrong hill to die on. It should be solved properly in the general case rather than having one off schema modification.
Comment on attachment 8981627 [details]
Bug 1465181 - Use releng-hardware/gecko-t-linux-talos workerType for js-bench tasks,

https://reviewboard.mozilla.org/r/247750/#review253970

thanks
Attachment #8981627 - Flags: review?(jmaher) → review+
The last commit in this series is going to conflict with bug 1465336 (which just landed on inbound).
Depends on: 1465336
Comment on attachment 8981625 [details]
Bug 1465181 - [taskgraph] Stop hardcoding the workdir to /builds/worker in 'job' tasks,

https://reviewboard.mozilla.org/r/247746/#review254134

FWIW, we still want to strive for using `/builds/worker` in as many tasks as possible, especially on the build side. That's because things like sccache cache according to the absolute path. Just something to keep in mind. Talos should be immune from these concerns.
Attachment #8981625 - Flags: review?(gps) → review+
Good to know! The only reason I did this was because the jsshell task I was setting up didn't have permission to write to /builds/worker, and it doesn't do any compiling. Fwiw, I don't plan on using this feature again (unless a similar situation happens in the future).
Comment on attachment 8981624 [details]
Bug 1465181 - [taskgraph] Support use-artifacts with native-engine in run-task,

https://reviewboard.mozilla.org/r/247744/#review254428

sorry for the review delay
Attachment #8981624 - Flags: review?(dustin) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f84d6a6ce047
[taskgraph] Support use-artifacts with native-engine in run-task, r=dustin
https://hg.mozilla.org/integration/autoland/rev/2c68830a03a0
[taskgraph] Stop hardcoding the workdir to /builds/worker in 'job' tasks, r=gps
https://hg.mozilla.org/integration/autoland/rev/f75a5959d692
[run-task] Remove requirement to run as root on POSIX systems, r=gps
https://hg.mozilla.org/integration/autoland/rev/71ab37336d27
Use releng-hardware/gecko-t-linux-talos workerType for js-bench tasks, r=jmaher
Depends on: 1469095
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.