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

tomberek commented Dec 17, 2019

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

Tom Bereknyei
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

nixos-discourse commented Dec 17, 2019

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

@jonringer jonringer requested a review from grahamc Dec 17, 2019
Copy link
Member

zimbatm left a comment

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'

This comment has been minimized.

Copy link
@zimbatm

zimbatm Dec 17, 2019

Member

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.

This comment has been minimized.

Copy link
@tomberek

tomberek Dec 17, 2019

Author Contributor

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 and others added 2 commits Dec 17, 2019
Co-Authored-By: zimbatm <zimbatm@zimbatm.com>
@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
Contributor

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

tomberek commented Jun 25, 2020

@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
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.