FreeBSD port by akhramov · Pull Request #2376 · moby/buildkit

akhramov · 2021-09-24T17:56:04Z

Buildkit code is mostly generic enough to support FreeBSD, however
there are some quirks / infrastructural pieces that need to be
addressed for full support, to name some

contenthash.NewFromStat attempts to set Devmajor / Devminor for
regular files, assuming that RDev is zero for regular
files. Unlike on Linux, it's not the case for FreeBSD.
containerdexecutor.Run uses bind mounts for rootfs. Bind mounts
are not supported in FreeBSD and we should use nullfs instead
There is no CI job to run tests on FreeBSD
Some dependencies weren't ported

This change

ports buildkit to FreeBSD
bumps tonistiigi/fsutil and docker/docker versions
introduces a CI job to run unit tests on FreeBSD.

Signed-off-by: Artem Khramov akhramov@pm.me

akhramov · 2021-09-25T06:19:14Z

A question: is there a way to tell containerd to use a specific runtime?

tonistiigi

CI seems to be failing

tonistiigi · 2021-09-26T01:40:08Z

+	"github.com/pkg/errors"
+)
+
+func (lm *localMounter) Mount() (string, error) {


Why is this copy needed? And why is it missing the single layer direct mount that exists in linux?

why is it missing the single layer direct mount that exists in linux?

Bind mounts are not supported in FreeBSD. Rather, we use nullfs instead. So the condition like (lm.mounts[0].Type == "bind" || lm.mounts[0].Type == "rbind") must return false.

Perhaps I am missing something, but the rest of the function should handle the single layer direct mount case.

But I don't think that the copy itself is necessary, thanks!

I don't know exactly what is the difference between nullfs and bind but if there is no technical limitation then the freebsd code should detect the single layer as well, eg. lm.mounts[0].Type == "nullfs". For better performance and to avoid privileged call in tests when it is not needed. Plus to just be consistent with linux.

Gotcha, thanks.

We still need root privileges to mount nullfs, though

tonistiigi · 2021-09-26T01:41:45Z

A question: is there a way to tell containerd to use a specific runtime?

https://pkg.go.dev/github.com/containerd/containerd#WithDefaultRuntime

akhramov · 2021-09-26T14:36:48Z

Thanks for the review!

To outline things left to be done:

For FreeBSD default the runtime to runj ("wtf.sbk.runj.v1")
Investigate and fix CI failures

tonistiigi · 2021-09-26T18:08:43Z

To outline things left to be done:

Heads up that if you also want to include buildkitd in binaries then (at least most of the) integration tests need to also run in CI first(skipped atm). For only buildctl this is not needed. So you may want to do this in 2 parts: fix unit tests in current PR and we will merge the client support and then work on buildkitd, runtime and enabling integration tests.

Full matrix is not needed for freebsd integration tests. Single worker or random worker is fine. For the workers, oci worker needs to be supported, containerd is optional.

akhramov · 2021-10-02T06:29:27Z

For the workers, oci worker needs to be supported, containerd is optional.

Honestly, I tested just containerd support locally. I am going to test OCI workers & enable both in CI.

As for unit tests, it depends on moby/moby#42903

tonistiigi · 2021-10-02T06:34:21Z

Feel free to add linux build tag to https://github.com/moby/buildkit/blob/master/util/testutil/integration/dockerd.go unless you actually want to run tests through docker as well.

akhramov · 2021-11-27T17:34:38Z

Rebased the branch. Moving to draft, since I didn't have adequate amount of resources lately to finish this.

AkihiroSuda · 2022-03-08T15:07:44Z

Needs rebase

arrowd · 2023-05-20T19:14:58Z

Just curious if there any movement on this?

crazy-max · 2023-06-06T22:06:57Z

After rebasing on my side, I see some errors when building buildkit: https://github.com/crazy-max/buildkit/actions/runs/5193670842/jobs/9364508523#step:4:348

    fbsd_13_2: # github.com/docker/docker/pkg/chrootarchive
    fbsd_13_2: vendor/github.com/docker/docker/pkg/chrootarchive/archive_unix.go:31:8: undefined: goInChroot
    fbsd_13_2: vendor/github.com/docker/docker/pkg/chrootarchive/archive_unix.go:53:8: undefined: goInChroot
    fbsd_13_2: vendor/github.com/docker/docker/pkg/chrootarchive/diff_unix.go:45:8: undefined: goInChroot

We might need upstream changes in moby. Looks related to moby/moby#44210 (cc @corhere)

akhramov · 2023-06-10T17:45:42Z

@crazy-max confirming.

Created a simple patch to fix that: moby/moby#45724

tonistiigi · 2023-06-28T06:11:26Z

So this is blocked on Moby after the chroot updates there? Do we need to remove it from the milestone?

crazy-max · 2023-06-28T13:36:05Z

So this is blocked on Moby after the chroot updates there? Do we need to remove it from the milestone?

Yes I guess we need to move this out of the milestone while waiting for upstream changes in moby.

tonistiigi · 2023-06-28T17:37:50Z

Assuming we would follow up with the correct fix, I think I would be ok with switching from chrootarchive to the unsafe archive pkg with a build tag if you like. The FreeBSD support would be experimental and not supported anyway. Looks like the only place is https://github.com/moby/buildkit/blob/master/solver/llbsolver/file/unpack.go#L38

akhramov · 2023-07-13T01:34:37Z

I think I would be ok with switching from chrootarchive to the unsafe archive pkg with a build tag if you like.

We could do that. But I think we're pretty close to reaching a proper solution in moby/moby#45724

crazy-max · 2023-07-23T19:59:56Z

@akhramov #4052 got merged if you want to do a rebase.

crazy-max

This minimal impl looks good to me to start with but like @tonistiigi said in #2376 (comment) we should look at the integration tests if you want to ship freebsd binaries in our release pipeline. We probably also need runj in the vagrant file to use it as runtime in the integration tests and create an executor for it.

Let's tackle this in follow-up.

And thanks for your patience and sorry it took so much time!

PTAL @AkihiroSuda @jedevc

jedevc · 2023-07-26T10:14:55Z

 	}

