Merge pull request #3446 from dtrudg/pick-oci-fixes

Pick OCI-mode XDG var handling improvements (release-4.2)

CHANGELOG.md

@@ -6,6 +6,20 @@
 
 - Fix regression from 4.1.5 that overwrites source image runscript, environment
   etc. in build from local image.
+- Fall back to `$TMPDIR` as singularity-buildkitd root directory if
+  `~/.singularity` is on a filesystem that does not fully support overlay.
+- Add more intuitive error message for rootless `build --oci` when required
+  `XDG_RUNTIME_DIR` env var is not set.
+
+### New Features & Functionality
+
+- In OCI-Mode, accommodate systems configured so that they do not create a
+  `/run/user` session directory. OCI-Mode will now attempt to use
+  `$TMPDIR/singularity-oci-<uid>` for runtime state on systems where
+  `$XDG_RUNTIME_DIR` is not set and the default user session path of
+  `/run/user/<uid>` does not exist. Note that the `$TMPDIR/singularity-oci-<uid>`
+  directory is shared between concurrent `--oci` mode invocations, and will not
+  be removed on exit - an empty directory will remain.
 
 ## 4.2.1 \[2024-09-13\]
 

cmd/singularity-buildkitd/main.go

@@ -9,21 +9,27 @@ import (
 	"context"
 	"os"
 
+	"github.com/spf13/pflag"
 	bkdaemon "github.com/sylabs/singularity/v4/internal/pkg/build/buildkit/daemon"
 	"github.com/sylabs/singularity/v4/internal/pkg/buildcfg"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
 	"github.com/sylabs/singularity/v4/pkg/util/singularityconf"
 )
 
