Create shared master mounts for volumes with share=container.

We need to create shared mounts (that are owned by the sentry) for volumes with share=container too. The admission plugin sets share=container when only one container in a pod is using the said volume. As of right now, all such shared volumes are mounted as tmpfs. If the tmpfs is mounted as a regular mount belonging to the container, then when the container restarts, the tmpfs is not preserved and the contents are lost. Furthermore, if the same shared volume is mounted multiple times within the same container, then we need to share the tmpfs mount, so that all mount points present the same contents. PiperOrigin-RevId: 574665432
2026-05-22 17:12:49 -07:00 · 2023-10-18 17:55:26 -07:00
parent f286d71166
commit 29a8856da7
3 changed files with 151 additions and 93 deletions
@@ -171,7 +171,11 @@ func (m *MountHint) setShare(val string) error {
 func (m *MountHint) ShouldShareMount() bool {
 	// Only support tmpfs for now. Bind mounts require a common gofer to mount
 	// all shared volumes.
-	return m.Mount.Type == tmpfs.Name && m.Share == pod
+	return m.Mount.Type == tmpfs.Name &&
+		// A shared mount should be configured for share=container too so:
+		// 1. Restarting the container does not lose the tmpfs data.
+		// 2. Repeated mounts in the container reuse the same tmpfs instance.
+		(m.Share == container || m.Share == pod)
 }

 // checkCompatible verifies that shared mount is compatible with master.
@@ -820,7 +820,7 @@ func (c *Container) Destroy() error {
 			if hint := sb.MountHints.FindMount(mountSrc); hint != nil && hint.ShouldShareMount() {
 				// Don't delete filestore file for shared mounts. The sandbox owns a
 				// shared master mount which uses this filestore and is shared with
-				// multiple containers.
+				// multiple mount points.
 				return
 			}
 		}
@@ -125,9 +125,18 @@ func execMany(t *testing.T, conf *config.Config, execs []execDesc) {
 }

 func createSharedMount(mount specs.Mount, name string, pod ...*specs.Spec) {
-	share := "pod"
-	if len(pod) == 1 {
-		share = "container"
+	numContainers := 0
+	for _, spec := range pod {
+		for i := range spec.Mounts {
+			if spec.Mounts[i].Source == mount.Source {
+				numContainers++
+				break
+			}
+		}
+	}
+	share := "container"
+	if numContainers > 1 {
+		share = "pod"
 	}
 	for _, spec := range pod {
 		spec.Annotations[boot.MountPrefix+name+".source"] = mount.Source
@@ -1560,101 +1569,90 @@ func TestMultiContainerSharedMountCompatible(t *testing.T) {

 // Test that shared pod mounts continue to work after container is restarted.
 func TestMultiContainerSharedMountRestart(t *testing.T) {
-	testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) {
-		// Setup the containers.
-		sleep := []string{"sleep", "100"}
-		podSpec, ids := createSpecs(sleep, sleep)
-		mnt0 := specs.Mount{
-			Destination: "/mydir/test",
-			Source:      sourceDir,
-			Type:        mntType,
-			Options:     nil,
-		}
-		podSpec[0].Mounts = append(podSpec[0].Mounts, mnt0)
+	for numSubConts := 1; numSubConts <= 2; numSubConts++ {
+		testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) {
+			// Setup the containers.
+			sleep := []string{"sleep", "100"}
+			cmds := [][]string{sleep}
+			for i := 1; i <= numSubConts; i++ {
+				cmds = append(cmds, sleep)
+			}
+			podSpec, ids := createSpecs(cmds...)

-		mnt1 := mnt0
-		mnt1.Destination = "/mydir2/test2"
-		podSpec[1].Mounts = append(podSpec[1].Mounts, mnt1)
+			// Add a shared mount to all subcontainers.
+			mnt := specs.Mount{
+				Source:  sourceDir,
+				Type:    mntType,
+				Options: nil,
+			}
+			for i := 1; i <= numSubConts; i++ {
+				mnt.Destination = fmt.Sprintf("/mydir/test%d", i)
+				podSpec[i].Mounts = append(podSpec[i].Mounts, mnt)
+			}

-		createSharedMount(mnt0, "test-mount", podSpec...)
+			createSharedMount(mnt, "test-mount", podSpec...)

-		containers, cleanup, err := startContainers(conf, podSpec, ids)
-		if err != nil {
-			t.Fatalf("error starting containers: %v", err)
-		}
-		defer cleanup()
+			containers, cleanup, err := startContainers(conf, podSpec, ids)
+			if err != nil {
+				t.Fatalf("error starting containers: %v", err)
+			}
+			defer cleanup()

-		file0 := path.Join(mnt0.Destination, "abc")
-		file1 := path.Join(mnt1.Destination, "abc")
-		execs := []execDesc{
-			{
-				c:    containers[0],
-				cmd:  []string{"/bin/touch", file0},
-				name: "create file in container0",
-			},
-			{
-				c:    containers[0],
-				cmd:  []string{"/usr/bin/test", "-f", file0},
-				name: "file appears in container0",
-			},
-			{
-				c:    containers[1],
-				cmd:  []string{"/usr/bin/test", "-f", file1},
-				name: "file appears in container1",
-			},
-		}
-		execMany(t, conf, execs)
+			// Create file in first subcontainer.
+			file1 := "/mydir/test1/abc"
+			execs := []execDesc{
+				{
+					c:    containers[1],
+					cmd:  []string{"/bin/touch", file1},
+					name: "create file in container1",
+				},
+			}
+			// Check it appears in all subcontainers.
+			for i := 1; i <= numSubConts; i++ {
+				fileName := fmt.Sprintf("/mydir/test%d/abc", i)
+				execs = append(execs, execDesc{
+					c:    containers[i],
+					cmd:  []string{"/usr/bin/test", "-f", fileName},
+					name: fmt.Sprintf("file appears in container%d", i),
+				})
+			}
+			execMany(t, conf, execs)

-		containers[1].Destroy()
+			// Restart first subcontainer.
+			containers[1].Destroy()

-		bundleDir, cleanup, err := testutil.SetupBundleDir(podSpec[1])
-		if err != nil {
-			t.Fatalf("error restarting container: %v", err)
-		}
-		defer cleanup()
+			bundleDir, cleanup, err := testutil.SetupBundleDir(podSpec[1])
+			if err != nil {
+				t.Fatalf("error restarting container: %v", err)
+			}
+			defer cleanup()

-		args := Args{
-			ID:        ids[1],
-			Spec:      podSpec[1],
-			BundleDir: bundleDir,
-		}
-		containers[1], err = New(conf, args)
-		if err != nil {
-			t.Fatalf("error creating container: %v", err)
-		}
-		if err := containers[1].Start(conf); err != nil {
-			t.Fatalf("error starting container: %v", err)
-		}
+			args := Args{
+				ID:        ids[1],
+				Spec:      podSpec[1],
+				BundleDir: bundleDir,
+			}
+			containers[1], err = New(conf, args)
+			if err != nil {
+				t.Fatalf("error creating container: %v", err)
+			}
+			if err := containers[1].Start(conf); err != nil {
+				t.Fatalf("error starting container: %v", err)
+			}

-		execs = []execDesc{
-			{
-				c:    containers[0],
-				cmd:  []string{"/usr/bin/test", "-f", file0},
-				name: "file is still in container0",
-			},
-			{
-				c:    containers[1],
-				cmd:  []string{"/usr/bin/test", "-f", file1},
-				name: "file is still in container1",
-			},
-			{
-				c:    containers[1],
-				cmd:  []string{"/bin/rm", file1},
-				name: "remove file from container1",
-			},
-			{
-				c:    containers[0],
-				cmd:  []string{"/usr/bin/test", "!", "-f", file0},
-				name: "file removed from container0",
-			},
-			{
-				c:    containers[1],
-				cmd:  []string{"/usr/bin/test", "!", "-f", file1},
-				name: "file removed from container1",
-			},
-		}
-		execMany(t, conf, execs)
-	})
+			// Ensure that the file exists in all subcontainers.
+			execs = nil
+			for i := 1; i <= numSubConts; i++ {
+				fileName := fmt.Sprintf("/mydir/test%d/abc", i)
+				execs = append(execs, execDesc{
+					c:    containers[i],
+					cmd:  []string{"/usr/bin/test", "-f", fileName},
+					name: fmt.Sprintf("file appears in container%d", i),
+				})
+			}
+			execMany(t, conf, execs)
+		})
+	}
 }

 // Test that unsupported pod mounts options are ignored when matching master and
@@ -1701,6 +1699,62 @@ func TestMultiContainerSharedMountUnsupportedOptions(t *testing.T) {
 	})
 }

