From f8ede828a0245ef17c9e82a5cb7115603867eb32 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Sun, 5 Feb 2023 09:09:02 -0800 Subject: [PATCH] Create separate overlay filestore files for each overlay mount. c16cbd3f6efd ("Create separate overlay filestore for subcontainers.") moved the ownership of pgalloc.MemoryFile to tmpfs mount (upper layer in overlay). However, this also means that each tmpfs mount needs its own filestore file. Currently if there are multiple overlay mounts in a container that are backed by the same host file. Since they use different MemoryFiles, they will overwrite each other. This change creates a separate overlay file per overlay mount per container and plumbs the FDs to their respective tmpfs mounts. This is similar to how gofer FDs are dealt with. Added a regression test for this too. PiperOrigin-RevId: 507289704 --- Makefile | 2 +- runsc/boot/controller.go | 23 +++--- runsc/boot/loader.go | 26 +++--- runsc/boot/loader_test.go | 15 ++-- runsc/boot/mount_hints.go | 4 +- runsc/boot/vfs.go | 63 ++++++++------- runsc/cmd/boot.go | 38 ++++----- runsc/container/container.go | 114 +++++++++++++++++++-------- runsc/sandbox/sandbox.go | 23 +++--- test/e2e/integration_runtime_test.go | 38 +++++++++ 10 files changed, 222 insertions(+), 124 deletions(-) diff --git a/Makefile b/Makefile index 1ba3743c4..ee48a22e2 100644 --- a/Makefile +++ b/Makefile @@ -270,7 +270,7 @@ docker-tests: load-basic $(RUNTIME_BIN) @$(call install_runtime,$(RUNTIME)-fdlimit,--fdlimit=2000) # Used by TestRlimitNoFile. @$(call install_runtime,$(RUNTIME)-dcache,--fdlimit=2000 --dcache=100) # Used by TestDentryCacheLimit. @$(call install_runtime,$(RUNTIME)-host-uds,--host-uds=all) # Used by TestHostSocketConnect. - @$(call install_runtime,$(RUNTIME)-overlay,--overlay2=root:dir=/tmp) # Used by TestOverlay*. + @$(call install_runtime,$(RUNTIME)-overlay,--overlay2=all:dir=/tmp) # Used by TestOverlay*. @$(call test_runtime,$(RUNTIME),$(INTEGRATION_TARGETS) //test/e2e:integration_runtime_test) .PHONY: docker-tests diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index f0babe326..32ffcc983 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -262,9 +262,13 @@ type StartArgs struct { // CID is the ID of the container to start. CID string + // NumOverlayFilestoreFDs is the number of overlay filestore FDs donated. + // Optionally configured with the overlay2 flag. + NumOverlayFilestoreFDs int + // FilePayload contains, in order: // * stdin, stdout, and stderr (optional: if terminal is disabled). - // * file descriptor to overlay-backing host file (optional: for overlay2). + // * file descriptors to overlay-backing host files (optional: for overlay2). // * file descriptors to connect to gofer to serve the root filesystem. urpc.FilePayload } @@ -286,13 +290,10 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro return errors.New("start argument missing container ID") } expectedFDs := 1 // At least one FD for the root filesystem. + expectedFDs += args.NumOverlayFilestoreFDs if !args.Spec.Process.Terminal { expectedFDs += 3 } - overlay2 := args.Conf.GetOverlay2() - if overlay2.IsBackedByHostFile() { - expectedFDs++ - } if len(args.Files) < expectedFDs { return fmt.Errorf("start arguments must contain at least %d FDs, but only got %d", expectedFDs, len(args.Files)) } @@ -318,15 +319,15 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro } }() - var overlayFilestoreFD *fd.FD - if overlay2.IsBackedByHostFile() { - var err error - overlayFilestoreFD, err = fd.NewFromFile(goferFiles[0]) + var overlayFilestoreFDs []*fd.FD + for i := 0; i < args.NumOverlayFilestoreFDs; i++ { + overlayFilestoreFD, err := fd.NewFromFile(goferFiles[i]) if err != nil { return fmt.Errorf("error dup'ing overlay filestore file: %w", err) } - goferFiles = goferFiles[1:] + overlayFilestoreFDs = append(overlayFilestoreFDs, overlayFilestoreFD) } + goferFiles = goferFiles[args.NumOverlayFilestoreFDs:] goferFDs, err := fd.NewFromFiles(goferFiles) if err != nil { @@ -338,7 +339,7 @@ func (cm *containerManager) StartSubcontainer(args *StartArgs, _ *struct{}) erro } }() - if err := cm.l.startSubcontainer(args.Spec, args.Conf, args.CID, stdios, goferFDs, overlayFilestoreFD); err != nil { + if err := cm.l.startSubcontainer(args.Spec, args.Conf, args.CID, stdios, goferFDs, overlayFilestoreFDs); err != nil { log.Debugf("containerManager.StartSubcontainer failed, cid: %s, args: %+v, err: %v", args.CID, args, err) return err } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 7e40f20be..780cf036b 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -101,9 +101,9 @@ type containerInfo struct { // goferFDs are the FDs that attach the sandbox to the gofers. goferFDs []*fd.FD - // overlayFilestoreFD is the FD for the memory file that will back the - // overlay mount's upper tmpfs layer. - overlayFilestoreFD *fd.FD + // overlayFilestoreFDs are the FDs to the regular files that will back the + // tmpfs upper mount in the overlay mounts. + overlayFilestoreFDs []*fd.FD } // Loader keeps state needed to start the kernel and run the container. @@ -203,9 +203,9 @@ type Args struct { // StdioFDs is the stdio for the application. The Loader takes ownership of // these FDs and may close them at any time. StdioFDs []int - // OverlayFilestoreFD is the host FD to a regular file which will be used to - // back the overlay mount's upper tmpfs layer. - OverlayFilestoreFD int + // OverlayFilestoreFDs are the FDs to the regular files that will back the + // tmpfs upper mount in the overlay mounts. + OverlayFilestoreFDs []int // NumCPU is the number of CPUs to create inside the sandbox. NumCPU int // TotalMem is the initial amount of total memory to report back to the @@ -273,8 +273,8 @@ func New(args Args) (*Loader, error) { for _, goferFD := range args.GoferFDs { info.goferFDs = append(info.goferFDs, fd.New(goferFD)) } - if args.OverlayFilestoreFD >= 0 { - info.overlayFilestoreFD = fd.New(args.OverlayFilestoreFD) + for _, overlayFD := range args.OverlayFilestoreFDs { + info.overlayFilestoreFDs = append(info.overlayFilestoreFDs, fd.New(overlayFD)) } // Create kernel and platform. @@ -701,7 +701,7 @@ func (l *Loader) createSubcontainer(cid string, tty *fd.FD) error { // startSubcontainer starts a child container. It returns the thread group ID of // the newly created process. Used FDs are either closed or released. It's safe // for the caller to close any remaining files upon return. -func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdioFDs, goferFDs []*fd.FD, overlayFilestoreFD *fd.FD) error { +func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdioFDs, goferFDs, overlayFilestoreFDs []*fd.FD) error { // Create capabilities. caps, err := specutils.Capabilities(conf.EnableRaw, spec.Process.Capabilities) if err != nil { @@ -752,10 +752,10 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st } info := &containerInfo{ - conf: conf, - spec: spec, - goferFDs: goferFDs, - overlayFilestoreFD: overlayFilestoreFD, + conf: conf, + spec: spec, + goferFDs: goferFDs, + overlayFilestoreFDs: overlayFilestoreFDs, } info.procArgs, err = createProcessArgs(cid, spec, creds, l.k, pidns) if err != nil { diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 3aec336f6..3939dd5e1 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -125,14 +125,13 @@ func createLoader(spec *specs.Spec) (*Loader, func(), error) { } args := Args{ - ID: "foo", - Spec: spec, - Conf: conf, - ControllerFD: fd, - GoferFDs: []int{sandEnd}, - StdioFDs: stdio, - OverlayFilestoreFD: -1, - PodInitConfigFD: -1, + ID: "foo", + Spec: spec, + Conf: conf, + ControllerFD: fd, + GoferFDs: []int{sandEnd}, + StdioFDs: stdio, + PodInitConfigFD: -1, } l, err := New(args) if err != nil { diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go index 3b84fd1ad..60833afae 100644 --- a/runsc/boot/mount_hints.go +++ b/runsc/boot/mount_hints.go @@ -191,8 +191,8 @@ func (m *mountHint) isSupported() bool { // Master options must be the same or less restrictive than the container mount, // e.g. master can be 'rw' while container mounts as 'ro'. func (m *mountHint) checkCompatible(replica *specs.Mount) error { - masterOpts := parseMountOptions(m.mount.Options) - replicaOpts := parseMountOptions(replica.Options) + masterOpts := ParseMountOptions(m.mount.Options) + replicaOpts := ParseMountOptions(replica.Options) if masterOpts.ReadOnly && !replicaOpts.ReadOnly { return fmt.Errorf("cannot mount read-write shared mount because master is read-only, mount: %+v", replica) diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 84458027d..d849254cb 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -51,10 +51,10 @@ import ( "gvisor.dev/gvisor/runsc/specutils" ) +// Supported filesystems that map to different internal filesystems. const ( - // Supported filesystems that map to different internal filesystems. - bind = "bind" - nonefs = "none" + Bind = "bind" + Nonefs = "none" ) // tmpfs has some extra supported options that we must pass through. @@ -293,10 +293,14 @@ type fdDispenser struct { } func (f *fdDispenser) remove() int { + return f.removeAsFD().Release() +} + +func (f *fdDispenser) removeAsFD() *fd.FD { if f.empty() { panic("fdDispenser out of fds") } - rv := f.fds[0].Release() + rv := f.fds[0] f.fds = f.fds[1:] return rv } @@ -315,9 +319,9 @@ type containerMounter struct { // fds is the list of FDs to be dispensed for mounts that require it. fds fdDispenser - // overlayFilestoreFD is the FD for the memory file that will back the - // overlay mount's upper tmpfs layer. - overlayFilestoreFD *fd.FD + // overlayFilestoreFDs are the FDs to the regular files that will back the + // tmpfs upper mount in the overlay mounts. + overlayFilestoreFDs fdDispenser k *kernel.Kernel @@ -330,13 +334,13 @@ type containerMounter struct { func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountHints, productName string) *containerMounter { return &containerMounter{ - root: info.spec.Root, - mounts: compileMounts(info.spec, info.conf), - fds: fdDispenser{fds: info.goferFDs}, - overlayFilestoreFD: info.overlayFilestoreFD, - k: k, - hints: hints, - productName: productName, + root: info.spec.Root, + mounts: compileMounts(info.spec, info.conf), + fds: fdDispenser{fds: info.goferFDs}, + overlayFilestoreFDs: fdDispenser{fds: info.overlayFilestoreFDs}, + k: k, + hints: hints, + productName: productName, } } @@ -423,11 +427,11 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi } fsName := gofer.Name - if conf.GetOverlay2().RootMount && !c.root.Readonly { + if overlay2 := conf.GetOverlay2(); overlay2.RootMount && !c.root.Readonly { log.Infof("Adding overlay on top of root") var err error var cleanup func() - opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName) + opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName, overlay2.IsBackedByHostFile()) if err != nil { return nil, fmt.Errorf("mounting root with overlay: %w", err) } @@ -446,7 +450,7 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi // layer using tmpfs, and return overlay mount options. "cleanup" must be called // after the options have been used to mount the overlay, to release refs on // lower and upper mounts. -func (c *containerMounter) configureOverlay(ctx context.Context, creds *auth.Credentials, lowerOpts *vfs.MountOptions, lowerFSName string) (*vfs.MountOptions, func(), error) { +func (c *containerMounter) configureOverlay(ctx context.Context, creds *auth.Credentials, lowerOpts *vfs.MountOptions, lowerFSName string, useFilestoreFD bool) (*vfs.MountOptions, func(), error) { // First copy options from lower layer to upper layer and overlay. Clear // filesystem specific options. upperOpts := *lowerOpts @@ -484,10 +488,13 @@ func (c *containerMounter) configureOverlay(ctx context.Context, creds *auth.Cre } // Upper is a tmpfs mount to keep all modifications inside the sandbox. - upperOpts.GetFilesystemOptions.InternalData = tmpfs.FilesystemOpts{ + tmpfsOpts := tmpfs.FilesystemOpts{ RootFileType: uint16(rootType), - FilestoreFD: c.overlayFilestoreFD, } + if useFilestoreFD { + tmpfsOpts.FilestoreFD = c.overlayFilestoreFDs.removeAsFD() + } + upperOpts.GetFilesystemOptions.InternalData = tmpfsOpts upper, err := c.k.VFS().MountDisconnected(ctx, creds, "" /* source */, tmpfs.Name, &upperOpts) if err != nil { return nil, nil, fmt.Errorf("failed to create upper layer for overlay, opts: %+v: %v", upperOpts, err) @@ -612,7 +619,7 @@ func (c *containerMounter) prepareMounts() ([]mountAndFD, error) { // Only bind mounts use host FDs; see // containerMounter.getMountNameAndOptions. fd := -1 - if m.Type == bind { + if m.Type == Bind { fd = c.fds.remove() } mounts = append(mounts, mountAndFD{ @@ -648,8 +655,9 @@ func (c *containerMounter) mountSubmount(ctx context.Context, conf *config.Confi if useOverlay { log.Infof("Adding overlay on top of mount %q", submount.mount.Destination) + overlay2 := conf.GetOverlay2() var cleanup func() - opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName) + opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName, overlay2.IsBackedByHostFile()) if err != nil { return nil, fmt.Errorf("mounting volume with overlay at %q: %w", submount.mount.Destination, err) } @@ -688,7 +696,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA case devpts.Name, devtmpfs.Name, proc.Name: // Nothing to do. - case nonefs: + case Nonefs: fsName = sys.Name case sys.Name: @@ -703,7 +711,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA return "", nil, false, err } - case bind: + case Bind: fsName = gofer.Name if m.fd == 0 { // Check that an FD was provided to fails fast. Technically FD=0 is valid, @@ -716,7 +724,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA } // If configured, add overlay to all writable mounts. - useOverlay = conf.GetOverlay2().SubMounts && !parseMountOptions(m.mount.Options).ReadOnly + useOverlay = conf.GetOverlay2().SubMounts && !ParseMountOptions(m.mount.Options).ReadOnly case cgroupfs.Name: var err error @@ -730,7 +738,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA return "", nil, false, nil } - opts := parseMountOptions(m.mount.Options) + opts := ParseMountOptions(m.mount.Options) opts.GetFilesystemOptions = vfs.GetFilesystemOptions{ Data: strings.Join(data, ","), InternalData: internalData, @@ -739,7 +747,8 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA return fsName, opts, useOverlay, nil } -func parseMountOptions(opts []string) *vfs.MountOptions { +// ParseMountOptions converts specs.Mount.Options to vfs.MountOptions. +func ParseMountOptions(opts []string) *vfs.MountOptions { mountOpts := &vfs.MountOptions{ InternalMount: true, } @@ -884,7 +893,7 @@ func (c *containerMounter) mountSharedMaster(ctx context.Context, conf *config.C if useOverlay { log.Infof("Adding overlay on top of shared mount %q", mntFD.mount.Destination) var cleanup func() - opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName) + opts, cleanup, err = c.configureOverlay(ctx, creds, opts, fsName, false /* useFilestoreFD */) if err != nil { return nil, fmt.Errorf("mounting shared volume with overlay at %q: %w", mntFD.mount.Destination, err) } diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index 7b144f0b8..54840295d 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -58,9 +58,9 @@ type Boot struct { // ioFDs is the list of FDs used to connect to FS gofers. ioFDs intFlags - // overlayFilestoreFD is the host FD to the regular file which will back the - // overlay's upper tmpfs mount for all containers. - overlayFilestoreFD int + // overlayFilestoreFDs are FDs to the regular files that will back the tmpfs + // upper mount in the overlay mounts. + overlayFilestoreFDs intFlags // stdioFDs are the fds for stdin, stdout, and stderr. They must be // provided in that order. @@ -148,7 +148,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.IntVar(&b.deviceFD, "device-fd", -1, "FD for the platform device file") f.Var(&b.ioFDs, "io-fds", "list of FDs to connect gofer clients. They must follow this order: root first, then mounts as defined in the spec") f.Var(&b.stdioFDs, "stdio-fds", "list of FDs containing sandbox stdin, stdout, and stderr in that order") - f.IntVar(&b.overlayFilestoreFD, "overlay-filestore-fd", -1, "FD to a regular file which will be used to back the overlay's tmpfs upper mount.") + f.Var(&b.overlayFilestoreFDs, "overlay-filestore-fds", "FDs to the regular files that will back the tmpfs upper mount in the overlay mounts.") f.IntVar(&b.userLogFD, "user-log-fd", 0, "file descriptor to write user logs to. 0 means no logging.") f.IntVar(&b.startSyncFD, "start-sync-fd", -1, "required FD to used to synchronize sandbox startup") f.IntVar(&b.mountsFD, "mounts-fd", -1, "mountsFD is the file descriptor to read list of mounts after they have been resolved (direct paths, no symlinks).") @@ -324,21 +324,21 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma // Create the loader. bootArgs := boot.Args{ - ID: f.Arg(0), - Spec: spec, - Conf: conf, - ControllerFD: b.controllerFD, - Device: os.NewFile(uintptr(b.deviceFD), "platform device"), - GoferFDs: b.ioFDs.GetArray(), - StdioFDs: b.stdioFDs.GetArray(), - OverlayFilestoreFD: b.overlayFilestoreFD, - NumCPU: b.cpuNum, - TotalMem: b.totalMem, - UserLogFD: b.userLogFD, - ProductName: b.productName, - PodInitConfigFD: b.podInitConfigFD, - SinkFDs: b.sinkFDs.GetArray(), - ProfileOpts: b.profileFDs.ToOpts(), + ID: f.Arg(0), + Spec: spec, + Conf: conf, + ControllerFD: b.controllerFD, + Device: os.NewFile(uintptr(b.deviceFD), "platform device"), + GoferFDs: b.ioFDs.GetArray(), + StdioFDs: b.stdioFDs.GetArray(), + OverlayFilestoreFDs: b.overlayFilestoreFDs.GetArray(), + NumCPU: b.cpuNum, + TotalMem: b.totalMem, + UserLogFD: b.userLogFD, + ProductName: b.productName, + PodInitConfigFD: b.podInitConfigFD, + SinkFDs: b.sinkFDs.GetArray(), + ProfileOpts: b.profileFDs.ToOpts(), } l, err := boot.New(bootArgs) if err != nil { diff --git a/runsc/container/container.go b/runsc/container/container.go index a6fa7ad59..a95335117 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -131,6 +131,10 @@ type Container struct { // processes. Saver StateFile `json:"saver"` + // OverlayConf is the overlay configuration with which this container was + // started. + OverlayConf config.Overlay2 `json:"overlayConf"` + // // Fields below this line are not saved in the state file and will not // be preserved across commands. @@ -211,6 +215,7 @@ func New(conf *config.Config, args Args) (*Container, error) { ContainerID: args.ID, }, }, + OverlayConf: conf.GetOverlay2(), } // The Cleanup object cleans up partially created containers when an error // occurs. Any errors occurring during cleanup itself are ignored. @@ -264,7 +269,7 @@ func New(conf *config.Config, args Args) (*Container, error) { } } c.CompatCgroup = cgroup.CgroupJSON{Cgroup: subCgroup} - overlayFilestoreFile, err := createOverlayFilestore(conf) + overlayFilestoreFiles, err := c.createOverlayFilestores() if err != nil { return nil, err } @@ -277,16 +282,16 @@ func New(conf *config.Config, args Args) (*Container, error) { // Start a new sandbox for this container. Any errors after this point // must destroy the container. sandArgs := &sandbox.Args{ - ID: sandboxID, - Spec: args.Spec, - BundleDir: args.BundleDir, - ConsoleSocket: args.ConsoleSocket, - UserLog: args.UserLog, - IOFiles: ioFiles, - MountsFile: specFile, - Cgroup: containerCgroup, - Attached: args.Attached, - OverlayFilestoreFile: overlayFilestoreFile, + ID: sandboxID, + Spec: args.Spec, + BundleDir: args.BundleDir, + ConsoleSocket: args.ConsoleSocket, + UserLog: args.UserLog, + IOFiles: ioFiles, + MountsFile: specFile, + Cgroup: containerCgroup, + Attached: args.Attached, + OverlayFilestoreFiles: overlayFilestoreFiles, } sand, err := sandbox.New(conf, sandArgs) if err != nil { @@ -403,7 +408,7 @@ func (c *Container) Start(conf *config.Config) error { } else { // Create an overlay filestore for the subcontainer if its overlay is // backed by a host file. - overlayFilestoreFile, err := createOverlayFilestore(conf) + overlayFilestoreFiles, err := c.createOverlayFilestores() if err != nil { return err } @@ -435,7 +440,7 @@ func (c *Container) Start(conf *config.Config) error { stdios = []*os.File{os.Stdin, os.Stdout, os.Stderr} } - return c.Sandbox.StartSubcontainer(c.Spec, conf, c.ID, stdios, goferFiles, overlayFilestoreFile) + return c.Sandbox.StartSubcontainer(c.Spec, conf, c.ID, stdios, goferFiles, overlayFilestoreFiles) }); err != nil { return err } @@ -782,12 +787,27 @@ func (c *Container) Destroy() error { return fmt.Errorf(strings.Join(errs, "\n")) } -func createOverlayFilestore(conf *config.Config) (*os.File, error) { - overlay2 := conf.GetOverlay2() - if !overlay2.IsBackedByHostFile() { +// 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. +func (c *Container) createOverlayFilestores() ([]*os.File, error) { + if !c.OverlayConf.IsBackedByHostFile() { return nil, nil } - filestoreDir := overlay2.HostFileDir() + filestoreFiles, err := c.createOverlayFilestoreInDir() + if err != nil { + return nil, err + } + for _, f := range filestoreFiles { + // Perform this work around outside the sandbox. The sandbox may already be + // running with seccomp filters that do not allow this. + pgalloc.IMAWorkAroundForMemFile(f.Fd()) + } + return filestoreFiles, nil +} + +func (c *Container) createOverlayFilestoreInDir() ([]*os.File, error) { + filestoreDir := c.OverlayConf.HostFileDir() fileInfo, err := os.Stat(filestoreDir) if err != nil { return nil, fmt.Errorf("failed to stat overlay filestore directory %q: %v", filestoreDir, err) @@ -795,22 +815,54 @@ func createOverlayFilestore(conf *config.Config) (*os.File, error) { if !fileInfo.IsDir() { return nil, fmt.Errorf("overlay2 flag should specify an existing directory") } - // Create an unnamed temporary file in filestore directory which will be - // deleted when the last FD on it is closed. We don't use O_TMPFILE because - // it is not supported on all filesystems. So we simulate it by creating a - // named file and then immediately unlinking it while keeping an FD on it. - // This file will be deleted when the container exits. - filestoreFile, err := os.CreateTemp(filestoreDir, "runsc-overlay-filestore-*") - if err != nil { - return nil, fmt.Errorf("failed to create a temporary file inside %q: %v", filestoreDir, err) + var filestoreFiles []*os.File + if err := c.forEachOverlayMount(func(_ string) error { + // Create an unnamed temporary file in filestore directory which will be + // deleted when the last FD on it is closed. We don't use O_TMPFILE because + // it is not supported on all filesystems. So we simulate it by creating a + // named file and then immediately unlinking it while keeping an FD on it. + // This file will be deleted when the container exits. + filestoreFile, err := os.CreateTemp(filestoreDir, "runsc-overlay-filestore-") + if err != nil { + return fmt.Errorf("failed to create a temporary file inside %q: %v", filestoreDir, err) + } + if err := unix.Unlink(filestoreFile.Name()); err != nil { + return fmt.Errorf("failed to unlink temporary file %q: %v", filestoreFile.Name(), err) + } + filestoreFiles = append(filestoreFiles, filestoreFile) + return nil + }); err != nil { + return nil, err } - if err := unix.Unlink(filestoreFile.Name()); err != nil { - return nil, fmt.Errorf("failed to unlink temporary file %q: %v", filestoreFile.Name(), err) + return filestoreFiles, nil +} + +// forEachOverlayMount calls fn on all mounts that runsc/boot/vfs.go will +// configure filestore-based overlays on. See containerMounter.configureOverlay(). +// +// Precondition: Overlay2.IsBackedByHostFile(). +func (c *Container) forEachOverlayMount(fn func(srcDir string) error) error { + if c.OverlayConf.RootMount && !c.Spec.Root.Readonly { + if err := fn(c.Spec.Root.Path); err != nil { + return err + } } - // Perform this work around outside the sandbox. The sandbox may already be - // running with seccomp filters that do not allow this. - pgalloc.IMAWorkAroundForMemFile(filestoreFile.Fd()) - return filestoreFile, nil + if !c.OverlayConf.SubMounts { + return nil + } + for i := range c.Spec.Mounts { + if c.Spec.Mounts[i].Type != boot.Bind { + continue + } + if boot.ParseMountOptions(c.Spec.Mounts[i].Options).ReadOnly { + continue + } + mountSrc := c.Spec.Mounts[i].Source + if err := fn(mountSrc); err != nil { + return err + } + } + return nil } // saveLocked saves the container metadata to a file. diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index b4ce092cd..1977c5418 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -222,9 +222,9 @@ type Args struct { // appear in the spec. IOFiles []*os.File - // OverlayFilestoreFile is the regular file that will back the tmpfs upper + // OverlayFilestoreFiles are the regular files that will back the tmpfs upper // mount in the overlay mounts. - OverlayFilestoreFile *os.File + OverlayFilestoreFiles []*os.File // MountsFile is a file container mount information from the spec. It's // equivalent to the mounts from the spec, except that all paths have been @@ -376,7 +376,7 @@ func (s *Sandbox) StartRoot(spec *specs.Spec, conf *config.Config) error { } // StartSubcontainer starts running a sub-container inside the sandbox. -func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles []*os.File, overlayFilestoreFile *os.File) error { +func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles, overlayFilestoreFiles []*os.File) error { log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load()) if err := s.configureStdios(conf, stdios); err != nil { @@ -385,22 +385,21 @@ func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid s // The payload contains (in this specific order): // * stdin/stdout/stderr (optional: only present when not using TTY) - // * The subcontainer's overlay filestore file (optional: only present when + // * The subcontainer's overlay filestore files (optional: only present when // host file backed overlay is configured) // * Gofer files. payload := urpc.FilePayload{} payload.Files = append(payload.Files, stdios...) - if overlayFilestoreFile != nil { - payload.Files = append(payload.Files, overlayFilestoreFile) - } + payload.Files = append(payload.Files, overlayFilestoreFiles...) payload.Files = append(payload.Files, goferFiles...) // Start running the container. args := boot.StartArgs{ - Spec: spec, - Conf: conf, - CID: cid, - FilePayload: payload, + Spec: spec, + Conf: conf, + CID: cid, + NumOverlayFilestoreFDs: len(overlayFilestoreFiles), + FilePayload: payload, } if err := s.call(boot.ContMgrStartSubcontainer, &args, nil); err != nil { return fmt.Errorf("starting sub-container %v: %w", spec.Process.Args, err) @@ -640,7 +639,7 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn // If there is a gofer, sends all socket ends to the sandbox. donations.DonateAndClose("io-fds", args.IOFiles...) - donations.DonateAndClose("overlay-filestore-fd", args.OverlayFilestoreFile) + donations.DonateAndClose("overlay-filestore-fds", args.OverlayFilestoreFiles...) donations.DonateAndClose("mounts-fd", args.MountsFile) donations.Donate("start-sync-fd", startSyncFile) if err := donations.OpenAndDonate("user-log-fd", args.UserLog, os.O_CREATE|os.O_WRONLY|os.O_APPEND); err != nil { diff --git a/test/e2e/integration_runtime_test.go b/test/e2e/integration_runtime_test.go index add04aeea..b08e5db36 100644 --- a/test/e2e/integration_runtime_test.go +++ b/test/e2e/integration_runtime_test.go @@ -196,3 +196,41 @@ func TestOverlayNameTooLong(t *testing.T) { t.Errorf("container output %q does not contain %q", got, want) } } + +// 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) + } +}