manifest: zstd

[supakeen]

Implements the existing zstd stage from osbuild and creates the appropriate scaffolding to use it.

Also enables a new image type for Fedora Minimal that uses it 1.

[achilleas-k]

Neat!

[achilleas-k]

Conflict :(

[mvo5]

This looks fine, some suggestions inline (could be followups but the duplication is something that might be worth doing now).

[mvo5]

(wondering) should this be minimal-raw-zstd (notice the leading d) ? I know the extension if zst (which I personally find very confusing) so maybe fine? idk

[supakeen]

I indeed picked it based on the extension so the image type is related to the output artifact, I did things in the same style in #1233.

[mvo5]

This duplicates a lot of the existing minimal-raw with only minimal changes - including this workaround which means we will (if we ever remove it) keep the two in sync. How about just making a "maker" function to keep them in sync automatically? Something like:

diff --git a/pkg/distro/fedora/distro.go b/pkg/distro/fedora/distro.go
index 5959db425..9921a24e4 100644
--- a/pkg/distro/fedora/distro.go
+++ b/pkg/distro/fedora/distro.go
@@ -424,42 +424,16 @@ var (
                exports:                []string{"container"},
                requiredPartitionSizes: requiredDirectorySizes,
        }
+       minimalrawImgType     = makeMinimalRaw("minimal-raw", "xz", "xz")
+       minimalrawZstdImgType = makeMinimalRaw("minimal-raw-zst", "zstd", "zst")
+)
 
-       minimalrawImgType = imageType{
-               name:        "minimal-raw",
-               filename:    "disk.raw.xz",
-               compression: "xz",
-               mimeType:    "application/xz",
-               packageSets: map[string]packageSetFunc{
-                       osPkgsKey: packageSetLoader,
-               },
-               defaultImageConfig: &distro.ImageConfig{
-                       EnabledServices: minimalRawServices,
-                       // NOTE: temporary workaround for a bug in initial-setup that
-                       // requires a kickstart file in the root directory.
-                       Files: []*fsnode.File{initialSetupKickstart()},
-                       Grub2Config: &osbuild.GRUB2Config{
-                               // Overwrite the default Grub2 timeout value.
-                               Timeout: 5,
-                       },
-               },
-               rpmOstree:              false,
-               kernelOptions:          defaultKernelOptions,
-               bootable:               true,
-               defaultSize:            2 * datasizes.GibiByte,
-               image:                  diskImage,
-               buildPipelines:         []string{"build"},
-               payloadPipelines:       []string{"os", "image", "xz"},
-               exports:                []string{"xz"},
-               basePartitionTables:    minimalrawPartitionTables,
-               requiredPartitionSizes: requiredDirectorySizes,
-       }
-
-       minimalrawZstdImgType = imageType{
-               name:        "minimal-raw-zst",
-               filename:    "disk.raw.zst",
-               compression: "zstd",
-               mimeType:    "application/zstd",
+func makeMinimalRaw(name, compression, ext string) imageType {
+       return imageType{
+               name:        name,
+               filename:    "disk.raw." + ext,
+               compression: compression,
+               mimeType:    "application/" + compression,
                packageSets: map[string]packageSetFunc{
                        osPkgsKey: packageSetLoader,
                },
@@ -479,12 +453,12 @@ var (
                defaultSize:            2 * datasizes.GibiByte,
                image:                  diskImage,
                buildPipelines:         []string{"build"},
-               payloadPipelines:       []string{"os", "image", "zstd"},
-               exports:                []string{"zstd"},
+               payloadPipelines:       []string{"os", "image", compression},
+               exports:                []string{compression},
                basePartitionTables:    minimalrawPartitionTables,
                requiredPartitionSizes: requiredDirectorySizes,
        }
-)
+}
 
 type distribution struct {
        name               string

[supakeen]

This sort of thing keeps popping up and we do it in various ways (we also just 'copy' image types and then adjust some of their properties in the newDistro function as well so the above would introduce a '3rd way'.

I don't particularly care which way we pick; do you want me to do your suggestion or the newDistro way?

[mvo5]

Good point, yeah, the newDistro() way seems fine then.

[supakeen]

Added a commit to do this.

[supakeen]

Rebased, resolved conflicts, addressed comments.

[ondrejbudai]

Awesome, thanks! :) I still think that the final compression should be just configurable per build (with a sane default per image type), but this is a step in the correct direction.

How would you feel about adding a minimal-raw-zstd alias for minimal-raw-zst? I think it may remove some friction...

[ondrejbudai]

It's interesting that the image type name is "minimal-raw-zst", but the variable name is "minimalRawZstdImgType". My gut feeling is that the d shouldn't be there.