+// Test that shared mounts can be repeated within a container.
+func TestMultiContainerSharedMountsRepeated(t *testing.T) {
+	testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) {
+		// Setup the containers.
+		sleep := []string{"/bin/sleep", "100"}
+		podSpec, ids := createSpecs(sleep)
+		mnt0 := specs.Mount{
+			Destination: "/mydir/test1",
+			Source:      sourceDir,
+			Type:        mntType,
+			Options:     []string{"rw", "relatime"},
+		}
+		mnt1 := specs.Mount{
+			Destination: "/mydir/test2",
+			Source:      sourceDir,
+			Type:        mntType,
+			Options:     []string{"ro"},
+		}
+		podSpec[0].Mounts = append(podSpec[0].Mounts, mnt0, mnt1)
+
+		// Set annotations using less-restrictive mnt0.
+		createSharedMount(mnt0, "test-mount", podSpec...)
+
+		containers, cleanup, err := startContainers(conf, podSpec, ids)
+		if err != nil {
+			t.Fatalf("error starting containers: %v", err)
+		}
+		defer cleanup()
+
+		execs := []execDesc{
+			{
+				c:    containers[0],
+				cmd:  []string{"/bin/touch", path.Join(mnt1.Destination, "fail")},
+				want: 1,
+				name: "fails write to read-only mount",
+			},
+			{
+				c:    containers[0],
+				cmd:  []string{"/bin/cp", "/usr/bin/test", mnt0.Destination},
+				name: "writes to writable mount",
+			},
+			{
+				c:    containers[0],
+				cmd:  []string{"/usr/bin/test", "-f", path.Join(mnt1.Destination, "test")},
+				name: "file appears in read-only mount",
+			},
+			{
+				c:    containers[0],
+				cmd:  []string{"/usr/bin/test", "-f", path.Join(mnt0.Destination, "test")},
+				name: "file appears in writable mount",
+			},
+		}
+		execMany(t, conf, execs)
+	})
+}
+
 // This test checks that a bind mount that is "shared" is overlaid correctly
 // with a self-backed tmpfs.
 func TestMultiContainerSharedBindMount(t *testing.T) {
@@ -1832,9 +1886,9 @@ func TestMultiContainerMultiRootCanHandleFDs(t *testing.T) {
 			[]string{app, "fd_sender", "--socket", socketPath},
 			[]string{app, "fd_receiver", "--socket", socketPath},
 		)
-		createSharedMount(sharedMnt, "shared-mount", specs...)
 		specs[1].Mounts = append(specs[1].Mounts, sharedMnt, writeableMnt)
 		specs[2].Mounts = append(specs[2].Mounts, sharedMnt)
+		createSharedMount(sharedMnt, "shared-mount", specs...)

 		containers, cleanup, err := startContainers(conf, specs, ids)
 		if err != nil {