darwin runtime support #4526
darwin runtime support #4526
Conversation
|
Build succeeded.
|
There was a problem hiding this comment.
I'd prefer this to be called io.containerd.runu.v1 and maintained in runu's repo
There was a problem hiding this comment.
Or just extend the existing io.containerd.runc.v2 to support Darwin
There was a problem hiding this comment.
agree; will move those parts (cmd/containerd-shim-darwin-v1) to runu repo.
There was a problem hiding this comment.
did remove cmd/containerd-shim-darwin-v1. I will update the runu part later.
mount/mount_darwin.go
Outdated
There was a problem hiding this comment.
Do we have snapshotters natively work on darwin? Should we have a new one?
There was a problem hiding this comment.
at least, make integration reported some of errors related to snapshotters (which I was struggled to interpret them).
It would be helpful if somebody let me know what I have to do for a proper snapshotter.
There was a problem hiding this comment.
Because bind mount is faked with a symbolic link, some features are not available. But the current integration test for the native snapshot looks okay to me (I disabled TestSnapshotterClient/ViewReadonly for darwin); how do you think ?
--- PASS: TestSnapshotterClient (0.01s)
--- PASS: TestSnapshotterClient/Chown (1.07s)
--- PASS: TestSnapshotterClient/Basic (1.11s)
--- PASS: TestSnapshotterClient/RemoveDirectoryInLowerLayer (1.57s)
--- PASS: TestSnapshotterClient/Remove (1.22s)
--- PASS: TestSnapshotterClient/Walk (1.76s)
--- PASS: TestSnapshotterClient/PreareViewFailingtest (0.89s)
--- PASS: TestSnapshotterClient/TransitivityTest (0.56s)
--- PASS: TestSnapshotterClient/StatComitted (0.23s)
--- SKIP: TestSnapshotterClient/ViewReadonly (0.00s)
--- PASS: TestSnapshotterClient/StatActive (0.16s)
--- PASS: TestSnapshotterClient/RootPermission (0.16s)
--- PASS: TestSnapshotterClient/CloseTwice (0.42s)
--- PASS: TestSnapshotterClient/Update (3.24s)
--- PASS: TestSnapshotterClient/StatInWalk (1.11s)
--- SKIP: TestSnapshotterClient/Rename (0.00s)
--- PASS: TestSnapshotterClient/MoveFileFromLowerLayer (1.00s)
--- PASS: TestSnapshotterClient/DeletedFilesInChildSnapshot (1.76s)
--- PASS: TestSnapshotterClient/RemoveIntermediateSnapshot (1.05s)
--- PASS: TestSnapshotterClient/DirectoryPermissionOnCommit (1.37s)
--- PASS: TestSnapshotterClient/LayerFileupdate (8.69s)
--- PASS: TestSnapshotterClient/128LayersMount (21.91s)
There was a problem hiding this comment.
This should check to make sure the mount is a bind mount before doing the symlink hack.
We should find a better snapshot solution here, what type of block device/file support does runu have here? That and the unpacking are what need to be figured out here to have something closer to a real solution.
There was a problem hiding this comment.
This should check to make sure the mount is a bind mount before doing the symlink hack.
Okay, I will add the check before the hack.
We should find a better snapshot solution here, what type of block device/file support does runu have here? That and the unpacking are what need to be figured out here to have something closer to a real solution.
Currently, runu supports two modes for mounting filesystems:
- a disk image file, formatted with Linux-capable filesystems (ext4, btrfs, etc)
- 9pfs hosted by server (experimental, running inside runu runtime, still a bit buggy)
So, because applications (with LKL) running over runu mount filesystem by themselves, runtimes (containerd, runu) are required to expose those files or 9pfs server ports to offer file services.
I'm not well understanding what part of unpacking is not a real solution right now.
There was a problem hiding this comment.
The unpacking today is done on the host. If the host is unable to mount the snapshot filesystem, then the unpacking will need to be done inside the runtime environment. Using the native snapshotter and simulating bind mounts with symlinks is not a real solution that we would recommend to use, but gets us a test environment.
So I take it from your response that this is using the experimental 9pfs inside the runu runtime.
There was a problem hiding this comment.
Thanks, I got the point of what the native snapshotter is.
A decent snapshotter for darwin would be nice rather than the one of testing purpose, but I would expect this to further contribution because it's not likely to be a trivial component.
Or, is not having a real snapshotter a blocker for this pull request ?
So I take it from your response that this is using the experimental 9pfs inside the runu runtime.
Yes, plus a disk image file unpacked by the snapshotter also can be mounted.
There was a problem hiding this comment.
Does the native snapshotter work here? It'll be slower, but it's very simple.
There was a problem hiding this comment.
Does the
nativesnapshotter work here? It'll be slower, but it's very simple.
Yes. This PR uses native snapshotter indeed. On darwin there is no bind mount nor nullfs (yes it actually has but depends on fuse/bindfs etc), thus emulates with symlinks.
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
8db7790 to
6bc6efa
Compare
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
6bc6efa to
a9a3af2
Compare
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
a9a3af2 to
c3ab5bc
Compare
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
c3ab5bc to
114d3ca
Compare
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
114d3ca to
9948b54
Compare
|
Currently, I'm waiting for a merge containerd/go-runc#64; once it's upstreamed, I'll update this PR too. |
|
Build succeeded.
|
container_test.go
Outdated
There was a problem hiding this comment.
Should we just exclude this file on darwin? Seems to be a lot cleaner solution.
There was a problem hiding this comment.
thanks for the comment.
As I commented at #4526 (comment), there are still valid tests in container_test.go even with darwin and it's worthwhile to have those tests I believe.
Excluding the tests is indeed much cleaner but I'd like to test if it's fine.
thehajime
force-pushed
the
feature-darwin-runtime
branch
2 times, most recently
from
e920bbb to
d4dfd5b
Compare
|
I have (force) pushed the branch to fix several issues. |
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
d4dfd5b to
dce2648
Compare
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
ee24ce0 to
dce2648
Compare
|
Build succeeded.
|
|
Build succeeded.
|
thehajime
force-pushed
the
feature-darwin-runtime
branch
from
dce2648 to
ea87ad8
Compare
|
Build succeeded.
|
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I don't think the helper binaries with names resolved by the mount type is the right plugin interface to support custom mount types. The goal of a plugin interface is that they would be easy to register and work with other plugins, such as a snapshotter that uses a custom mount type and able to register the custom mount type. Also such a plugin interface needs to worry about lifecycle and device activation which is not relevant for system mount types. I do think we could have a solution which is less complicated though, we should have support for mounting block devices with natively supported filesystems using hdiutil and bind mounts using bindfs (possibly able to be overwritten via environment but using bindfs cli interface). Currently the helpers just wrap these utilities but I don't see a reason why we can't use them directly in our mount package. Then we can defer the debate/development over custom mount types supported across all OSes. |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
Up to avoid stale. |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
Keepalive bump! |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
ensure this survives the bot |
|
The most important bits of this PR were already merged in #5936. Containerd can run on Darwin. The key missing part to be usable is lack of bind-mounts. But they are not going to happen because there is no native support. But you can use https://github.com/darwin-containers/containerd with |
We couldn't reach consensus on introducing the dependency on macFUSE because it is a third-party kernel extension, but probably it is acceptable to depend on Apple's FSKit |
|
So, I think this PR can be closed. The only useful changes left here are related to tests. But until we have bind-mounts implementation, there's no way to actually run them. And all core changes were already merged. |
|
Starting with Xcode 15, Apple adds FSKit in betas, and then removes in RC/release. Then adds back in next beta. |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
mac os 15.4 released, is FSKit available? and what else blocking this PR? |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 7 days unless new comments are made or the stale label is removed. |
|
This PR was closed because it has been stalled for 7 days with no activity. |
20 participants
This PR tries to add new platform support with darwin, since we have OCI runtime (https://github.com/ukontainer/runu) running on macos (w/o hypervisor.framework).
The commits include several specific issues (thus draft PR); several are already addressed but I would like to ask opinions from more people if there are better ways to handle.
reap process handling is tricky since there are no PR_SET_CHILD_SUBREAPER alike on darwin
I'm completely unfamiliar with native snapshotter; thus the current patch (16845ac0b64362186673b6af5873f549904fea01) is a bandaid to pass
ctr runwithout failures.make integrationfails right now, thus ignored for the moment(will revert it)(reverted)go-runc package needs to be updated (thus skip validate/vendor for the moment;
will revert as well) (reverted)skip critest since grpc.v1.cri for darwin is not ported yet; disabled the test as wellI have another patchset for dockerd: will work on when everything required is upstreamed to containerd.