Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for zstd compression #2313

Merged
merged 4 commits into from
Apr 2, 2023

Conversation

LFrobeen
Copy link
Contributor

@LFrobeen LFrobeen commented Nov 8, 2022

Description

This PR adds support for zstandard compression to kaniko. We have modified the go-containerregistry package to add support for zstd compressed tarball image layers. The PR (google/go-containerregistry#1487) is currently being reviewed.

We have added two additional command line arguments --compression and --compression-level which can be used to select the compression algorithm (gzip or zstd) and specify a numeric compression level. To ensure full backwards compatibility, gzip will be used by default.

Please note that we had to bump the version of google/go-containerregistry to v0.13.0 and cloud.google.com/go/storage to v1.26.0 which caused a couple of other dependencies to be upgraded as well.

So far, we ran some manual tests but haven't added automated tests yet. If you have any suggestions or pointers regarding test, especially integration tests, please let us know.

To give some more context:
We are adding zstd support to Northflank CI/CD & Images. We have built support for multiple build engines including Buildkit, Kaniko and CNBPs. Buildkit already supports zstd, but Kaniko does not. This PR implements zstd support for Kaniko which outperformed gzip in both performance and compression ratio in our tests.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

- kaniko adds a new argument `--compression` which can be used to select the compression algorithm for layers (`zstd`, `gzip`). If omitted, `gzip` will be used by default.
- kaniko adds a new argument `--compression-level` which can be used to specify the desired compression level (0 for fastest to 9 for slowest but most compressed output)

@@ -68,6 +68,8 @@ type KanikoOptions struct {
ImageNameDigestFile string
ImageNameTagDigestFile string
OCILayoutPath string
Compression string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be a CompressionLevel type like you have in the go-containerregistry change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we expose the Compression type in the go-containerregistry package I'll just use this type. Otherwise I'll create a new one in this codebase.

@LFrobeen LFrobeen force-pushed the feature/zstd-support branch 2 times, most recently from dbc0ffa to 69f3e3f Compare January 26, 2023 11:10
@LFrobeen LFrobeen marked this pull request as ready for review February 23, 2023 15:22
@PiersRBME
Copy link

Is there much activity on this project these days? I'd love to see this PR merged.

(Should the README be updated with new flags, by the way?)

@LFrobeen
Copy link
Contributor Author

Yes, I'd like to proceed with this PR. @imjasonh could you trigger the automatic checks please?

@LFrobeen LFrobeen requested a review from imjasonh March 20, 2023 15:28
@LFrobeen
Copy link
Contributor Author

Thanks @imjasonh for triggering the tests. I'm having a hard time fixing the unit tests on my machine. It seems like the vagrant setup doesn't work on M2 cpus :/ Did anyone have this issue before?
Anyway, I'll try to debug this on another machine for now.

@LFrobeen LFrobeen force-pushed the feature/zstd-support branch 3 times, most recently from 390fc06 to e93ce25 Compare March 21, 2023 15:51
@LFrobeen
Copy link
Contributor Author

Ok, I've now created a proper test setup on debian. I'm pretty sure that i'm doing everything right, but the make test step still fails for me.

I tried running the make test step in the main branch of this repo without any modifications and i get pretty much the same errors as in this branch. The unit tests seem to be fine, but the gofmt and linter checks fail. I was able to fix the gofmt errors but I'm having trouble with the linter error which looks like this:

WARN [runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/go/src/github.com/GoogleContainerTools/kaniko/pkg/config/args.go:20:2: could not import fmt (/usr/local/go/src/fmt/errors.go:8:2: could not import errors (/usr/local/go/src/errors/wrap.go:8:2: could not import internal/reflectlite (/usr/local/go/src/internal/reflectlite/swapper.go:8:2: could not import internal/goarch (-: could not load export data: cannot import "internal/goarch" (unknown bexport format version -1 ("u\x01\x00\x00\x00\x 
...

I tried upgrading the linter to a more recent version and while that seems to resolve the cryptic error message above, it results in new lint errors.

Could it be that there is still something wrong with my test setup? I don't quite understand why i'm getting lint and gofmt errors on a version of the repo that has already been released.

…ompression level

We want to make the layer compression in kaniko configurable, so we have added two optional command line arguments “--compression” and “--compression-level”. The former allows the user to specify a compression algorithm (zstd, gzip) and the latter can be used to specify the compression level.

Depending on the selected compression algorithm and level we modify the set of layerOptions that are used to create tarball layers in `push.go` and `build.go`.

The actual implementation of the zstd support can be found in our fork of the go-containerregistry package for which we have filed this PR: google/go-containerregistry#1487

The changes should be fully backwards compatible.
This change will ensure that users can only specify supported compression algorithms (`zstd`, `gzip`) to the `--compression` flag.
@LFrobeen
Copy link
Contributor Author

LFrobeen commented Mar 22, 2023

Regarding my previous message: I realized that my changes weren't based on the v1.9.1 release, but on a more recent commit on the main branch which also suffers from the same linting issues. I thought I made my changes on top of v1.9.1 which is why i was so confused.

Earlier today I saw that there was some activity on main that seems to address the issues (many thanks to everyone who is working on this), so I rebased my changed onto the latest commit on main.

@PiersRBME
Copy link

Glad to see this picking up again :)

As and when there's a good time, I'd be glad to test for my use case.

Not sure if an actual gcr.io image will be generated from this PR -- or even if merged -- but if I'd have to build the image myself, I can work that out if needed.

@LFrobeen
Copy link
Contributor Author

Sorry to be a bother with this, but could you trigger the checks again please, @imjasonh?

@LFrobeen
Copy link
Contributor Author

Thanks @imjasonh for triggering the tests. The results looked much better than the test runs before that. I fixed the issues that popped up in the previous unit test. Would you mind restarting the tests once again? Thanks a lot!

@imjasonh imjasonh merged commit 76afb70 into GoogleContainerTools:main Apr 2, 2023
kylecarbs pushed a commit to coder/kaniko that referenced this pull request Jul 12, 2023
* Add support for configurable compression algorithm (gzip, zstd) and compression level

We want to make the layer compression in kaniko configurable, so we have added two optional command line arguments “--compression” and “--compression-level”. The former allows the user to specify a compression algorithm (zstd, gzip) and the latter can be used to specify the compression level.

Depending on the selected compression algorithm and level we modify the set of layerOptions that are used to create tarball layers in `push.go` and `build.go`.

The actual implementation of the zstd support can be found in our fork of the go-containerregistry package for which we have filed this PR: google/go-containerregistry#1487

The changes should be fully backwards compatible.

* Restrict inputs for compression flag to gzip and zstd

This change will ensure that users can only specify supported compression algorithms (`zstd`, `gzip`) to the `--compression` flag.

* Fix incorrect type for switch statements on config.Compression
@slonopotamus
Copy link

It it true that this PR doesn't add zstd:chunked support?

@LFrobeen
Copy link
Contributor Author

LFrobeen commented Oct 25, 2023

It only adds regular zstd compression. The underlying compression logic can be found here: google/go-containerregistry#1487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants