Merge pull request containerd#10274 from fuweid/fix-10228

*: export RemoveVolatileOption for CRI image volumes

Author: fuweid

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)
 		}

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" {

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"))
+}

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

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 {