deduplicate mounts

Signed-off-by: CrazyMax <[email protected]>
moby · Oct 18, 2021 · 32d95c8 · 32d95c8
1 parent 87e1fa7
commit 32d95c8
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 39 deletions.
diff --git a/executor/oci/mounts.go b/executor/oci/mounts.go
@@ -100,3 +100,17 @@ func withBoundProc() oci.SpecOpts {
 		return nil
 	}
 }
+
+func dedupMounts(mnts []specs.Mount) []specs.Mount {
+	ret := make([]specs.Mount, 0, len(mnts))
+	visited := make(map[string]int)
+	for i, mnt := range mnts {
+		if j, ok := visited[mnt.Destination]; ok {
+			ret[j] = mnt
+		} else {
+			visited[mnt.Destination] = i
+			ret = append(ret, mnt)
+		}
+	}
+	return ret
+}
diff --git a/executor/oci/mounts_test.go b/executor/oci/mounts_test.go
@@ -8,8 +8,56 @@ import (
 	"github.com/moby/buildkit/util/appcontext"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+// The default mount-list from containerd
+// https://github.com/containerd/containerd/blob/main/oci/mounts.go
+var containerdDefMounts = []specs.Mount{
+	{
+		Destination: "/proc",
+		Type:        "proc",
+		Source:      "proc",
+		Options:     []string{"nosuid", "noexec", "nodev"},
+	},
+	{
+		Destination: "/dev",
+		Type:        "tmpfs",
+		Source:      "tmpfs",
+		Options:     []string{"nosuid", "strictatime", "mode=755", "size=65536k"},
+	},
+	{
+		Destination: "/dev/pts",
+		Type:        "devpts",
+		Source:      "devpts",
+		Options:     []string{"nosuid", "noexec", "newinstance", "ptmxmode=0666", "mode=0620", "gid=5"},
+	},
+	{
+		Destination: "/dev/shm",
+		Type:        "tmpfs",
+		Source:      "shm",
+		Options:     []string{"nosuid", "noexec", "nodev", "mode=1777", "size=65536k"},
+	},
+	{
+		Destination: "/dev/mqueue",
+		Type:        "mqueue",
+		Source:      "mqueue",
+		Options:     []string{"nosuid", "noexec", "nodev"},
+	},
+	{
+		Destination: "/sys",
+		Type:        "sysfs",
+		Source:      "sysfs",
+		Options:     []string{"nosuid", "noexec", "nodev", "ro"},
+	},
+	{
+		Destination: "/run",
+		Type:        "tmpfs",
+		Source:      "tmpfs",
+		Options:     []string{"nosuid", "strictatime", "mode=755", "size=65536k"},
+	},
+}
+
 func TestHasPrefix(t *testing.T) {
 	type testCase struct {
 		path     string
@@ -99,56 +147,53 @@ func TestHasPrefix(t *testing.T) {
 }
 
 func TestWithRemovedMounts(t *testing.T) {
-	// The default mount-list from containerd
 	s := oci.Spec{
-		Mounts: []specs.Mount{
-			{
-				Destination: "/proc",
-				Type:        "proc",
-				Source:      "proc",
-				Options:     []string{"nosuid", "noexec", "nodev"},
-			},
-			{
-				Destination: "/dev",
-				Type:        "tmpfs",
-				Source:      "tmpfs",
-				Options:     []string{"nosuid", "strictatime", "mode=755", "size=65536k"},
-			},
-			{
-				Destination: "/dev/pts",
-				Type:        "devpts",
-				Source:      "devpts",
-				Options:     []string{"nosuid", "noexec", "newinstance", "ptmxmode=0666", "mode=0620", "gid=5"},
-			},
+		Mounts: containerdDefMounts,
+	}
+
+	oldLen := len(s.Mounts)
+	err := withRemovedMount("/run")(appcontext.Context(), nil, nil, &s)
+	assert.NoError(t, err)
+	assert.Equal(t, oldLen-1, len(s.Mounts))
+}
+
+func TestDedupMounts(t *testing.T) {
+	s := oci.Spec{
+		Mounts: append(containerdDefMounts, []specs.Mount{
 			{
 				Destination: "/dev/shm",
 				Type:        "tmpfs",
 				Source:      "shm",
-				Options:     []string{"nosuid", "noexec", "nodev", "mode=1777", "size=65536k"},
+				Options:     []string{"nosuid", "size=131072k"},
+			},
+			{
+				Destination: "/foo",
+				Type:        "bind",
+				Source:      "/bar",
+				Options:     []string{"nosuid", "noexec", "nodev", "rbind", "ro"},
 			},
 			{
 				Destination: "/dev/mqueue",
 				Type:        "mqueue",
 				Source:      "mqueue",
-				Options:     []string{"nosuid", "noexec", "nodev"},
-			},
-			{
-				Destination: "/sys",
-				Type:        "sysfs",
-				Source:      "sysfs",
-				Options:     []string{"nosuid", "noexec", "nodev", "ro"},
+				Options:     []string{"nosuid"},
 			},
-			{
-				Destination: "/run",
-				Type:        "tmpfs",
-				Source:      "tmpfs",
-				Options:     []string{"nosuid", "strictatime", "mode=755", "size=65536k"},
-			},
-		},
+		}...),
 	}
 
-	oldLen := len(s.Mounts)
-	err := withRemovedMount("/run")(appcontext.Context(), nil, nil, &s)
-	assert.NoError(t, err)
-	assert.Equal(t, oldLen-1, len(s.Mounts))
+	mntsLen := len(s.Mounts)
+	s.Mounts = dedupMounts(s.Mounts)
+	require.Equal(t, mntsLen-2, len(s.Mounts))
+	assert.Equal(t, specs.Mount{
+		Destination: "/dev/shm",
+		Type:        "tmpfs",
+		Source:      "shm",
+		Options:     []string{"nosuid", "size=131072k"},
+	}, s.Mounts[3])
+	assert.Equal(t, specs.Mount{
+		Destination: "/foo",
+		Type:        "bind",
+		Source:      "/bar",
+		Options:     []string{"nosuid", "noexec", "nodev", "rbind", "ro"},
+	}, s.Mounts[len(s.Mounts)-1])
 }
diff --git a/executor/oci/spec.go b/executor/oci/spec.go
@@ -164,6 +164,7 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
 		})
 	}
 
+	s.Mounts = dedupMounts(s.Mounts)
 	return s, releaseAll, nil
 }
-Original file line number
+Diff line change
@@ Expand Up @@
     		})
     	}
+    	s.Mounts = dedupMounts(s.Mounts)
     	return s, releaseAll, nil
     }
@@ Expand Down @@