+var (
+	rootDir  string
+	arch     string
+	bkSocket string
+)
+
 func main() {
-	if len(os.Args) < 2 || len(os.Args) > 3 {
-		sylog.Fatalf("%s: usage: %s <socket-uri> [architecture]", bkdaemon.DaemonName, os.Args[0])
-	}
+	pflag.StringVar(&rootDir, "root", "", "buildkitd root directory")
+	pflag.StringVar(&arch, "arch", "", "build architecture")
+	pflag.StringVar(&bkSocket, "socket", "", "socket path")
+	pflag.Parse()
 
-	bkSocket := os.Args[1]
-	bkArch := ""
-	if len(os.Args) == 3 {
-		bkArch = os.Args[2]
+	if bkSocket == "" {
+		sylog.Fatalf("%s: usage: %s [--root <dir>] [--arch <arch>] --socket <socket-uri>", bkdaemon.DaemonName, os.Args[0])
 	}
 
 	sylog.Debugf("%s: parsing configuration file %s", bkdaemon.DaemonName, buildcfg.SINGULARITY_CONF_FILE)
@@ -34,8 +40,10 @@ func main() {
 	singularityconf.SetCurrentConfig(config)
 
 	daemonOpts := &bkdaemon.Opts{
-		ReqArch: bkArch,
+		ReqArch: arch,
+		RootDir: rootDir,
 	}
+
 	if err := bkdaemon.Run(context.Background(), daemonOpts, bkSocket); err != nil {
 		sylog.Fatalf("%s: %v", bkdaemon.DaemonName, err)
 	}

internal/pkg/build/buildkit/client/client.go

@@ -47,14 +47,17 @@ import (
 	"github.com/sylabs/singularity/v4/internal/pkg/ociplatform"
 	"github.com/sylabs/singularity/v4/internal/pkg/remote/credential/ociauth"
 	"github.com/sylabs/singularity/v4/internal/pkg/util/bin"
+	fsoverlay "github.com/sylabs/singularity/v4/internal/pkg/util/fs/overlay"
+	"github.com/sylabs/singularity/v4/internal/pkg/util/rootless"
+	"github.com/sylabs/singularity/v4/pkg/syfs"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
 	"golang.org/x/sync/errgroup"
 )
 
 const (
 	buildTag          = "tag"
 	bkDefaultSocket   = "unix:///run/buildkit/buildkitd.sock"
-	bkLaunchTimeout   = 120 * time.Second
+	bkLaunchTimeout   = 10 * time.Second
 	bkShutdownTimeout = 10 * time.Second
 	bkMinVersion      = "v0.12.3"
 )
@@ -161,13 +164,32 @@ func startBuildkitd(ctx context.Context, opts *Opts) (bkSocket string, cleanup f
 		return "", nil, err
 	}
 
-	bkSocket = generateSocketAddress()
+	bkSocket, err = generateSocketAddress()
+	if err != nil {
+		return "", nil, err
+	}
+
+	args := []string{}
+	tmpRoot := ""
+	// Check the user .singularity dir is in a location supporting overlayfs etc. If not, use a tmpdir.
+	if err := fsoverlay.CheckUpper(syfs.ConfigDir()); err != nil {
+		tmpRoot, err = os.MkdirTemp("", "singularity-buildkitd-")
+		if err != nil {
+			sylog.Fatalf("while creating singularity-buildkitd temporary root dir: %v", err)
+		}
+		if err := fsoverlay.CheckUpper(tmpRoot); err != nil {
+			sylog.Fatalf("Temporary directory does not support buildkit. Please set $TMPDIR to a local filesystem.")
+		}
+
+		sylog.Warningf("~/.singularity filesystem does not support buildkit. Using temporary directory %s. Layers will not be cached for future builds.", tmpRoot)
+		args = append(args, "--root="+tmpRoot)
+	}
 
-	// singularity-buildkitd <socket-uri> [architecture]
-	args := []string{bkSocket}
 	if opts.ReqArch != "" {
-		args = append(args, opts.ReqArch)
+		args = append(args, "--arch="+opts.ReqArch)
 	}
+	args = append(args, "--socket="+bkSocket)
+
 	cmd := exec.CommandContext(ctx, bkCmd, args...)
 	cmd.WaitDelay = bkShutdownTimeout
 	cmd.Cancel = func() error {
@@ -182,8 +204,15 @@ func startBuildkitd(ctx context.Context, opts *Opts) (bkSocket string, cleanup f
 			sylog.Errorf("while canceling buildkit daemon process: %v", err)
 		}
 		cmd.Wait()
+		if tmpRoot != "" {
+			sylog.Warningf("removing singularity-buildkitd temporary directory %s", tmpRoot)
+			if err := os.RemoveAll(tmpRoot); err != nil {
+				sylog.Errorf("while removing singularity-buildkitd temp dir: %v", err)
+			}
+		}
 	}
 
+	sylog.Debugf("starting %s %v", bkCmd, args)
 	if err := cmd.Start(); err != nil {
 		return "", nil, err
 	}
@@ -378,15 +407,22 @@ func writeDockerTar(r io.Reader, outputFile *os.File) error {
 	return err
 }
 
-func generateSocketAddress() string {
+func generateSocketAddress() (string, error) {
+	uid, err := rootless.Getuid()
+	if err != nil {
+		return "", err
+	}
+
 	socketPath := "/run/singularity-buildkitd"
+	if uid == 0 {
+		return "unix://" + filepath.Join(socketPath, fmt.Sprintf("singularity-buildkitd-%d.sock", os.Getpid())), nil
+	}
 
-	//  pam_systemd sets XDG_RUNTIME_DIR but not other dirs.
 	xdgRuntimeDir := os.Getenv("XDG_RUNTIME_DIR")
-	if xdgRuntimeDir != "" {
-		dirs := strings.Split(xdgRuntimeDir, ":")
-		socketPath = filepath.Join(dirs[0], "singularity-buildkitd")
+	if xdgRuntimeDir == "" {
+		return "", fmt.Errorf("rootless build --oci requires XDG_RUNTIME_DIR is set")
 	}
-
-	return "unix://" + filepath.Join(socketPath, fmt.Sprintf("singularity-buildkitd-%d.sock", os.Getpid()))
+	dirs := strings.Split(xdgRuntimeDir, ":")
+	socketPath = filepath.Join(dirs[0], "singularity-buildkitd")
+	return "unix://" + filepath.Join(socketPath, fmt.Sprintf("singularity-buildkitd-%d.sock", os.Getpid())), nil
 }

internal/pkg/build/buildkit/daemon/daemon.go

@@ -80,6 +80,8 @@ const DaemonName = "singularity-buildkitd"
 type Opts struct {
 	// Requested build architecture
 	ReqArch string
+	// Override the location of the singularity-buildkitd root with specified directory
+	RootDir string
 }
 
 type workerInitializerOpt struct {
@@ -149,7 +151,7 @@ func waitLock(ctx context.Context, lockPath string) (*flock.Flock, error) {
 func Run(ctx context.Context, opts *Opts, socketPath string) error {
 	// If we need to, enter a new cgroup now, to workaround an issue with crun container cgroup creation (#1538).
 	if err := oci.CrunNestCgroup(); err != nil {
-		sylog.Fatalf("%s: while applying crun cgroup workaround: %v", DaemonName, err)
+		return fmt.Errorf("%s: while applying crun cgroup workaround: %v", DaemonName, err)
 	}
 
 	cfg, err := config.LoadFile(defaultConfigPath())
@@ -166,6 +168,14 @@ func Run(ctx context.Context, opts *Opts, socketPath string) error {
 
 	server := grpc.NewServer()
 
+	if opts.RootDir != "" {
+		ptr := func(v bool) *bool {
+			return &v
+		}
+		cfg.Root = opts.RootDir
+		cfg.Workers.OCI.GC = ptr(false)
+	}
+
 	// relative path does not work with nightlyone/lockfile
 	root, err := filepath.Abs(cfg.Root)
 	if err != nil {
@@ -181,7 +191,7 @@ func Run(ctx context.Context, opts *Opts, socketPath string) error {
 	sylog.Debugf("%s: path for buildkitd lock file: %s", DaemonName, lockPath)
 	lock, err := waitLock(ctx, lockPath)
 	if err != nil {
-		sylog.Fatalf("%s: while creating lock file: %v", DaemonName, err)
+		return fmt.Errorf("%s: while creating lock file: %v", DaemonName, err)
 	}
 	defer func() {
 		lock.Unlock()

internal/pkg/runtime/launcher/oci/oci_linux.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2018-2022, Sylabs Inc. All rights reserved.
+// Copyright (c) 2018-2024, Sylabs Inc. All rights reserved.
 // This software is licensed under a 3-clause BSD license. Please consult the
 // LICENSE.md file distributed with the sources of this project regarding your
 // rights to use or distribute this software.
@@ -13,9 +13,11 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"syscall"
 	"time"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/sylabs/singularity/v4/internal/pkg/cgroups"
 	"github.com/sylabs/singularity/v4/internal/pkg/util/bin"
 	"github.com/sylabs/singularity/v4/internal/pkg/util/fs"
 	"github.com/sylabs/singularity/v4/internal/pkg/util/rootless"
@@ -58,10 +60,52 @@ func runtimeStateDir() (path string, err error) {
 	if err != nil {
 		return "", err
 	}
+
+	// Root - use our own /run directory
 	if u.Uid == "0" {
 		return "/run/singularity-oci", nil
 	}
-	return fmt.Sprintf("/run/user/%s/singularity-oci", u.Uid), nil
+
+	// Prefer XDG_RUNTIME_DIR for non-root, if set and usable.
+	if ok, _ := cgroups.HasXDGRuntimeDir(); ok {
+		d := filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "singularity-oci")
+		sylog.Debugf("Using XDG_RUNTIME_DIR for runtime state (%s)", d)
+		return d, nil
+	}
+
+	// If no XDG_RUNTIME_DIR, then try standard user session directory location.
+	runDir := fmt.Sprintf("/run/user/%s/", u.Uid)
+	if fs.IsDir(runDir) {
+		d := filepath.Join(runDir, "singularity-oci")
+		sylog.Debugf("Using /run/user default for runtime state (%s)", d)
+		return d, nil
+	}
+
+	// If standard user session directory not available, use TMPDIR as a last resort.
+	runDir = filepath.Join(os.TempDir(), "singularity-oci-"+u.Uid)
+	sylog.Infof("No /run/user session directory for user. Using %q for runtime state.", runDir)
+
+	// Create if not present
+	st, err := os.Stat(runDir)
+	if os.IsNotExist(err) {
+		return runDir, os.Mkdir(runDir, 0o700)
+	}
+	if err != nil {
+		return "", err
+	}
+
+	// If it exists, verify it's a directory with correct ownership, perms.
+	if !st.IsDir() {
+		return "", fmt.Errorf("%s exists, but is not a directory", runDir)
+	}
+	if st.Sys().(*syscall.Stat_t).Uid != uint32(os.Geteuid()) { //nolint:forcetypeassert
+		return "", fmt.Errorf("%s exists, but is not owned by correct user", runDir)
+	}
+	if st.Mode().Perm() != 0o700 {
+		return "", fmt.Errorf("%s exists, but does not have correct permissions (700)", runDir)
+	}
+
+	return runDir, nil
 }
 
 // stateDir returns the path to container state handled by conmon/singularity