From 1a5aee55393806862ba2bc775a1c469e38f01b0e Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Fri, 13 Oct 2023 16:36:12 -0700 Subject: [PATCH] Handle deletion of self-backed filestores for shared mounts. Currently shared mounts do not have a file backend, but future changes will introduce it. Shared mounts are owned by the sandbox process and they persist for the entire lifecycle of the sandbox. When containers using them are start, they simply reuse the shared mount's filesystem instance. During the lifecycle of the sandbox, containers using a shared mount can be started/restarted/destroyed multiple times. So we should only delete the shared mount's filestore when the sandbox is being destroyed. PiperOrigin-RevId: 573347590 --- runsc/boot/mount_hints.go | 10 +++++----- runsc/boot/vfs.go | 10 +++++----- runsc/boot/vfs_test.go | 3 +-- runsc/container/container.go | 29 +++++++++++++++++++++++++++-- 4 files changed, 38 insertions(+), 14 deletions(-) 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 {