diff --git a/pkg/shim/utils/BUILD b/pkg/shim/utils/BUILD index 85608afde..717861b66 100644 --- a/pkg/shim/utils/BUILD +++ b/pkg/shim/utils/BUILD @@ -16,7 +16,10 @@ go_library( "//pkg/shim:__subpackages__", "//shim:__subpackages__", ], - deps = ["@com_github_opencontainers_runtime_spec//specs-go:go_default_library"], + deps = [ + "//runsc/specutils", + "@com_github_opencontainers_runtime_spec//specs-go:go_default_library", + ], ) go_test( diff --git a/pkg/shim/utils/volumes.go b/pkg/shim/utils/volumes.go index 8af2081da..de776e653 100644 --- a/pkg/shim/utils/volumes.go +++ b/pkg/shim/utils/volumes.go @@ -20,6 +20,7 @@ import ( "strings" specs "github.com/opencontainers/runtime-spec/specs-go" + "gvisor.dev/gvisor/runsc/specutils" ) const ( @@ -160,7 +161,7 @@ func UpdateVolumeAnnotations(s *specs.Spec) (bool, error) { // matching. if yes, _ := isVolumePath(volume, s.Mounts[i].Source); yes { // Container mount type must match the sandbox's mount type. - changeMountType(&s.Mounts[i], v) + specutils.ChangeMountType(&s.Mounts[i], v) updated = true } } @@ -218,7 +219,7 @@ func configureShm(s *specs.Spec) (bool, error) { s.Annotations[volumeKeyPrefix+devshmName+".options"] = "rw" } - changeMountType(m, devshmType) + specutils.ChangeMountType(m, devshmType) updated = true // Remove the duplicate entry now that we found the shared /dev/shm mount. @@ -230,22 +231,3 @@ func configureShm(s *specs.Spec) (bool, error) { } return updated, nil } - -func changeMountType(m *specs.Mount, newType string) { - m.Type = newType - - // OCI spec allows bind mounts to be specified in options only. So if new type - // is not bind, remove bind/rbind from options. - // - // "For bind mounts (when options include either bind or rbind), the type is - // a dummy, often "none" (not listed in /proc/filesystems)." - if newType != "bind" { - newOpts := make([]string, 0, len(m.Options)) - for _, opt := range m.Options { - if opt != "rbind" && opt != "bind" { - newOpts = append(newOpts, opt) - } - } - m.Options = newOpts - } -} diff --git a/runsc/boot/gofer_conf.go b/runsc/boot/gofer_conf.go index 4801bf30a..49177accb 100644 --- a/runsc/boot/gofer_conf.go +++ b/runsc/boot/gofer_conf.go @@ -38,16 +38,20 @@ const ( // AnonOverlay indicates that this gofer mount should be overlaid with an // overlayfs backed by a host file in an anonymous directory. AnonOverlay + + // SelfTmpfs indicates that this gofer mount should be overlaid with a tmpfs + // mount backed by a host file in the mount's source directory. + SelfTmpfs ) // IsFilestorePresent returns true if a filestore file was associated with this. func (g GoferMountConf) IsFilestorePresent() bool { - return g == SelfOverlay || g == AnonOverlay + return g == SelfOverlay || g == AnonOverlay || g == SelfTmpfs } // IsSelfBacked returns true if this mount is backed by a filestore in itself. func (g GoferMountConf) IsSelfBacked() bool { - return g == SelfOverlay + return g == SelfOverlay || g == SelfTmpfs } // ShouldUseOverlayfs returns true if an overlayfs should be applied. @@ -55,6 +59,11 @@ func (g GoferMountConf) ShouldUseOverlayfs() bool { return g == MemoryOverlay || g == SelfOverlay || g == AnonOverlay } +// ShouldUseLisafs returns true if a lisafs client/server should be set up. +func (g GoferMountConf) ShouldUseLisafs() bool { + return g == VanillaGofer || g.ShouldUseOverlayfs() +} + // GoferMountConfFlags can be used with GoferMountConf flags that appear // multiple times. type GoferMountConfFlags []GoferMountConf @@ -87,7 +96,7 @@ func (g *GoferMountConfFlags) Set(s string) error { if err != nil { return fmt.Errorf("invalid GoferMountConf value (%d): %v", confVal, err) } - if confVal > int(AnonOverlay) { + if confVal > int(SelfTmpfs) { return fmt.Errorf("invalid GoferMountConf value (%d)", confVal) } *g = append(*g, GoferMountConf(confVal)) diff --git a/runsc/boot/gofer_conf_test.go b/runsc/boot/gofer_conf_test.go index 269278d36..f4e00b717 100644 --- a/runsc/boot/gofer_conf_test.go +++ b/runsc/boot/gofer_conf_test.go @@ -23,22 +23,32 @@ func TestGoferConf(t *testing.T) { ovl GoferMountConf wantOverlay bool wantHostFile bool + wantLisafs bool }{{ ovl: VanillaGofer, wantOverlay: false, wantHostFile: false, + wantLisafs: true, }, { ovl: MemoryOverlay, wantOverlay: true, wantHostFile: false, + wantLisafs: true, }, { ovl: SelfOverlay, wantOverlay: true, wantHostFile: true, + wantLisafs: true, }, { ovl: AnonOverlay, wantOverlay: true, wantHostFile: true, + wantLisafs: true, + }, { + ovl: SelfTmpfs, + wantOverlay: false, + wantHostFile: true, + wantLisafs: false, }} for _, tc := range tcs { if got := tc.ovl.ShouldUseOverlayfs(); got != tc.wantOverlay { @@ -47,11 +57,14 @@ func TestGoferConf(t *testing.T) { if got := tc.ovl.IsFilestorePresent(); got != tc.wantHostFile { t.Errorf("gofer conf = %d, IsFilestorePresent() = %t, want = %t", tc.ovl, got, tc.wantHostFile) } + if got := tc.ovl.ShouldUseLisafs(); got != tc.wantLisafs { + t.Errorf("gofer conf = %d, ShouldUseLisafs() = %t, want = %t", tc.ovl, got, tc.wantLisafs) + } } } func TestGoferConfFlags(t *testing.T) { - want := GoferMountConfFlags{VanillaGofer, MemoryOverlay, SelfOverlay, AnonOverlay} + want := GoferMountConfFlags{VanillaGofer, MemoryOverlay, SelfOverlay, AnonOverlay, SelfTmpfs} var got GoferMountConfFlags got.Set(want.String()) if len(got) != len(want) { diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go index 1a353eb0b..036018e6b 100644 --- a/runsc/boot/mount_hints.go +++ b/runsc/boot/mount_hints.go @@ -141,6 +141,20 @@ func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) { } } + // Convert mount types. + for _, m := range mnts { + if m.Mount.Type == Bind && + // This mount is only accessed within the sandbox. + (m.Share == container || m.Share == pod) && + // The mount is created and deleted within the pod's lifecycle. + (m.Lifecycle == containerLife || m.Lifecycle == podLife) { + // Use a file-backed tmpfs mount for such a mount, because it is isolated + // to the sandbox and is empty on startup. + log.Infof("Converting %s hint to tmpfs", m.Name) + m.Mount.Type = tmpfs.Name + } + } + return &PodMountHints{Mounts: mnts}, nil } @@ -217,18 +231,11 @@ func (m *MountHint) setLifecycle(val string) error { // ShouldShareMount returns true if this mount should be configured as a shared // mount that is shared among multiple containers in a pod. func (m *MountHint) ShouldShareMount() bool { - // TODO(b/142076984): Only support tmpfs for now. Bind mounts require a - // common gofer to mount all shared volumes. + // 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 } -// ShouldOverlay returns true if this mount should be overlaid. -func (m *MountHint) ShouldOverlay() bool { - // TODO(b/142076984): Only support share=container for now. Once shared gofer - // support is added, we can overlay shared bind mounts too. - return m.Mount.Type == Bind && m.Share == container && m.Lifecycle != sharedLife -} - // checkCompatible verifies that shared mount is compatible with master. // Master options must be the same or less restrictive than the container mount, // e.g. master can be 'rw' while container mounts as 'ro'. @@ -248,7 +255,6 @@ func (m *MountHint) checkCompatible(replica *specs.Mount) error { return nil } -// Precondition: m.mount.Type == Bind. func (m *MountHint) fileAccessType() config.FileAccessType { if m.Share == shared { return config.FileAccessShared diff --git a/runsc/boot/mount_hints_test.go b/runsc/boot/mount_hints_test.go index bd59f756c..395c2808d 100644 --- a/runsc/boot/mount_hints_test.go +++ b/runsc/boot/mount_hints_test.go @@ -69,7 +69,7 @@ func TestPodMountHintsHappy(t *testing.T) { if want := "bar"; want != mount2.Mount.Source { t.Errorf("mount2 source, want: %q, got: %q", want, mount2.Mount.Source) } - if want := "bind"; want != mount2.Mount.Type { + if want := "tmpfs"; want != mount2.Mount.Type { t.Errorf("mount2 type, want: %q, got: %q", want, mount2.Mount.Type) } if want := container; want != mount2.Share { diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 1e56dd198..7ba5c2e8c 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -425,6 +425,9 @@ func (c *containerMounter) checkDispenser() error { if !c.goferFDs.empty() { return fmt.Errorf("not all gofer FDs were consumed, remaining: %v", c.goferFDs) } + if !c.goferFilestoreFDs.empty() { + return fmt.Errorf("not all gofer Filestore FDs were consumed, remaining: %v", c.goferFilestoreFDs) + } return nil } @@ -494,6 +497,9 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi fsName := gofer.Name rootfsConf := c.goferMountConfs[0] + if rootfsConf == SelfTmpfs { + panic("SelfTmpfs is not possible for rootfs") + } if rootfsConf.ShouldUseOverlayfs() { log.Infof("Adding overlay on top of root") var ( @@ -728,21 +734,22 @@ func (c *containerMounter) prepareMounts() ([]mountInfo, error) { var mounts []mountInfo goferMntIdx := 1 // First index is for rootfs. for i := range c.mounts { - m := &c.mounts[i] - specutils.MaybeConvertToBindMount(m) - - // Only bind mounts use host FDs; see - // containerMounter.getMountNameAndOptions. info := mountInfo{ - mount: m, - hint: c.hints.FindMount(m.Source), + mount: &c.mounts[i], + hint: c.hints.FindMount(c.mounts[i].Source), } - if specutils.IsGoferMount(*m) { - info.goferFD = c.goferFDs.removeAsFD() + specutils.MaybeConvertToBindMount(info.mount) + if specutils.IsGoferMount(*info.mount) { info.goferMountConf = c.goferMountConfs[goferMntIdx] + if info.goferMountConf.ShouldUseLisafs() { + info.goferFD = c.goferFDs.removeAsFD() + } if info.goferMountConf.IsFilestorePresent() { info.filestoreFD = c.goferFilestoreFDs.removeAsFD() } + if info.goferMountConf == SelfTmpfs { + specutils.ChangeMountType(info.mount, tmpfs.Name) + } goferMntIdx++ } mounts = append(mounts, info) @@ -829,6 +836,14 @@ func getMountNameAndOptions(conf *config.Config, m *mountInfo, productName strin if err != nil { return "", nil, err } + if m.filestoreFD != nil { + internalData = tmpfs.FilesystemOpts{ + FilestoreFD: m.filestoreFD, + // If a mount is being overlaid with tmpfs, it should not be limited by + // the default tmpfs size limit. + DisableDefaultSizeLimit: true, + } + } case Bind: fsName = gofer.Name @@ -972,6 +987,12 @@ func (c *containerMounter) getSharedMount(ctx context.Context, conf *config.Conf sharedMount, ok := c.sharedMounts[mount.hint.Mount.Source] if ok { log.Infof("Using existing shared mount %q from %q type %q", mount.hint.Name, mount.hint.Mount.Source, mount.hint.Mount.Type) + if mount.goferFD != nil { + panic(fmt.Errorf("extra goferFD provided for shared mount %q", mount.hint.Name)) + } + if mount.filestoreFD != nil { + mount.filestoreFD.Close() + } return sharedMount, nil } log.Infof("Mounting master of shared mount %q from %q type %q", mount.hint.Name, mount.hint.Mount.Source, mount.hint.Mount.Type) diff --git a/runsc/boot/vfs_test.go b/runsc/boot/vfs_test.go index a53ac10da..3322d3691 100644 --- a/runsc/boot/vfs_test.go +++ b/runsc/boot/vfs_test.go @@ -54,7 +54,7 @@ func TestGetMountAccessType(t *testing.T) { MountPrefix + "mount1.share": "pod", MountPrefix + "mount1.lifecycle": "pod", }, - want: config.FileAccessShared, + want: config.FileAccessExclusive, }, { name: "shared=shared", diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index bdf497079..aed4bc4f7 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -199,7 +199,7 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm // // Note that all mount points have been mounted in the proper location in // setupRootFS(). - cleanMounts, err := resolveMounts(conf, spec.Mounts, root) + cleanMounts, err := g.resolveMounts(conf, spec.Mounts, root) if err != nil { util.Fatalf("Failure to resolve mounts: %v", err) } @@ -276,32 +276,37 @@ func (g *Gofer) serve(spec *specs.Spec, conf *config.Config, root string) subcom log.Infof("Serving %q mapped to %q on FD %d (ro: %t)", "/", root, g.ioFDs[0], cfgs[0].readonly) mountIdx := 1 // first one is the root + submountIoFDs := g.ioFDs[1:] for _, m := range spec.Mounts { if !specutils.IsGoferMount(m) { continue } - + mountConf := g.mountConfs[mountIdx] + mountIdx++ + if !mountConf.ShouldUseLisafs() { + continue + } if !filepath.IsAbs(m.Destination) { util.Fatalf("mount destination must be absolute: %q", m.Destination) } - if mountIdx >= len(g.ioFDs) { + + if len(submountIoFDs) == 0 { util.Fatalf("no FD found for mount. Did you forget --io-fd? FDs: %d, Mount: %+v", len(g.ioFDs), m) } - + ioFD := submountIoFDs[0] + submountIoFDs = submountIoFDs[1:] + readonly := specutils.IsReadonlyMount(m.Options) || mountConf.ShouldUseOverlayfs() cfgs = append(cfgs, connectionConfig{ - sock: newSocket(g.ioFDs[mountIdx]), + sock: newSocket(ioFD), mountPath: m.Destination, - readonly: specutils.IsReadonlyMount(m.Options) || g.mountConfs[mountIdx].ShouldUseOverlayfs(), + readonly: readonly, }) - - log.Infof("Serving %q mapped on FD %d (ro: %t)", m.Destination, g.ioFDs[mountIdx], cfgs[mountIdx].readonly) - mountIdx++ + log.Infof("Serving %q mapped on FD %d (ro: %t)", m.Destination, ioFD, readonly) } - if mountIdx != len(g.ioFDs) { - util.Fatalf("too many FDs passed for mounts. mounts: %d, FDs: %d", mountIdx, len(g.ioFDs)) + if len(submountIoFDs) > 0 { + util.Fatalf("too many FDs passed for mounts. mounts: %d, FDs: %d", len(cfgs), len(g.ioFDs)) } - cfgs = cfgs[:mountIdx] for _, cfg := range cfgs { conn, err := server.CreateConnection(cfg.sock, cfg.mountPath, cfg.readonly) @@ -447,11 +452,16 @@ func (g *Gofer) setupRootFS(spec *specs.Spec, conf *config.Config) error { // location inside root. It will resolve relative paths and symlinks. It also // creates directories as needed. func (g *Gofer) setupMounts(conf *config.Config, mounts []specs.Mount, root, procPath string) error { - goferMntIdx := 1 // First index is for rootfs. + mountIdx := 1 // First index is for rootfs. for _, m := range mounts { if !specutils.IsGoferMount(m) { continue } + mountConf := g.mountConfs[mountIdx] + mountIdx++ + if !mountConf.ShouldUseLisafs() { + continue + } dst, err := resolveSymlinks(root, m.Destination) if err != nil { @@ -459,7 +469,7 @@ func (g *Gofer) setupMounts(conf *config.Config, mounts []specs.Mount, root, pro } flags := specutils.OptionsToFlags(m.Options) | unix.MS_BIND - if g.mountConfs[goferMntIdx].ShouldUseOverlayfs() { + if mountConf.ShouldUseOverlayfs() { // Force mount read-only if writes are not going to be sent to it. flags |= unix.MS_RDONLY } @@ -476,7 +486,6 @@ func (g *Gofer) setupMounts(conf *config.Config, mounts []specs.Mount, root, pro return fmt.Errorf("mount dst: %q, flags: %#x, err: %v", dst, flags, err) } } - goferMntIdx++ } return nil } @@ -487,13 +496,20 @@ func (g *Gofer) setupMounts(conf *config.Config, mounts []specs.Mount, root, pro // Otherwise, it may follow symlinks to locations that would be overwritten // with another mount point and return the wrong location. In short, make sure // setupMounts() has been called before. -func resolveMounts(conf *config.Config, mounts []specs.Mount, root string) ([]specs.Mount, error) { +func (g *Gofer) resolveMounts(conf *config.Config, mounts []specs.Mount, root string) ([]specs.Mount, error) { + mountIdx := 1 // First index is for rootfs. cleanMounts := make([]specs.Mount, 0, len(mounts)) for _, m := range mounts { if !specutils.IsGoferMount(m) { cleanMounts = append(cleanMounts, m) continue } + mountConf := g.mountConfs[mountIdx] + mountIdx++ + if !mountConf.ShouldUseLisafs() { + cleanMounts = append(cleanMounts, m) + continue + } dst, err := resolveSymlinks(root, m.Destination) if err != nil { return nil, fmt.Errorf("resolving symlinks to %q: %v", m.Destination, err) diff --git a/runsc/container/container.go b/runsc/container/container.go index 8819e69da..bee7bbd47 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -949,9 +949,15 @@ func (c *Container) createGoferFilestores(ovlConf config.Overlay2, mountHints *b } func (c *Container) createGoferFilestore(ovlConf config.Overlay2, mountSrc string, shouldOverlay bool, hint *boot.MountHint) (*os.File, boot.GoferMountConf, error) { - if hint != nil && hint.ShouldOverlay() { - // MountHint information takes precedence over shouldOverlay. - return c.createOverlayFilestoreInSelf(mountSrc) + // MountHint information takes precedence over shouldOverlay. + if hint != nil && !specutils.IsGoferMount(hint.Mount) { + switch hint.Mount.Type { + case "tmpfs": + // Create self-backed tmpfs. + return c.createGoferFilestoreInSelf(mountSrc, hint, boot.SelfTmpfs) + default: + return nil, boot.VanillaGofer, fmt.Errorf("unsupported mount type %q in mount hint", hint.Mount.Type) + } } switch { case !shouldOverlay: @@ -959,13 +965,13 @@ func (c *Container) createGoferFilestore(ovlConf config.Overlay2, mountSrc strin case ovlConf.IsBackedByMemory(): return nil, boot.MemoryOverlay, nil case ovlConf.IsBackedBySelf(): - return c.createOverlayFilestoreInSelf(mountSrc) + return c.createGoferFilestoreInSelf(mountSrc, hint, boot.SelfOverlay) default: return c.createGoferFilestoreInDir(ovlConf) } } -func (c *Container) createOverlayFilestoreInSelf(mountSrc string) (*os.File, boot.GoferMountConf, error) { +func (c *Container) createGoferFilestoreInSelf(mountSrc string, hint *boot.MountHint, successConf boot.GoferMountConf) (*os.File, boot.GoferMountConf, error) { mountSrcInfo, err := os.Stat(mountSrc) if err != nil { return nil, boot.VanillaGofer, fmt.Errorf("failed to stat mount %q to see if it were a directory: %v", mountSrc, err) @@ -975,17 +981,21 @@ func (c *Container) createOverlayFilestoreInSelf(mountSrc string) (*os.File, boo return nil, boot.MemoryOverlay, nil } // Create the self filestore file. + createFlags := unix.O_RDWR | unix.O_CREAT | unix.O_CLOEXEC + if !(hint != nil && hint.ShouldShareMount()) { + // Allow shared mounts to reuse existing filestore. A previous shared user + // may have already set up the filestore. + createFlags |= unix.O_EXCL + } filestorePath := boot.SelfFilestorePath(mountSrc, c.sandboxID()) - filestoreFD, err := unix.Open(filestorePath, unix.O_RDWR|unix.O_CREAT|unix.O_EXCL|unix.O_CLOEXEC, 0666) + filestoreFD, err := unix.Open(filestorePath, createFlags, 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 nil, boot.VanillaGofer, fmt.Errorf("%q mount source already has a filestore file at %q; repeated submounts are not suppported with self medium", mountSrc, filestorePath) + // same sandbox, and is not shared, then the overlay option doesn't work + // correctly. Because each overlay mount is independent and changes to + // one are not visible to the other. + return nil, boot.VanillaGofer, fmt.Errorf("%q mount source already has a filestore file at %q; repeated submounts are not supported with overlay optimizations", mountSrc, filestorePath) } return nil, boot.VanillaGofer, fmt.Errorf("failed to create filestore file inside %q: %v", mountSrc, err) } @@ -995,7 +1005,7 @@ func (c *Container) createOverlayFilestoreInSelf(mountSrc string) (*os.File, boo // and apply any limits appropriately (like local ephemeral storage // limits). So don't delete it. These files will be unlinked when the // container is destroyed. This makes self medium appropriate for k8s. - return os.NewFile(uintptr(filestoreFD), filestorePath), boot.SelfOverlay, nil + return os.NewFile(uintptr(filestoreFD), filestorePath), successConf, nil } func (c *Container) createGoferFilestoreInDir(ovlConf config.Overlay2) (*os.File, boot.GoferMountConf, error) { @@ -1177,16 +1187,16 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *config.Config, bu } donations.DonateAndClose("mounts-fd", mountsGofer) - // Add root mount and then add any other additional mounts. - mountCount := 1 - for _, m := range spec.Mounts { - if specutils.IsGoferMount(m) { - mountCount++ + // Count the number of mounts using lisafs. + lisafsCount := 0 + for _, cfg := range c.GoferMountConfs { + if cfg.ShouldUseLisafs() { + lisafsCount++ } } - sandEnds := make([]*os.File, 0, mountCount) - for i := 0; i < mountCount; i++ { + sandEnds := make([]*os.File, 0, lisafsCount) + for i := 0; i < lisafsCount; i++ { fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM|unix.SOCK_CLOEXEC, 0) if err != nil { return nil, nil, err diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index ffcae7804..4aa46d8b2 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -45,7 +45,6 @@ import ( "gvisor.dev/gvisor/pkg/state/statefile" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/test/testutil" - "gvisor.dev/gvisor/runsc/boot" "gvisor.dev/gvisor/runsc/cgroup" "gvisor.dev/gvisor/runsc/config" "gvisor.dev/gvisor/runsc/flag" @@ -3039,105 +3038,6 @@ func TestExecFDExec(t *testing.T) { } } -// This test checks that a bind mount which is annotated to be fully owned by -// the sandbox is overlaid using "self" overlay medium. -func TestOverlayByMountAnnotation(t *testing.T) { - conf := testutil.TestConfig(t) - // Disable overlay settings. - conf.Overlay2.Set("none") - - // We just sleep here because we want to test execution in an already - // running container. - spec := testutil.NewSpecWithArgs("bash", "-c", "sleep infinity") - - // Set up a bind mount at "/submount". - subMount, err := ioutil.TempDir(testutil.TmpDir(), "submount") - if err != nil { - t.Fatalf("ioutil.TempDir failed: %v", err) - } - defer os.RemoveAll(subMount) - spec.Mounts = append(spec.Mounts, specs.Mount{ - Destination: subMount, - Source: subMount, - Type: "bind", - }) - - // Add mount annotation to self-overlay the submount. - volumeName := "mount1" - if spec.Annotations == nil { - spec.Annotations = make(map[string]string) - } - spec.Annotations[boot.MountPrefix+volumeName+".source"] = subMount - spec.Annotations[boot.MountPrefix+volumeName+".type"] = "bind" - spec.Annotations[boot.MountPrefix+volumeName+".share"] = "container" - spec.Annotations[boot.MountPrefix+volumeName+".lifecycle"] = "pod" - - _, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf) - if err != nil { - t.Fatalf("error setting up container: %v", err) - } - defer cleanup() - - args := Args{ - ID: testutil.RandomContainerID(), - Spec: spec, - BundleDir: bundleDir, - } - - cont, err := New(conf, args) - if err != nil { - t.Fatalf("Creating container: %v", err) - } - destroyed := false - destroy := func() { - if destroyed { - return - } - destroyed = true - cont.Destroy() - } - defer destroy() - - if err := cont.Start(conf); err != nil { - t.Fatalf("starting container: %v", err) - } - - // Create a file in submount with a few bytes. - testFilePath := path.Join(subMount, "testfile") - if ws, err := execute(conf, cont, "/bin/sh", "-c", "echo hello > "+testFilePath); err != nil || ws != 0 { - t.Fatalf("exec command failed to write a file in submount, ws: %v, err: %v", ws, err) - } - - // Check that the filestore file is created and is not empty. - filestoreFile := boot.SelfFilestorePath(subMount, cont.Sandbox.ID) - var stat unix.Stat_t - if err := unix.Stat(filestoreFile, &stat); err != nil { - t.Fatalf("unix.Stat(%q) failed for submount filestore: %v", filestoreFile, err) - } - if stat.Blocks == 0 { - t.Errorf("submount filestore file %q is empty", filestoreFile) - } - - // Check that the file is not created on the host. - if err := unix.Stat(path.Join(subMount, testFilePath), &stat); err == nil { - t.Errorf("%q file created on the host in spite of overlay", testFilePath) - } - - got, err := executeCombinedOutput(conf, cont, nil, "/bin/sh", "-c", "mount | grep /submount") - if err != nil { - t.Fatalf("failed to grep mount(1) from container: %v", err) - } - if !strings.Contains(string(got), "type overlay (rw)") { - t.Errorf("expected /submount to be an overlay mount, but it is not. mount(1) reports its type as:\n%s", string(got)) - } - - // Destroying the container should delete the filestore file. - destroy() - if err := unix.Stat(filestoreFile, &stat); err == nil { - t.Fatalf("overlay filestore at %q was not deleted after container.Destroy()", filestoreFile) - } -} - // TestMountEROFS checks that the checksums from the target directory in container // are identical with the ones from the source directory on host. func TestMountEROFS(t *testing.T) { diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go index cfaf79081..961c8c5b8 100644 --- a/runsc/container/multi_container_test.go +++ b/runsc/container/multi_container_test.go @@ -1339,9 +1339,6 @@ func TestMultiContainerContainerDestroyStress(t *testing.T) { // changes from one container is reflected in the other. func TestMultiContainerSharedMount(t *testing.T) { testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } // Setup the containers. sleep := []string{"sleep", "100"} podSpec, ids := createSpecs(sleep, sleep) @@ -1446,9 +1443,6 @@ func TestMultiContainerSharedMount(t *testing.T) { // Test that pod mounts are mounted as readonly when requested. func TestMultiContainerSharedMountReadonly(t *testing.T) { testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } // Setup the containers. sleep := []string{"sleep", "100"} podSpec, ids := createSpecs(sleep, sleep) @@ -1506,9 +1500,6 @@ func TestMultiContainerSharedMountReadonly(t *testing.T) { // container mounts. func TestMultiContainerSharedMountCompatible(t *testing.T) { testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } sleep := []string{"sleep", "100"} podSpec, ids := createSpecs(sleep, sleep) @@ -1571,9 +1562,6 @@ 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) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } // Setup the containers. sleep := []string{"sleep", "100"} podSpec, ids := createSpecs(sleep, sleep) @@ -1674,9 +1662,6 @@ func TestMultiContainerSharedMountRestart(t *testing.T) { // replica mounts. func TestMultiContainerSharedMountUnsupportedOptions(t *testing.T) { testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } // Setup the containers. sleep := []string{"/bin/sleep", "100"} podSpec, ids := createSpecs(sleep, sleep) @@ -1717,6 +1702,100 @@ func TestMultiContainerSharedMountUnsupportedOptions(t *testing.T) { }) } +// This test checks that a bind mount that is "shared" is overlaid correctly +// with a self-backed tmpfs. +func TestMultiContainerSharedBindMount(t *testing.T) { + for numContainers := 1; numContainers <= 2; numContainers++ { + testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { + if mntType != "bind" { + t.Skipf("This test is only for shared bind mounts, skipping %q mount type", mntType) + } + t.Run(fmt.Sprintf("containers-%d", numContainers), func(t *testing.T) { + // Setup the containers. + sleep := []string{"sleep", "100"} + var cmds [][]string + for i := 0; i < numContainers; i++ { + cmds = append(cmds, sleep) + } + podSpec, ids := createSpecs(cmds...) + + sharedMount := specs.Mount{ + Destination: "/mydir/test", + Source: sourceDir, + Type: mntType, + } + for _, spec := range podSpec { + spec.Mounts = append(spec.Mounts, sharedMount) + } + + createSharedMount(sharedMount, "test-mount", podSpec...) + + containers, cleanup, err := startContainers(conf, podSpec, ids) + if err != nil { + t.Fatalf("error starting containers: %v", err) + } + destroyed := false + destroy := func() { + if destroyed { + return + } + destroyed = true + cleanup() + } + defer destroy() + + // Create a file in shared mount with a few bytes in each container. + var execs []execDesc + for i, c := range containers { + testFileName := fmt.Sprintf("testfile-%d", i) + testFilePath := path.Join(sharedMount.Destination, testFileName) + execs = append(execs, execDesc{ + c: c, + cmd: []string{"/bin/sh", "-c", "echo hello > " + testFilePath}, + name: fmt.Sprintf("file created in container %d", i), + }) + } + execMany(t, conf, execs) + + // Check that the file is not created on the host. + for i := 0; i < numContainers; i++ { + testFileName := fmt.Sprintf("testfile-%d", i) + if _, err := os.Stat(path.Join(sourceDir, testFileName)); err == nil { + t.Errorf("%q file created on the host in spite of tmpfs", testFileName) + } + } + + // Check that the filestore file is created and is not empty. + filestoreFile := boot.SelfFilestorePath(sourceDir, containers[0].sandboxID()) + var stat unix.Stat_t + if err := unix.Stat(filestoreFile, &stat); err != nil { + t.Fatalf("unix.Stat(%q) failed for submount filestore: %v", filestoreFile, err) + } + if stat.Blocks == 0 { + t.Errorf("submount filestore file %q is empty", filestoreFile) + } + + // Ensure the shared mount is tmpfs. + for i, c := range containers { + got, err := executeCombinedOutput(conf, c, nil, "/bin/sh", "-c", "mount | grep "+sharedMount.Destination) + if err != nil { + t.Fatalf("failed to grep mount(1) from container %d: %v", i, err) + } + if !strings.Contains(string(got), "type tmpfs (rw)") { + t.Errorf("expected %s to be a tmpfs mount in container %d. mount(1) reports its type as:\n%s", sharedMount.Destination, i, string(got)) + } + } + + // Destroying the containers should delete the filestore file. + destroy() + if err := unix.Stat(filestoreFile, &stat); err == nil { + t.Fatalf("overlay filestore at %q was not deleted after container.Destroy()", filestoreFile) + } + }) + }) + } +} + // Test that one container can send an FD to another container, even though // they have distinct MountNamespaces. func TestMultiContainerMultiRootCanHandleFDs(t *testing.T) { @@ -1726,9 +1805,6 @@ func TestMultiContainerMultiRootCanHandleFDs(t *testing.T) { } testSharedMount(t, func(t *testing.T, conf *config.Config, sourceDir string, mntType string) { - if mntType != "tmpfs" { - t.Skipf("Only tmpfs shared mounts support for now, got %q", mntType) - } // We set up two containers with one shared mount that is used for a // shared socket. The first container will send an FD over the socket // to the second container. The FD corresponds to a file in the first diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index ad0de31e2..7f2bd22c3 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -287,6 +287,27 @@ func ReadMounts(f *os.File) ([]specs.Mount, error) { return mounts, nil } +// ChangeMountType changes m.Type to the specified type. It may do necessary +// amends to m.Options. +func ChangeMountType(m *specs.Mount, newType string) { + m.Type = newType + + // OCI spec allows bind mounts to be specified in options only. So if new type + // is not bind, remove bind/rbind from options. + // + // "For bind mounts (when options include either bind or rbind), the type is + // a dummy, often "none" (not listed in /proc/filesystems)." + if newType != "bind" { + newOpts := make([]string, 0, len(m.Options)) + for _, opt := range m.Options { + if opt != "rbind" && opt != "bind" { + newOpts = append(newOpts, opt) + } + } + m.Options = newOpts + } +} + // Capabilities takes in spec and returns a TaskCapabilities corresponding to // the spec. func Capabilities(enableRaw bool, specCaps *specs.LinuxCapabilities) (*auth.TaskCapabilities, error) {