From 35dab382a68ebf4b02092ee28118b8297ad54fa6 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Wed, 24 Jan 2024 14:14:04 -0800 Subject: [PATCH] Add container name to unique path keys This is to allow multi-containers to be saved/restored without conflicting "unique" IDs. Updates #1956 PiperOrigin-RevId: 601231193 --- pkg/sentry/fsimpl/erofs/erofs.go | 2 +- pkg/sentry/fsimpl/erofs/save_restore.go | 2 +- pkg/sentry/fsimpl/gofer/gofer.go | 2 +- pkg/sentry/fsimpl/gofer/save_restore.go | 6 ++-- pkg/sentry/fsimpl/tmpfs/save_restore.go | 4 +-- pkg/sentry/fsimpl/tmpfs/tmpfs.go | 8 ++--- pkg/sentry/kernel/kernel.go | 10 +++--- pkg/sentry/vfs/save_restore.go | 16 +++++++++ runsc/boot/loader.go | 8 +++-- runsc/boot/vfs.go | 46 +++++++++++++++++-------- runsc/specutils/specutils.go | 16 ++++----- 11 files changed, 77 insertions(+), 43 deletions(-) diff --git a/pkg/sentry/fsimpl/erofs/erofs.go b/pkg/sentry/fsimpl/erofs/erofs.go index 658c3ba4b..ef51b5db7 100644 --- a/pkg/sentry/fsimpl/erofs/erofs.go +++ b/pkg/sentry/fsimpl/erofs/erofs.go @@ -81,7 +81,7 @@ type filesystem struct { type InternalFilesystemOptions struct { // If UniqueID is non-empty, it is an opaque string used to reassociate the // filesystem with a new image FD during restoration from checkpoint. - UniqueID string + UniqueID vfs.RestoreID } // Name implements vfs.FilesystemType.Name. diff --git a/pkg/sentry/fsimpl/erofs/save_restore.go b/pkg/sentry/fsimpl/erofs/save_restore.go index d1566caa6..399acf8e8 100644 --- a/pkg/sentry/fsimpl/erofs/save_restore.go +++ b/pkg/sentry/fsimpl/erofs/save_restore.go @@ -38,7 +38,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest if fdmapv == nil { return fmt.Errorf("no image FD map available") } - fdmap := fdmapv.(map[string]int) + fdmap := fdmapv.(map[vfs.RestoreID]int) fd, ok := fdmap[fs.iopts.UniqueID] if !ok { return fmt.Errorf("no image FD available for filesystem with unique ID %q", fs.iopts.UniqueID) diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 93fdbd333..d88f8dd2e 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -363,7 +363,7 @@ const ( type InternalFilesystemOptions struct { // If UniqueID is non-empty, it is an opaque string used to reassociate the // filesystem with a new server FD during restoration from checkpoint. - UniqueID string + UniqueID vfs.RestoreID // If LeakConnection is true, do not close the connection to the server // when the Filesystem is released. This is necessary for deployments in diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 09810fb45..3bd193df9 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -37,7 +37,7 @@ type savedDentryRW struct { // PrepareSave implements vfs.FilesystemImplSaveRestoreExtension.PrepareSave. func (fs *filesystem) PrepareSave(ctx context.Context) error { - if len(fs.iopts.UniqueID) == 0 { + if len(fs.iopts.UniqueID.Path) == 0 { return fmt.Errorf("gofer.filesystem with no UniqueID cannot be saved") } @@ -176,10 +176,10 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest if fdmapv == nil { return fmt.Errorf("no server FD map available") } - fdmap := fdmapv.(map[string]int) + fdmap := fdmapv.(map[vfs.RestoreID]int) fd, ok := fdmap[fs.iopts.UniqueID] if !ok { - return fmt.Errorf("no server FD available for filesystem with unique ID %q", fs.iopts.UniqueID) + return fmt.Errorf("no server FD available for filesystem with unique ID %+v, map: %v", fs.iopts.UniqueID, fdmap) } fs.opts.fd = fd fs.inoByKey = make(map[inoKey]uint64) diff --git a/pkg/sentry/fsimpl/tmpfs/save_restore.go b/pkg/sentry/fsimpl/tmpfs/save_restore.go index 261621e0d..7c99eb358 100644 --- a/pkg/sentry/fsimpl/tmpfs/save_restore.go +++ b/pkg/sentry/fsimpl/tmpfs/save_restore.go @@ -48,7 +48,7 @@ func (fs *filesystem) PrepareSave(ctx context.Context) error { if mfmapv == nil { return fmt.Errorf("CtxFilesystemMemoryFileMap was not provided") } - mfmap := mfmapv.(map[string]*pgalloc.MemoryFile) + mfmap := mfmapv.(map[vfs.RestoreID]*pgalloc.MemoryFile) mfmap[fs.uniqueID] = fs.mf return nil } @@ -63,7 +63,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest if mfmapv == nil { return fmt.Errorf("CtxFilesystemMemoryFileMap was not provided") } - mfmap := mfmapv.(map[string]*pgalloc.MemoryFile) + mfmap := mfmapv.(map[vfs.RestoreID]*pgalloc.MemoryFile) mf, ok := mfmap[fs.uniqueID] if !ok { return fmt.Errorf("memory file for %q not found in CtxFilesystemMemoryFileMap", fs.uniqueID) diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 0595b2a08..d5359d7d6 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -72,7 +72,7 @@ type filesystem struct { // uniqueID is an opaque string used to reassociate the filesystem with its // private MemoryFile during checkpoint and restore. - uniqueID string + uniqueID vfs.RestoreID // mfp is used to provide mf, when privateMF == false. This is required to // re-provide mf on restore. mfp is immutable. @@ -159,7 +159,7 @@ type FilesystemOpts struct { // If UniqueID is non-empty, it is an opaque string used to reassociate the // filesystem with its private MemoryFile during checkpoint and restore. - UniqueID string + UniqueID vfs.RestoreID } // Default size limit mount option. It is immutable after initialization. @@ -192,7 +192,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt } mf := mfp.MemoryFile() privateMF := false - uniqueID := "" + var uniqueID vfs.RestoreID rootFileType := uint16(linux.S_IFDIR) disableDefaultSizeLimit := false newFSType := vfs.FilesystemType(&fstype) @@ -225,7 +225,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt allowXattrPrefix[xattr] = struct{}{} } } - if privateMF && uniqueID == "" { + if privateMF && len(uniqueID.Path) == 0 { ctx.Warningf("tmpfs.FilesystemType.GetFilesystem: privateMF requires uniqueID to be set") return nil, nil, linuxerr.EINVAL } diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 929340668..83f670842 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -505,10 +505,10 @@ func (k *Kernel) Init(args InitKernelArgs) error { // +stateify savable type privateMemoryFileMetadata struct { - owners []string + owners []vfs.RestoreID } -func savePrivateMFs(ctx context.Context, w wire.Writer, mfsToSave map[string]*pgalloc.MemoryFile) error { +func savePrivateMFs(ctx context.Context, w wire.Writer, mfsToSave map[vfs.RestoreID]*pgalloc.MemoryFile) error { var meta privateMemoryFileMetadata // Generate the order in which private memory files are saved. for fsID := range mfsToSave { @@ -533,9 +533,9 @@ func loadPrivateMFs(ctx context.Context, r wire.Reader) error { if _, err := state.Load(ctx, r, &meta); err != nil { return err } - var mfmap map[string]*pgalloc.MemoryFile + var mfmap map[vfs.RestoreID]*pgalloc.MemoryFile if mfmapv := ctx.Value(vfs.CtxFilesystemMemoryFileMap); mfmapv != nil { - mfmap = mfmapv.(map[string]*pgalloc.MemoryFile) + mfmap = mfmapv.(map[vfs.RestoreID]*pgalloc.MemoryFile) } // Ensure that it is consistent with CtxFilesystemMemoryFileMap. if len(mfmap) != len(meta.owners) { @@ -578,7 +578,7 @@ func (k *Kernel) SaveTo(ctx context.Context, w wire.Writer) error { } // Capture all private memory files. - mfsToSave := make(map[string]*pgalloc.MemoryFile) + mfsToSave := make(map[vfs.RestoreID]*pgalloc.MemoryFile) vfsCtx := context.WithValue(ctx, vfs.CtxFilesystemMemoryFileMap, mfsToSave) // Prepare filesystems for saving. This must be done after // invalidateUnsavableMappings(), since dropping memory mappings may diff --git a/pkg/sentry/vfs/save_restore.go b/pkg/sentry/vfs/save_restore.go index a3b736f8b..fe677530f 100644 --- a/pkg/sentry/vfs/save_restore.go +++ b/pkg/sentry/vfs/save_restore.go @@ -15,6 +15,7 @@ package vfs import ( + "fmt" "sync/atomic" "gvisor.dev/gvisor/pkg/context" @@ -133,3 +134,18 @@ func (epi *epollInterest) afterLoad() { // EpollInstance.ReadEvents() rechecks their readiness. epi.waiter.NotifyEvent(waiter.EventMaskFromLinux(epi.mask)) } + +// RestoreID is a unique ID that is used to identify resources between save/restore sessions. +// Example of resources are host files, gofer connection for mount points, etc. +// +// +stateify savable +type RestoreID struct { + // ContainerName is the name of the container that the resource belongs to. + ContainerName string + // Path is the path of the resource. + Path string +} + +func (f RestoreID) String() string { + return fmt.Sprintf("%s:%s", f.ContainerName, f.Path) +} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 0cec500e6..e20dc4919 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -95,6 +95,8 @@ import ( type containerInfo struct { cid string + containerName string + conf *config.Config // spec is the base configuration for the root container. @@ -328,6 +330,7 @@ func New(args Args) (*Loader, error) { info := containerInfo{ cid: args.ID, + containerName: specutils.ContainerName(args.Spec), conf: args.Conf, spec: args.Spec, goferMountConfs: args.GoferMountConfs, @@ -899,6 +902,7 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st info := &containerInfo{ cid: cid, + containerName: specutils.ContainerName(spec), conf: conf, spec: spec, goferFDs: goferFDs, @@ -973,7 +977,7 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st func (l *Loader) createContainerProcess(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) + fdTable, ttyFile, err := createFDTable(ctx, info.spec.Process.Terminal, info.stdioFDs, info.passFDs, info.spec.Process.User, info.containerName) if err != nil { return nil, nil, fmt.Errorf("importing fds: %w", err) } @@ -1572,7 +1576,7 @@ func (l *Loader) ttyFromIDLocked(key execID) (*host.TTYFileDescription, error) { return ep.tty, nil } -func createFDTable(ctx context.Context, console bool, stdioFDs []*fd.FD, passFDs []fdMapping, user specs.User) (*kernel.FDTable, *host.TTYFileDescription, error) { +func createFDTable(ctx context.Context, console bool, stdioFDs []*fd.FD, passFDs []fdMapping, user specs.User, containerName string) (*kernel.FDTable, *host.TTYFileDescription, error) { if len(stdioFDs) != 3 { return nil, nil, fmt.Errorf("stdioFDs should contain exactly 3 FDs (stdin, stdout, and stderr), but %d FDs received", len(stdioFDs)) } diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index b44b67736..fe62558f7 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -402,7 +402,8 @@ type containerMounter struct { containerID string // sandboxID is the ID for the whole sandbox. - sandboxID string + sandboxID string + containerName string // cgroupMounts is a map of cgroup mounts that can be reused across // containers. Key is the cgroup controller name string. @@ -434,6 +435,7 @@ func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *PodMountH productName: productName, containerID: info.procArgs.ContainerID, sandboxID: sandboxID, + containerName: info.containerName, cgroupMounts: cgroupMounts, } } @@ -519,7 +521,10 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi InternalMount: true, Data: strings.Join(data, ","), InternalData: gofer.InternalFilesystemOptions{ - UniqueID: "/", + UniqueID: vfs.RestoreID{ + ContainerName: c.containerName, + Path: "/", + }, }, }, } @@ -532,7 +537,10 @@ func (c *containerMounter) createMountNamespace(ctx context.Context, conf *confi InternalMount: true, Data: fmt.Sprintf("ifd=%d", ioFD), InternalData: erofs.InternalFilesystemOptions{ - UniqueID: "/", + UniqueID: vfs.RestoreID{ + ContainerName: c.containerName, + Path: "/", + }, }, }, } @@ -645,7 +653,7 @@ func (c *containerMounter) configureOverlay(ctx context.Context, conf *config.Co return nil, nil, fmt.Errorf("failed to create memory file for overlay: %v", err) } tmpfsOpts.MemoryFile = mf - tmpfsOpts.UniqueID = dst + tmpfsOpts.UniqueID = vfs.RestoreID{ContainerName: c.containerName, Path: dst} } upperOpts.GetFilesystemOptions.InternalData = tmpfsOpts upper, err := c.k.VFS().MountDisconnected(ctx, creds, "" /* source */, tmpfs.Name, &upperOpts) @@ -829,7 +837,7 @@ func (c *containerMounter) prepareMounts() ([]mountInfo, error) { } func (c *containerMounter) mountSubmount(ctx context.Context, spec *specs.Spec, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, submount *mountInfo) (*vfs.Mount, error) { - fsName, opts, err := getMountNameAndOptions(spec, conf, submount, c.productName) + fsName, opts, err := getMountNameAndOptions(spec, conf, submount, c.productName, c.containerName) if err != nil { return nil, fmt.Errorf("mountOptions failed: %w", err) } @@ -870,7 +878,7 @@ func (c *containerMounter) mountSubmount(ctx context.Context, spec *specs.Spec, // getMountNameAndOptions retrieves the fsName, opts, and useOverlay values // used for mounts. -func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, productName string) (string, *vfs.MountOptions, error) { +func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, productName, containerName string) (string, *vfs.MountOptions, error) { fsName := m.mount.Type var ( mopts = m.mount.Options @@ -906,7 +914,7 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, } internalData = tmpfs.FilesystemOpts{ MemoryFile: mf, - UniqueID: m.mount.Destination, + UniqueID: vfs.RestoreID{ContainerName: containerName, Path: m.mount.Destination}, // If a mount is being overlaid with tmpfs, it should not be limited by // the default tmpfs size limit. DisableDefaultSizeLimit: true, @@ -926,7 +934,10 @@ func getMountNameAndOptions(spec *specs.Spec, conf *config.Config, m *mountInfo, } data = append(data, goferMountData(m.goferFD.Release(), getMountAccessType(conf, m.hint), conf)...) internalData = gofer.InternalFilesystemOptions{ - UniqueID: m.mount.Destination, + UniqueID: vfs.RestoreID{ + ContainerName: containerName, + Path: m.mount.Destination, + }, } case cgroupfs.Name: @@ -1204,7 +1215,7 @@ func (c *containerMounter) mountSharedMaster(ctx context.Context, spec *specs.Sp // 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(spec, conf, mntInfo, c.productName) + fsName, opts, err := getMountNameAndOptions(spec, conf, mntInfo, c.productName, c.containerName) mntInfo.mount.Options = origOpts if err != nil { return nil, err @@ -1271,15 +1282,18 @@ func (c *containerMounter) makeMountPoint(ctx context.Context, creds *auth.Crede func (c *containerMounter) configureRestore(ctx context.Context) (context.Context, error) { // Compare createMountNamespace(); rootfs always consumes a gofer FD and a // filestore FD is consumed if the rootfs GoferMountConf indicates so. - fdmap := make(map[string]int) - fdmap["/"] = c.goferFDs.remove() - mfmap := make(map[string]*pgalloc.MemoryFile) + fdmap := make(map[vfs.RestoreID]int) + + rootKey := vfs.RestoreID{ContainerName: c.containerName, Path: "/"} + fdmap[rootKey] = c.goferFDs.remove() + + mfmap := make(map[vfs.RestoreID]*pgalloc.MemoryFile) if rootfsConf := c.goferMountConfs[0]; rootfsConf.IsFilestorePresent() { mf, err := createPrivateMemoryFile(c.goferFilestoreFDs.removeAsFD().ReleaseToFile("overlay-filestore")) if err != nil { return ctx, fmt.Errorf("failed to create private memory file for mount rootfs: %w", err) } - mfmap["/"] = mf + mfmap[rootKey] = mf } // prepareMounts() consumes the remaining FDs for submounts. mounts, err := c.prepareMounts() @@ -1289,14 +1303,16 @@ func (c *containerMounter) configureRestore(ctx context.Context) (context.Contex for i := range mounts { submount := &mounts[i] if submount.goferFD != nil { - fdmap[submount.mount.Destination] = submount.goferFD.Release() + key := vfs.RestoreID{ContainerName: c.containerName, Path: submount.mount.Destination} + fdmap[key] = submount.goferFD.Release() } if submount.filestoreFD != nil { mf, err := createPrivateMemoryFile(submount.filestoreFD.ReleaseToFile("overlay-filestore")) if err != nil { return ctx, fmt.Errorf("failed to create private memory file for mount %q: %w", submount.mount.Destination, err) } - mfmap[submount.mount.Destination] = mf + key := vfs.RestoreID{ContainerName: c.containerName, Path: submount.mount.Destination} + mfmap[key] = mf } } return context.WithValue(context.WithValue(ctx, vfs.CtxRestoreFilesystemFDMap, fdmap), vfs.CtxFilesystemMemoryFileMap, mfmap), nil diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 3609da000..72a43abd5 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -241,15 +241,7 @@ func fixSpec(spec *specs.Spec, bundleDir string, conf *config.Config) error { } } - // Check annotation to see if container name is available. - var containerName string - for key, val := range spec.Annotations { - if key == annotationContainerName { - containerName = val - log.Debugf("Container name: %q", containerName) - break - } - } + containerName := ContainerName(spec) for annotation, val := range spec.Annotations { if strings.HasPrefix(annotation, annotationFlagPrefix) { // Override flags using annotation to allow customization per sandbox @@ -758,3 +750,9 @@ func ResolveEnvs(envs ...[]string) ([]string, error) { func FaqErrorMsg(anchor, msg string) string { return fmt.Sprintf("%s; see https://gvisor.dev/faq#%s for more details", msg, anchor) } + +// ContainerName looks for an annotation in the spec with the container name. Returns empty string +// if no annotation is found. +func ContainerName(spec *specs.Spec) string { + return spec.Annotations[annotationContainerName] +}