macOS support #87

patricoferris · 2021-11-09T18:15:56Z

This PR is a draft for initial macOS support using users as sandboxes. This PR contains:

A new macos_sandbox.ml implementation which generates a new user and executes commands as the user, note at this time obuilder only expects a single user to be run at a time.
A new rsync storage backend -- this is for easier testing (especially on macOS where ZFS is only available using kernel extensions)

This PR is a re-branding of this branch that was proving to difficult to rebase and also the commit history was a bit wonky. To get in this "working" state I had a lot of help from @kit-ty-kate and @talex5, thanks!

Questions

The rsync backend is almost a complete copy of the btrfs backend which has a result and result-tmp directory and can quickly copy from one to the other with snapshotting. ZFS (due to limitations) builds directly in the result directory. I'm wondering if rsync should do the same which should make it significantly faster.
We should probably sandbox-exec everything, not sure how nicely that plays when nested (when opam invokes it).
Originally, we were going to choose the sandboxing environment at build time by choosing an implementation based on the OS but given the Docker backend could in theory run on Linux, Windows and macOS (with a bit of tweaking) I'm not sure if we still want to go with this approach ?
The User_temp.ml could probably just be removed now, everything can use the "Docker fetcher" or would it be good to support a file-path based fetch (not limited to /Users) just some tar:/path/to/tar, it can be useful on macOS for testing but not strictly necessary ?

TODOs

Test this newer branch more thoroughly by submitting jobs to it.
- Currently being tested as mac-cat-2 in opam-repo-ci and running a opam-health-check with the latest version of all packages too.
Work out the docker image story.
Add the todos from Add macOS support opam-repo-ci#116
- static home directory
- Check caching works properly
Investigate ZFS issues on Linux and macOS -- recently the Github actions seem a bit flaky (see Temporarily pin tar-unix #85). I noticed problems on macOS to and resulted to forcefully unmounting datasets in the original branch, but it would be good to actually fix the problem rather than do that. (Fix ZFS tests in CI #91)

kit-ty-kate · 2021-11-09T18:31:40Z

* Investigate ZFS issues on Linux and macOS

Do we really need to investigate if the rsync backend works fine or is zfs still used even with rsync?

patricoferris · 2021-11-09T19:13:08Z

Do we really need to investigate if the rsync backend works fine or is zfs still used even with rsync?

I was going to try and show using the Github runner that it is quite a bit slower but looks like it ran out of memory or something and I can't cancel the job :( Anyway, that's the main problem, rsync is slow. I used WSL to check the linux version worked and the stress test there says:

Ran 100 jobs (max 10 at once). 0 failures. Took 227.5 s (0.4 jobs/s)

I can't imagine that's all WSL/virtualisation's fault :/

When I get a chance I'll see if I can port the stress test to macOS and then do a head to head with ZFS maybe.

EDIT: Building directly in the directory gives

Ran 100 jobs (max 10 at once). 0 failures. Took 143.8 s (0.7 jobs/s)

talex5

Looks reasonable. Might be worth separating the rsync stuff out into a separate PR to make reviewing easier, but since that and the macos support is experimental it's not a big problem.

lib/macos_sandbox.mli

lib/os.ml

talex5 · 2021-11-10T11:39:06Z

lib/os.ml

+    let pp s ppf = Fmt.pf ppf "[ %s ]" s in
+    if String.length pid = 0 then (Log.warn (fun f -> f "Empty PID"); Lwt.return ())
+    else begin
+      let delete = ["kill"; "-9";  pid ] in


On Linux, kill says:

Negative PID values may be used to choose whole process groups; see the PGID column in ps command output.

Is that useful on macos too?

There's also setsid(1). I don't know much about this stuff, but I think it's the old POSIX way of managing groups of processes from before Linux had containers.

lib/os.ml

lib/rsync_store.ml

talex5 · 2021-11-10T12:01:10Z

lib/rsync_store.ml

+  let gen = cache.gen in
+  Rsync.copy_children ~src:snapshot ~dst:tmp >>= fun () ->
+  let { Obuilder_spec.uid; gid } = user in
+  Os.sudo ["chown"; Printf.sprintf "%d:%d" uid gid; tmp] >>= fun () ->


Would probably be faster to do rsync --chown above.

This change went into #88 but I've just realised that macOS ships with a much older version of rsync that doesn't support --chown. There are two options:

Go back to this implementation so Obuilder will work with the default rsync macOS comes with.

Force people to download a newer version -- note this is not necessarily super straightforward because brew install rsync will put it in /usr/local and won't be available when obuilderfs is active, so the user will probably have to build from source and put the binary and libs elsewhere.

Any thoughts ?

Yeah, I guess we can revert that then. I'm not used to things being older than debian/stable! (and, indeed, older than debian/oldstable too!!)

Might be a performance hit, though. I remember that running sudo chown in Dockerfiles is massively slower than setting the ownership in the COPY.

lib/rsync_store.ml

MisterDA · 2021-11-10T15:20:14Z

Originally, we were going to choose the sandboxing environment at build time by choosing an implementation based on the OS but given the Docker backend could in theory run on Linux, Windows and macOS (with a bit of tweaking) I'm not sure if we still want to go with this approach ?

This I have "solved" by having a Builder module using the usual Store and Sandbox modules, and a custom Docker_builder module. Two mutually-exclusive flags, --store=STORE and --docker-backend=PATH select respectively the Builder module or the Docker_builder module.

patricoferris · 2021-11-15T15:46:06Z

Thanks for the review @talex5 :)) I will investigate a cleaner kill solution based on your suggestions :)

