Skip to content

Commit

Permalink
de-duplicate fuse mounting/unmounting code (#2199)
Browse files Browse the repository at this point in the history
* replace call to sif's pkg/user code in squashfuseMount() with call to CE's squashfs code

* changed internal/pkg/util/fs/squashfs to use internal/pkg/util/fs/squashfs/fuse; add ctx args as needed

* replace call to sif's pkg/user code in CleanupHost() with call to CE's squashfs code

* address review comments

* error out instead of warning on invalid FUSE mount extra opt

* drop samber/mo dependency

* unit-test at Mount()/Unmount() level rather than pkg internals

* improve checking of extra opts: add "nosiud", "nodev"; some refactoring

* incorporate UID, GID as struct-fields; improve unit-test
  • Loading branch information
preminger authored Sep 18, 2023
1 parent 4dc233b commit bfb7a87
Show file tree
Hide file tree
Showing 16 changed files with 319 additions and 151 deletions.
14 changes: 2 additions & 12 deletions internal/pkg/runtime/engine/singularity/cleanup_host_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import (
"fmt"
"os"

sifuser "github.com/sylabs/sif/v2/pkg/user"
"github.com/sylabs/singularity/v4/internal/pkg/util/bin"
"github.com/sylabs/singularity/v4/internal/pkg/util/fs/squashfs"
"github.com/sylabs/singularity/v4/pkg/sylog"
)

Expand All @@ -20,16 +19,7 @@ import (
func (e *EngineOperations) CleanupHost(ctx context.Context) (err error) {
if e.EngineConfig.GetImageFuse() {
sylog.Infof("Unmounting SIF with FUSE...")
fusermountPath, err := bin.FindBin("fusermount")
if err != nil {
return fmt.Errorf("while unmounting fuse directory: %s: %w", e.EngineConfig.GetImage(), err)
}

err = sifuser.Unmount(ctx, e.EngineConfig.GetImage(),
sifuser.OptUnmountStdout(os.Stdout),
sifuser.OptUnmountStderr(os.Stderr),
sifuser.OptUnmountFusermountPath(fusermountPath))
if err != nil {
if err := squashfs.FUSEUnmount(ctx, e.EngineConfig.GetImage()); err != nil {
return fmt.Errorf("while unmounting fuse directory: %s: %w", e.EngineConfig.GetImage(), err)
}

Expand Down
20 changes: 11 additions & 9 deletions internal/pkg/runtime/launcher/native/launcher_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

lccgroups "github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runtime-spec/specs-go"
sifuser "github.com/sylabs/sif/v2/pkg/user"
"github.com/sylabs/sif/v2/pkg/sif"
"github.com/sylabs/singularity/v4/internal/pkg/buildcfg"
"github.com/sylabs/singularity/v4/internal/pkg/cgroups"
"github.com/sylabs/singularity/v4/internal/pkg/image/unpacker"
Expand All @@ -33,6 +33,7 @@ import (
"github.com/sylabs/singularity/v4/internal/pkg/util/bin"
"github.com/sylabs/singularity/v4/internal/pkg/util/env"
"github.com/sylabs/singularity/v4/internal/pkg/util/fs"
"github.com/sylabs/singularity/v4/internal/pkg/util/fs/squashfs"
"github.com/sylabs/singularity/v4/internal/pkg/util/gpu"
"github.com/sylabs/singularity/v4/internal/pkg/util/starter"
"github.com/sylabs/singularity/v4/internal/pkg/util/user"
Expand Down Expand Up @@ -1156,18 +1157,19 @@ func squashfuseMount(ctx context.Context, img *imgutil.Image, imageDir string) (
}
sylog.Infof("Mounting SIF with FUSE...")

squashfusePath, err := bin.FindBin("squashfuse")
f, err := sif.LoadContainerFromPath(img.Path, sif.OptLoadWithFlag(os.O_RDONLY))
if err != nil {
return fmt.Errorf("squashfuse is required: %w", err)
return fmt.Errorf("failed to load image: %w", err)
}
if _, err := bin.FindBin("fusermount"); err != nil {
return fmt.Errorf("fusermount is required: %w", err)

d, err := f.GetDescriptor(sif.WithPartitionType(sif.PartPrimSys))
if err != nil {
return fmt.Errorf("failed to get partition descriptor: %w", err)
}

return sifuser.Mount(ctx, img.Path, imageDir,
sifuser.OptMountStdout(os.Stdout),
sifuser.OptMountStderr(os.Stderr),
sifuser.OptMountSquashfusePath(squashfusePath))
_, err = squashfs.FUSEMount(ctx, uint64(d.Offset()), img.Path, imageDir)

return err
}

// starterInteractive executes the starter binary to run an image interactively, given the supplied engineConfig
Expand Down
8 changes: 4 additions & 4 deletions internal/pkg/runtime/launcher/oci/launcher_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ func (l *Launcher) RunWrapped(ctx context.Context, containerID, bundlePath, pidF
if err := os.MkdirAll(im.GetMountPoint(), 0o755); err != nil {
return err
}
if err := im.Mount(); err != nil {
if err := im.Mount(ctx); err != nil {
return err
}
}
Expand All @@ -784,17 +784,17 @@ func (l *Launcher) RunWrapped(ctx context.Context, containerID, bundlePath, pidF
err = Run(ctx, containerID, absBundle, pidFile, systemdCgroups)

for _, im := range l.imageMountsByMountpoint {
im.Unmount()
im.Unmount(ctx)
}

return err
}

if len(l.cfg.OverlayPaths) > 0 {
return WrapWithOverlays(runFunc, absBundle, l.cfg.OverlayPaths, l.cfg.AllowSUID)
return WrapWithOverlays(ctx, runFunc, absBundle, l.cfg.OverlayPaths, l.cfg.AllowSUID)
}

return WrapWithWritableTmpFs(runFunc, absBundle, l.cfg.AllowSUID)
return WrapWithWritableTmpFs(ctx, runFunc, absBundle, l.cfg.AllowSUID)
}

// getCgroup will return a cgroup path and resources for the runtime to create.
Expand Down
23 changes: 12 additions & 11 deletions internal/pkg/runtime/launcher/oci/oci_overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package oci

import (
"context"
"fmt"

"github.com/sylabs/singularity/v4/internal/pkg/util/fs/overlay"
Expand All @@ -18,8 +19,8 @@ import (
// tmpfs. This tmpfs is always writable so that the launcher and runtime are
// able to add content to the container. Whether it is writable from inside the
// container is controlled by the runtime config.
func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) error {
overlayDir, err := prepareWritableTmpfs(bundleDir, allowSetuid)
func WrapWithWritableTmpFs(ctx context.Context, f func() error, bundleDir string, allowSetuid bool) error {
overlayDir, err := prepareWritableTmpfs(ctx, bundleDir, allowSetuid)
sylog.Debugf("Done with prepareWritableTmpfs; overlayDir is: %q", overlayDir)
if err != nil {
return err
Expand All @@ -28,7 +29,7 @@ func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) e
err = f()

// Cleanup actions log errors, but don't return - so we get as much cleanup done as possible.
if cleanupErr := cleanupWritableTmpfs(bundleDir, overlayDir); cleanupErr != nil {
if cleanupErr := cleanupWritableTmpfs(ctx, bundleDir, overlayDir); cleanupErr != nil {
sylog.Errorf("While cleaning up writable tmpfs: %v", cleanupErr)
}

Expand All @@ -37,7 +38,7 @@ func WrapWithWritableTmpFs(f func() error, bundleDir string, allowSetuid bool) e
}

// WrapWithOverlays runs a function wrapped with prep / cleanup steps for overlays.
func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, allowSetuid bool) error {
func WrapWithOverlays(ctx context.Context, f func() error, bundleDir string, overlayPaths []string, allowSetuid bool) error {
writableOverlayFound := false
s := overlay.Set{}
for _, p := range overlayPaths {
Expand All @@ -64,36 +65,36 @@ func WrapWithOverlays(f func() error, bundleDir string, overlayPaths []string, a
}

rootFsDir := tools.RootFs(bundleDir).Path()
err := s.Mount(rootFsDir)
err := s.Mount(ctx, rootFsDir)
if err != nil {
return err
}

if writableOverlayFound {
err = f()
} else {
err = WrapWithWritableTmpFs(f, bundleDir, allowSetuid)
err = WrapWithWritableTmpFs(ctx, f, bundleDir, allowSetuid)
}

// Cleanup actions log errors, but don't return - so we get as much cleanup done as possible.
if cleanupErr := s.Unmount(rootFsDir); cleanupErr != nil {
if cleanupErr := s.Unmount(ctx, rootFsDir); cleanupErr != nil {
sylog.Errorf("While unmounting rootfs overlay: %v", cleanupErr)
}

// Return any error from the actual container payload - preserve exit code.
return err
}

func prepareWritableTmpfs(bundleDir string, allowSetuid bool) (string, error) {
func prepareWritableTmpfs(ctx context.Context, bundleDir string, allowSetuid bool) (string, error) {
sylog.Debugf("Configuring writable tmpfs overlay for %s", bundleDir)
c := singularityconf.GetCurrentConfig()
if c == nil {
return "", fmt.Errorf("singularity configuration is not initialized")
}
return tools.CreateOverlayTmpfs(bundleDir, int(c.SessiondirMaxSize), allowSetuid)
return tools.CreateOverlayTmpfs(ctx, bundleDir, int(c.SessiondirMaxSize), allowSetuid)
}

func cleanupWritableTmpfs(bundleDir, overlayDir string) error {
func cleanupWritableTmpfs(ctx context.Context, bundleDir, overlayDir string) error {
sylog.Debugf("Cleaning up writable tmpfs overlay for %s", bundleDir)
return tools.DeleteOverlayTmpfs(bundleDir, overlayDir)
return tools.DeleteOverlayTmpfs(ctx, bundleDir, overlayDir)
}
111 changes: 97 additions & 14 deletions internal/pkg/util/fs/fuse/fuse_mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,30 @@
package fuse

import (
"context"
"fmt"
"os"
"os/exec"
"strings"

"github.com/samber/lo"
"github.com/sylabs/singularity/v4/internal/pkg/util/bin"
"github.com/sylabs/singularity/v4/pkg/image"
"github.com/sylabs/singularity/v4/pkg/sylog"
"github.com/sylabs/singularity/v4/pkg/util/maps"
)

type ImageMount struct {
// Type represents what type of image this mount involves (from among the
// values in pkg/image)
Type int

// UID is the value to pass to the uid option when mounting
UID int

// GID is the value to pass to the gid option when mounting
GID int

// Readonly represents whether this is a Readonly overlay
Readonly bool

Expand All @@ -43,11 +52,15 @@ type ImageMount struct {

// AllowOther is set to true to mount the image with the "allow_other" option.
AllowOther bool

// ExtraOpts are options to be passed to the mount command (in the "-o"
// argument) beyond the ones autogenerated from other ImageMount fields.
ExtraOpts []string
}

// Mount mounts an image to a temporary directory. It also verifies that
// the fusermount utility is present before performing the mount.
func (i *ImageMount) Mount() (err error) {
func (i *ImageMount) Mount(ctx context.Context) (err error) {
fuseMountCmd, err := i.determineMountCmd()
if err != nil {
return err
Expand All @@ -60,7 +73,7 @@ func (i *ImageMount) Mount() (err error) {

fuseCmdLine := fmt.Sprintf("%s %s", fuseMountCmd, strings.Join(args, " "))
sylog.Debugf("Executing FUSE mount command: %q", fuseCmdLine)
execCmd := exec.Command(fuseMountCmd, args...)
execCmd := exec.CommandContext(ctx, fuseMountCmd, args...)
execCmd.Stderr = os.Stderr
_, err = execCmd.Output()
if err != nil {
Expand Down Expand Up @@ -124,33 +137,103 @@ func (i *ImageMount) generateCmdArgs() ([]string, error) {
}
}()

// TODO: Think through what makes sense for file ownership in FUSE-mounted
// images, vis a vis id-mappings and user-namespaces.
opts := []string{"uid=0", "gid=0"}
opts, err := i.generateMountOpts()
if err != nil {
return args, err
}

if len(opts) > 0 {
args = append(args, "-o", strings.Join(opts, ","))
}

args = append(args, i.SourcePath)
args = append(args, i.mountpoint)

return args, nil
}

func (i ImageMount) generateMountOpts() ([]string, error) {
// Create a map of the extra mount options that have been requested, so we
// can catch attempts to overwrite builtin struct fields.
extraOptsMap := lo.SliceToMap(i.ExtraOpts, func(s string) (string, *string) {
splitted := strings.SplitN(s, "=", 2)
if len(splitted) < 2 {
return strings.ToLower(s), nil
}

return strings.ToLower(splitted[0]), &splitted[1]
})

opts := []string{}

if err := checkProhibitedOpt(extraOptsMap, "uid"); err != nil {
return opts, err
}
opts = append(opts, fmt.Sprintf("uid=%d", i.UID))

if err := checkProhibitedOpt(extraOptsMap, "gid"); err != nil {
return opts, err
}
opts = append(opts, fmt.Sprintf("gid=%d", i.GID))

if err := checkProhibitedOpt(extraOptsMap, "ro"); err != nil {
return opts, err
}
if err := checkProhibitedOpt(extraOptsMap, "rw"); err != nil {
return opts, err
}
if i.Readonly {
// Not strictly necessary as will be read-only in assembled overlay,
// however this stops any erroneous writes through the stagingDir.
opts = append(opts, "ro")
}

// FUSE defaults to nosuid,nodev - attempt to reverse if AllowDev/Setuid requested.
if err := checkProhibitedOpt(extraOptsMap, "dev"); err != nil {
return opts, err
}
if err := checkProhibitedOpt(extraOptsMap, "nodev"); err != nil {
return opts, err
}
if i.AllowDev {
opts = append(opts, "dev")
}
if err := checkProhibitedOpt(extraOptsMap, "suid"); err != nil {
return opts, err
}
if err := checkProhibitedOpt(extraOptsMap, "nosuid"); err != nil {
return opts, err
}
if i.AllowSetuid {
opts = append(opts, "suid")
}

if err := checkProhibitedOpt(extraOptsMap, "allow_other"); err != nil {
return opts, err
}
if i.AllowOther {
opts = append(opts, "allow_other")
}

if len(opts) > 0 {
args = append(args, "-o", strings.Join(opts, ","))
filteredExtraOpts := lo.MapToSlice(extraOptsMap, rebuildOpt)
opts = append(opts, filteredExtraOpts...)

return opts, nil
}

func checkProhibitedOpt(extraOptsMap map[string]*string, opt string) error {
if maps.HasKey(extraOptsMap, opt) {
return fmt.Errorf("cannot pass %q as extra FUSE-mount option, as it is handled by an internal field", opt)
}

args = append(args, i.SourcePath)
args = append(args, i.mountpoint)
return nil
}

return args, nil
func rebuildOpt(k string, v *string) string {
if v == nil {
return k
}
return k + "=" + *v
}

func (i ImageMount) GetMountPoint() string {
Expand All @@ -161,21 +244,21 @@ func (i *ImageMount) SetMountPoint(mountpoint string) {
i.mountpoint = mountpoint
}

func (i ImageMount) Unmount() error {
return UnmountWithFuse(i.GetMountPoint())
func (i ImageMount) Unmount(ctx context.Context) error {
return UnmountWithFuse(ctx, i.GetMountPoint())
}

// UnmountWithFuse performs an unmount on the specified directory using
// fusermount -u.
func UnmountWithFuse(dir string) error {
func UnmountWithFuse(ctx context.Context, dir string) error {
fusermountCmd, err := bin.FindBin("fusermount")
if err != nil {
// We should not be creating FUSE-based mounts in the first place
// without checking that fusermount is available.
return fmt.Errorf("fusermount not available while trying to perform unmount: %w", err)
}
sylog.Debugf("Executing FUSE unmount command: %s -u %s", fusermountCmd, dir)
execCmd := exec.Command(fusermountCmd, "-u", dir)
execCmd := exec.CommandContext(ctx, fusermountCmd, "-u", dir)
execCmd.Stderr = os.Stderr
_, err = execCmd.Output()
return err
Expand Down
Loading

0 comments on commit bfb7a87

Please sign in to comment.