Support the native snapshotter on FreeBSD #4858

samuelkarp · 2020-12-17T08:19:50Z

This PR adds support for the native snapshotter on FreeBSD using FreeBSD nullfs.

The native snapshotter passes its test suite and is able to unpack an image for the freebsd/amd64 platform:

% freebsd-version
12.1-RELEASE-p4
% pwd
/home/sam/go/src/github.com/containerd/containerd/snapshots/native
% sudo go test -v -test.root .
[...trimmed output...]
--- PASS: TestNaive (0.01s)                                                                                                                                              
    --- PASS: TestNaive/Basic (0.03s)                                                                                                                                    
    --- PASS: TestNaive/128LayersMount (4.04s)                                                                                                                           
    --- PASS: TestNaive/RootPermission (0.01s)                                                                                                                           
    --- PASS: TestNaive/CloseTwice (0.00s)                                                                                                                               
    --- PASS: TestNaive/StatInWalk (0.01s)                                                                                                                               
    --- PASS: TestNaive/ViewReadonly (0.01s)                                                                                                                             
    --- SKIP: TestNaive/Rename (0.00s)                                                                                                                                   
    --- PASS: TestNaive/MoveFileFromLowerLayer (0.03s)                                                                                                                   
    --- PASS: TestNaive/DeletedFilesInChildSnapshot (0.03s)                                                                                                              
    --- PASS: TestNaive/RemoveIntermediateSnapshot (0.01s)                                                                                                               
    --- PASS: TestNaive/DirectoryPermissionOnCommit (0.04s)                                                                                                              
    --- PASS: TestNaive/Chown (0.03s)                                                                                                                                    
    --- PASS: TestNaive/RemoveDirectoryInLowerLayer (0.04s)                                                                                                              
    --- PASS: TestNaive/LayerFileupdate (3.85s)                                                                                                                          
    --- PASS: TestNaive/Walk (0.01s)                                                                                                                                     
    --- PASS: TestNaive/Remove (0.01s)                                                                                                                                   
    --- PASS: TestNaive/Update (0.03s)                                                                                                                                   
    --- PASS: TestNaive/PreareViewFailingtest (0.01s)                                                                                                                    
    --- PASS: TestNaive/TransitivityTest (0.01s)                                                                                                                         
    --- PASS: TestNaive/StatComitted (0.01s)                                                                                                                             
    --- PASS: TestNaive/StatActive (0.01s)                                                                                                                               
PASS
ok      github.com/containerd/containerd/snapshots/native       8.226s

% sudo ctr i ls
REF     TYPE                                    DIGEST                                                                  SIZE      PLATFORMS     LABELS 
freebsd application/vnd.oci.image.index.v1+json sha256:960c76846cd112e09032c88914458faee8d03c04b8260dfbc4da70b25227534a 236.8 MiB freebsd/amd64 -

samuelkarp · 2020-12-17T08:21:39Z

mount/mount_freebsd.go

+	// cmd.CombinedOutput() may intermittently return ECHILD because of our signal handling in shim.
+	// See #4387 and wait(2).


The behavior around ECHILD and retries is copied from mount_linux.go. There isn't (yet) a shim on FreeBSD, so I don't know if the logic is really necessary, but I left it in to start. If there's a preference to remove it, I'm happy to do so.

samuelkarp · 2020-12-17T08:22:24Z

mount/mount_freebsd.go

+			case unix.EBUSY:
+				time.Sleep(50 * time.Millisecond)


Similarly, the behavior around EBUSY and retries is copied from mount_linux.go. Again, I don't know if the logic is really necessary, but I left it in to start. If there's a preference to remove it, I'm happy to do so.

AkihiroSuda · 2020-12-17T09:21:47Z

mount/mount_freebsd.go