kit-ty-kate · 2021-11-15T20:54:36Z

We should probably sandbox-exec everything, not sure how nicely that plays when nested (when opam invokes it).

Nested sandbox-exec is not permitted. When tried it returns:

sandbox-exec: sandbox_apply: Operation not permitted

It could be a good idea to enable sandboxing by default and to have some kind of parameter to disable it, e.g.

(run (sandbox disabled) (shell "opam install pkg")) # disabled for opam (allows us to test the opam sandbox)
(run (shell "dune build -p pkg")) # enabled by default for all other programs

patricoferris · 2021-12-02T15:08:32Z

ocurrent/ocluster#152 is now in opam-repo-ci as mac-cat-2 to start testing this PR more thoroughly (note we're still using the User_temp fetcher and not Docker for the moment).

kit-ty-kate · 2021-12-06T21:51:35Z

lib/rsync_store.ml

@@ -125,7 +125,9 @@ let cache ~user t name =
  (* Create writeable clone. *)
  let gen = cache.gen in
  let { Obuilder_spec.uid; gid } = user in
-  Rsync.copy_children ~chown:(Printf.sprintf "%d:%d" uid gid) ~src:snapshot ~dst:tmp () >>= fun () ->
+  (* rsync --chown not supported by the rsync that macOS ships with *)


Shouldn’t rsync be using -a (which includes -o and -g, aka. preserve owner/group) instead of plain rsync + chown? It should be more efficient.

Or are there some issues with owner/group needing changing between 2 snapshots?

By default all rsync commands have -aHq

obuilder/lib/rsync_store.ml

Line 27 in ea78b35

let rsync = [ "rsync"; "-aHq" ]

The extra chown iiuc is for differences between uids/gids when restoring caches. It is not used in macOS where we ignore the config.user because we're using users to build, unlike in runc where it could be anyone

obuilder/lib/runc_sandbox.ml

Lines 110 to 115 in ea78b35

let user =

let { Obuilder_spec.uid; gid } = user in

`Assoc [

"uid", `Int uid;

