diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go index e07ff2627..1a353eb0b 100644 --- a/runsc/boot/mount_hints.go +++ b/runsc/boot/mount_hints.go @@ -214,9 +214,9 @@ func (m *MountHint) setLifecycle(val string) error { return nil } -// shouldShareMount returns true if this mount should be configured as a shared +// 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 { +func (m *MountHint) ShouldShareMount() bool { // TODO(b/142076984): 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 @@ -253,7 +253,7 @@ func (m *MountHint) fileAccessType() config.FileAccessType { if m.Share == shared { return config.FileAccessShared } - if m.shouldShareMount() { + if m.ShouldShareMount() { return config.FileAccessExclusive } if m.Share == container { @@ -263,9 +263,9 @@ func (m *MountHint) fileAccessType() config.FileAccessType { } // FindMount finds the MountHint that applies to this mount. -func (p *PodMountHints) FindMount(mount *specs.Mount) *MountHint { +func (p *PodMountHints) FindMount(mountSrc string) *MountHint { for _, m := range p.Mounts { - if m.Mount.Source == mount.Source { + if m.Mount.Source == mountSrc { return m } } diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index a8ede4cf5..9a2ebfcd0 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -429,7 +429,7 @@ func (c *containerMounter) checkDispenser() error { return nil } -func getMountAccessType(conf *config.Config, mount *specs.Mount, hint *MountHint) config.FileAccessType { +func getMountAccessType(conf *config.Config, hint *MountHint) config.FileAccessType { if hint != nil { return hint.fileAccessType() } @@ -677,7 +677,7 @@ func (c *containerMounter) mountSubmounts(ctx context.Context, conf *config.Conf err error ) - if submount.hint != nil && submount.hint.shouldShareMount() { + if submount.hint != nil && submount.hint.ShouldShareMount() { sharedMount, ok := c.sharedMounts[submount.hint.Mount.Source] if !ok { return fmt.Errorf("shared mount %q not found", submount.hint.Name) @@ -740,7 +740,7 @@ func (c *containerMounter) prepareMounts() ([]mountInfo, error) { info := mountInfo{ mount: m, fd: -1, - hint: c.hints.FindMount(m), + hint: c.hints.FindMount(m.Source), overlayMedium: NoOverlay, } if specutils.IsGoferMount(*m) { @@ -842,7 +842,7 @@ func getMountNameAndOptions(conf *config.Config, m *mountInfo, productName strin // Check that an FD was provided to fails fast. return "", nil, fmt.Errorf("gofer mount requires a connection FD") } - data = goferMountData(m.fd, getMountAccessType(conf, m.mount, m.hint), conf) + data = goferMountData(m.fd, getMountAccessType(conf, m.hint), conf) internalData = gofer.InternalFilesystemOptions{ UniqueID: m.mount.Destination, } @@ -983,7 +983,7 @@ func (l *Loader) processHints(conf *config.Config, creds *auth.Credentials) erro ctx := l.k.SupervisorContext() var sharedMounts map[string]*vfs.Mount for _, hint := range l.mountHints.Mounts { - if !hint.shouldShareMount() { + if !hint.ShouldShareMount() { continue } diff --git a/runsc/boot/vfs_test.go b/runsc/boot/vfs_test.go index 2d568f984..a53ac10da 100644 --- a/runsc/boot/vfs_test.go +++ b/runsc/boot/vfs_test.go @@ -91,8 +91,7 @@ func TestGetMountAccessType(t *testing.T) { t.Fatalf("newPodMountHints failed: %v", err) } conf := &config.Config{FileAccessMounts: config.FileAccessShared} - mnt := &specs.Mount{Source: source} - if got := getMountAccessType(conf, mnt, podHints.FindMount(mnt)); got != tst.want { + if got := getMountAccessType(conf, podHints.FindMount(source)); got != tst.want { t.Errorf("getMountAccessType(), got: %v, want: %v", got, tst.want) } }) diff --git a/runsc/container/container.go b/runsc/container/container.go index 70e945ff1..e50381e43 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -816,6 +816,14 @@ func (c *Container) Destroy() error { // Clean up overlay filestore files created in their respective mounts. c.forEachSelfOverlay(func(mountSrc string) { + if sb != nil { + 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. + return + } + } 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) @@ -823,6 +831,23 @@ func (c *Container) Destroy() error { errs = append(errs, err.Error()) } }) + if sb != nil && sb.IsRootContainer(c.ID) { + // When the root container is being destroyed, we can clean up filestores + // used by shared mounts. + for _, hint := range sb.MountHints.Mounts { + if !hint.ShouldShareMount() { + continue + } + // Assume this is a self-backed shared mount and try to delete the + // filestore. Subsequently ignore the ENOENT if the assumption is wrong. + filestorePath := boot.SelfOverlayFilestorePath(hint.Mount.Source, c.sandboxID()) + if err := os.Remove(filestorePath); err != nil && !os.IsNotExist(err) { + err = fmt.Errorf("failed to delete shared filestore file %q: %v", filestorePath, err) + log.Warningf("%v", err) + errs = append(errs, err.Error()) + } + } + } c.changeStatus(Stopped) @@ -863,7 +888,7 @@ func (c *Container) sandboxID() string { func (c *Container) forEachSelfOverlay(fn func(mountSrc string)) { if c.OverlayMediums == nil { - // Sub container not started? Skip. + // Container not started? Skip. return } if c.OverlayMediums[0] == boot.SelfMedium { @@ -904,7 +929,7 @@ func (c *Container) createOverlayFilestores(conf config.Overlay2, mountHints *bo if !specutils.IsGoferMount(c.Spec.Mounts[i]) { continue } - hint := mountHints.FindMount(&c.Spec.Mounts[i]) + hint := mountHints.FindMount(c.Spec.Mounts[i].Source) shouldOverlay := conf.SubMountEnabled() && !specutils.IsReadonlyMount(c.Spec.Mounts[i].Options) filestore, medium, err := c.createOverlayFilestore(conf, c.Spec.Mounts[i].Source, shouldOverlay, hint) if err != nil {