Add a Taskcluster job to build with the tup backend

RESOLVED FIXED in Firefox 61

Status

RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: mshal, Assigned: mshal)

Tracking

(Blocks: 1 bug)

unspecified
mozilla61
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
There should be a separate Taskcluster job that builds the tree with the tup backend. We'll need to have tup in tooltool for this.
Assignee: nobody → cmanchester
I got pretty far with this, but the builds hit an issue to do with Docker: "fuse: failed to open /dev/fuse: Operation not permitted".

From searching around about this, the workaround I can find suggests adding "--cap-add SYS_ADMIN --device /dev/fuse" to the `docker run` command line, which sounds like it will significantly compromise the container's isolation. I'm not sure if this is an option for TC or if there is a better workaround available.
Ok, I found the section in the docker docs: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities, which says we need "--cap-add SYS_ADMIN" for this.

To add this would require patching docker-worker I expect, as in bug 1221661, but more to the point we were ok with adding that feature because it was only for testers, this could be a significant hurdle when we get to the point of evaluating a Tup build's suitability for shipping.
CAP_SYS_ADMIN is basically root and would almost completely undermine the process sandbox used by docker-worker. I reckon you'll have a hard time convincing people to give Docker containers that capability. See http://man7.org/linux/man-pages/man7/capabilities.7.html for more.

I suspect there's a way to make fuse work without CAP_SYS_ADMIN. CAP_SYS_ADMIN is probably the most convenient. Although it likely requires changes to docker-worker to expose fuse to containers.
I bet the first problem here is that cgroups isn't allowing access to the /dev/fuse device. You can fix that as a one-off. Why it needs CAP_SYS_ADMIN, I dunno. Extended attributes?
mount()
mount() itself /might/ be safe to expose to the container because presumably the container was started with its own mount namespace. But, yeah, if you can't call mount() without CAP_SYS_ADMIN, then you are in a predicament. We may have to solve this by having the container started with a fuse filesystem pre-mounted. That requires docker-worker cooperation. That also means moving the tup fuse bits into docker-worker. That'll be a rabbit hole.
(Assignee)

Comment 7

a year ago
I'll also be looking at using an LD_PRELOAD shim again in tup instead of fuse, assuming that will work inside docker. If it does, that would get around us having to fiddle with capabilities I believe. (tup used to use LD_PRELOAD, but I switched it to fuse a while back for various reasons, like staticically-linked binary support).
LD_PRELOAD will work inside Docker as long as the library being loaded doesn't require system calls that the process doesn't have privileges to call :)
(Assignee)

Updated

a year ago
Depends on: 1387098

Updated

9 months ago
Product: Core → Firefox Build System
(Assignee)

Comment 9

8 months ago
Tup now has an LD_PRELOAD shim available again, and it appears to work fine in Docker. This is also nice because when we get to linking libxul, we won't have the same performance issues that we had with FUSE. I'll handle that in bug 1387098.
(Assignee)

Updated

8 months ago
Depends on: 1449623
(Assignee)

Updated

8 months ago
Assignee: cmanchester → mshal
Comment hidden (mozreview-request)

Comment 11

8 months ago
mozreview-review
Comment on attachment 8963393 [details]
Bug 1319230 - Add a Taskcluster job to build with the tup backend;

https://reviewboard.mozilla.org/r/232300/#review237742

Looks sensible to me, but see my one small comment.  Exciting to see this moving forward!

::: testing/mozharness/configs/builds/releng_sub_linux_configs/64_tup.py:3
(Diff revision 1)
> +import os
> +
> +config = {

I expect you want something like https://searchfox.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_linux_configs/64_rusttests.py#29 here, since artifact builds probably don't make sense for Tup-backend builds.  (Yet?)
Attachment #8963393 - Flags: review+
Attachment #8963393 - Flags: review?(core-build-config-reviews)
(Assignee)

Comment 12

8 months ago
(In reply to Nick Alexander :nalexander from comment #11)
> I expect you want something like
> https://searchfox.org/mozilla-central/source/testing/mozharness/configs/
> builds/releng_sub_linux_configs/64_rusttests.py#29 here, since artifact
> builds probably don't make sense for Tup-backend builds.  (Yet?)

Ahh, good catch! I think chmanchester had artifact+tup working at one point, but it appears they don't work together now.
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Attachment #8963393 - Flags: review?(core-build-config-reviews)

Comment 14

8 months ago
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/18a14e2a17c6
Add a Taskcluster job to build with the tup backend; r=nalexander

Comment 15

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/18a14e2a17c6
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.