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

dockerTools: split raw and cooked images #75810

Closed
wants to merge 4 commits into from

Conversation

tomberek
Copy link
Contributor

Uses passthru and lazy evaluation to separately create raw and cooked
images. Also caches sha256sum of intermediate layers to greatly speed up
creation of larger layers.

Motivation for this change

Building large images with multiple layers requires several repeated iterations of tar/untar and sha256sum of layers resulting in O(n^2) performance. This is a WIP to bring much faster builds to dockerTools.

Things done

Ideally this should not change existing semantics, just speed up builds and provide some useful caching behavior and passthru attributes.

eg:

nix-build -A dockerTools.examples.layersOrder.raw

As a WIP there are some extra echo/printf for debugging and understanding purposes.

  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @grahamc @zimbatm @shlevy @adnelson @antoine

Uses passthru and lazy evaluation to seperately create raw and cooked
images. Also caches sha256sum of intermediate layers to greatly speed up
creation of larger layers.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/docker-buildimage-workflow-improvement/5086/2

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

On principle, this is great. I always wished that the outputs weren't compressed since it creates so much churn in the nix store and is not really necessary when using skopeo.

This needs a bit more testing to make sure that it works.

pkgs/build-support/docker/default.nix Outdated Show resolved Hide resolved
echo "Finished."
echo "To cook and load:"
printf "\e[32m%s\e[0m" 'tar -C result/image --dereference --hard-dereference --sort=name --mtime="1" --owner=0 --group=0 --mode=a-w --xform s:'^./':: -c . | docker load'
Copy link
Member

Choose a reason for hiding this comment

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

More of an idea but this should also work nicely:

skopeo copy dir:./result/image docker-daemon://image-name:tag-name

Skopeo requires to pass the image-name and tag-name to the destination so those could be extracted from the result as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into some problems using skopeo for this purpose related to it expecting Image Manifest V 2, Schema 2. We're using an older schema. But that is/was the goal.

@tomberek
Copy link
Contributor Author

tomberek commented Feb 2, 2020

Anything this needs? @grahamc ? Is there a better way to do this, or should this split off into it's own tool (buildRawImage)?

@zimbatm
Copy link
Member

zimbatm commented Feb 2, 2020

Yeah it would be easier to merge if it didn't change the existing functions. Or build a set of tests that show that there are no regressions for the old usage.

@misuzu
Copy link
Contributor

misuzu commented Jun 17, 2020

This is really great! @tomberek any update on this please?

@purcell
Copy link
Member

purcell commented Jun 25, 2020

I suspect this is superseded by #91084, and could be closed when that is merged?

@tomberek
Copy link
Contributor Author

@purcell, yes. It seems they accomplished a similar goal as well as improving its maintainability.

@flokli
Copy link
Contributor

flokli commented Jun 29, 2020

#91084 was merged, closing.

@flokli flokli closed this Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants