From 4123170a39835b8eb877b01875660147479cf490 Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Tue, 28 May 2024 22:17:23 +0800 Subject: [PATCH] *: export RemoveVolatileOption for CRI image volumes Remove volatile option when CRI prepares image volumes. Signed-off-by: Wei Fu --- core/mount/mount_test.go | 2 +- core/mount/temp.go | 18 +++-- integration/container_volume_linux_test.go | 78 +++++++++++++++++++ .../images/volume-ownership/Dockerfile | 1 + internal/cri/opts/container.go | 1 + 5 files changed, 93 insertions(+), 7 deletions(-) create mode 100644 integration/container_volume_linux_test.go diff --git a/core/mount/mount_test.go b/core/mount/mount_test.go index 74592263a..7ae2d6a2f 100644 --- a/core/mount/mount_test.go +++ b/core/mount/mount_test.go @@ -250,7 +250,7 @@ func TestRemoveVolatileTempMount(t *testing.T) { for _, tc := range testCases { original := copyMounts(tc.input) - actual := removeVolatileTempMount(tc.input) + actual := RemoveVolatileOption(tc.input) if !reflect.DeepEqual(actual, tc.expected) { t.Fatalf("incorrectly modified mounts: %s.\n\n Expected: %v\n\n, Actual: %v", tc.desc, tc.expected, actual) } diff --git a/core/mount/temp.go b/core/mount/temp.go index 2e34023d9..ba37ba5f3 100644 --- a/core/mount/temp.go +++ b/core/mount/temp.go @@ -28,8 +28,11 @@ var tempMountLocation = getTempDir() // WithTempMount mounts the provided mounts to a temp dir, and pass the temp dir to f. // The mounts are valid during the call to the f. -// The volatile option of overlayfs doesn't allow to mount again using the same upper / work dirs. Since it's a temp mount, avoid using that option here if found. // Finally we will unmount and remove the temp dir regardless of the result of f. +// +// NOTE: The volatile option of overlayfs doesn't allow to mount again using the +// same upper / work dirs. Since it's a temp mount, avoid using that option here +// if found. func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) error) (err error) { root, uerr := os.MkdirTemp(tempMountLocation, "containerd-mount") if uerr != nil { @@ -60,7 +63,7 @@ func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) erro } }() - if uerr = All(removeVolatileTempMount(mounts), root); uerr != nil { + if uerr = All(RemoveVolatileOption(mounts), root); uerr != nil { return fmt.Errorf("failed to mount %s: %w", root, uerr) } if err := f(root); err != nil { @@ -69,12 +72,15 @@ func WithTempMount(ctx context.Context, mounts []Mount, f func(root string) erro return nil } -// removeVolatileTempMount The volatile option of overlayfs doesn't allow to mount again using the -// same upper / work dirs. Since it's a temp mount, avoid using that -// option here. Reference: https://docs.kernel.org/filesystems/overlayfs.html#volatile-mount +// RemoveVolatileOption copies and remove the volatile option for overlay +// type, since overlayfs doesn't allow to mount again using the same upper/work +// dirs. +// +// REF: https://docs.kernel.org/filesystems/overlayfs.html#volatile-mount +// // TODO: Make this logic conditional once the kernel supports reusing // overlayfs volatile mounts. -func removeVolatileTempMount(mounts []Mount) []Mount { +func RemoveVolatileOption(mounts []Mount) []Mount { var out []Mount for i, m := range mounts { if m.Type != "overlay" { diff --git a/integration/container_volume_linux_test.go b/integration/container_volume_linux_test.go new file mode 100644 index 000000000..708559018 --- /dev/null +++ b/integration/container_volume_linux_test.go @@ -0,0 +1,78 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package integration + +import ( + "os" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/containerd/containerd/v2/integration/images" + "github.com/containerd/containerd/v2/pkg/kernelversion" + "github.com/stretchr/testify/require" + criruntime "k8s.io/cri-api/pkg/apis/runtime/v1" +) + +func TestRunContainerWithVolatileOption(t *testing.T) { + if ok, err := kernelversion.GreaterEqualThan(kernelversion.KernelVersion{ + Kernel: 5, + Major: 10, + }); !ok { + t.Skipf("Only test it when kernel >= 5.10: %v", err) + } + + workDir := t.TempDir() + + t.Log("Prepare containerd config with volatile option") + cfgPath := filepath.Join(workDir, "config.toml") + err := os.WriteFile( + cfgPath, + []byte(` +version = 3 + +[plugins.'io.containerd.internal.v1.cri'] + ignore_image_defined_volumes = false + +[plugins."io.containerd.snapshotter.v1.overlayfs"] + mount_options = ["volatile"] +`), + 0600) + require.NoError(t, err) + + t.Logf("Starting containerd") + currentProc := newCtrdProc(t, "containerd", workDir) + require.NoError(t, currentProc.isReady()) + t.Cleanup(func() { + t.Log("Cleanup all the pods") + cleanupPods(t, currentProc.criRuntimeService(t)) + + t.Log("Stopping containerd process") + require.NoError(t, currentProc.kill(syscall.SIGTERM)) + require.NoError(t, currentProc.wait(5*time.Minute)) + }) + + imageName := images.Get(images.VolumeOwnership) + pullImagesByCRI(t, currentProc.criImageService(t), imageName) + + podCtx := newPodTCtx(t, currentProc.criRuntimeService(t), "running-pod", "sandbox") + + podCtx.createContainer("running", imageName, + criruntime.ContainerState_CONTAINER_RUNNING, + WithCommand("sleep", "1d")) +} diff --git a/integration/images/volume-ownership/Dockerfile b/integration/images/volume-ownership/Dockerfile index 66f51e78b..75a47e90d 100644 --- a/integration/images/volume-ownership/Dockerfile +++ b/integration/images/volume-ownership/Dockerfile @@ -15,4 +15,5 @@ FROM ubuntu RUN mkdir -p /test_dir && \ chown -R nobody:nogroup /test_dir +# NOTE: The volume is required by ../../container_volume_linux_test.go#TestRunContainerWithVolatileOption. VOLUME /test_dir diff --git a/internal/cri/opts/container.go b/internal/cri/opts/container.go index ba37006f9..e31e796e6 100644 --- a/internal/cri/opts/container.go +++ b/internal/cri/opts/container.go @@ -74,6 +74,7 @@ func WithVolumes(volumeMounts map[string]string, platform imagespec.Platform) co if len(mounts) == 1 && mounts[0].Type == "overlay" { mounts[0].Options = append(mounts[0].Options, "ro") } + mounts = mount.RemoveVolatileOption(mounts) root, err := os.MkdirTemp("", "ctd-volume") if err != nil {