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

compress SDK with zstd #243

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

bcressey
Copy link
Contributor

@bcressey bcressey commented Feb 3, 2025

Issue number:
Fixes #242

Description of changes:
Rather than implicitly using BuildKit underneath docker build, switch to explicitly using it via docker buildx build with a custom builder.

The default builder loads builds into Docker after they finish, which causes certain options - like zstd compression - to be ignored when pushing to a registry.

docker buildx build doesn't really distinguish between "build" and "push" steps; a "push" is just a build where the output is sent to a registry rather than written to a tar archive or loaded into Docker. This breaks one of the main assumptions of the publish-sdk script, which expects the build to be done already.

Rather than wiring up the build arguments as additional arguments to publish-sdk, replace it with docker buildx imagetools create as the tool for creating and replacing remote manifests.

Testing done:
Built and pushed the SDK using the new Makefile tasks.

Compression Size (MiB)
v0.50.0 (gzip) 1805
zstd level 3 1612
zstd level 22 1403

I opted for the maximum compression level given that the size reduction going from zstd level 3 to level 22 was about the same as going from gzip to zstd level 3.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@bcressey bcressey requested review from cbgbt, jpculp and jmt-lab February 3, 2025 22:03
@bcressey
Copy link
Contributor Author

bcressey commented Feb 3, 2025

@jpculp the old publish-sdk supported a few flags that aren't implemented in quite the same way:

  • --short-sha - this is no longer possible to override, but I didn't see a use case for overriding it
  • --skip-manifest - this should now be make build-push REGISTRY=... REPOSITORY=...
  • --only-manifest - not implemented (is it needed?)

To implement --only-manifest, I would do:

publish-manifest:
	docker buildx imagetools create \
		--tag $(REGISTRY)/$(MANIFEST) \
		$(REGISTRY)/$(IMAGE_NAME) \
		$(REGISTRY)/$(IMAGE_ALT_NAME)

In other words, don't link it to the build step at all, and make it unconditionally expect both manifests to exist.

@jpculp
Copy link
Member

jpculp commented Feb 3, 2025

Those flags were added mainly to get the original Bottlerocket SDK to work with some release automation. Since the Bottlerocket SDK unified back in 0.40.0, they are no longer in use.

Copy link
Contributor

@cbgbt cbgbt left a comment

Choose a reason for hiding this comment

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

🚀

@bcressey bcressey marked this pull request as draft February 14, 2025 21:43
@bcressey
Copy link
Contributor Author

Moving this back to draft as @jpculp has some concerns about whether the release automation is ready for this.

@jpculp
Copy link
Member

jpculp commented Feb 14, 2025

We might want to hold off on this one for a bit since our release automation bypasses the Makefile to publish to ECR. It does this to maintain compatibility with all the containers we vend, so we'll just need to make an escape hatch first (or add this logic to our other containers).

Rather than implicitly using BuildKit underneath `docker build`,
switch to explicitly using it via `docker buildx build` with a custom
builder.

The default builder loads builds into Docker after they finish, which
causes certain options - like zstd compression - to be ignored when
pushing to a registry.

`docker buildx build` doesn't really distinguish between "build" and
"push" steps; a "push" is just a build where the output is sent to a
registry rather than written to a tar archive or loaded into Docker.
This breaks one of the main assumptions of the `publish-sdk` script,
which expects the build to be done already.

Rather than wiring up the build arguments as additional arguments to
`publish-sdk`, replace it with `docker buildx imagetools create` as
the tool for creating and replacing remote manifests.

Signed-off-by: Ben Cressey <[email protected]>
@bcressey
Copy link
Contributor Author

⬆️ force push fixes logging for the custom buildx builder

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.

compress SDK with zstd
3 participants