diff --git a/pkg/shim/utils/volumes.go b/pkg/shim/utils/volumes.go index 5dd7abc63..1e0576ff9 100644 --- a/pkg/shim/utils/volumes.go +++ b/pkg/shim/utils/volumes.go @@ -185,14 +185,14 @@ func configureShm(s *specs.Spec) (bool, error) { m := &s.Mounts[i] if m.Destination == shmPath && m.Type == "bind" { if IsSandbox(s) { - s.Annotations["dev.gvisor.spec.mount."+devshmName+".source"] = m.Source - s.Annotations["dev.gvisor.spec.mount."+devshmName+".type"] = devshmType - s.Annotations["dev.gvisor.spec.mount."+devshmName+".share"] = "pod" + s.Annotations[volumeKeyPrefix+devshmName+".source"] = m.Source + s.Annotations[volumeKeyPrefix+devshmName+".type"] = devshmType + s.Annotations[volumeKeyPrefix+devshmName+".share"] = "pod" // Given that we don't have visibility into mount options for all // containers, assume broad access for the master mount (it's tmpfs // inside the sandbox anyways) and apply options to subcontainers as // they bind mount individually. - s.Annotations["dev.gvisor.spec.mount."+devshmName+".options"] = "rw" + s.Annotations[volumeKeyPrefix+devshmName+".options"] = "rw" } changeMountType(m, devshmType) diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index f78dfcb7b..8f15f4f26 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -147,7 +147,7 @@ type Loader struct { // mountHints provides extra information about mounts for containers that // apply to the entire pod. - mountHints *podMountHints + mountHints *PodMountHints // productName is the value to show in // /sys/devices/virtual/dmi/id/product_name. @@ -442,7 +442,7 @@ func New(args Args) (*Loader, error) { return nil, fmt.Errorf("initializing compat logs: %w", err) } - mountHints, err := newPodMountHints(args.Spec) + mountHints, err := NewPodMountHints(args.Spec) if err != nil { return nil, fmt.Errorf("creating pod mount hints: %w", err) } diff --git a/runsc/boot/mount_hints.go b/runsc/boot/mount_hints.go index 6c7817a00..f89a5d471 100644 --- a/runsc/boot/mount_hints.go +++ b/runsc/boot/mount_hints.go @@ -74,13 +74,14 @@ func (s shareType) String() string { } } -// podMountHints contains a collection of mountHints for the pod. -type podMountHints struct { - mounts map[string]*mountHint +// PodMountHints contains a collection of mountHints for the pod. +type PodMountHints struct { + mounts map[string]*MountHint } -func newPodMountHints(spec *specs.Spec) (*podMountHints, error) { - mnts := make(map[string]*mountHint) +// NewPodMountHints instantiates PodMountHints using spec. +func NewPodMountHints(spec *specs.Spec) (*PodMountHints, error) { + mnts := make(map[string]*MountHint) for k, v := range spec.Annotations { // Look for 'dev.gvisor.spec.mount' annotations and parse them. if strings.HasPrefix(k, MountPrefix) { @@ -95,7 +96,7 @@ func newPodMountHints(spec *specs.Spec) (*podMountHints, error) { } mnt := mnts[name] if mnt == nil { - mnt = &mountHint{name: name} + mnt = &MountHint{name: name} mnts[name] = mnt } if err := mnt.setField(parts[1], v); err != nil { @@ -121,13 +122,13 @@ func newPodMountHints(spec *specs.Spec) (*podMountHints, error) { } } - return &podMountHints{mounts: mnts}, nil + return &PodMountHints{mounts: mnts}, nil } -// mountHint represents extra information about mounts that are provided via +// MountHint represents extra information about mounts that are provided via // annotations. They can override mount type, and provide sharing information // so that mounts can be correctly shared inside the pod. -type mountHint struct { +type MountHint struct { name string share shareType mount specs.Mount @@ -137,7 +138,7 @@ type mountHint struct { vfsMount *vfs.Mount } -func (m *mountHint) setField(key, val string) error { +func (m *MountHint) setField(key, val string) error { switch key { case "source": if len(val) == 0 { @@ -160,9 +161,9 @@ func (m *mountHint) setField(key, val string) error { return nil } -func (m *mountHint) setType(val string) error { +func (m *MountHint) setType(val string) error { switch val { - case "tmpfs", "bind": + case tmpfs.Name, Bind: m.mount.Type = val default: return fmt.Errorf("invalid type %q", val) @@ -170,7 +171,8 @@ func (m *mountHint) setType(val string) error { return nil } -func (m *mountHint) isSupported() bool { +// isShared returns true if this mount should be configured as a shared mount. +func (m *MountHint) isShared() 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 @@ -179,7 +181,7 @@ func (m *mountHint) isSupported() bool { // checkCompatible verifies that shared mount is compatible with master. // Master options must be the same or less restrictive than the container mount, // e.g. master can be 'rw' while container mounts as 'ro'. -func (m *mountHint) checkCompatible(replica *specs.Mount) error { +func (m *MountHint) checkCompatible(replica *specs.Mount) error { masterOpts := ParseMountOptions(m.mount.Options) replicaOpts := ParseMountOptions(replica.Options) @@ -195,14 +197,15 @@ func (m *mountHint) checkCompatible(replica *specs.Mount) error { return nil } -func (m *mountHint) fileAccessType() config.FileAccessType { +func (m *MountHint) fileAccessType() config.FileAccessType { if m.share == container { return config.FileAccessExclusive } return config.FileAccessShared } -func (p *podMountHints) findMount(mount *specs.Mount) *mountHint { +// FindMount finds the MountHint that applies to this mount. +func (p *PodMountHints) FindMount(mount *specs.Mount) *MountHint { for _, m := range p.mounts { if m.mount.Source == mount.Source { return m diff --git a/runsc/boot/mount_hints_test.go b/runsc/boot/mount_hints_test.go index a0a31239a..bc8a59a1c 100644 --- a/runsc/boot/mount_hints_test.go +++ b/runsc/boot/mount_hints_test.go @@ -35,7 +35,7 @@ func TestPodMountHintsHappy(t *testing.T) { MountPrefix + "mount2.options": "rw,private", }, } - podHints, err := newPodMountHints(spec) + podHints, err := NewPodMountHints(spec) if err != nil { t.Fatalf("newPodMountHints failed: %v", err) } @@ -112,7 +112,7 @@ func TestPodMountHintsErrors(t *testing.T) { } { t.Run(tst.name, func(t *testing.T) { spec := &specs.Spec{Annotations: tst.annotations} - podHints, err := newPodMountHints(spec) + podHints, err := NewPodMountHints(spec) if err == nil || !strings.Contains(err.Error(), tst.error) { t.Errorf("newPodMountHints invalid error, want: .*%s.*, got: %v", tst.error, err) } @@ -157,7 +157,7 @@ func TestPodMountHintsIgnore(t *testing.T) { } { t.Run(tst.name, func(t *testing.T) { spec := &specs.Spec{Annotations: tst.annotations} - podHints, err := newPodMountHints(spec) + podHints, err := NewPodMountHints(spec) if err != nil { t.Errorf("newPodMountHints() failed: %v", err) } else if podHints != nil { @@ -178,7 +178,7 @@ func TestIgnoreInvalidMountOptions(t *testing.T) { MountPrefix + "mount1.options": "rw,invalid,private", }, } - podHints, err := newPodMountHints(spec) + podHints, err := NewPodMountHints(spec) if err != nil { t.Fatalf("newPodMountHints failed: %v", err) } @@ -233,7 +233,7 @@ func TestHintsCheckCompatible(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - master := mountHint{mount: specs.Mount{Options: tc.masterOpts}} + master := MountHint{mount: specs.Mount{Options: tc.masterOpts}} replica := specs.Mount{Options: tc.replicaOpts} if err := master.checkCompatible(&replica); err != nil { if !strings.Contains(err.Error(), tc.err) { diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index c0a8c38f8..510cf7d99 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -351,7 +351,7 @@ type containerMounter struct { k *kernel.Kernel - hints *podMountHints + hints *PodMountHints // productName is the value to show in // /sys/devices/virtual/dmi/id/product_name. @@ -361,7 +361,7 @@ type containerMounter struct { sandboxID string } -func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *podMountHints, productName string, sandboxID string) *containerMounter { +func newContainerMounter(info *containerInfo, k *kernel.Kernel, hints *PodMountHints, productName string, sandboxID string) *containerMounter { return &containerMounter{ root: info.spec.Root, mounts: compileMounts(info.spec, info.conf), @@ -381,8 +381,8 @@ func (c *containerMounter) checkDispenser() error { return nil } -func (c *containerMounter) getMountAccessType(conf *config.Config, mount *specs.Mount) config.FileAccessType { - if hint := c.hints.findMount(mount); hint != nil { +func (c *containerMounter) getMountAccessType(conf *config.Config, mount *specs.Mount, hint *MountHint) config.FileAccessType { + if hint != nil { return hint.fileAccessType() } return conf.FileAccessMounts @@ -624,10 +624,10 @@ func (c *containerMounter) mountSubmounts(ctx context.Context, conf *config.Conf err error ) - if hint := c.hints.findMount(submount.mount); hint != nil && hint.isSupported() { - mnt, err = c.mountSharedSubmount(ctx, conf, mns, creds, submount.mount, hint) + if submount.hint != nil && submount.hint.isShared() { + mnt, err = c.mountSharedSubmount(ctx, conf, mns, creds, submount.mount, submount.hint) if err != nil { - return fmt.Errorf("mount shared mount %q to %q: %v", hint.name, submount.mount.Destination, err) + return fmt.Errorf("mount shared mount %q to %q: %v", submount.hint.name, submount.mount.Destination, err) } } else { mnt, err = c.mountSubmount(ctx, conf, mns, creds, submount) @@ -656,20 +656,21 @@ func (c *containerMounter) mountSubmounts(ctx context.Context, conf *config.Conf return nil } -type mountAndFD struct { +type mountInfo struct { mount *specs.Mount fd int + hint *MountHint } -func newNonGoferMountAndFD(mnt *specs.Mount) *mountAndFD { - return &mountAndFD{mount: mnt, fd: -1} +func newNonGoferMountInfo(mount *specs.Mount) *mountInfo { + return &mountInfo{mount: mount, fd: -1} } -func (c *containerMounter) prepareMounts() ([]mountAndFD, error) { +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 // they are required by mounts. - var mounts []mountAndFD + var mounts []mountInfo for i := range c.mounts { m := &c.mounts[i] specutils.MaybeConvertToBindMount(m) @@ -680,9 +681,10 @@ func (c *containerMounter) prepareMounts() ([]mountAndFD, error) { if m.Type == Bind { fd = c.fds.remove() } - mounts = append(mounts, mountAndFD{ + mounts = append(mounts, mountInfo{ mount: m, fd: fd, + hint: c.hints.FindMount(m), }) } if err := c.checkDispenser(); err != nil { @@ -697,7 +699,7 @@ func (c *containerMounter) prepareMounts() ([]mountAndFD, error) { return mounts, nil } -func (c *containerMounter) mountSubmount(ctx context.Context, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, submount *mountAndFD) (*vfs.Mount, error) { +func (c *containerMounter) mountSubmount(ctx context.Context, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, submount *mountInfo) (*vfs.Mount, error) { fsName, opts, useOverlay, err := c.getMountNameAndOptions(conf, submount) if err != nil { return nil, fmt.Errorf("mountOptions failed: %w", err) @@ -740,7 +742,7 @@ func (c *containerMounter) mountSubmount(ctx context.Context, conf *config.Confi // getMountNameAndOptions retrieves the fsName, opts, and useOverlay values // used for mounts. -func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountAndFD) (string, *vfs.MountOptions, bool, error) { +func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountInfo) (string, *vfs.MountOptions, bool, error) { fsName := m.mount.Type useOverlay := false var ( @@ -774,7 +776,7 @@ func (c *containerMounter) getMountNameAndOptions(conf *config.Config, m *mountA // Check that an FD was provided to fails fast. return "", nil, false, fmt.Errorf("gofer mount requires a connection FD") } - data = goferMountData(m.fd, c.getMountAccessType(conf, m.mount), conf) + data = goferMountData(m.fd, c.getMountAccessType(conf, m.mount, m.hint), conf) internalData = gofer.InternalFilesystemOptions{ UniqueID: m.mount.Destination, } @@ -896,7 +898,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, newNonGoferMountAndFD(&tmpMount)); err != nil { + if _, err := c.mountSubmount(ctx, conf, mns, creds, newNonGoferMountInfo(&tmpMount)); err != nil { return fmt.Errorf("mountSubmount failed: %v", err) } return nil @@ -916,7 +918,7 @@ func (c *containerMounter) mountTmp(ctx context.Context, conf *config.Config, cr func (c *containerMounter) processHints(conf *config.Config, creds *auth.Credentials) error { ctx := c.k.SupervisorContext() for _, hint := range c.hints.mounts { - if !hint.isSupported() { + if !hint.isShared() { continue } @@ -932,11 +934,11 @@ func (c *containerMounter) processHints(conf *config.Config, creds *auth.Credent // mountSharedMaster mounts the master of a volume that is shared among // containers in a pod. -func (c *containerMounter) mountSharedMaster(ctx context.Context, conf *config.Config, hint *mountHint, creds *auth.Credentials) (*vfs.Mount, error) { +func (c *containerMounter) 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. - mntFD := newNonGoferMountAndFD(&hint.mount) - fsName, opts, useOverlay, err := c.getMountNameAndOptions(conf, mntFD) + mntInfo := newNonGoferMountInfo(&hint.mount) + fsName, opts, useOverlay, err := c.getMountNameAndOptions(conf, mntInfo) if err != nil { return nil, err } @@ -945,14 +947,14 @@ func (c *containerMounter) mountSharedMaster(ctx context.Context, conf *config.C } if useOverlay { - log.Infof("Adding overlay on top of shared mount %q", mntFD.mount.Destination) + log.Infof("Adding overlay on top of shared mount %q", mntInfo.mount.Destination) var cleanup func() // TODO(b/142076984): Use an overlay for a shared EmptyDir mount. Such a // mount should be backed by a self filestore, so limits can be enforced // by k8s on the host. For now pass nil for useFilestoreFD. opts, cleanup, err = c.configureOverlay(ctx, conf, creds, opts, fsName, nil /* useFilestoreFD */) if err != nil { - return nil, fmt.Errorf("mounting shared volume with overlay at %q: %w", mntFD.mount.Destination, err) + return nil, fmt.Errorf("mounting shared volume with overlay at %q: %w", mntInfo.mount.Destination, err) } defer cleanup() fsName = overlay.Name @@ -963,14 +965,14 @@ func (c *containerMounter) mountSharedMaster(ctx context.Context, conf *config.C // 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, source *mountHint) (*vfs.Mount, error) { +func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config.Config, mns *vfs.MountNamespace, creds *auth.Credentials, mount *specs.Mount, source *MountHint) (*vfs.Mount, error) { if err := source.checkCompatible(mount); err != nil { return nil, err } // Ignore data and useOverlay because these were already applied to // the master mount. - _, opts, _, err := c.getMountNameAndOptions(conf, newNonGoferMountAndFD(mount)) + _, opts, _, err := c.getMountNameAndOptions(conf, newNonGoferMountInfo(mount)) if err != nil { return nil, err } diff --git a/runsc/boot/vfs_test.go b/runsc/boot/vfs_test.go index a06c5ada3..477eb9473 100644 --- a/runsc/boot/vfs_test.go +++ b/runsc/boot/vfs_test.go @@ -67,13 +67,14 @@ func TestGetMountAccessType(t *testing.T) { } { t.Run(tst.name, func(t *testing.T) { spec := &specs.Spec{Annotations: tst.annotations} - podHints, err := newPodMountHints(spec) + podHints, err := NewPodMountHints(spec) if err != nil { t.Fatalf("newPodMountHints failed: %v", err) } mounter := containerMounter{hints: podHints} conf := &config.Config{FileAccessMounts: config.FileAccessShared} - if got := mounter.getMountAccessType(conf, &specs.Mount{Source: source}); got != tst.want { + mnt := &specs.Mount{Source: source} + if got := mounter.getMountAccessType(conf, mnt, podHints.FindMount(mnt)); got != tst.want { t.Errorf("getMountAccessType(), want: %v, got: %v", tst.want, got) } })