From 81609a45f181e4616b1ebef8b6634e2c47ae604f Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Fri, 22 Nov 2024 09:48:19 +0000 Subject: [PATCH 1/3] oci: buildkit TMPDIR fallback Detect whether the `~/singularity.d/singularity-buildkit` location can be used for buildkit. Full overlay support is required. If not compatible, try falling back to a temporary directory with a warning that there will be no persistent cache. If TMPDIR is not compatible, give a sensible fatal error. Fixes #3382 --- CHANGELOG.md | 2 ++ cmd/singularity-buildkitd/main.go | 24 +++++++++----- internal/pkg/build/buildkit/client/client.go | 33 +++++++++++++++++--- internal/pkg/build/buildkit/daemon/daemon.go | 14 +++++++-- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76874f4d41..697e88beb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - 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. ## 4.2.1 \[2024-09-13\] diff --git a/cmd/singularity-buildkitd/main.go b/cmd/singularity-buildkitd/main.go index 5146f66db8..c3d3efd643 100644 --- a/cmd/singularity-buildkitd/main.go +++ b/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 [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 ] [--arch ] --socket ", 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) } diff --git a/internal/pkg/build/buildkit/client/client.go b/internal/pkg/build/buildkit/client/client.go index c335461942..480827a6cd 100644 --- a/internal/pkg/build/buildkit/client/client.go +++ b/internal/pkg/build/buildkit/client/client.go @@ -47,6 +47,8 @@ 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/pkg/syfs" "github.com/sylabs/singularity/v4/pkg/sylog" "golang.org/x/sync/errgroup" ) @@ -54,7 +56,7 @@ import ( 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" ) @@ -163,11 +165,27 @@ func startBuildkitd(ctx context.Context, opts *Opts) (bkSocket string, cleanup f bkSocket = generateSocketAddress() - // singularity-buildkitd [architecture] - args := []string{bkSocket} + 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) + } + 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 +200,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 } diff --git a/internal/pkg/build/buildkit/daemon/daemon.go b/internal/pkg/build/buildkit/daemon/daemon.go index 6602f5e17a..c27479a103 100644 --- a/internal/pkg/build/buildkit/daemon/daemon.go +++ b/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() From 984e2723b93a5df9fd78ab23509df0c4cf20d9c1 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Thu, 28 Nov 2024 14:24:29 +0000 Subject: [PATCH 2/3] fix: improve build --oci error for missing XDG_RUNTIME_DIR The buildkit code that we call as a dependency uses XDG_RUNTIME_DIR in multiple locations, so we cannot handle it being unset. Add a specific error for rootless build --oci with XDG_RUNTIME_DIR unset, so it is clear that this env var must be set. I don't think we should be setting XDG_RUNTIME_DIR ourselves, to a tmpdir we create. There is a strong assumption that this value is a genuine session specific directory that is configured by systemd, or by the user manually. Fixes #3410 --- CHANGELOG.md | 2 ++ internal/pkg/build/buildkit/client/client.go | 27 ++++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 697e88beb5..bfe154dd9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ 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. ## 4.2.1 \[2024-09-13\] diff --git a/internal/pkg/build/buildkit/client/client.go b/internal/pkg/build/buildkit/client/client.go index 480827a6cd..d9d9dbf711 100644 --- a/internal/pkg/build/buildkit/client/client.go +++ b/internal/pkg/build/buildkit/client/client.go @@ -48,6 +48,7 @@ import ( "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" @@ -163,7 +164,10 @@ 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 := "" @@ -403,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 } From f5010b13c16167fb7a53e758aa8719a2979feda7 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Wed, 20 Nov 2024 13:55:38 +0000 Subject: [PATCH 3/3] feat: accommodate missing /run/user dir in OCI mode On systems where `pam-systemd` is disabled, or there is other configuration preventing the creation of a /run/user/ directory, we need to use a different location for the OCI runtime state. Change the runtimeStateDir handling to: * Use XDG_RUNTIME_DIR if set. * Otherwise, try to use /run/user/. * Otherwise, use a location under $TMPDIR. Fixes #3393 --- CHANGELOG.md | 10 ++++ .../pkg/runtime/launcher/oci/oci_linux.go | 48 ++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe154dd9f..b6ac012ba0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,16 @@ - 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-` for runtime state on systems where + `$XDG_RUNTIME_DIR` is not set and the default user session path of + `/run/user/` does not exist. Note that the `$TMPDIR/singularity-oci-` + 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\] ### Bug Fixes diff --git a/internal/pkg/runtime/launcher/oci/oci_linux.go b/internal/pkg/runtime/launcher/oci/oci_linux.go index 0d280af7cc..f9843ac4fd 100644 --- a/internal/pkg/runtime/launcher/oci/oci_linux.go +++ b/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