Merge pull request #22507 from ashley-cui/cache

Clean machine pull cache

pkg/machine/e2e/machine_pull_test.go

@@ -26,7 +26,7 @@ func pullOCITestDisk(finalDir string, vmType define.VMType) error {
 	if err != nil {
 		return err
 	}
-	err = ociArtPull.GetNoCompress()
+	_, err = ociArtPull.GetNoCompress()
 	if err != nil {
 		return err
 	}

pkg/machine/ocipull/ociartifact.go

@@ -32,6 +32,7 @@ const (
 )
 
 type OCIArtifactDisk struct {
+	cache                    bool
 	cachedCompressedDiskPath *define.VMFile
 	name                     string
 	ctx                      context.Context
@@ -91,12 +92,15 @@ func NewOCIArtifactPull(ctx context.Context, dirs *define.MachineDirs, endpoint
 		os:       machineOS,
 	}
 
+	cache := false
 	if endpoint == "" {
 		endpoint = fmt.Sprintf("docker://%s/%s/%s:%s", artifactRegistry, artifactRepo, artifactImageName, artifactVersion.majorMinor())
+		cache = true
 	}
 
 	ociDisk := OCIArtifactDisk{
 		ctx:              ctx,
+		cache:            cache,
 		dirs:             dirs,
 		diskArtifactOpts: &diskOpts,
 		finalPath:        finalPath.GetPath(),
@@ -114,36 +118,46 @@ func (o *OCIArtifactDisk) OriginalFileName() (string, string) {
 }
 
 func (o *OCIArtifactDisk) Get() error {
-	if err := o.get(); err != nil {
+	cleanCache, err := o.get()
+	if err != nil {
 		return err
 	}
+	if cleanCache != nil {
+		defer cleanCache()
+	}
 	return o.decompress()
 }
 
-func (o *OCIArtifactDisk) GetNoCompress() error {
+func (o *OCIArtifactDisk) GetNoCompress() (func(), error) {
 	return o.get()
 }
 
-func (o *OCIArtifactDisk) get() error {
+func (o *OCIArtifactDisk) get() (func(), error) {
+	cleanCache := func() {}
+
 	destRef, artifactDigest, err := o.getDestArtifact()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// Note: the artifactDigest here is the hash of the most recent disk image available
 	cachedImagePath, err := o.dirs.ImageCacheDir.AppendToNewVMFile(fmt.Sprintf("%s.%s", artifactDigest.Encoded(), o.vmType.ImageFormat().KindWithCompression()), nil)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// check if we have the latest and greatest disk image
 	if _, err = os.Stat(cachedImagePath.GetPath()); err != nil {
 		if !errors.Is(err, os.ErrNotExist) {
-			return fmt.Errorf("unable to access cached image path %q: %q", cachedImagePath.GetPath(), err)
+			return nil, fmt.Errorf("unable to access cached image path %q: %q", cachedImagePath.GetPath(), err)
 		}
+
+		// On cache misses, we clean out the cache
+		cleanCache = o.cleanCache(cachedImagePath.GetPath())
+
 		// pull the image down to our local filesystem
 		if err := o.pull(destRef, artifactDigest); err != nil {
-			return fmt.Errorf("failed to pull %s: %w", destRef.DockerReference(), err)
+			return nil, fmt.Errorf("failed to pull %s: %w", destRef.DockerReference(), err)
 		}
 		// grab the artifact disk out of the cache and lay
 		// it into our local cache in the format of
@@ -153,13 +167,46 @@ func (o *OCIArtifactDisk) get() error {
 		//
 		// i.e. 91d1e51...d28974.qcow2.xz
 		if err := o.unpack(artifactDigest); err != nil {
-			return err
+			return nil, err
 		}
 	} else {
 		logrus.Debugf("cached image exists and is latest: %s", cachedImagePath.GetPath())
 		o.cachedCompressedDiskPath = cachedImagePath
 	}
-	return nil
+	return cleanCache, nil
+}
+
+func (o *OCIArtifactDisk) cleanCache(cachedImagePath string) func() {
+	// cache miss while using an image that we cache, ie the default image
+	// clean out all old files fron the cache dir
+	if o.cache {
+		files, err := os.ReadDir(o.dirs.ImageCacheDir.GetPath())
+		if err != nil {
+			logrus.Warn("failed to clean machine image cache: ", err)
+			return nil
+		}
+
+		return func() {
+			for _, file := range files {
+				path := filepath.Join(o.dirs.ImageCacheDir.GetPath(), file.Name())
+				logrus.Debugf("cleaning cached file: %s", path)
+				err := utils.GuardedRemoveAll(path)
+				if err != nil && !errors.Is(err, os.ErrNotExist) {
+					logrus.Warn("failed to clean machine image cache: ", err)
+				}
+			}
+		}
+	} else {
+		// using an image that we don't cache, ie not the default image
+		// delete image after use and don't cache
+		return func() {
+			logrus.Debugf("cleaning cache: %s", o.dirs.ImageCacheDir.GetPath())
+			err := os.Remove(cachedImagePath)
+			if err != nil && !errors.Is(err, os.ErrNotExist) {
+				logrus.Warn("failed to clean pulled machine image: ", err)
+			}
+		}
+	}
 }
 
 func (o *OCIArtifactDisk) getDestArtifact() (types.ImageReference, digest.Digest, error) {

pkg/machine/shim/diskpull/diskpull.go

@@ -20,7 +20,7 @@ func GetDisk(userInputPath string, dirs *define.MachineDirs, imagePath *define.V
 	} else {
 		if strings.HasPrefix(userInputPath, "http") {
 			// TODO probably should use tempdir instead of datadir
-			mydisk, err = stdpull.NewDiskFromURL(userInputPath, imagePath, dirs.DataDir, nil)
+			mydisk, err = stdpull.NewDiskFromURL(userInputPath, imagePath, dirs.DataDir, nil, false)
 		} else {
 			mydisk, err = stdpull.NewStdDiskPull(userInputPath, imagePath)
 		}

pkg/machine/stdpull/url.go

@@ -22,9 +22,10 @@ type DiskFromURL struct {
 	u            *url2.URL
 	finalPath    *define.VMFile
 	tempLocation *define.VMFile
+	cache        bool
 }
 
-func NewDiskFromURL(inputPath string, finalPath *define.VMFile, tempDir *define.VMFile, optionalTempFileName *string) (*DiskFromURL, error) {
+func NewDiskFromURL(inputPath string, finalPath *define.VMFile, tempDir *define.VMFile, optionalTempFileName *string, cache bool) (*DiskFromURL, error) {
 	var (
 		err error
 	)
@@ -57,6 +58,7 @@ func NewDiskFromURL(inputPath string, finalPath *define.VMFile, tempDir *define.
 		u:            u,
 		finalPath:    finalPath,
 		tempLocation: tempLocation,
+		cache:        cache,
 	}, nil
 }
 
@@ -65,6 +67,16 @@ func (d *DiskFromURL) Get() error {
 	if err := d.pull(); err != nil {
 		return err
 	}
+	if !d.cache {
+		defer func() {
+			if err := utils.GuardedRemoveAll(d.tempLocation.GetPath()); err != nil {
+				if !errors.Is(err, os.ErrNotExist) {
+					logrus.Warn("failed to clean machine image cache: ", err)
+				}
+			}
+		}()
+	}
+
 	logrus.Debugf("decompressing (if needed) %s to %s", d.tempLocation.GetPath(), d.finalPath.GetPath())
 	return compression.Decompress(d.tempLocation, d.finalPath.GetPath())
 }

pkg/machine/wsl/stubber.go

@@ -15,6 +15,7 @@ import (
 	"github.com/containers/podman/v5/pkg/machine/shim/diskpull"
 	"github.com/containers/podman/v5/pkg/machine/stdpull"
 	"github.com/containers/podman/v5/pkg/machine/wsl/wutil"
+	"github.com/containers/podman/v5/utils"
 
 	gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
 	"github.com/containers/podman/v5/pkg/machine"
@@ -303,8 +304,7 @@ func (w WSLStubber) GetDisk(userInputPath string, dirs *define.MachineDirs, mc *
 	// i.e.v39.0.31-rootfs.tar.xz
 	versionedBase := fmt.Sprintf("%s-%s", downloadVersion, filepath.Base(downloadURL.Path))
 
-	// TODO we need a mechanism for "flushing" old cache files
-	cachedFile, err := dirs.DataDir.AppendToNewVMFile(versionedBase, nil)
+	cachedFile, err := dirs.ImageCacheDir.AppendToNewVMFile(versionedBase, nil)
 	if err != nil {
 		return err
 	}
@@ -313,12 +313,30 @@ func (w WSLStubber) GetDisk(userInputPath string, dirs *define.MachineDirs, mc *
 	if _, err = os.Stat(cachedFile.GetPath()); err == nil {
 		logrus.Debugf("%q already exists locally", cachedFile.GetPath())
 		myDisk, err = stdpull.NewStdDiskPull(cachedFile.GetPath(), mc.ImagePath)
+		if err != nil {
+			return err
+		}
 	} else {
-		// no cached file
-		myDisk, err = stdpull.NewDiskFromURL(downloadURL.String(), mc.ImagePath, dirs.DataDir, &versionedBase)
-	}
-	if err != nil {
-		return err
+		files, err := os.ReadDir(dirs.ImageCacheDir.GetPath())
+		if err != nil {
+			logrus.Warn("failed to clean machine image cache: ", err)
+		} else {
+			defer func() {
+				for _, file := range files {
+					path := filepath.Join(dirs.ImageCacheDir.GetPath(), file.Name())
+					logrus.Debugf("cleaning cached image: %s", path)
+					err := utils.GuardedRemoveAll(path)
+					if err != nil && !errors.Is(err, os.ErrNotExist) {
+						logrus.Warn("failed to clean machine image cache: ", err)
+					}
+				}
+			}()
+		}
+
+		myDisk, err = stdpull.NewDiskFromURL(downloadURL.String(), mc.ImagePath, dirs.ImageCacheDir, &versionedBase, true)
+		if err != nil {
+			return err
+		}
 	}
 	// up until now, nothing has really happened
 	// pull if needed and decompress to image location