Create shared mounts on demand.

Earlier we were creating them upfront when the root container started.
However, later some of these shared mounts might require additional
configuration that is more easily available when the sub-container using that
shared mount is being started.

So move to an on-demand model. This also has the benefit that mounts will not
be created until needed (hence resource utilization will be more optimal).

Consequently, failures in setting up shared mounts will not cause the sandbox
from failing to start up. It will instead cause the impacted containers from
failing to start up.

PiperOrigin-RevId: 574533960

runsc/boot/loader.go

@@ -159,10 +159,6 @@ type Loader struct {
 	// apply to the entire pod.
 	mountHints *PodMountHints
 
-	// sharedMountKey holds VFS mounts that may be shared between containers
-	// within the same pod. It is mapped by mount source.
-	sharedMounts map[string]*vfs.Mount
-
 	// productName is the value to show in
 	// /sys/devices/virtual/dmi/id/product_name.
 	productName string
@@ -170,9 +166,13 @@ type Loader struct {
 	// nvidiaUVMDevMajor is the device major number used for nvidia-uvm.
 	nvidiaUVMDevMajor uint32
 
-	// mu guards processes and porForwardProxies.
+	// mu guards the fields below.
 	mu sync.Mutex
 
+	// sharedMounts holds VFS mounts that may be shared between containers within
+	// the same pod. It is mapped by mount source.
+	sharedMounts map[string]*vfs.Mount
+
 	// processes maps containers init process and invocation of exec. Root
 	// processes are keyed with container ID and pid=0, while exec invocations
 	// have the corresponding pid set.
@@ -508,6 +508,7 @@ func New(args Args) (*Loader, error) {
 		sandboxID:         args.ID,
 		processes:         map[execID]*execProcess{eid: {}},
 		mountHints:        mountHints,
+		sharedMounts:      make(map[string]*vfs.Mount),
 		root:              info,
 		stopProfiling:     stopProfiling,
 		productName:       args.ProductName,
@@ -720,7 +721,7 @@ func (l *Loader) run() error {
 			tg  *kernel.ThreadGroup
 			err error
 		)
-		tg, ep.tty, err = l.createContainerProcess(true, l.sandboxID, &l.root)
+		tg, ep.tty, err = l.createContainerProcess(l.sandboxID, &l.root)
 		if err != nil {
 			return err
 		}
@@ -873,7 +874,7 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st
 		info.stdioFDs = stdioFDs
 	}
 
-	ep.tg, ep.tty, err = l.createContainerProcess(false, cid, info)
+	ep.tg, ep.tty, err = l.createContainerProcess(cid, info)
 	if err != nil {
 		return err
 	}
@@ -902,7 +903,8 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st
 	return nil
 }
 
-func (l *Loader) createContainerProcess(root bool, cid string, info *containerInfo) (*kernel.ThreadGroup, *host.TTYFileDescription, error) {
+// +checklocks:l.mu
+func (l *Loader) createContainerProcess(cid string, info *containerInfo) (*kernel.ThreadGroup, *host.TTYFileDescription, error) {
 	// Create the FD map, which will set stdin, stdout, and stderr.
 	ctx := info.procArgs.NewContext(l.k)
 	fdTable, ttyFile, err := createFDTable(ctx, info.spec.Process.Terminal, info.stdioFDs, info.passFDs, info.spec.Process.User)
@@ -939,11 +941,8 @@ func (l *Loader) createContainerProcess(root bool, cid string, info *containerIn
 	}
 	l.startGoferMonitor(cid, int32(info.goferFDs[0].FD()))
 
-	if root {
-		if err := l.processHints(info.conf, info.procArgs.Credentials); err != nil {
-			return nil, nil, err
-		}
-	}
+	// We can share l.sharedMounts with containerMounter since l.mu is locked.
+	// Hence, mntr must only be used within this function (while l.mu is locked).
 	mntr := newContainerMounter(info, l.k, l.mountHints, l.sharedMounts, l.productName, l.sandboxID)
 	if err := setupContainerVFS(ctx, info, mntr, &info.procArgs); err != nil {
 		return nil, nil, err

runsc/boot/loader_test.go

@@ -475,11 +475,7 @@ func TestCreateMountNamespace(t *testing.T) {
 			defer l.Destroy()
 			defer loaderCleanup()
 
-			if err := l.processHints(l.root.conf, l.root.procArgs.Credentials); err != nil {
-				t.Fatalf("failed process hints: %v", err)
-			}
 			mntr := newContainerMounter(&l.root, l.k, l.mountHints, l.sharedMounts, "", l.sandboxID)
-
 			ctx := l.k.SupervisorContext()
 			creds := auth.NewRootCredentials(l.root.procArgs.Credentials.UserNamespace)
 			mns, err := mntr.mountAll(ctx, creds, l.root.conf, &l.root.procArgs)

runsc/boot/vfs.go

@@ -678,11 +678,11 @@ func (c *containerMounter) mountSubmounts(ctx context.Context, conf *config.Conf
 		)
 
 		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)
+			sharedMount, err := c.getSharedMount(ctx, conf, submount, creds)
+			if err != nil {
+				return fmt.Errorf("getting shared mount %q: %w", submount.hint.Name, err)
 			}
-			mnt, err = c.mountSharedSubmount(ctx, conf, mns, creds, submount.mount, submount.hint, sharedMount)
+			mnt, err = c.mountSharedSubmount(ctx, conf, mns, creds, submount, sharedMount)
 			if err != nil {
 				return fmt.Errorf("mount shared mount %q to %q: %v", submount.hint.Name, submount.mount.Destination, err)
 			}
@@ -715,16 +715,12 @@ func (c *containerMounter) mountSubmounts(ctx context.Context, conf *config.Conf
 
 type mountInfo struct {
 	mount          *specs.Mount
-	goferFD        int
+	goferFD        *fd.FD
 	hint           *MountHint
 	goferMountConf GoferMountConf
 	filestoreFD    *fd.FD
 }
 
-func newNonGoferMountInfo(mount *specs.Mount) *mountInfo {
-	return &mountInfo{mount: mount, goferFD: -1}
-}
-
 func (c *containerMounter) prepareMounts() ([]mountInfo, error) {
 	// Associate bind mounts with their FDs before sorting since there is an
 	// undocumented assumption that FDs are dispensed in the order in which
@@ -738,12 +734,11 @@ func (c *containerMounter) prepareMounts() ([]mountInfo, error) {
 		// Only bind mounts use host FDs; see
 		// containerMounter.getMountNameAndOptions.
 		info := mountInfo{
-			mount:   m,
-			goferFD: -1,
-			hint:    c.hints.FindMount(m.Source),
+			mount: m,
+			hint:  c.hints.FindMount(m.Source),
 		}
 		if specutils.IsGoferMount(*m) {
-			info.goferFD = c.goferFDs.remove()
+			info.goferFD = c.goferFDs.removeAsFD()
 			info.goferMountConf = c.goferMountConfs[goferMntIdx]
 			if info.goferMountConf.IsFilestorePresent() {
 				info.filestoreFD = c.goferFilestoreFDs.removeAsFD()
@@ -837,11 +832,11 @@ func getMountNameAndOptions(conf *config.Config, m *mountInfo, productName strin
 
 	case Bind:
 		fsName = gofer.Name
-		if m.goferFD < 0 {
+		if m.goferFD == nil {
 			// Check that an FD was provided to fails fast.
 			return "", nil, fmt.Errorf("gofer mount requires a connection FD")
 		}
-		data = goferMountData(m.goferFD, getMountAccessType(conf, m.hint), conf)
+		data = goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf)
 		internalData = gofer.InternalFilesystemOptions{
 			UniqueID: m.mount.Destination,
 		}
@@ -959,7 +954,7 @@ func (c *containerMounter) mountTmp(ctx context.Context, conf *config.Config, cr
 			// another user. This is normally done for /tmp.
 			Options: []string{"mode=01777"},
 		}
-		if _, err := c.mountSubmount(ctx, conf, mns, creds, newNonGoferMountInfo(&tmpMount)); err != nil {
+		if _, err := c.mountSubmount(ctx, conf, mns, creds, &mountInfo{mount: &tmpMount}); err != nil {
 			return fmt.Errorf("mountSubmount failed: %v", err)
 		}
 		return nil
@@ -973,63 +968,48 @@ func (c *containerMounter) mountTmp(ctx context.Context, conf *config.Config, cr
 	}
 }
 
-// processHints processes annotations that container hints about how volumes
-// should be mounted (e.g. a volume shared between containers).
-// Precondition: Must be only called once during the loader sequence
-// for the root container.
-// Postcondition: Initialized l.sharedMounts on success.
-func (l *Loader) processHints(conf *config.Config, creds *auth.Credentials) error {
-	ctx := l.k.SupervisorContext()
-	var sharedMounts map[string]*vfs.Mount
-	for _, hint := range l.mountHints.Mounts {
-		if !hint.ShouldShareMount() {
-			continue
-		}
-
-		log.Infof("Mounting master of shared mount %q from %q type %q", hint.Name, hint.Mount.Source, hint.Mount.Type)
-		mnt, err := l.mountSharedMaster(ctx, conf, hint, creds)
-		if err != nil {
-			return fmt.Errorf("mounting shared master %q: %v", hint.Name, err)
-		}
-		if sharedMounts == nil {
-			sharedMounts = make(map[string]*vfs.Mount)
-		}
-		sharedMounts[hint.Mount.Source] = mnt
+func (c *containerMounter) getSharedMount(ctx context.Context, conf *config.Config, mount *mountInfo, creds *auth.Credentials) (*vfs.Mount, error) {
+	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)
+		return sharedMount, nil
 	}
-	l.sharedMounts = sharedMounts
-	return nil
+	log.Infof("Mounting master of shared mount %q from %q type %q", mount.hint.Name, mount.hint.Mount.Source, mount.hint.Mount.Type)
+	sharedMount, err := c.mountSharedMaster(ctx, conf, mount, creds)
+	if err != nil {
+		return nil, fmt.Errorf("mounting shared master %q: %v", mount.hint.Name, err)
+	}
+	c.sharedMounts[mount.hint.Mount.Source] = sharedMount
+	return sharedMount, nil
 }
 
 // mountSharedMaster mounts the master of a volume that is shared among
 // containers in a pod.
-func (l *Loader) mountSharedMaster(ctx context.Context, conf *config.Config, hint *MountHint, creds *auth.Credentials) (*vfs.Mount, error) {
-	// Map mount type to filesystem name, and parse out the options that we are
-	// capable of dealing with.
-	mntInfo := newNonGoferMountInfo(&hint.Mount)
-	fsName, opts, err := getMountNameAndOptions(conf, mntInfo, l.productName)
+func (c *containerMounter) mountSharedMaster(ctx context.Context, conf *config.Config, mntInfo *mountInfo, creds *auth.Credentials) (*vfs.Mount, error) {
+	// Mount the master using the options from the hint (mount annotations).
+	origOpts := mntInfo.mount.Options
+	mntInfo.mount.Options = mntInfo.hint.Mount.Options
+	fsName, opts, err := getMountNameAndOptions(conf, mntInfo, c.productName)
+	mntInfo.mount.Options = origOpts
 	if err != nil {
 		return nil, err
 	}
 	if len(fsName) == 0 {
-		return nil, fmt.Errorf("mount type not supported %q", hint.Mount.Type)
+		return nil, fmt.Errorf("mount type not supported %q", mntInfo.hint.Mount.Type)
 	}
-	return l.k.VFS().MountDisconnected(ctx, creds, "", fsName, opts)
+	return c.k.VFS().MountDisconnected(ctx, creds, "", fsName, opts)
 }
 
 // mountSharedSubmount binds mount to a previously mounted volume that is shared
 // among containers in the same pod.
-func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, mount *specs.Mount, srcHint *MountHint, srcMount *vfs.Mount) (*vfs.Mount, error) {
-	if err := srcHint.checkCompatible(mount); err != nil {
+func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, mntInfo *mountInfo, sharedMount *vfs.Mount) (*vfs.Mount, error) {
+	if err := mntInfo.hint.checkCompatible(mntInfo.mount); err != nil {
 		return nil, err
 	}
 
-	// Ignore data and useOverlay because these were already applied to
-	// the master mount.
-	_, opts, err := getMountNameAndOptions(conf, newNonGoferMountInfo(mount), c.productName)
-	if err != nil {
-		return nil, err
-	}
-	newMnt := c.k.VFS().NewDisconnectedMount(srcMount.Filesystem(), srcMount.Root(), opts)
+	// Generate mount point specific opts using mntInfo.mount.
+	opts := ParseMountOptions(mntInfo.mount.Options)
+	newMnt := c.k.VFS().NewDisconnectedMount(sharedMount.Filesystem(), sharedMount.Root(), opts)
 	defer newMnt.DecRef(ctx)
 
 	root := mns.Root(ctx)
@@ -1037,17 +1017,17 @@ func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config
 	target := &vfs.PathOperation{
 		Root:  root,
 		Start: root,
-		Path:  fspath.Parse(mount.Destination),
+		Path:  fspath.Parse(mntInfo.mount.Destination),
 	}
 
-	if err := c.makeMountPoint(ctx, creds, mns, mount.Destination); err != nil {
-		return nil, fmt.Errorf("creating mount point %q: %w", mount.Destination, err)
+	if err := c.makeMountPoint(ctx, creds, mns, mntInfo.mount.Destination); err != nil {
+		return nil, fmt.Errorf("creating mount point %q: %w", mntInfo.mount.Destination, err)
 	}
 
 	if err := c.k.VFS().ConnectMountAt(ctx, creds, newMnt, target); err != nil {
 		return nil, err
 	}
-	log.Infof("Mounted %q type shared bind to %q", mount.Destination, srcHint.Name)
+	log.Infof("Mounted %q type shared bind to %q", mntInfo.mount.Destination, mntInfo.hint.Name)
 	return newMnt, nil
 }
 
@@ -1080,10 +1060,10 @@ func (c *containerMounter) configureRestore(ctx context.Context) (context.Contex
 	if err != nil {
 		return ctx, err
 	}
-	for i := range c.mounts {
+	for i := range mounts {
 		submount := &mounts[i]
-		if submount.goferFD >= 0 {
-			fdmap[submount.mount.Destination] = submount.goferFD
+		if submount.goferFD != nil {
+			fdmap[submount.mount.Destination] = submount.goferFD.Release()
 		}
 	}
 	return context.WithValue(ctx, gofer.CtxRestoreServerFDMap, fdmap), nil