diff --git a/CHANGELOG.md b/CHANGELOG.md index b11146af82..6ec7fc26e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ - Correctly escape ENV vars when importing OCI containers to native SIF, so that they match podman / docker behaviour. - Clarify error when trying to build --oci from a non-Dockerfile spec. +- When images are pulled implicitly by actions (run/shell/exec...), and the + cache is disabled, correctly clean up the temporary files. ### New Features & Functionality diff --git a/cmd/internal/cli/actions.go b/cmd/internal/cli/actions.go index 0fc6f0defd..4035919bc4 100644 --- a/cmd/internal/cli/actions.go +++ b/cmd/internal/cli/actions.go @@ -1,5 +1,5 @@ // Copyright (c) 2020, Control Command Inc. All rights reserved. -// Copyright (c) 2018-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2026, 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. @@ -11,6 +11,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "github.com/spf13/cobra" @@ -53,6 +54,7 @@ type contextKey string const ( keyOrigImageURI contextKey = "origImageURI" + keyPullTempDir contextKey = "pullTempDir" ) // actionPreRun will: @@ -60,7 +62,7 @@ const ( // - and implement flag inferences for: // --compat // --hostname -// - run replaceURIWithImage; +// - retrieve remote images to the cache or a temporary directory for execution func actionPreRun(cmd *cobra.Command, args []string) { // For compatibility - we still set USER_PATH so it will be visible in the // container, and can be used there if needed. USER_PATH is not used by @@ -87,39 +89,76 @@ func actionPreRun(cmd *cobra.Command, args []string) { utsNamespace = true } - origImageURI := replaceURIWithImage(cmd.Context(), cmd, args) + // Store the original image URI in the command context, so it can be used by + // any fallback logic. + origImageURI := args[0] cmd.SetContext(context.WithValue(cmd.Context(), keyOrigImageURI, &origImageURI)) -} -func handleOCI(ctx context.Context, imgCache *cache.Handle, cmd *cobra.Command, pullFrom string) (string, error) { - ociAuth, err := makeOCICredentials(cmd) - if err != nil { - sylog.Fatalf("While creating Docker credentials: %v", err) - } + // Replace remote URI with a local image path, pulling to cache or a + // temporary directory as needed. + localImage, pullTempDir := uriToImage(cmd.Context(), cmd, origImageURI) + args[0] = localImage - pullOpts := oci.PullOptions{ - TmpDir: tmpDir, - OciAuth: ociAuth, - DockerHost: dockerHost, - NoHTTPS: noHTTPS, - OciSif: isOCI, - KeepLayers: keepLayers, - Platform: getOCIPlatform(), - ReqAuthFile: reqAuthFile, - } + // Track the pullTempDir (if set) in the context, so it can be cleaned up on container exit. + cmd.SetContext(context.WithValue(cmd.Context(), keyPullTempDir, &pullTempDir)) +} - return oci.Pull(ctx, imgCache, pullFrom, pullOpts) +func uriToCacheImage(ctx context.Context, refType string, cmd *cobra.Command, imgCache *cache.Handle, pullFrom string) (string, error) { + switch refType { + case uri.Library: + return handleLibrary(ctx, imgCache, "", pullFrom) + case uri.Oras: + ociAuth, err := makeOCICredentials(cmd) + if err != nil { + return "", fmt.Errorf("while creating docker credentials: %v", err) + } + return oras.PullToCache(ctx, imgCache, pullFrom, ociAuth, reqAuthFile) + case uri.Shub: + return shub.PullToCache(ctx, imgCache, pullFrom, noHTTPS) + case ociimage.SupportedTransport(refType): + return handleOCI(ctx, cmd, imgCache, "", pullFrom) + case uri.HTTP: + return net.PullToCache(ctx, imgCache, pullFrom) + case uri.HTTPS: + return net.PullToCache(ctx, imgCache, pullFrom) + default: + return "", fmt.Errorf("unsupported transport type: %s", refType) + } } -func handleOras(ctx context.Context, imgCache *cache.Handle, cmd *cobra.Command, pullFrom string) (string, error) { - ociAuth, err := makeOCICredentials(cmd) +func uriToTempImage(ctx context.Context, refType string, cmd *cobra.Command, imgCache *cache.Handle, pullFrom string) (string, string, error) { + pullTempDir, err := os.MkdirTemp(tmpDir, "singularity-action-pull-") if err != nil { - return "", fmt.Errorf("while creating docker credentials: %v", err) + return "", "", fmt.Errorf("unable to create temporary directory: %w", err) } - return oras.Pull(ctx, imgCache, pullFrom, tmpDir, ociAuth, reqAuthFile) + tmpImage := filepath.Join(pullTempDir, "image") + sylog.Debugf("Cache disabled, pulling image to temporary file: %s", tmpImage) + + switch refType { + case uri.Library: + _, err = handleLibrary(ctx, imgCache, tmpImage, pullFrom) + case uri.Oras: + ociAuth, authErr := makeOCICredentials(cmd) + if authErr != nil { + return "", "", fmt.Errorf("while creating docker credentials: %v", authErr) + } + _, err = oras.PullToFile(ctx, imgCache, tmpImage, pullFrom, ociAuth, reqAuthFile) + case uri.Shub: + _, err = shub.PullToFile(ctx, imgCache, tmpImage, pullFrom, noHTTPS) + case ociimage.SupportedTransport(refType): + _, err = handleOCI(ctx, cmd, imgCache, tmpImage, pullFrom) + case uri.HTTP: + _, err = net.PullToFile(ctx, imgCache, tmpImage, pullFrom) + case uri.HTTPS: + _, err = net.PullToFile(ctx, imgCache, tmpImage, pullFrom) + default: + return "", "", fmt.Errorf("unsupported transport type: %s", refType) + } + + return tmpImage, pullTempDir, err } -func handleLibrary(ctx context.Context, imgCache *cache.Handle, pullFrom string) (string, error) { +func handleLibrary(ctx context.Context, imgCache *cache.Handle, tmpImage, pullFrom string) (string, error) { r, err := library.NormalizeLibraryRef(pullFrom) if err != nil { return "", err @@ -149,50 +188,71 @@ func handleLibrary(ctx context.Context, imgCache *cache.Handle, pullFrom string) TmpDir: tmpDir, Platform: getOCIPlatform(), } - return library.Pull(ctx, imgCache, r, pullOpts) -} -func handleShub(ctx context.Context, imgCache *cache.Handle, pullFrom string) (string, error) { - return shub.Pull(ctx, imgCache, pullFrom, tmpDir, noHTTPS) + var imagePath string + if tmpImage == "" { + imagePath, err = library.PullToCache(ctx, imgCache, r, pullOpts) + } else { + imagePath, err = library.PullToFile(ctx, imgCache, tmpImage, r, pullOpts) + } + + if err != nil && err != library.ErrLibraryPullUnsigned { + return "", err + } + if err == library.ErrLibraryPullUnsigned { + sylog.Warningf("Skipping container verification") + } + return imagePath, nil } -func handleNet(ctx context.Context, imgCache *cache.Handle, pullFrom string) (string, error) { - return net.Pull(ctx, imgCache, pullFrom, tmpDir) +func handleOCI(ctx context.Context, cmd *cobra.Command, imgCache *cache.Handle, tmpImage, pullFrom string) (string, error) { + ociAuth, err := makeOCICredentials(cmd) + if err != nil { + sylog.Fatalf("While creating Docker credentials: %v", err) + } + + pullOpts := oci.PullOptions{ + TmpDir: tmpDir, + OciAuth: ociAuth, + DockerHost: dockerHost, + NoHTTPS: noHTTPS, + OciSif: isOCI, + KeepLayers: keepLayers, + Platform: getOCIPlatform(), + ReqAuthFile: reqAuthFile, + } + + if tmpImage == "" { + return oci.PullToCache(ctx, imgCache, pullFrom, pullOpts) + } + return oci.PullToFile(ctx, imgCache, tmpImage, pullFrom, pullOpts) } -func replaceURIWithImage(ctx context.Context, cmd *cobra.Command, args []string) string { - origImageURI := args[0] - t, _ := uri.Split(origImageURI) +// uriToImage will pull a remote image to the cache, or a temporary directory if +// the cache is disabled. It returns a path to the pulled image, and the +// temporary directory that should be removed when the container exits, where +// applicable. +func uriToImage(ctx context.Context, cmd *cobra.Command, origImageURI string) (imagePath, tempDir string) { + refType, _ := uri.Split(origImageURI) // If joining an instance (instance://xxx), or we have a bare filename then // no retrieval / conversion is required. - if t == "instance" || t == "" { - return origImageURI + if refType == "instance" || refType == "" { + return origImageURI, "" } - var image string - var err error - - // Create a cache handle only when we know we are using a URI imgCache := getCacheHandle(cache.Config{Disable: disableCache}) if imgCache == nil { sylog.Fatalf("failed to create a new image cache handle") } - switch t { - case uri.Library: - image, err = handleLibrary(ctx, imgCache, origImageURI) - case uri.Oras: - image, err = handleOras(ctx, imgCache, cmd, origImageURI) - case uri.Shub: - image, err = handleShub(ctx, imgCache, origImageURI) - case ociimage.SupportedTransport(t): - image, err = handleOCI(ctx, imgCache, cmd, origImageURI) - case uri.HTTP: - image, err = handleNet(ctx, imgCache, origImageURI) - case uri.HTTPS: - image, err = handleNet(ctx, imgCache, origImageURI) - default: - sylog.Fatalf("Unsupported transport type: %s", t) + // If the cache is disabled, then we pull to a temporary location, which + // will need to be removed on container exit. Otherwise, we pull to the + // cache and run directly from there. + var err error + if disableCache { + imagePath, tempDir, err = uriToTempImage(ctx, refType, cmd, imgCache, origImageURI) + } else { + imagePath, err = uriToCacheImage(ctx, refType, cmd, imgCache, origImageURI) } // If we are in OCI mode, then we can still attempt to run from a directory @@ -206,16 +266,26 @@ func replaceURIWithImage(ctx context.Context, cmd *cobra.Command, args []string) } sylog.Warningf("%v", err) sylog.Warningf("OCI-SIF could not be created, falling back to unpacking OCI bundle in temporary sandbox dir") - return origImageURI + return origImageURI, "" } if err != nil { sylog.Fatalf("Unable to handle %s uri: %v", origImageURI, err) } - args[0] = image + return imagePath, tempDir +} - return origImageURI +func pullTempDirFromContext(ctx context.Context) string { + pullTempDirPtr := ctx.Value(keyPullTempDir) + if pullTempDirPtr != nil { + pullTempDir, ok := pullTempDirPtr.(*string) + if !ok { + sylog.Fatalf("unable to recover pull temp dir (expected string, found: %T) from context", pullTempDirPtr) + } + return *pullTempDir + } + return "" } // ExecCmd represents the exec command @@ -227,10 +297,11 @@ var ExecCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { // singularity exec [args...] ep := launcher.ExecParams{ - Image: args[0], - Action: "exec", - Process: args[1], - Args: args[2:], + Image: args[0], + PullTempDir: pullTempDirFromContext(cmd.Context()), + Action: "exec", + Process: args[1], + Args: args[2:], } if err := launchContainer(cmd, ep); err != nil { sylog.Fatalf("%s", err) @@ -252,8 +323,9 @@ var ShellCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { // singularity shell ep := launcher.ExecParams{ - Image: args[0], - Action: "shell", + Image: args[0], + PullTempDir: pullTempDirFromContext(cmd.Context()), + Action: "shell", } if err := launchContainer(cmd, ep); err != nil { sylog.Fatalf("%s", err) @@ -275,9 +347,10 @@ var RunCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { // singularity run [args...] ep := launcher.ExecParams{ - Image: args[0], - Action: "run", - Args: args[1:], + Image: args[0], + PullTempDir: pullTempDirFromContext(cmd.Context()), + Action: "run", + Args: args[1:], } if err := launchContainer(cmd, ep); err != nil { sylog.Fatalf("%s", err) @@ -299,9 +372,10 @@ var TestCmd = &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { // singularity test [args...] ep := launcher.ExecParams{ - Image: args[0], - Action: "test", - Args: args[1:], + Image: args[0], + PullTempDir: pullTempDirFromContext(cmd.Context()), + Action: "test", + Args: args[1:], } if err := launchContainer(cmd, ep); err != nil { sylog.Fatalf("%s", err) @@ -396,6 +470,7 @@ func launchContainer(cmd *cobra.Command, ep launcher.ExecParams) error { launcher.OptNoCompat(noCompat), launcher.OptTmpSandbox(tmpSandbox), launcher.OptNoTmpSandbox(noTmpSandbox), + launcher.OptPullTempDir(ep.PullTempDir), } // Explicitly use the interface type here, as we will add alternative launchers later... diff --git a/e2e/actions/actions.go b/e2e/actions/actions.go index 2db65341fd..2975bc07be 100644 --- a/e2e/actions/actions.go +++ b/e2e/actions/actions.go @@ -2960,6 +2960,81 @@ func (c actionTests) actionAuthTester(t *testing.T, withCustomAuthFile bool, pro } } +// actionCleanup tests that an image pulled implicitly to a temp dir, due to +// --disable-cache, is cleaned up. +func (c actionTests) actionCleanup(t *testing.T) { + e2e.EnsureORASImage(t, c.env) + + tests := []struct { + name string + profile e2e.Profile + flags []string + }{ + { + name: "User", + profile: e2e.UserProfile, + flags: []string{}, + }, + { + name: "UserTmpSandbox", + profile: e2e.UserProfile, + flags: []string{"--tmp-sandbox"}, + }, + { + name: "UserNamespace", + profile: e2e.UserNamespaceProfile, + flags: []string{}, + }, + { + name: "UserNamespaceTmpSandbox", + profile: e2e.UserNamespaceProfile, + flags: []string{"--tmp-sandbox"}, + }, + { + name: "Fakeroot", + profile: e2e.FakerootProfile, + }, + { + name: "FakerootTmpSandbox", + profile: e2e.FakerootProfile, + flags: []string{"--tmp-sandbox"}, + }, + { + name: "OCIUser", + profile: e2e.OCIUserProfile, + }, + { + name: "OCIFakeroot", + profile: e2e.OCIFakerootProfile, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir, cleanup := e2e.MakeTempDir(t, c.env.TestDir, "action-cleanup-", "") + defer cleanup(t) + tempEnv := "SINGULARITY_TMPDIR=" + tempDir + + c.env.RunSingularity( + t, + e2e.WithProfile(tt.profile), + e2e.WithCommand("exec"), + e2e.WithArgs(append(tt.flags, "--disable-cache", c.env.OrasTestImage, "/bin/true")...), + e2e.WithEnv(append(os.Environ(), tempEnv)), + e2e.ExpectExit(0), + ) + + entries, err := os.ReadDir(tempDir) + if err != nil { + t.Fatalf("could not read contents of temp dir %q: %s", tempDir, err) + } + if len(entries) != 0 { + t.Errorf("SINGULARITY_TMPDIR had %d entries after execution, expected 0\n%v", len(entries), entries) + } + }) + } +} + // E2ETests is the main func to trigger the test suite func E2ETests(env e2e.TestEnv) testhelper.Tests { c := actionTests{ @@ -3015,6 +3090,7 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests { "relWorkdirScratch": np(c.relWorkdirScratch), // test relative --workdir with --scratch "ociRelWorkdirScratch": np(c.actionOciRelWorkdirScratch), // test relative --workdir with --scratch in OCI mode "auth": np(c.actionAuth), // tests action cmds w/authenticated pulls from OCI registries + "actionCleanup": c.actionCleanup, // test cleanup of temporary files from action // // OCI Runtime Mode // diff --git a/internal/pkg/build/sources/conveyorPacker_library.go b/internal/pkg/build/sources/conveyorPacker_library.go index 4828542a59..7dd1c05dca 100644 --- a/internal/pkg/build/sources/conveyorPacker_library.go +++ b/internal/pkg/build/sources/conveyorPacker_library.go @@ -1,5 +1,5 @@ // Copyright (c) 2020, Control Command Inc. All rights reserved. -// Copyright (c) 2018-2024, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2026, 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. @@ -8,7 +8,9 @@ package sources import ( "context" + "errors" "fmt" + "path/filepath" golog "github.com/go-log/log" @@ -72,8 +74,15 @@ func (cp *LibraryConveyorPacker) Get(ctx context.Context, b *types.Bundle) (err TmpDir: cp.b.TmpDir, Platform: cp.b.Opts.Platform, } - imagePath, err := library.Pull(ctx, b.Opts.ImgCache, imageRef, pullOpts) - if err != nil { + + var imagePath string + if b.Opts.ImgCache.IsDisabled() { + imageTemp := filepath.Join(b.TmpDir, "library-image") + imagePath, err = library.PullToFile(ctx, b.Opts.ImgCache, imageTemp, imageRef, pullOpts) + } else { + imagePath, err = library.PullToCache(ctx, b.Opts.ImgCache, imageRef, pullOpts) + } + if err != nil && !errors.Is(err, library.ErrLibraryPullUnsigned) { return fmt.Errorf("while fetching library image: %v", err) } diff --git a/internal/pkg/build/sources/conveyorPacker_oras.go b/internal/pkg/build/sources/conveyorPacker_oras.go index 11912ce618..371bab8db4 100644 --- a/internal/pkg/build/sources/conveyorPacker_oras.go +++ b/internal/pkg/build/sources/conveyorPacker_oras.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2024, Sylabs Inc. All rights reserved. +// Copyright (c) 2020-2026, 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. @@ -8,6 +8,7 @@ package sources import ( "context" "fmt" + "path/filepath" "github.com/google/go-containerregistry/pkg/authn" "github.com/sylabs/singularity/v4/internal/pkg/client/oras" @@ -39,7 +40,13 @@ func (cp *OrasConveyorPacker) Get(ctx context.Context, b *types.Bundle) (err err } } - imagePath, err := oras.Pull(ctx, b.Opts.ImgCache, fullRef, b.Opts.TmpDir, authConfig, b.Opts.DockerAuthFile) + var imagePath string + if b.Opts.ImgCache.IsDisabled() { + imageTemp := filepath.Join(b.TmpDir, "library-image") + imagePath, err = oras.PullToFile(ctx, b.Opts.ImgCache, imageTemp, fullRef, authConfig, b.Opts.DockerAuthFile) + } else { + imagePath, err = oras.PullToCache(ctx, b.Opts.ImgCache, fullRef, authConfig, b.Opts.DockerAuthFile) + } if err != nil { return fmt.Errorf("while fetching library image: %v", err) } diff --git a/internal/pkg/build/sources/conveyorPacker_shub.go b/internal/pkg/build/sources/conveyorPacker_shub.go index c82fa3ed94..64d1ac6d75 100644 --- a/internal/pkg/build/sources/conveyorPacker_shub.go +++ b/internal/pkg/build/sources/conveyorPacker_shub.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2024, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2026, 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. @@ -8,6 +8,7 @@ package sources import ( "context" "fmt" + "path/filepath" shub "github.com/sylabs/singularity/v4/internal/pkg/client/shub" "github.com/sylabs/singularity/v4/pkg/build/types" @@ -28,7 +29,13 @@ func (cp *ShubConveyorPacker) Get(ctx context.Context, b *types.Bundle) (err err src := `shub://` + b.Recipe.Header["from"] - imagePath, err := shub.Pull(ctx, b.Opts.ImgCache, src, b.Opts.TmpDir, b.Opts.NoHTTPS) + var imagePath string + if b.Opts.ImgCache.IsDisabled() { + imageTemp := filepath.Join(b.TmpDir, "library-image") + imagePath, err = shub.PullToFile(ctx, b.Opts.ImgCache, imageTemp, src, b.Opts.NoHTTPS) + } else { + imagePath, err = shub.PullToCache(ctx, b.Opts.ImgCache, src, b.Opts.NoHTTPS) + } if err != nil { return fmt.Errorf("while fetching library image: %v", err) } diff --git a/internal/pkg/client/library/pull.go b/internal/pkg/client/library/pull.go index d8531d2a13..e661d92917 100644 --- a/internal/pkg/client/library/pull.go +++ b/internal/pkg/client/library/pull.go @@ -159,24 +159,17 @@ func pullOCI(ctx context.Context, imgCache *cache.Handle, directTo string, pullF return ocisif.PullOCISIF(ctx, imgCache, directTo, pullRef, ocisifOpts) } -// Pull will pull a library image to the cache or direct to a temporary file if cache is disabled -func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom *scslibrary.Ref, opts PullOptions) (imagePath string, err error) { - directTo := "" - +// PullToCache will pull a library image to the cache, returning the path to the cached image. +func PullToCache(ctx context.Context, imgCache *cache.Handle, pullFrom *scslibrary.Ref, opts PullOptions) (imagePath string, err error) { if imgCache.IsDisabled() { - file, err := os.CreateTemp(opts.TmpDir, "sbuild-tmp-cache-") - if err != nil { - return "", fmt.Errorf("unable to create tmp file: %v", err) - } - directTo = file.Name() - sylog.Infof("Downloading library image to tmp cache: %s", directTo) + return "", fmt.Errorf("cache is disabled, cannot pull to cache") } if opts.RequireOciSif { - return pullOCI(ctx, imgCache, directTo, pullFrom, opts) + return pullOCI(ctx, imgCache, "", pullFrom, opts) } - return pull(ctx, imgCache, directTo, pullFrom, opts) + return pull(ctx, imgCache, "", pullFrom, opts) } // PullToFile will pull a library image to the specified location, through the cache, or directly if cache is disabled diff --git a/internal/pkg/client/net/pull.go b/internal/pkg/client/net/pull.go index 6d84374a06..ab72278894 100644 --- a/internal/pkg/client/net/pull.go +++ b/internal/pkg/client/net/pull.go @@ -171,20 +171,13 @@ func pull(ctx context.Context, imgCache *cache.Handle, directTo, pullFrom string return imagePath, nil } -// Pull will pull a http(s) image to the cache or direct to a temporary file if cache is disabled -func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom string, tmpDir string) (imagePath string, err error) { - directTo := "" - +// PullToCache will pull a http(s) image to the cache, returning the path to the cached image. +func PullToCache(ctx context.Context, imgCache *cache.Handle, pullFrom string) (imagePath string, err error) { if imgCache.IsDisabled() { - file, err := os.CreateTemp(tmpDir, "sbuild-tmp-cache-") - if err != nil { - return "", fmt.Errorf("unable to create tmp file: %v", err) - } - directTo = file.Name() - sylog.Infof("Downloading library image to tmp cache: %s", directTo) + return "", fmt.Errorf("cache is disabled, cannot pull to cache") } - return pull(ctx, imgCache, directTo, pullFrom) + return pull(ctx, imgCache, "", pullFrom) } // PullToFile will pull an http(s) image to the specified location, through the cache, or directly if cache is disabled diff --git a/internal/pkg/client/oci/pull.go b/internal/pkg/client/oci/pull.go index 767586d9c4..6f14667d14 100644 --- a/internal/pkg/client/oci/pull.go +++ b/internal/pkg/client/oci/pull.go @@ -9,7 +9,6 @@ package oci import ( "context" "fmt" - "os" "github.com/google/go-containerregistry/pkg/authn" gccrv1 "github.com/google/go-containerregistry/pkg/v1" @@ -48,17 +47,12 @@ func transportOptions(opts PullOptions) *ociimage.TransportOptions { } } -// Pull will create a SIF / OCI-SIF image to the cache or direct to a temporary file if cache is disabled -func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom string, opts PullOptions) (imagePath string, err error) { - directTo := "" +// PullToCache will create a SIF / OCI-SIF image in the cache, and return the path to the cached image. +func PullToCache(ctx context.Context, imgCache *cache.Handle, pullFrom string, opts PullOptions) (imagePath string, err error) { if imgCache.IsDisabled() { - file, err := os.CreateTemp(opts.TmpDir, "sbuild-tmp-cache-") - if err != nil { - return "", fmt.Errorf("unable to create tmp file: %v", err) - } - directTo = file.Name() - sylog.Infof("Downloading image to tmp cache: %s", directTo) + return "", fmt.Errorf("cache is disabled, cannot pull to cache") } + if opts.OciSif { ocisifOpts := ocisif.PullOptions{ TmpDir: opts.TmpDir, @@ -71,10 +65,10 @@ func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom string, opts Pul KeepLayers: opts.KeepLayers, WithCosign: opts.WithCosign, } - return ocisif.PullOCISIF(ctx, imgCache, directTo, pullFrom, ocisifOpts) + return ocisif.PullOCISIF(ctx, imgCache, "", pullFrom, ocisifOpts) } - return pullNativeSIF(ctx, imgCache, directTo, pullFrom, opts) + return pullNativeSIF(ctx, imgCache, "", pullFrom, opts) } // PullToFile will create a SIF / OCI-SIF image from the specified oci URI and place it at the specified dest diff --git a/internal/pkg/client/oras/pull.go b/internal/pkg/client/oras/pull.go index ae8768debc..ba12e0349e 100644 --- a/internal/pkg/client/oras/pull.go +++ b/internal/pkg/client/oras/pull.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2020-2026, 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. @@ -8,7 +8,6 @@ package oras import ( "context" "fmt" - "os" "github.com/google/go-containerregistry/pkg/authn" "github.com/sylabs/singularity/v4/internal/pkg/cache" @@ -60,20 +59,12 @@ func pull(ctx context.Context, imgCache *cache.Handle, directTo, pullFrom string return imagePath, nil } -// Pull will pull an oras image to the cache or direct to a temporary file if cache is disabled -func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom, tmpDir string, ociAuth *authn.AuthConfig, reqAuthFile string) (imagePath string, err error) { - directTo := "" - +// PullToCache will pull an oras image to the cache, and return the path to the cached image. +func PullToCache(ctx context.Context, imgCache *cache.Handle, pullFrom string, ociAuth *authn.AuthConfig, reqAuthFile string) (imagePath string, err error) { if imgCache.IsDisabled() { - file, err := os.CreateTemp(tmpDir, "sbuild-tmp-cache-") - if err != nil { - return "", fmt.Errorf("unable to create tmp file: %v", err) - } - directTo = file.Name() - sylog.Infof("Downloading oras image to tmp cache: %s", directTo) + return "", fmt.Errorf("cache is disabled, cannot pull to cache") } - - return pull(ctx, imgCache, directTo, pullFrom, ociAuth, reqAuthFile) + return pull(ctx, imgCache, "", pullFrom, ociAuth, reqAuthFile) } // PullToFile will pull an oras image to the specified location, through the cache, or directly if cache is disabled diff --git a/internal/pkg/client/shub/pull.go b/internal/pkg/client/shub/pull.go index ffc982577c..6c8aecc2ad 100644 --- a/internal/pkg/client/shub/pull.go +++ b/internal/pkg/client/shub/pull.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2026, 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. @@ -173,20 +173,12 @@ func pull(ctx context.Context, imgCache *cache.Handle, directTo, pullFrom string return imagePath, nil } -// Pull will pull a shub image to the cache or direct to a temporary file if cache is disabled -func Pull(ctx context.Context, imgCache *cache.Handle, pullFrom, tmpDir string, noHTTPS bool) (imagePath string, err error) { - directTo := "" - +// PullToCache will pull a shub image to the cache, and return the path to the cached image. +func PullToCache(ctx context.Context, imgCache *cache.Handle, pullFrom string, noHTTPS bool) (imagePath string, err error) { if imgCache.IsDisabled() { - file, err := os.CreateTemp(tmpDir, "sbuild-tmp-cache-") - if err != nil { - return "", fmt.Errorf("unable to create tmp file: %v", err) - } - directTo = file.Name() - sylog.Infof("Downloading shub image to tmp cache: %s", directTo) + return "", fmt.Errorf("cache is disabled, cannot pull to cache") } - - return pull(ctx, imgCache, directTo, pullFrom, noHTTPS) + return pull(ctx, imgCache, "", pullFrom, noHTTPS) } // PullToFile will pull a shub image to the specified location, through the cache, or directly if cache is disabled diff --git a/internal/pkg/runtime/engine/singularity/cleanup_linux.go b/internal/pkg/runtime/engine/singularity/cleanup_linux.go index 499b2e763c..14103fb909 100644 --- a/internal/pkg/runtime/engine/singularity/cleanup_linux.go +++ b/internal/pkg/runtime/engine/singularity/cleanup_linux.go @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2024, Sylabs Inc. All rights reserved. +// Copyright (c) 2018-2026, 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. @@ -51,24 +51,44 @@ func (e *EngineOperations) CleanupContainer(ctx context.Context, _ error, _ sysc } } - if tempDir := e.EngineConfig.GetDeleteTempDir(); tempDir != "" && !e.EngineConfig.GetImageFuse() { - sylog.Verbosef("Removing image tempDir %s", tempDir) - sylog.Infof("Cleaning up image...") - - var err error - - if e.EngineConfig.GetFakeroot() && os.Getuid() != 0 { - // this is required when we are using SUID workflow - // because master process is not in the fakeroot - // context and can get permission denied error during - // image removal, so we execute "rm -rf /tmp/image" via - // the fakeroot engine - err = fakerootCleanup(tempDir) - } else { - err = os.RemoveAll(tempDir) + // If our image is not FUSE mounted then: + // + // * GetDeleteTempDir being set indicates the rootfs is in a temporary + // directory, which should be removed. + // * GetDeletePullTempDir being set indicates the image was implicitly + // pulled to a temporary directory, due to disabled cache, and this should + // be removed. + // + // If the image is FUSE mounted, this is handled in the HOST_CLEANUP + // instead. + if !e.EngineConfig.GetImageFuse() { + rmdirs := []string{} + if tempDir := e.EngineConfig.GetDeleteTempDir(); tempDir != "" { + rmdirs = append(rmdirs, tempDir) } - if err != nil { - sylog.Errorf("failed to delete container image tempDir %s: %s", tempDir, err) + if tempDir := e.EngineConfig.GetDeletePullTempDir(); tempDir != "" { + rmdirs = append(rmdirs, tempDir) + } + if len(rmdirs) > 0 { + sylog.Infof("Cleaning up image temporary dir(s)...") + } + + for _, tempDir := range rmdirs { + sylog.Verbosef("Removing %s", tempDir) + var err error + if e.EngineConfig.GetFakeroot() && os.Getuid() != 0 { + // this is required when we are using SUID workflow + // because master process is not in the fakeroot + // context and can get permission denied error during + // image removal, so we execute "rm -rf /tmp/image" via + // the fakeroot engine + err = fakerootCleanup(tempDir) + } else { + err = os.RemoveAll(tempDir) + } + if err != nil { + sylog.Errorf("failed to delete temporary dir %s: %s", tempDir, err) + } } } diff --git a/internal/pkg/runtime/engine/singularity/host_linux.go b/internal/pkg/runtime/engine/singularity/host_linux.go index 4419ccf011..3454a51626 100644 --- a/internal/pkg/runtime/engine/singularity/host_linux.go +++ b/internal/pkg/runtime/engine/singularity/host_linux.go @@ -1,4 +1,4 @@ -// Copyright (c) 2022, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2026, 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. @@ -25,17 +25,37 @@ func (e *EngineOperations) PostStartHost(ctx context.Context) (err error) { return nil } -// CleanupHost cleans up a SIF FUSE image mount and the temporary directory that -// holds it. If container creation fails early, in STAGE 1, it will be called +// CleanupHost cleans up a SIF FUSE image mount and related temporary +// directories. If container creation fails early, in STAGE 1, it will be called // directly from STAGE 1. Otherwise, it will be called from a CLEANUP_HOST // process, when the container cleanly exits, or is killed. func (e *EngineOperations) CleanupHost(ctx context.Context) (err error) { - tmpDir := e.EngineConfig.GetDeleteTempDir() - if e.EngineConfig.GetImageFuse() && tmpDir != "" { + if !e.EngineConfig.GetImageFuse() { + return nil + } + + // GetDeleteTempDir being set indicates the rootfs is FUSE mounted in a + // temporary directory, which should unmounted and removed. It should have + // been cleaned up with a lazy unmount in PostStartHost, but if something + // went wrong there, we try again here. + if tmpDir := e.EngineConfig.GetDeleteTempDir(); tmpDir != "" { if fs.IsDir(tmpDir) { + sylog.Debugf("Cleaning up image FUSE mount temporary directory %s", tmpDir) return cleanFUSETempDir(ctx, e) } } + + // GetDeletePullTempDir being set indicates the underlying image was + // implicitly pulled to a temporary directory, due to disabled cache, and + // this should be removed. + if tmpDir := e.EngineConfig.GetDeletePullTempDir(); tmpDir != "" { + sylog.Debugf("Cleaning up image pull temporary directory %s", tmpDir) + err := os.RemoveAll(tmpDir) + if err != nil { + return fmt.Errorf("failed to delete temporary directory %s: %s", tmpDir, err) + } + } + return nil } @@ -48,7 +68,7 @@ func cleanFUSETempDir(ctx context.Context, e *EngineOperations) error { if tmpDir != "" { err := os.RemoveAll(tmpDir) if err != nil { - return fmt.Errorf("failed to delete container image tempDir %s: %s", tmpDir, err) + return fmt.Errorf("failed to delete temporary directory %s: %s", tmpDir, err) } } return nil diff --git a/internal/pkg/runtime/launcher/launcher.go b/internal/pkg/runtime/launcher/launcher.go index 06d86ca155..82b8eddeba 100644 --- a/internal/pkg/runtime/launcher/launcher.go +++ b/internal/pkg/runtime/launcher/launcher.go @@ -1,4 +1,4 @@ -// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2026, 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. @@ -28,6 +28,10 @@ type Launcher interface { type ExecParams struct { // Image is the container image to execute, as a bare path, or :. Image string + // PullTempDir is a temporary directory used to store an image that was + // implicitly pulled with cache disabled, and which must be cleaned up on + // container exit. + PullTempDir string // Action is one of exec/run/shell/start/test as specified on the CLI. Action string // Process is the command to execute as the container process, where applicable. diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index 530cfa6ae2..24b372f48c 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2025, Sylabs Inc. All rights reserved. +// Copyright (c) 2019-2026, 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. @@ -362,6 +362,12 @@ func (l *Launcher) Exec(ctx context.Context, ep launcher.ExecParams) error { return fmt.Errorf("while preparing image: %s", err) } + // If the image we are running was pulled into a temp dir, because cache is + // disabled, make sure to clean that up on exit. + if ep.PullTempDir != "" { + l.engineConfig.SetDeletePullTempDir(ep.PullTempDir) + } + // Call the starter binary using our prepared config. if l.engineConfig.GetInstance() { err = l.starterInstance(ep.Instance, useSuid) diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index 6c2d48b662..f1dc030445 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -799,6 +799,15 @@ func (l *Launcher) Exec(ctx context.Context, ep launcher.ExecParams) error { sylog.Errorf("Couldn't unmount session directory: %v", err) } + // If the original image was implicitly pulled to a temporary dir due to + // disabled cache, remove it. + if l.cfg.PullTempDir != "" { + sylog.Debugf("Removing image pull temp dir: %s", l.cfg.PullTempDir) + if cleanupErr := fs.ForceRemoveAll(l.cfg.PullTempDir); cleanupErr != nil { + sylog.Errorf("Couldn't remove image pull temp dir %s: %v", l.cfg.PullTempDir, cleanupErr) + } + } + if e, ok := err.(*exec.ExitError); ok { status, ok := e.Sys().(syscall.WaitStatus) if ok && status.Signaled() { diff --git a/internal/pkg/runtime/launcher/options.go b/internal/pkg/runtime/launcher/options.go index a280976b0a..318079814e 100644 --- a/internal/pkg/runtime/launcher/options.go +++ b/internal/pkg/runtime/launcher/options.go @@ -1,4 +1,4 @@ -// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved. +// Copyright (c) 2022-2026, 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. @@ -150,6 +150,10 @@ type Options struct { // userns flows we will need to delete the redundant temporary pulled image after // conversion to sandbox. CacheDisabled bool + // PullTempDir is a temporary directory used to store an image that was + // implicitly pulled with cache disabled, and which must be cleaned up on + // container exit. + PullTempDir string // TransportOptions holds Docker/OCI image transport configuration (auth etc.) // This will be used by a launcher handling OCI images directly. @@ -569,6 +573,16 @@ func OptCacheDisabled(b bool) Option { } } +// OptPullTempDir sets a temporary directory used to store an image that was +// implicitly pulled with cache disabled, and which must be cleaned up on +// container exit. +func OptPullTempDir(d string) Option { + return func(lo *Options) error { + lo.PullTempDir = d + return nil + } +} + // OptTransportOptions sets Docker/OCI image transport options (auth etc.) func OptTransportOptions(tOpts *ociimage.TransportOptions) Option { return func(lo *Options) error { diff --git a/pkg/runtime/engine/singularity/config/config.go b/pkg/runtime/engine/singularity/config/config.go index e65b26bb72..d994e4f156 100644 --- a/pkg/runtime/engine/singularity/config/config.go +++ b/pkg/runtime/engine/singularity/config/config.go @@ -134,6 +134,7 @@ type JSONConfig struct { SignalPropagation bool `json:"signalPropagation,omitempty"` RestoreUmask bool `json:"restoreUmask,omitempty"` DeleteTempDir string `json:"deleteTempDir,omitempty"` + DeletePullTempDir string `json:"deletePullTempDir,omitempty"` ImageFuse bool `json:"imageFuse,omitempty"` Umask int `json:"umask,omitempty"` XdgRuntimeDir string `json:"xdgRuntimeDir,omitempty"` @@ -666,18 +667,35 @@ func (e *EngineConfig) GetFakeroot() bool { return e.JSON.Fakeroot } -// GetDeleteTempDir returns the path of the temporary directory containing the root filesystem -// which must be deleted after use. If no deletion is required, the empty string is returned. +// GetDeleteTempDir returns the path of a temporary directory containing the +// root filesystem which must be deleted after use, if a temporary sandbox was +// created from an image or an image was FUSE mounted. If no deletion is +// required, the empty string is returned. func (e *EngineConfig) GetDeleteTempDir() string { return e.JSON.DeleteTempDir } -// SetDeleteTempDir sets dir as the path of the temporary directory containing the root filesystem, -// which must be deleted after use. +// SetDeleteTempDir sets dir as the path of a temporary directory containing +// the root filesystem which must be deleted after use, if a temporary sandbox +// was created from an image or an image was FUSE mounted. func (e *EngineConfig) SetDeleteTempDir(dir string) { e.JSON.DeleteTempDir = dir } +// GetDeletePullTempDir returns the path of a temporary directory containing +// an image that was implicitly pulled with cache disabled, and which must be +// deleted after use. If no deletion is required, the empty string is returned. +func (e *EngineConfig) GetDeletePullTempDir() string { + return e.JSON.DeletePullTempDir +} + +// SetDeletePullTempDir sets the path of a temporary directory containing +// an image that was implicitly pulled with cache disabled, and which must be +// deleted after use. +func (e *EngineConfig) SetDeletePullTempDir(dir string) { + e.JSON.DeletePullTempDir = dir +} + // SetImageFuse sets whether the ImageDir is a FUSE mount. func (e *EngineConfig) SetImageFuse(fuse bool) { e.JSON.ImageFuse = fuse