diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 72a7b1017..49748fe95 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -822,7 +822,7 @@ func (l *Loader) createContainerProcess(root bool, cid string, info *containerIn } l.startGoferMonitor(cid, int32(info.goferFDs[0].FD())) - mntr := newContainerMounter(info, l.k, l.mountHints, l.productName, cid) + mntr := newContainerMounter(info, l.k, l.mountHints, l.productName, l.sandboxID) if root { if err := mntr.processHints(info.conf, info.procArgs.Credentials); err != nil { return nil, nil, err diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index f63e177ac..d6aa1b880 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -58,22 +58,22 @@ const ( Nonefs = "none" ) -// SelfOverlayFilestoreDirPrefix is the prefix in the directory name of the -// self overlay filestore directory. -const SelfOverlayFilestoreDirPrefix = ".gvisor.overlay.img." +// SelfOverlayFilestorePrefix is the prefix in the file name of the +// self overlay filestore file. +const SelfOverlayFilestorePrefix = ".gvisor.overlay.img." -// SelfOverlayFilestoreDir returns the directory path in which self overlay filestore -// files are stored for a given mount. -func SelfOverlayFilestoreDir(mountSrc, cid string) string { - // We will place filestore files in a gvisor specific hidden directory inside - // the mount being overlayed itself. The same volume can be overlay-ed by - // multiple containers. So make the filestore directory unique to container - // by suffixing the container ID. - return path.Join(mountSrc, selfOverlayFilestoreDirName(cid)) +// SelfOverlayFilestorePath returns the path at which the self overlay +// filestore file is stored for a given mount. +func SelfOverlayFilestorePath(mountSrc, sandboxID string) string { + // We will place the filestore file in a gVisor specific hidden file inside + // the mount being overlay-ed itself. The same volume can be overlay-ed by + // multiple sandboxes. So make the filestore file unique to a sandbox by + // suffixing the sandbox ID. + return path.Join(mountSrc, selfOverlayFilestoreName(sandboxID)) } -func selfOverlayFilestoreDirName(cid string) string { - return SelfOverlayFilestoreDirPrefix + cid +func selfOverlayFilestoreName(sandboxID string) string { + return SelfOverlayFilestorePrefix + sandboxID } // tmpfs has some extra supported options that we must pass through. @@ -350,11 +350,11 @@ type containerMounter struct { // /sys/devices/virtual/dmi/id/product_name. productName string - // cid is the container ID for the container. - cid string + // sandboxID is the ID for the whole sandbox. + sandboxID string } -func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountHints, productName string, cid string) *containerMounter { +func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountHints, productName string, sandboxID string) *containerMounter { return &containerMounter{ root: info.spec.Root, mounts: compileMounts(info.spec, info.conf), @@ -363,7 +363,7 @@ func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountH k: k, hints: hints, productName: productName, - cid: cid, + sandboxID: sandboxID, } } @@ -572,7 +572,7 @@ func (c *containerMounter) configureOverlay(ctx context.Context, conf *config.Co if err := overlay.CreateWhiteout(ctx, c.k.VFS(), creds, &vfs.PathOperation{ Root: upperRootVD, Start: upperRootVD, - Path: fspath.Parse(selfOverlayFilestoreDirName(c.cid)), + Path: fspath.Parse(selfOverlayFilestoreName(c.sandboxID)), }); err != nil { return nil, nil, fmt.Errorf("failed to create whiteout to hide self overlay filestore: %w", err) } diff --git a/runsc/container/container.go b/runsc/container/container.go index 936d0f172..e68fdc2b3 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -766,13 +766,13 @@ func (c *Container) Destroy() error { errs = append(errs, err.Error()) return nil } - // mountSrc only contains the filestore directory if it is a directory. + // mountSrc only contains the filestore file if it is a directory. if !mountSrcInfo.IsDir() { return nil } - filestoreDir := c.SelfOverlayFilestoreDir(mountSrc) - if err := os.RemoveAll(filestoreDir); err != nil { - err = fmt.Errorf("failed to delete filestore directory %q: %v", filestoreDir, err) + filestorePath := boot.SelfOverlayFilestorePath(mountSrc, c.sandboxID()) + if err := os.Remove(filestorePath); err != nil { + err = fmt.Errorf("failed to delete filestore file %q: %v", filestorePath, err) log.Warningf("%v", err) errs = append(errs, err.Error()) } @@ -813,6 +813,10 @@ func (c *Container) Destroy() error { return fmt.Errorf(strings.Join(errs, "\n")) } +func (c *Container) sandboxID() string { + return c.Saver.ID.SandboxID +} + // createOverlayFilestores creates the regular files that will back the tmpfs // upper mount for overlay mounts. It may return (nil, nil) if overlay is not // configured to be backed by host files. @@ -840,12 +844,6 @@ func (c *Container) createOverlayFilestores() ([]*os.File, error) { return filestoreFiles, nil } -// SelfOverlayFilestoreDir returns the directory path in which self overlay filestore -// files are stored for a given mount. -func (c *Container) SelfOverlayFilestoreDir(mountSrc string) string { - return boot.SelfOverlayFilestoreDir(mountSrc, c.ID) -} - // Precondition: Overlay2.IsBackedByHostFile() && Overlay2.IsBackedBySelf(). func (c *Container) createOverlayFilestoreInSelf() ([]*os.File, error) { var filestoreFiles []*os.File @@ -858,16 +856,19 @@ func (c *Container) createOverlayFilestoreInSelf() ([]*os.File, error) { log.Warningf("overlay2 self medium is only supported for directory mounts, but mount %q is not a directory, falling back to memory", mountSrc) return nil } - // Create SelfFilestoreDir() directory. Note that it may already exist from - // previous iteration for the same mountSrc. - filestoreDir := c.SelfOverlayFilestoreDir(mountSrc) - if err := os.Mkdir(filestoreDir, 0755); err != nil && !os.IsExist(err) { - return fmt.Errorf("failed to create filestore directory %q: %v", filestoreDir, err) - } - // The same volume can be mounted at various places within the same - // container. So use os.CreateTemp to create a random filename. - filestoreFile, err := os.CreateTemp(filestoreDir, "filestore-") + // Create the self overlay filestore file. + filestorePath := boot.SelfOverlayFilestorePath(mountSrc, c.sandboxID()) + filestoreFD, err := unix.Open(filestorePath, unix.O_RDWR|unix.O_CREAT|unix.O_EXCL, 0666) if err != nil { + if err == unix.EEXIST { + // Note that if the same submount is mounted multiple times within the + // same sandbox, then the overlay option doesn't work correctly. + // Because each overlay mount is independent and changes to one are not + // visible to the other. Given "overlay on repeated submounts" is + // already broken, we don't support such a scenario with the self + // medium. The filestore file will already exist for such a case. + return fmt.Errorf("%q mount source already has a filestore file at %q; repeated submounts are not suppported with self medium", mountSrc, filestorePath) + } return fmt.Errorf("failed to create filestore file inside %q: %v", mountSrc, err) } // Filestore in self should be a named path because it needs to be @@ -875,7 +876,7 @@ func (c *Container) createOverlayFilestoreInSelf() ([]*os.File, error) { // and apply any limits appropriately (like local ephemeral storage // limits). So don't delte it. These files will be unlinked when the // container is destroyed. This makes self medium appropriate for k8s. - filestoreFiles = append(filestoreFiles, filestoreFile) + filestoreFiles = append(filestoreFiles, os.NewFile(uintptr(filestoreFD), filestorePath)) return nil }) return filestoreFiles, err diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index 942d8cd59..4fa717d5f 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -2274,6 +2274,7 @@ func TestMultiContainerOverlayLeaks(t *testing.T) { } defer cleanup() + sandboxID := conts[0].Sandbox.ID for i, c := range conts { if i == 0 { // Don't wait for the root container which just sleeps. @@ -2295,30 +2296,8 @@ func TestMultiContainerOverlayLeaks(t *testing.T) { continue } - // Fetch the overlay filestore directory. - filestoreDir := conts[i].SelfOverlayFilestoreDir(s.Root.Path) - dirFD, err := os.Open(filestoreDir) - if err != nil { - t.Fatalf("os.Open(%q) failed for filestore dir: %v", filestoreDir, err) - } - - // Find the filestore file. - names, err := dirFD.Readdirnames(0) - if err != nil { - t.Fatalf("dirFD.Readdirnames(0) failed for filestore dir: %v", err) - } - filestoreFile := "" - for _, name := range names { - if strings.HasPrefix(name, "filestore") { - filestoreFile = path.Join(filestoreDir, name) - break - } - } - if filestoreFile == "" { - t.Fatalf("could not find filestore file in root directory entries: %v", names) - } - // Stat filestoreFile to see its usage. It should have been cleaned up. + filestoreFile := boot.SelfOverlayFilestorePath(s.Root.Path, sandboxID) var stat unix.Stat_t if err := unix.Stat(filestoreFile, &stat); err != nil { t.Errorf("unix.Stat(%q) failed for rootfs filestore: %v", filestoreFile, err) diff --git a/test/e2e/integration_runtime_test.go b/test/e2e/integration_runtime_test.go index 28157c384..0010197da 100644 --- a/test/e2e/integration_runtime_test.go +++ b/test/e2e/integration_runtime_test.go @@ -197,44 +197,6 @@ func TestOverlayNameTooLong(t *testing.T) { } } -// TestMultipleOverlayMounts tests having multiple overlay mounts works -// correctly when using host file backed overlays. All overlay mount should -// have their own MemoryFile backed different host files. -func TestMultipleOverlayMounts(t *testing.T) { - ctx := context.Background() - d := dockerutil.MakeContainerWithRuntime(ctx, t, "-overlay") - defer d.CleanUp(ctx) - - tmpDir := testutil.TmpDir() - opts := dockerutil.RunOpts{ - Image: "basic/ubuntu", - Mounts: []mount.Mount{ - { - Type: mount.TypeBind, - Source: tmpDir, - Target: "/submount1", - }, - { - Type: mount.TypeBind, - Source: tmpDir, - Target: "/submount2", - }, - }, - } - if got, err := d.Run(ctx, opts, "bash", "-c", "echo one > /submount1/file && echo two > /submount2/file && grep -Fxq one /submount1/file && grep -Fxq two /submount2/file && echo success"); err != nil { - t.Fatalf("docker run failed: %v", err) - } else if want := "success"; !strings.Contains(got, want) { - t.Errorf("container output %q does not contain %q", got, want) - } - - // Ensure overlay was applied to both bind mounts and no changes were made - // to the host filesystem. - filePath := filepath.Join(tmpDir, "file") - if _, err := os.Stat(filePath); !os.IsNotExist(err) { - t.Errorf("overlay not applied to both bind mounts, %q file exists", filePath) - } -} - // Tests that the overlay backing host file inside the container's rootfs is // hidden from the application. func TestOverlayRootfsWhiteout(t *testing.T) { @@ -245,9 +207,9 @@ func TestOverlayRootfsWhiteout(t *testing.T) { opts := dockerutil.RunOpts{ Image: "basic/ubuntu", } - if got, err := d.Run(ctx, opts, "bash", "-c", fmt.Sprintf("ls -al / | grep %q || true", boot.SelfOverlayFilestoreDirPrefix)); err != nil { + if got, err := d.Run(ctx, opts, "bash", "-c", fmt.Sprintf("ls -al / | grep %q || true", boot.SelfOverlayFilestorePrefix)); err != nil { t.Fatalf("docker run failed: %s, %v", got, err) } else if got != "" { - t.Errorf("root directory contains a file/directory whose name contains %q: output = %q", boot.SelfOverlayFilestoreDirPrefix, got) + t.Errorf("root directory contains a file/directory whose name contains %q: output = %q", boot.SelfOverlayFilestorePrefix, got) } }