"gid", `Int gid;

]

patricoferris · 2021-12-08T10:43:12Z

Just adding an update here that this PR now uses "a static home directory", this is needed to avoid relocation errors (the OCaml compiler is not relocatable, nor are quite a few packages). This comes at the expense of 2 additional copies from and to the static home directory. A typical build step is now:

Store clone: /<store>/result/<hash> -> /<store>/result-tmp/<new-hash>
Delete contents of /Users/<static-home-dir-unique-to-uid>
Rsync: /<store>/result-tmp/<new-hash>/ -> /Users/<static-home-dir-unique-to-uid>
Build using /Users/<static-home-dir-unique-to-uid>
Rsync: /Users/<static-home-dir-unique-to-uid>/ -> /<store>/result-tmp/<new-hash>
Store snapshot

The extra copies do slow down each build step. An alternative solution that could be explored would be to bindfs the static home directory to the store. Obuilderfs is largely based on bindfs, except it also has the added cost of checking the calling uid and redirecting based on that via the "scoreboard", which we don't need for this particular redirection. It would add yet another fuse filesystem though, so it's a tradeoff between the speed of copying vs. the speed of having all FS actions go through userspace.

Co-authored-by: Kate <[email protected]>

patricoferris · 2022-10-11T08:08:50Z

Closing in favour of #122

@mtelvers

CHANGES: - Add --fuse-path to allow selection of the path redirected by FUSE (@mtelvers ocurrent/obuilder#128, reviewed by @MisterDA ) - Pre-requisites for Windows support using docker for Windows (@MisterDA ocurrent/obuilder#116, reviewed by @tmcgilchrist) - Additional tests and prerequistes for Windows support (@MisterDA ocurrent/obuilder#130, reviewed by @tmcgilchrist) - Add support for Docker/Windows spec (@MisterDA ocurrent/obuilder#117, reviewed by @tmcgilchrist) - Depend on Lwt.5.6.1 for bugfixes (@MisterDA ocurrent/obuilder#108, reviewed by @tmcgilchrist) - Add macOS support (@patricoferris ocurrent/obuilder#87, reviewed by @tmcgilchrist @talex5 @kit-ty-kate) - Enable macOS tests only on macOS (@MisterDA ocurrent/obuilder#126, reviewed by @tmcgilchrist) - Dune 3.0 generates empty intf for executables (@MisterDA ocurrent/obuilder#111, reviewed by @talex5) - Fix warnings and CI failure (@MisterDA ocurrent/obuilder#110, reviewed by @talex5) - Expose store root and cmdliner term with non-required store (@MisterDA ocurrent/obuilder#119, reviewed by @tmcgilchrist) - Expose Rsync_store module (@MisterDA ocurrent/obuilder#114, reviewed by @talex5) - Rsync hard-links to save space (@art-w ocurrent/obuilder#102, reviewed by @patricoferris)

talex5 reviewed Nov 10, 2021

View reviewed changes

patricoferris force-pushed the macos-v2 branch from bcab0da to 45e811f Compare November 15, 2021 15:44

patricoferris mentioned this pull request Nov 17, 2021

Rsync store backend #88

Merged

patricoferris force-pushed the macos-v2 branch from bc2f34a to 0382e9a Compare December 1, 2021 18:02

patricoferris mentioned this pull request Dec 1, 2021

Update Ocluster to be macOS capable ocurrent/ocluster#152

Closed

kit-ty-kate reviewed Dec 6, 2021

View reviewed changes

patricoferris force-pushed the macos-v2 branch from 200580c to 9c910f0 Compare January 26, 2022 16:26

patricoferris mentioned this pull request Apr 30, 2022

Rsync hard-links to save space #102

Merged

patricoferris and others added 9 commits May 5, 2022 12:06

Initial macOS backend work.

4467309

Co-authored-by: Kate <[email protected]>

swap macOS to use docker fetcher

56dc16f

fix tests

1f2e7f5

make Os.Macos a top-level module

8283b9c

pass pid as int not string

50dcc66

choose runc or macos sandbox at build time

1544def

make macOS user's home directory static

d96bb8d

Delete contents of result-tmp when copying back

26fd270

Add macOS test and no-fuse flag

8c3f200

patricoferris force-pushed the macos-v2 branch from 9c910f0 to 8c3f200 Compare May 5, 2022 11:18

patricoferris mentioned this pull request Sep 19, 2022

Add support for FreeBSD #120

Closed

2 tasks

Merge branch 'master' into macos-v2

b1036a0

tmcgilchrist mentioned this pull request Oct 10, 2022

Macos v2 #122

Merged

patricoferris closed this Oct 11, 2022

tmcgilchrist mentioned this pull request Nov 7, 2022

[new release] obuilder and obuilder-spec (0.5) ocaml/opam-repository#22467

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

macOS support #87

macOS support #87

patricoferris commented Nov 9, 2021 •

edited by benmandrew

Loading

kit-ty-kate commented Nov 9, 2021

patricoferris commented Nov 9, 2021 •

edited

Loading

talex5 left a comment

talex5 Nov 10, 2021

talex5 Nov 10, 2021

patricoferris Dec 1, 2021

talex5 Dec 2, 2021

MisterDA commented Nov 10, 2021 •

edited

Loading

patricoferris commented Nov 15, 2021

kit-ty-kate commented Nov 15, 2021

patricoferris commented Dec 2, 2021

kit-ty-kate Dec 6, 2021

patricoferris Dec 6, 2021

patricoferris commented Dec 8, 2021

patricoferris commented Oct 11, 2022

	let user =
	let { Obuilder_spec.uid; gid } = user in
	`Assoc [
	"uid", `Int uid;
	"gid", `Int gid;
	]

macOS support #87

macOS support #87

Conversation

patricoferris commented Nov 9, 2021 • edited by benmandrew Loading

Questions

TODOs

kit-ty-kate commented Nov 9, 2021

patricoferris commented Nov 9, 2021 • edited Loading

talex5 left a comment

Choose a reason for hiding this comment

talex5 Nov 10, 2021

Choose a reason for hiding this comment

talex5 Nov 10, 2021

Choose a reason for hiding this comment

patricoferris Dec 1, 2021

Choose a reason for hiding this comment

talex5 Dec 2, 2021

Choose a reason for hiding this comment

MisterDA commented Nov 10, 2021 • edited Loading

patricoferris commented Nov 15, 2021

kit-ty-kate commented Nov 15, 2021

patricoferris commented Dec 2, 2021

kit-ty-kate Dec 6, 2021

Choose a reason for hiding this comment

patricoferris Dec 6, 2021

Choose a reason for hiding this comment

patricoferris commented Dec 8, 2021

patricoferris commented Oct 11, 2022

patricoferris commented Nov 9, 2021 •

edited by benmandrew

Loading

patricoferris commented Nov 9, 2021 •

edited

Loading

MisterDA commented Nov 10, 2021 •

edited

Loading