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

Better sparse image tar for podman #128

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

matejvasek
Copy link
Contributor

This library uses the fact that uploaded tar may miss some layers when
pushed to docker provided the missing layers are already in daemon.
It works also with podman but the layers have to be present in the tar
at least as empty entries.

Signed-off-by: Matej Vasek [email protected]

@matejvasek matejvasek requested a review from a team as a code owner September 23, 2021 23:09
@matejvasek
Copy link
Contributor Author

This make build faster and it also means fewer temporary files. This is especially important because there is leak a buildpacks/pack#1167.

@matejvasek
Copy link
Contributor Author

this is based on findings of @vlk-charles

This library uses the fact that uploaded tar may miss some layers when
pushed to docker provided the missing layers are already in daemon.
It works also with podman but the layers have to be present in the tar
at least as empty entries.

Signed-off-by: Matej Vasek <[email protected]>
@matejvasek
Copy link
Contributor Author

for more reference containers/podman#8132

@natalieparellano
Copy link
Member

@matejvasek thanks for the PR! It would be great to get the same performance improvement with podman that we have with docker. Did you by chance happen to test this out against a docker daemon as well? I just want to make sure there is no unexpected performance impact (when reusing a large number of layers).

@matejvasek
Copy link
Contributor Author

I'll check that.

@matejvasek
Copy link
Contributor Author

@natalieparellano I tried to run build on Java mvn project using gcr.io/paketo-buildpacks/builder:base and there seems to be no time performance drop.

Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you @matejvasek for the contribution!

It would be nice to be able to test this, but that would require some reworking of our doSave() method. I think we could leave it off for now.

@jabrown85 what do you think?

@natalieparellano natalieparellano merged commit cf7ae41 into buildpacks:main Oct 1, 2021
@matejvasek
Copy link
Contributor Author

@natalieparellano will you or somebody else update the dep in pack latter?

@natalieparellano
Copy link
Member

@matejvasek we pulled it into the lifecycle (v0.12.0) so if you update the lifecycle in your builder you should see the performance benefit in re-using base image layers for exported app images. However pack should update imgutil to get the same benefit when creating builders.

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.

3 participants