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

Support fetching docker images from V2 registries #32248

Merged
merged 10 commits into from Mar 4, 2018

Conversation

ixmatus
Copy link
Contributor

@ixmatus ixmatus commented Dec 2, 2017

Motivation for this change

The stock dockerTools in nixpkgs support fetching docker images from a V1 docker registry and not a V2 registry; dockerTools pull the whole docker image into the nix store instead of pulling the layers, config, and manifest into the nix store. This is problematic because dockerTools only work with an out-of-date docker registry version and it is storage-inefficient; inefficient because multiple docker images sharing base layers do not take advantage of deduplication in the nix store.

hocker and the fetchdocker utilities fix these issues. hocker implements its own client against the docker distribution REST API and it does not use the docker client or docker daemon to pull an image from a registry. Each utility of hocker is designed to do one thing well, this translates naturally to a set of Nix functions that produce derivations that fetch docker image layers, the docker image configuration json, and produce an image compositor script. The image compositor script assembles the downloaded and the generated pieces of the docker image from the nix store, streaming it to stdout as a tar archive for use with docker load.

Things TODO next, after merging
  • Write a tutorial
  • Contribute a manual entry
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

This change adds granular, non-docker daemon docker image fetchers and
a docker image layer compositor to be used in conjunction with the
`docker2nix` utility provided by the `haskellPackages.hocker` package.

This change includes a hackage package version bump and updated sha256
for recent fixes released to `hocker` resulting from formulating this
patch.
This change adds a simple integration test exercising the fetchdocker
Nix code and hocker utilities for the simple `hello-world` docker
container. We exercise:

- Fetching the docker image configuration json
- Fetching the docker image layers
- Building a compositor script
- Loading the `hello-world` docker image into docker using the
  compositor script and `docker load`
- Running that loaded container
@ixmatus ixmatus requested a review from peti as a code owner December 2, 2017 06:29
@ixmatus
Copy link
Contributor Author

ixmatus commented Dec 2, 2017

CC: @fpletz @grahamc

@@ -103952,8 +103952,8 @@ self: {
}:
mkDerivation {
pname = "hocker";
version = "1.0.0";
sha256 = "16indvxpf2zzdkb7hp09zfnn1zkjwc1pcg2560x2vj7x4akh25mv";
version = "1.0.2";
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing that out, I had a late night oopsie and forgot to include an override that fixes this issue. I also need to take the temporary override I put in place in the test's machine.nix.

I also see the hackage-packages.nix has been updated so I'll drop this hunk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's failing its test suite due to a missing data file :-/ I thought I'd fixed that but apparently not. Since we need to override the derivation to wrap the programs with a PATH to nix I added a dontCheck for now; when I fix this fully I'll open another PR to remove the dontCheck.

( dontCheck super.hocker )
( oldDerivation: {
testToolDepends =
(oldDerivation.testToolDepends or []) ++[ pkgs.nix ];
Copy link
Member

Choose a reason for hiding this comment

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

I guess the package should declare its dependency on Nix, i.e. by depending on http://hackage.haskell.org/package/nix-paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only utility from the hocker package that depends on Nix (other than the tests) is docker2nix; I think that's clear enough for now. I agree that a build-time dependency on Nix is better than a run-time dependency, so thank you for pointing me at nix-paths. Here are my reasons why I don't think we should gate this PR on that, though:

  • I don't entirely understand how nix-paths works; it looks like it has magic constants in it but I don't understand where those come from; a minimal README.md for the project would help a lot I think (note that if you explain it all to me, I'm also happy to submit a PR with a README.md contribution)
  • The utility that we need for docker2nix is nix-hash, which is not provided by nix-paths. I can submit a PR to fix that but I don't think we should gate this PR on that and I also want to understand how nix-paths works first before contributing more functionality

Copy link
Member

Choose a reason for hiding this comment

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

http://hackage.haskell.org/package/nix-paths-1.0.1 adds support for nix-hash. The paths are determined are configure-time by Cabal. Look at Setup.lhs for the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for adding support for nix-hash @peti. I'll probably get to that soon. In the meantime can I get a review of the other parts of this PR?

I don't think using nix-paths is a blocker for this PR but if it is, could you say so?

Copy link
Member

Choose a reason for hiding this comment

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

I would not call the hocker-related overrides a blocker. That's too strong a term, IMHO. What I can say though is that I won't merge the PR as-is. Other Nixpkgs maintainers may very well feel differently about it.

Copy link
Contributor Author

@ixmatus ixmatus Dec 11, 2017

Choose a reason for hiding this comment

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

Okay, thanks for clarifying!

I'd like to clarify my understanding of nix-paths (would you accept a PR adding a README.md with anything I learn? That may help newcomers and provide a reference to point people at).

I see now that Setup.lhs discovers the absolute path to, say, nix-hash using lookupProgram which is great because if nix-hash is not available, it becomes a compile-time error and not a run-time error (I think this is great, btw, I wish this were more easily discovered!) Doing this removes the need for the wrapProgram invocations but it does require that nix-paths is included in the buildInputs of hocker's derivation (as a build-time and run-time dependency). Do I have this right?

Additionally, I think the generated derivation for the nix-paths hackage package only includes the Nix package in the nativeBuildInputs; shouldn't it be in the propagatedBuildInputs so that the same Nix used as a build-time dependency is included in the run-time closure of any program that depends on nix-paths?

Also, the generated derivation for the nix-paths hackage package restricts the hydra build to Linux: hydraPlatforms = [ "i686-linux" "x86_64-linux" ]; what's the reason for this? hocker is usable on Darwin systems, will the hydraPlatforms line prevent hocker from being integrated on a Darwin worker?

@ixmatus
Copy link
Contributor Author

ixmatus commented Jan 10, 2018

@peti hocker-1.0.4 addresses your concerns (it uses nix-paths). As part of merging master into my branch and resolving the conflict in configuration-common.nix I removed the hocker override that wrapped the executables with the output from the nix package.

@ixmatus
Copy link
Contributor Author

ixmatus commented Jan 30, 2018

@peti this is ready for re-review. Perhaps @grahamc would like to look at this too since he was interested in this work.

@fpletz fpletz self-requested a review January 30, 2018 20:10
@fpletz fpletz added this to the 18.03 milestone Jan 30, 2018
@@ -998,5 +998,4 @@ self: super: {
cp $data/share/${self.ghc.name}/${pkgs.stdenv.system}-${self.ghc.name}/*/*.info $out/share/info/
'';
});

Copy link
Member

Choose a reason for hiding this comment

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

That change doesn't seem useful. :-)

@ixmatus
Copy link
Contributor Author

ixmatus commented Feb 11, 2018

Note that the cabal tests for hocker are failing because I didn't correctly include the tests data dir in hocker's cabal file. I have this fixed and am releasing to Hackage soon.

I discovered this by building the nixos test myself off of this branch which means I think the nixos test on this PR isn't included in the checks run by CI.

@grahamc is there anyway to mark small or light-weight nixos tests on a PR as required for merge?

@ixmatus
Copy link
Contributor Author

ixmatus commented Feb 12, 2018

The fixes for the tests have been uploaded to hackage so the next time the hackage packages are updated the nixos test in this PR should be working (I will double check it.)

@ixmatus
Copy link
Contributor Author

ixmatus commented Feb 13, 2018

I've run the included nixos test and verified it works.

@adelbertc
Copy link
Contributor

Ping 👍 am interested in using this as well

@fpletz
Copy link
Member

fpletz commented Mar 4, 2018

@GrahamcOfBorg test hocker-fetchdocker

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Partial log (click to expand)

Package ‘jailbreak-cabal-1.3.2’ in /var/lib/gc-of-borg/nix-test-rs-4/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/grahamc-aarch64-community-4/pkgs/development/haskell-modules/hackage-packages.nix:120699 is not supported on ‘aarch64-linux’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Partial log (click to expand)

while evaluating the attribute ‘drvPath’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/lib/customisation.nix:179:13:
while evaluating the attribute ‘drvPath’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/lib/customisation.nix:146:13:
while evaluating the attribute ‘buildCommand’ of the derivation ‘vm-test-run-test-hocker-fetchdocker’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘buildCommand’ of the derivation ‘nixos-test-driver-test-hocker-fetchdocker’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘buildCommand’ of the derivation ‘nixos-vm’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating the attribute ‘buildCommand’ of the derivation ‘nixos-system-machine-18.03.git.010a831’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating ‘optionalString’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/lib/strings.nix:138:26, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/nixos/modules/system/activation/top-level.nix:40:9:
while evaluating the attribute ‘closure’ of the derivation ‘initrd’ at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/stdenv/generic/make-derivation.nix:148:11:
while evaluating anonymous function at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/build-support/closure-info.nix:9:1, called from /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/build-support/kernel/make-initrd.nix:33:13:
assertion failed at /var/lib/gc-of-borg/.nix-test-rs/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/ogden/pkgs/build-support/closure-info.nix:11:1

@fpletz
Copy link
Member

fpletz commented Mar 4, 2018

Not sure why @GrahamcOfBorg is failing but this works for me. 👍

cc @grahamc

@fpletz fpletz merged commit 0f78afd into NixOS:master Mar 4, 2018
@ixmatus ixmatus deleted the parnell/fetchdocker branch March 4, 2018 23:32
@ixmatus
Copy link
Contributor Author

ixmatus commented Mar 4, 2018

Thank you @fpletz!

@grahamc
Copy link
Member

grahamc commented Mar 4, 2018

Is this possibly a Nix 2 vs. Nix 1 problem?

@ixmatus
Copy link
Contributor Author

ixmatus commented Mar 4, 2018

@grahamc I'm not sure, I will test it tomorrow morning using Nix 2 to be sure.

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

7 participants