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
This commit is contained in:
Ayush Ranjan
2023-10-13 16:38:59 -07:00
committed by gVisor bot
parent 7949478688
commit 1a5aee5539
4 changed files with 38 additions and 14 deletions
+5 -5
View File
@@ -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
}
}
+5 -5
View File
@@ -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
}
+1 -2
View File
@@ -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)
}
})
+27 -2
View File
@@ -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 {