+	// See #4387 and wait(2).
+	const retriesOnECHILD = 10
+	for i := 0; i < retriesOnECHILD; i++ {
+		cmd := exec.Command("mount", args...)


Is there any reason not to use mount(2) syscall directly?

See the comment on line 37:

// The "syscall" and "golang.org/x/sys/unix" packages do not define a Mount // function for FreeBSD, so instead we execute mount(8) and trust it to do // the right thing

We could use syscall.RawSyscall but using mount(8) felt more reliable to me.

kzys · 2020-12-17T17:42:37Z

mount/mount_freebsd.go

+
+var (
+	// ErrNotImplementOnUnix is returned for methods that are not implemented
+	ErrNotImplementOnUnix = errors.New("not implemented under unix")


You haven't used this constant on the PR.

Would it be ErrNotImplemented or ErrNotImplementedOnPlatform? This might be useful for other non-Unix platforms.

This constant is pre-existing (was in mount/mount_unix.go) and used elsewhere in the codebase when compiled for FreeBSD (specifically in pkg/os/mount_unix.go). I left the name as-is to avoid touching other places in the codebase.

Oh I see. LGTM then.

samuelkarp · 2020-12-18T17:52:52Z

samuelkarp force-pushed the samuelkarp:freebsd-native-snapshotter branch from 2fb04a0 to b667c15

Rebased on top of the current master; no changes to the diff.

theopenlab-ci · 2020-12-18T17:59:44Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 6m 11s (non-voting)

Signed-off-by: Samuel Karp <me@samuelkarp.com>

samuelkarp · 2020-12-23T05:26:55Z

samuelkarp force-pushed the samuelkarp:freebsd-native-snapshotter branch from b667c15 to b624486

Rebased on top of the current master; no changes to the diff.

theopenlab-ci · 2020-12-23T05:33:46Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 6m 00s (non-voting)

samuelkarp · 2020-12-23T05:41:07Z

I believe the devicemapper failures are unrelated to my change as the only change on Linux is a reordering of mount options for the native snapshotter.

samuelkarp commented Dec 17, 2020

View reviewed changes

samuelkarp force-pushed the freebsd-native-snapshotter branch from 8e9280b to 2fb04a0 Compare December 17, 2020 08:31

AkihiroSuda reviewed Dec 17, 2020

View reviewed changes

kzys reviewed Dec 17, 2020

View reviewed changes

kzys approved these changes Dec 17, 2020

View reviewed changes

AkihiroSuda approved these changes Dec 17, 2020

View reviewed changes

samuelkarp force-pushed the freebsd-native-snapshotter branch from 2fb04a0 to b667c15 Compare December 18, 2020 17:51

samuelkarp added 2 commits December 22, 2020 21:26

mount: basic support for FreeBSD

d6baafa

Signed-off-by: Samuel Karp <me@samuelkarp.com>

native: support for FreeBSD

b624486

Signed-off-by: Samuel Karp <me@samuelkarp.com>

samuelkarp force-pushed the freebsd-native-snapshotter branch from b667c15 to b624486 Compare December 23, 2020 05:26

mxpv approved these changes Jan 11, 2021

View reviewed changes

mxpv merged commit 04df60d into containerd:master Jan 11, 2021

kzys mentioned this pull request Mar 25, 2021

darwin runtime support #4526

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support the native snapshotter on FreeBSD #4858

Support the native snapshotter on FreeBSD #4858

samuelkarp commented Dec 17, 2020

samuelkarp Dec 17, 2020

samuelkarp Dec 17, 2020

AkihiroSuda Dec 17, 2020

samuelkarp Dec 17, 2020

kzys Dec 17, 2020 •

edited

Loading

samuelkarp Dec 17, 2020

kzys Dec 17, 2020

samuelkarp commented Dec 18, 2020

theopenlab-ci bot commented Dec 18, 2020

samuelkarp commented Dec 23, 2020

theopenlab-ci bot commented Dec 23, 2020

samuelkarp commented Dec 23, 2020

		// cmd.CombinedOutput() may intermittently return ECHILD because of our signal handling in shim.
		// See #4387 and wait(2).

Support the native snapshotter on FreeBSD #4858

Support the native snapshotter on FreeBSD #4858

Conversation

samuelkarp commented Dec 17, 2020

samuelkarp Dec 17, 2020

Choose a reason for hiding this comment

samuelkarp Dec 17, 2020

Choose a reason for hiding this comment

AkihiroSuda Dec 17, 2020

Choose a reason for hiding this comment

samuelkarp Dec 17, 2020

Choose a reason for hiding this comment

kzys Dec 17, 2020 • edited Loading

Choose a reason for hiding this comment

samuelkarp Dec 17, 2020

Choose a reason for hiding this comment

kzys Dec 17, 2020

Choose a reason for hiding this comment

samuelkarp commented Dec 18, 2020

theopenlab-ci bot commented Dec 18, 2020

samuelkarp commented Dec 23, 2020

theopenlab-ci bot commented Dec 23, 2020

samuelkarp commented Dec 23, 2020

kzys Dec 17, 2020 •

edited

Loading