Skip to content

c8d/builder-next: Skip unpacking when push=true #51494

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.

Sign up for GitHub

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

Jump to bottom
Closed
vvoland wants to merge 1 commit into from
Closed

c8d/builder-next: Skip unpacking when push=true #51494

vvoland wants to merge 1 commit into from

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Nov 12, 2025 •
edited
Loading

c8d/builder-next: Skip unpacking when push=true

The image exporter wrapper was unconditionally setting unpack=true for
all build exports when no explicit unpack option was provided. This
caused unnecessary unpacking overhead when images were only being pushed
to a registry and not intended for local execution.

containerd image store: Images built with `docker build --push` will no longer be unpacked automatically.

@vvoland vvoland self-assigned this Nov 12, 2025
@vvoland vvoland added area/builder Build kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/builder/buildkit Build containerd-integration Issues and PRs related to containerd integration labels Nov 12, 2025
@github-actions github-actions bot added the area/daemon Core Engine label Nov 12, 2025
@vvoland vvoland marked this pull request as draft November 12, 2025 13:44
@vvoland
c8d/builder-next: Skip unpacking when push=true
28c7364 
The image exporter wrapper was unconditionally setting `unpack=true` for
all build exports when no explicit unpack option was provided. This
caused unnecessary unpacking overhead when images were only being pushed
to a registry and not intended for local execution.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the c8d-build-pushnounpack branch from a8d52fe to 28c7364 Compare November 12, 2025 15:40
@vvoland vvoland marked this pull request as ready for review November 12, 2025 15:40
thaJeztah
thaJeztah approved these changes Nov 12, 2025
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

SGTM

@cpuguy83 @tonistiigi PTAL

@cpuguy83
Copy link
Member

cpuguy83 commented Nov 12, 2025

I feel like not unpacking is going to surprising.
Should we expose the option on the API?

@tonistiigi
Copy link
Member

tonistiigi commented Nov 12, 2025

I don't think we should change the unpack behavior based on push. They are both similar independent options to the image exporter in buildkit.

If this is desirable we could consider buildx cli setting unpack=false by default if --push or -o type=registry are used there. These are buildx-specific shorthands that just configure the image exporter internally but we can update the definition of the shorthand if it is used without other options.

@vvoland
Copy link
Contributor Author

vvoland commented Nov 13, 2025

I feel like not unpacking is going to surprising. Should we expose the option on the API?

This is already exposed - with #51493 you can set unpack option to whatever you want.

It might be a bit surprising that the first image use will take a bit longer.. but for build that pushes immediately, I think it would be fairly safe to assume that the main use-case is not running the image locally.

I don't think we should change the unpack behavior based on push. They are both similar independent options to the image exporter in buildkit.

If this is desirable we could consider buildx cli setting unpack=false by default if --push or -o type=registry are used there. These are buildx-specific shorthands that just configure the image exporter internally but we can update the definition of the shorthand if it is used without other options.

Doing this on buildx side seems good to me too. Opened: docker/buildx#3519

@vvoland vvoland mentioned this pull request Nov 13, 2025
Merged
@thompson-shaun
Copy link
Contributor

thompson-shaun commented Nov 13, 2025

Closing in favor of docker/buildx#3519

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@vvoland vvoland

Labels

area/builder/buildkit Build area/builder Build area/daemon Core Engine containerd-integration Issues and PRs related to containerd integration impact/changelog kind/enhancement Enhancements are not bugs or new features but can improve usability or performance.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

docker build --push always unpacks into moby snapshotter

5 participants

@vvoland @cpuguy83 @tonistiigi @thompson-shaun @thaJeztah