Merge pull request #21768 from baude/zstd

zstd now default compression for podman machine

Author: openshift-merge-bot[bot]

go.mod

@@ -41,6 +41,7 @@ require (
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/hugelgupf/p9 v0.3.1-0.20230822151754-54f5c5530921
 	github.com/json-iterator/go v1.1.12
+	github.com/klauspost/compress v1.17.5
 	github.com/linuxkit/virtsock v0.0.0-20220523201153-1a23e78aa7a2
 	github.com/mattn/go-shellwords v1.0.12
 	github.com/mattn/go-sqlite3 v1.14.21
@@ -149,7 +150,6 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/jinzhu/copier v0.4.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
-	github.com/klauspost/compress v1.17.5 // indirect
 	github.com/klauspost/cpuid/v2 v2.2.6 // indirect
 	github.com/klauspost/pgzip v1.2.6 // indirect
 	github.com/kr/fs v0.1.0 // indirect

pkg/machine/compression/compression_test.go

@@ -33,11 +33,11 @@ func Test_compressionFromFile(t *testing.T) {
 			want: Bz2,
 		},
 		{
-			name: "default is xz",
+			name: "default is zstd",
 			args: args{
 				path: "/tmp/foo",
 			},
-			want: Xz,
+			want: Zstd,
 		},
 	}
 	for _, tt := range tests {
@@ -76,9 +76,9 @@ func TestImageCompression_String(t *testing.T) {
 			want: "zip",
 		},
 		{
-			name: "xz is default",
+			name: "zstd is default",
 			c:    99,
-			want: "xz",
+			want: "zst",
 		},
 	}
 	for _, tt := range tests {

pkg/machine/compression/config.go

@@ -9,6 +9,7 @@ const (
 	Zip
 	Gz
 	Bz2
+	Zstd
 )
 
 func KindFromFile(path string) ImageCompression {
@@ -19,8 +20,10 @@ func KindFromFile(path string) ImageCompression {
 		return Gz
 	case strings.HasSuffix(path, Zip.String()):
 		return Zip
+	case strings.HasSuffix(path, Xz.String()):
+		return Xz
 	}
-	return Xz
+	return Zstd
 }
 
 func (c ImageCompression) String() string {
@@ -31,6 +34,8 @@ func (c ImageCompression) String() string {
 		return "zip"
 	case Bz2:
 		return "bz2"
+	case Xz:
+		return "xz"
 	}
-	return "xz"
+	return "zst"
 }

pkg/machine/compression/decompress.go

@@ -18,6 +18,7 @@ import (
 	"github.com/containers/podman/v5/utils"
 	"github.com/containers/storage/pkg/archive"
 	crcOs "github.com/crc-org/crc/v2/pkg/os"
+	"github.com/klauspost/compress/zstd"
 	"github.com/sirupsen/logrus"
 	"github.com/ulikunitz/xz"
 )
@@ -59,12 +60,22 @@ func Decompress(localPath *define.VMFile, uncompressedPath string) error {
 		if err != nil {
 			return err
 		}
+		// darwin really struggles with sparse files. being diligent here
 		fmt.Printf("Copying uncompressed file %q to %q/n", localPath.GetPath(), dstFile.Name())
+
+		// Keeping CRC implementation for now, but ideally this could be pruned and
+		// sparsewriter could be used.  in that case, this area needs rework or
+		// sparsewriter be made to honor the *file interface
 		_, err = crcOs.CopySparse(uncompressedFileWriter, dstFile)
 		return err
 	case archive.Gzip:
 		if runtime.GOOS == "darwin" {
-			return decompressGzWithSparse(prefix, localPath, uncompressedPath)
+			return decompressGzWithSparse(prefix, localPath, uncompressedFileWriter)
+		}
+		fallthrough
+	case archive.Zstd:
+		if runtime.GOOS == "darwin" {
+			return decompressZstdWithSparse(prefix, localPath, uncompressedFileWriter)
 		}
 		fallthrough
 	default:
@@ -225,22 +236,31 @@ func decompressZip(prefix string, src string, output io.WriteCloser) error {
 	return err
 }
 
-func decompressGzWithSparse(prefix string, compressedPath *define.VMFile, uncompressedPath string) error {
-	stat, err := os.Stat(compressedPath.GetPath())
-	if err != nil {
-		return err
-	}
-
-	dstFile, err := os.OpenFile(uncompressedPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, stat.Mode())
-	if err != nil {
-		return err
-	}
+func decompressWithSparse(prefix string, compressedReader io.Reader, uncompressedFile *os.File) error {
+	dstFile := NewSparseWriter(uncompressedFile)
 	defer func() {
 		if err := dstFile.Close(); err != nil {
-			logrus.Errorf("unable to close uncompressed file %s: %q", uncompressedPath, err)
+			logrus.Errorf("unable to close uncompressed file %s: %q", uncompressedFile.Name(), err)
 		}
 	}()
 
+	// TODO remove the following line when progress bars work
+	_ = prefix
+	// p, bar := utils.ProgressBar(prefix, stat.Size(), prefix+": done")
+	// proxyReader := bar.ProxyReader(f)
+	// defer func() {
+	// 	if err := proxyReader.Close(); err != nil {
+	// 		logrus.Error(err)
+	// 	}
+	// }()
+
+	// p.Wait()
+	_, err := io.Copy(dstFile, compressedReader)
+	return err
+}
+
+func decompressGzWithSparse(prefix string, compressedPath *define.VMFile, uncompressedFileWriter *os.File) error {
+	logrus.Debugf("decompressing %s", compressedPath.GetPath())
 	f, err := os.Open(compressedPath.GetPath())
 	if err != nil {
 		return err
@@ -260,20 +280,34 @@ func decompressGzWithSparse(prefix string, compressedPath *define.VMFile, uncomp
 			logrus.Errorf("unable to close gzreader: %q", err)
 		}
 	}()
+	// This way we get something to look at in debug mode
+	defer func() {
+		logrus.Debug("decompression complete")
+	}()
+	return decompressWithSparse(prefix, gzReader, uncompressedFileWriter)
+}
 
-	// TODO remove the following line when progress bars work
-	_ = prefix
-	// p, bar := utils.ProgressBar(prefix, stat.Size(), prefix+": done")
-	// proxyReader := bar.ProxyReader(f)
-	// defer func() {
-	// 	if err := proxyReader.Close(); err != nil {
-	// 		logrus.Error(err)
-	// 	}
-	// }()
-
+func decompressZstdWithSparse(prefix string, compressedPath *define.VMFile, uncompressedFileWriter *os.File) error {
 	logrus.Debugf("decompressing %s", compressedPath.GetPath())
-	_, err = crcOs.CopySparse(dstFile, gzReader)
-	logrus.Debug("decompression complete")
-	// p.Wait()
-	return err
+	f, err := os.Open(compressedPath.GetPath())
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			logrus.Errorf("unable to close on compressed file %s: %q", compressedPath.GetPath(), err)
+		}
+	}()
+
+	zstdReader, err := zstd.NewReader(f)
+	if err != nil {
+		return err
+	}
+	defer zstdReader.Close()
+
+	// This way we get something to look at in debug mode
+	defer func() {
+		logrus.Debug("decompression complete")
+	}()
+	return decompressWithSparse(prefix, zstdReader, uncompressedFileWriter)
 }

pkg/machine/define/image_format.go

@@ -1,5 +1,7 @@
 package define
 
+import "fmt"
+
 type ImageFormat int64
 
 const (
@@ -22,13 +24,9 @@ func (imf ImageFormat) Kind() string {
 }
 
 func (imf ImageFormat) KindWithCompression() string {
-	switch imf {
-	case Vhdx:
-		return "vhdx.zip"
-	case Tar:
+	// Tar uses xz; all others use zstd
+	if imf == Tar {
 		return "tar.xz"
-	case Raw:
-		return "raw.gz"
 	}
-	return "qcow2.xz"
+	return fmt.Sprintf("%s.zst", imf.Kind())
 }

pkg/machine/define/image_format_test.go

@@ -45,19 +45,19 @@ func TestImageFormat_KindWithCompression(t *testing.T) {
 		want string
 	}{
 		{
-			name: "vhdx.zip",
+			name: "vhdx",
 			imf:  Vhdx,
-			want: "vhdx.zip",
+			want: "vhdx.zst",
 		},
 		{
 			name: "qcow2",
 			imf:  Qcow,
-			want: "qcow2.xz",
+			want: "qcow2.zst",
 		},
 		{
-			name: "raw.gz",
+			name: "raw",
 			imf:  Raw,
-			want: "raw.gz",
+			want: "raw.zst",
 		}, {
 			name: "tar.xz",
 			imf:  Tar,

pkg/machine/ignition/ignition.go

@@ -178,24 +178,6 @@ ExecStart=
 ExecStart=-/usr/sbin/agetty --autologin root --noclear %I $TERM
 `
 
-	deMoby := parser.NewUnitFile()
-	deMoby.Add("Unit", "Description", "Remove moby-engine")
-	deMoby.Add("Unit", "After", "systemd-machine-id-commit.service")
-	deMoby.Add("Unit", "Before", "zincati.service")
-	deMoby.Add("Unit", "ConditionPathExists", "!/var/lib/%N.stamp")
-
-	deMoby.Add("Service", "Type", "oneshot")
-	deMoby.Add("Service", "RemainAfterExit", "yes")
-	deMoby.Add("Service", "ExecStart", "/usr/bin/rpm-ostree override remove moby-engine")
-	deMoby.Add("Service", "ExecStart", "/usr/bin/rpm-ostree ex apply-live --allow-replacement")
-	deMoby.Add("Service", "ExecStartPost", "/bin/touch /var/lib/%N.stamp")
-
-	deMoby.Add("Install", "WantedBy", "default.target")
-	deMobyFile, err := deMoby.ToString()
-	if err != nil {
-		return err
-	}
-
 	// This service gets environment variables that are provided
 	// through qemu fw_cfg and then sets them into systemd/system.conf.d,
 	// profile.d and environment.d files
@@ -252,11 +234,6 @@ ExecStart=-/usr/sbin/agetty --autologin root --noclear %I $TERM
 				Name:    "docker.socket",
 				Mask:    BoolToPtr(true),
 			},
-			{
-				Enabled:  BoolToPtr(true),
-				Name:     "remove-moby.service",
-				Contents: &deMobyFile,
-			},
 			{
 				// Disable auto-updating of fcos images
 				// https://github.com/containers/podman/issues/20122
@@ -871,7 +848,7 @@ func GetNetRecoveryUnitFile() *parser.UnitFile {
 
 func DefaultReadyUnitFile() parser.UnitFile {
 	u := parser.NewUnitFile()
-	u.Add("Unit", "After", "remove-moby.service sshd.socket sshd.service")
+	u.Add("Unit", "After", "sshd.socket sshd.service")
 	u.Add("Unit", "OnFailure", "emergency.target")
 	u.Add("Unit", "OnFailureJobMode", "isolate")
 	u.Add("Service", "Type", "oneshot")

pkg/machine/ocipull/ociartifact.go

@@ -27,7 +27,7 @@ const (
 	// TODO This is temporary until we decide on a proper image name
 	artifactRegistry     = "quay.io"
 	artifactRepo         = "baude"
-	artifactImageName    = "podman-machine-images-art"
+	artifactImageName    = "stage-podman-machine"
 	artifactOriginalName = "org.opencontainers.image.title"
 	machineOS            = "linux"
 )