Recursive is needed to catch mounts under the bind mount, for example
erofs mount points. i.e. with that in place things like:
/foo/erofs1
/foo/erofs2
bind mount + idmap /foo/ -> /foo-idmapped/
makes it so erofs1 and erofs2 are at /foo-idmapped/, allowing us to use
that when constructing the final overlay mount erofs snapshotters use.
This let's the snapshotter claim to support idmap support later without
issue (right now it goes through remapfs).
As part of this, we want to make the bind mount have private mount
propagation. If you umount /foo-idmapped/erofs1 it shouldn't affect
/foo/erofs1, which could be reused in another container's lowerdirs and
idmapped bind mount.
Signed-off-by: Andrew Halaney <ahalaney@netflix.com>
containerd will do a read only mount of the mount string to inspect
things like /etc/passwd for a username to uid mapping. It doesn't need
to do a uidmap/gidmap for this, that's entirely for the actual container
mount later.
Let's stop doing the idmap here, its extra work and not necessary.
Further, it causes complications when the mount manager is involved.
Mount manager will by default create erofs layers at
/run/containerd/io.containerd.mount-manager.v1.bolt/t/8103/1, and the
snapshotter's upperdir is at (if configured to use a different path)
/mnt/containerd/io.containerd.snapshotter.v1.erofs/snapshots/25795/fs.
During the read only mount this upperdir is shoved into the lowerdirs
list, and then the idmap code tries to find the common parent directory of
all lowerdirs. In this case its "/", which is invalid, so the process
fails.
Let's stop doing the extra work and get away from this class of
problems by doing less.
Signed-off-by: Andrew Halaney <ahalaney@netflix.com>
In CI we run make root-test via gotestsum, which executes multiple
package tests concurrently. TestAutoclearTrueLoop attempts to invoke
LOOP_CLR_FD using a device name, which introduces a race condition.
Example race:
Process P1 represents mount.test which runs TestAutoclearTrueLoop
Process P2 represents manager.test which runs TestLoopbackMount
T1: P1 closes fd of loop-device (loop3) (kernel unsets backing-file on close)
T2: P2 gets loop3 from /dev/loop-control
T3: P2 configures loop3 with backing file successfully
T4: P1 invokes removeLoop to clear backing file for loop3
You might see that failure like this
```
=== FAIL: core/mount/manager TestLoopbackMount (0.05s)
log_hook.go:47: time="2025-10-23T21:49:22.532811960Z" level=debug msg="activating mount" func="manager.(*mountManager).Activate" file="/home/runner/work/containerd/containerd/core/mount/manager/manager.go:134" mounts="[{loop /tmp/TestLoopbackMount989607109/001/fs-1621892597 []} {format/ext4 {{ mount 0 }} []}]" name=id1 testcase=TestLoopbackMount
helpers.go💯 unmount /tmp/TestLoopbackMount989607109/001/test-mount-3030342351
manager_linux_test.go:80:
Error Trace: /home/runner/work/containerd/containerd/core/mount/manager/manager_linux_test.go:80
/home/runner/work/containerd/containerd/core/mount/manager/manager_linux_test.go:105
Error: Received unexpected error:
failed to get loop device info: no such device or address
Test: TestLoopbackMount
```
To fix this, the test now compares backing-file's inode directly and does
not call removeLoop when autoclear is set.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Commit ee8ae9d569 ("Update erofs snapshotter to use mount manager")
temporarily removed the file-backed mount feature to adapt to the new
mount manager infrastructure as a quick start.
After the mount manager was introduced, a specific mount type can be
handled with a mount handler plugin to provide a dedicated mount
process (e.g. setup loopback devices in advance or calling external
mount helpers).
This commit adds a default EROFS mount handler for the Linux hosts
to set up loop devices for mount sources and "device=" external file
blobs if necessary (i.e. when file-backed mounts are unavailable),
allowing common runtimes such as runC to work directly, e.g.
``` sh
mount -t erofs /var/lib/containerd/io.containerd.snapshotter.v1.erofs/snapshots/1/layer.erofs \
/run/containerd/io.containerd.mount-manager.v1.bolt/t/346/1
```
will be handled as
``` sh
mount -t erofs /dev/loop1 /run/containerd/io.containerd.mount-manager.v1.bolt/t/346/1
```
and
``` sh
mount -t erofs /var/lib/containerd/io.containerd.snapshotter.v1.erofs/snapshots/7/fsmeta.erofs \
-odevice=/var/lib/containerd/io.containerd.snapshotter.v1.erofs/snapshots/1/layer.erofs,\
device=/var/lib/containerd/io.containerd.snapshotter.v1.erofs/snapshots/2/layer.erofs,\
...
device=/var/lib/containerd/io.containerd.snapshotter.v1.erofs/snapshots/7/layer.erofs
/run/containerd/io.containerd.mount-manager.v1.bolt/t/335/1
```
will be handled as
``` sh
mount -t erofs /dev/loop1 -odevice=/dev/loop2,device=/dev/loop3,... \
/run/containerd/io.containerd.mount-manager.v1.bolt/t/335/1
```
if file-backed mounts are unavailable.
For other host platforms (e.g. Darwin hosts) or specific runtimes
that require EROFS raw mounts instead of parsed mounts, this plugin
can be explicitly masked off by users.
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Rename `default_size_mb` to `default_size` so that it's similar to
Docker's `--storage-opt size=30G` flag. For example,
``` toml
[plugins."io.containerd.snapshotter.v1.erofs"]
default_size = "5GB"
```
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
The mount manager GC seems to be broken on my local setup.
According to commit df87a8f71b ("Add support for backreferences
in gc"), the interface name should be "ActiveWithBackRefs" instead.
After this patch, erofs and ext4 mounts won't be GCed in advance.
Fixes: 184fae60fc ("Add backreference support to mount manager")
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Reduce the size of the test files. Even using sparse files, the reported
large size may cause issues in some test environments.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Adding a close method allows the mount manager to close any open file
descriptors. The method will also be called automatically by containerd
on shutdown.
Ensure the tests call Close to avoid leaking file descriptors or errors
on Windows cleaning up directories that are in use.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Extend the mount manager to support more transformers than format. The
transformers allow altering the mount before it is passed to the mount
handlers. These could be one-time actions which are needed to perform
the mount.
Adds mkdir and mkfs actions which can be used to prepare the arguments
for a mount. The actions can be limited to actions within the target
mount directory or plugin directories.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Allow the mount manager to skip handling of custom types. Ensure that
custom types are still working with formatted mounts.
Signed-off-by: Derek McGowan <derek@mcg.dev>
The autoclear may take a bit of time to clear out the file, check
multiple times for the file to get removed before returning an error.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Avoid keeping file descriptor open to directory which is getting
removed. Update error handling and wrapping to provide more clarity
around failures.
Signed-off-by: Derek McGowan <derek@mcg.dev>
This commit makes all of the recommended changes to use the `testing`
package helper functions instead of doing the equivalent longhand
versions of the same thing.
This change was needed in order to properly detect errors, as the code
would previously skip running `tenv` stating that it had been deprecated
in favor of `usetesting`.
Signed-off-by: Enji Cooper <yaneurabeya@gmail.com>
If a Dockerfile is using a `VOLUME` directive and the directory exists
in the rootfs, like in this example:
FROM docker.io/library/alpine:latest
VOLUME [ "/run" ]
The alpine container image already contains a "/run" directory. This
will force the code in WithVolumes() to copy its content to the new
volume created for the VOLUME directive. This copies the content as well
as the ownership.
However, as we perform the mounts from the host POV without being inside
a userns, the idmap option will just shift the IDs in ways that will
screw up the ownerships when copied. We should only use the idmap option
when running the container inside a userns, so the ownerships are fine
(the userns will do a shift and the idmap another, to make it all seem
as if there was no UID/GID shift in the first place).
This PR does just that, remove the idmap option from mounts so we copy
the files without any ID transformations. It's simpler and easier to
reason about if we just don't mount with the idmap option here: all
files are copied just fine without ID transformations and ID
transformation is applied via the idmap option at mount time when
running the pod.
Also, note that `VOLUME` directives that refer to directories that don't
exist on the rootfs work fine (`VOLUME [ "/rata" ]` for example), as
there is no copy done in that case so the permissions weren't changed.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
When a lot of pods are created all at the same time, the umount fails
with EBUSY sometimes. This causes some mounts to be leaked. A way to
repro this issue is here:
https://github.com/containerd/containerd/issues/12139#issuecomment-3165305904
While using lsof/fuser it was not possible to get the culprits on time
(the mount is busy for a few ms), @fuwei has found what is causing the
mount to be busy:
https://github.com/containerd/containerd/issues/12139#issuecomment-3184544534
When we fork in `GetUsernsFD()`, if a call to prepareIDMappedOverlay()
is ongoing and has an open fd to the path to idmap, then the unmount
callback will fail as the forked process still has an open fd to it.
Let's handle the idmap unmounts in the same way other temp mounts are
done, using the Unmount() helper, that retries the unmount.
This retry fixes it 100% of the times on my system and in the systems
of @halaney that reported the issue too.
This was originally fixed by #10721 by using a detached mount, but the
mount was switched to non-detached again in #10955 and we started to
leak mounts. As @fuwei mentioned, using a detached mount is quite
invisible for the admin and, therefore, a retry is a better alternative.
It seems these umounts are a great candidate for #11303, which will
manage the life-cycle of mounts and can handle these retries whenever
needed.
[1]: To do it, I just run as root "unshare -m", that creates a mntns
with private propagation, and then run the containerd daemon.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Before this patch, the cleanup was unconditionally trying an unmount,
which is not needed if the mount never succeeded in the first place.
This causes logs of failed stuff, that should never be there.
Also, one function was creating the tmpDir and the nested one removing
it (in the cleanup function), which complicates the reasoning about
not leaking resources.
This patch on one hand moves the creation of the tmpDir into the
function that will remove it later and renames the parameter. On the
other hand, it also separates the cleanup function in two functions:
cleanDir() and cleanMount(). Each one will clean the directory or the
mount, only if needed, and a new cleanup function that just calls
cleanDir() and cleanMount() is returned as the cleanup function handler.
Because we now create the tmp directory inside the function, we need to
adjust the test: the directory passed as param now will exist, but it
shoild be empty. Therefore, we change the os.Stat() to an os.Remove().
If it's not empty (the cleanup didn't work), the remove will fail.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
doPrepareIDMappedOverlay() can return nil as the cleanup function. As
the function mandates for the callback to be called, even when errors
are returned, we end-up calling the nil function that of course causes:
panic: runtime error: invalid memory address or nil pointer dereference
The containerd daemon continues to run fine, though.
With the current structure of always calling the cleanup function, we
would either need to check which error it is, to call it only on
specific errors (ugly) or return a "stub function" that doesn't do
anything on those cases (so the call works). None of these options seem
very nice.
Let's just switch the logic to a pattern more common on go: only need to
cleanup if it returned fine. This makes it easier for callers to not do
the wrong thing.
For this change we need to do the cleanup when returning errors on the
function itself, so the user doesn't need to do it for us.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Per layer idmap'ed bind mounts are costly to performance, as shown in
[0]. Each one requires taking various kernel locks, and each one shows
up in the host's mount table leading to some components like systemd
processing all these temporary mounts unnecessarily.
Let's instead go ahead and idmap the common directory of all the
layers to achieve the same effect. Now instead of being a function of
the number of layers, its a constant idmap per overlayfs! This can have
a big impact. For example, imagine running 100 containers at once, each
with 50 layers. That's going from doing 100 * 50 (5000) bind mounts, to
just 100. In reality both the shim and containerd proper do this, so its
actually double that!
[0]: https://github.com/containerd/containerd/issues/12048#issuecomment-3050444019
Signed-off-by: Andrew Halaney <ahalaney@netflix.com>
Add new snapshotter & differ plugins that can leverage the block CIM layer writers to
store pulled/imported images in the block CIM format.
Signed-off-by: Amit Barve <ambarve@microsoft.com>
Now that we have 1.24.x as go min version, I think
we can remove this go code specific to a lower
version.
Signed-off-by: Jin Dong <djdongjin95@gmail.com>
This is a planned follow-on from #10721 primarily at the request of
@fuweid, exchanging MNT_DETACH at unmount time for MOUNT_ATTR_RDONLY at
mount time. The effect is to increase risk of unmount failure due to
EBUSY (as observed in the wild) but add an additional protection that the
then-leaked bind mount does not act as a conduit for inadvertent modification
of the underlying data, including our own efforts to clean up the mountpoint.
Tests covering the lifecycle of the temporary idmap mounts and integrity
of the underlying lower layer data is also included in the normal and
failed-unmount case.
Fixes#10704
Signed-off-by: Mike Baynton <mike@mbaynton.com>
* container_update_resources.go: it is Windows and Linux that need special handling
* local*.go: all platforms use the same list of tasks
* temp_unix.go/temp_unsupported.go: Darwin is a Unix
* util_unix.go/util_unsupported.go: use generic unix tag
The only user-visible effect of these changes is that tempMountLocation is now properly handled on Darwin
Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
