From a31e8d2a230684bbde9e103f3509b7f919a725a7 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Fri, 16 Feb 2024 09:47:12 -0600 Subject: [PATCH] zstd now default compression for podman machine given that we are moving to building our own machine images, we have decided to use zstd compression as it is superior in speed to the alternatives. as such, this pr adds zstd to our machine code; and also has to account for dealing with sparseness on darwin; which the default zstd golang library does not. [NO NEW TESTS NEEDED] Signed-off-by: Brent Baude --- go.mod | 2 +- pkg/machine/compression/compression_test.go | 8 +- pkg/machine/compression/config.go | 9 ++- pkg/machine/compression/decompress.go | 90 ++++++++++++++------- pkg/machine/define/image_format.go | 12 ++- pkg/machine/define/image_format_test.go | 10 +-- pkg/machine/ignition/ignition.go | 25 +----- pkg/machine/ocipull/ociartifact.go | 2 +- 8 files changed, 86 insertions(+), 72 deletions(-) diff --git a/go.mod b/go.mod index cde495c397..cc25df2d4b 100644 --- a/go.mod +++ b/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 diff --git a/pkg/machine/compression/compression_test.go b/pkg/machine/compression/compression_test.go index afffce1e5b..4accca18c6 100644 --- a/pkg/machine/compression/compression_test.go +++ b/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 { diff --git a/pkg/machine/compression/config.go b/pkg/machine/compression/config.go index 0201f75331..db2ec0d002 100644 --- a/pkg/machine/compression/config.go +++ b/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" } diff --git a/pkg/machine/compression/decompress.go b/pkg/machine/compression/decompress.go index 5ab0221b3a..5111f5aa96 100644 --- a/pkg/machine/compression/decompress.go +++ b/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) } }() - - // 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) - // } - // }() - - logrus.Debugf("decompressing %s", compressedPath.GetPath()) - _, err = crcOs.CopySparse(dstFile, gzReader) - logrus.Debug("decompression complete") - // p.Wait() - return err + // This way we get something to look at in debug mode + defer func() { + logrus.Debug("decompression complete") + }() + return decompressWithSparse(prefix, gzReader, uncompressedFileWriter) +} + +func decompressZstdWithSparse(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 + } + 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) } diff --git a/pkg/machine/define/image_format.go b/pkg/machine/define/image_format.go index 9b2766d638..6adbe75c75 100644 --- a/pkg/machine/define/image_format.go +++ b/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()) } diff --git a/pkg/machine/define/image_format_test.go b/pkg/machine/define/image_format_test.go index 50ca6b55d2..8c07a6bbd7 100644 --- a/pkg/machine/define/image_format_test.go +++ b/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, diff --git a/pkg/machine/ignition/ignition.go b/pkg/machine/ignition/ignition.go index f0e5a38c53..1b337783c5 100644 --- a/pkg/machine/ignition/ignition.go +++ b/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") diff --git a/pkg/machine/ocipull/ociartifact.go b/pkg/machine/ocipull/ociartifact.go index 9d59fb843d..6f71a7f7a0 100644 --- a/pkg/machine/ocipull/ociartifact.go +++ b/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" )