-	if tracingSocket != "" {
+	if tracingSocket != "" && runtime.GOOS != "freebsd" {


It looks like this could maybe use nullfs as well? Or is there some other tracing limitation for freebsd, in which case we should add an explicit comment.

We should avoid the explicit check here anyways, we should change the signature of getTracingSocketMount to return a pointer, so we can return nil and skip adding the mount in this case (since we have the compile time inclusion).

is there some other tracing limitation for freebsd

~~It used to be, but not anymore. nullfs now supports file mounts (https://mastodon.world/@dfr/109541463582011998), so now we can have a mount here. Thanks!~~

Update: can't mount sockets with this: #2376 (comment)

jedevc · 2023-07-26T16:25:27Z

@akhramov test-freebsd-amd64 looks to be failing in CI 👀

akhramov · 2023-07-26T17:25:18Z

test-freebsd-amd64 looks to be failing in CI 👀

Yes, mount_nullfs is apparently for regular files and directories, and socket is not a regular file.
@dfr I'm just wondering if it would be a good idea to allow socket to socket mounts with nullfs?

Had to rollback the previous change (+ implemented a pointer-based solution to #2376 (comment))

Buildkit code is mostly generic enough to support FreeBSD, however there are some quirks / infrastructural pieces that need to be addressed for full support, to name some - contenthash.NewFromStat attempts to set Devmajor / Devminor for regular files, assuming that RDev is zero for regular files. Unlike on Linux, it's not the case for FreeBSD. - containerdexecutor.Run uses bind mounts for rootfs. Bind mounts are not supported in FreeBSD and we should use nullfs instead - There is no CI job to run tests on FreeBSD - Some dependencies weren't ported This change ports buildkit to FreeBSD Signed-off-by: Artem Khramov <akhramov@pm.me> Co-authored-by: Akihiro Suda <suda.kyoto@gmail.com>

dfr · 2023-07-28T14:07:11Z

test-freebsd-amd64 looks to be failing in CI 👀

Yes, mount_nullfs is apparently for regular files and directories, and socket is not a regular file. @dfr I'm just wondering if it would be a good idea to allow socket to socket mounts with nullfs?

When I made the nullfs file-mount change, I was being quite conservative and ended up just allowing regular files and directories. I think it would be reasonable to add sockets and maybe fifos to the list.

akhramov · 2023-08-01T10:39:41Z

@jedevc sorry to bother you, but is there anything I've missed that might prevent us from moving forward?

tonistiigi

follow-up: there are some runtime OS checks left that would be better with build tag so we don't ship the freebsd specific code(although only a couple of lines) on linux.

jedevc · 2023-08-02T10:20:57Z

@akhramov just noticed a failure in https://github.com/moby/buildkit/actions/runs/5737387826/job/15548937738.

It looks like the server crashed, but I'm having difficulty working out what exactly has gone wrong - is there someway in CI that we could grab the buildkitd logs?

akhramov · 2023-08-02T10:41:54Z

@jedevc will follow it up here: #4089

slonopotamus · 2023-08-02T17:26:15Z

+	defer lm.mu.Unlock()
+
+	if lm.target != "" {
+		if err := mount.Unmount(lm.target, 0); err != nil {


Shouldn't this be mount.UnmountRecursive?

slonopotamus · 2023-08-03T20:49:03Z

          path: ./bin/testreports
+
+  test-freebsd-amd64:
+    runs-on: macos-12


Not an issue: is there a reason why it uses macOS?

Because the standard Linux instances on GHA do not support nested virtualization

tonistiigi · 2023-08-25T16:08:27Z

@akhramov Looks like the CI side is completely down atm if you have a moment to check what is going on.

akhramov · 2023-08-25T17:53:42Z

@tonistiigi sure, #4180 seems to be a decent idea (also sped up the pipeline)

akhramov commented Sep 24, 2021

View reviewed changes

Comment thread Dockerfile Outdated

akhramov marked this pull request as ready for review September 24, 2021 17:57

akhramov marked this pull request as draft September 24, 2021 18:05

akhramov force-pushed the feature/freebsd-port branch 2 times, most recently from 65e58cb to 9ebbc95 Compare September 25, 2021 05:59

akhramov marked this pull request as ready for review September 25, 2021 06:01

akhramov force-pushed the feature/freebsd-port branch from 9ebbc95 to 1b3204b Compare September 25, 2021 07:14

tonistiigi reviewed Sep 26, 2021

View reviewed changes

akhramov force-pushed the feature/freebsd-port branch from 1b3204b to 877d64a Compare September 26, 2021 14:34

akhramov force-pushed the feature/freebsd-port branch from 877d64a to 733c929 Compare September 26, 2021 14:40

akhramov force-pushed the feature/freebsd-port branch from 733c929 to 1128a01 Compare October 2, 2021 06:25

akhramov force-pushed the feature/freebsd-port branch from 1128a01 to 30b3c08 Compare November 27, 2021 17:33

akhramov marked this pull request as draft November 27, 2021 17:34

AkihiroSuda added the status/needs-rebase label Apr 17, 2022

akhramov force-pushed the feature/freebsd-port branch 4 times, most recently from 875a392 to 019eb04 Compare May 15, 2022 16:48

akhramov force-pushed the feature/freebsd-port branch 3 times, most recently from d146aa7 to 56d9e11 Compare May 28, 2022 19:30

akhramov marked this pull request as ready for review May 28, 2022 19:39

akhramov mentioned this pull request Jun 10, 2023

pkg/chrootarchive: fix FreeBSD build moby/moby#45724

Merged

This was referenced Jul 20, 2023

executor/resource: stub out NewSysSampler on Windows #4040

Merged

vendor: github.com/docker/docker@master (afd4805) #4052

Merged

crazy-max mentioned this pull request Jul 24, 2023

Add macOS support #4059

Closed

4 tasks

crazy-max approved these changes Jul 25, 2023

View reviewed changes

jedevc reviewed Jul 26, 2023

View reviewed changes

jedevc approved these changes Aug 1, 2023

View reviewed changes

crazy-max approved these changes Aug 1, 2023

View reviewed changes

tonistiigi approved these changes Aug 2, 2023

View reviewed changes

akhramov mentioned this pull request Aug 2, 2023

FreeBSD CI Run logs retention #4089

Merged

slonopotamus reviewed Aug 2, 2023

View reviewed changes

slonopotamus reviewed Aug 3, 2023

View reviewed changes

jedevc mentioned this pull request Aug 16, 2023

chore: fix git freebsd to match file names #4151

Merged

Conversation

akhramov commented Sep 24, 2021

Uh oh!

Uh oh!

akhramov commented Sep 25, 2021

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi commented Sep 26, 2021

Uh oh!

akhramov commented Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonistiigi commented Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akhramov commented Oct 2, 2021

Uh oh!

tonistiigi commented Oct 2, 2021

Uh oh!

akhramov commented Nov 27, 2021

Uh oh!

AkihiroSuda commented Mar 8, 2022

Uh oh!

arrowd commented May 20, 2023

Uh oh!

crazy-max commented Jun 6, 2023

Uh oh!

akhramov commented Jun 10, 2023

Uh oh!

tonistiigi commented Jun 28, 2023

Uh oh!

crazy-max commented Jun 28, 2023

Uh oh!

tonistiigi commented Jun 28, 2023

Uh oh!

akhramov commented Jul 13, 2023

Uh oh!

crazy-max commented Jul 23, 2023

Uh oh!

crazy-max left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akhramov Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jedevc commented Jul 26, 2023

Uh oh!

akhramov commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dfr commented Jul 28, 2023

Uh oh!

akhramov commented Aug 1, 2023

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

jedevc commented Aug 2, 2023

Uh oh!

akhramov commented Aug 2, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

akhramov commented Sep 26, 2021 •

edited

Loading

tonistiigi commented Sep 26, 2021 •

edited

Loading

crazy-max left a comment •

edited

Loading

akhramov Jul 26, 2023 •

edited

Loading

akhramov commented Jul 26, 2023 •

edited

Loading