-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Conversation
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
@@ -103952,8 +103952,8 @@ self: { | |||
}: | |||
mkDerivation { | |||
pname = "hocker"; | |||
version = "1.0.0"; | |||
sha256 = "16indvxpf2zzdkb7hp09zfnn1zkjwc1pcg2560x2vj7x4akh25mv"; | |||
version = "1.0.2"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new version fails its test suite on NixOS: https://nix-cache.s3.amazonaws.com/log/fb2a5pp8l6k1ys5gjvj1djh9r8ba27bk-hocker-1.0.2.drv
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ]; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
isnix-hash
, which is not provided bynix-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 hownix-paths
works first before contributing more functionality
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
@peti |
@@ -998,5 +998,4 @@ self: super: { | |||
cp $data/share/${self.ghc.name}/${pkgs.stdenv.system}-${self.ghc.name}/*/*.info $out/share/info/ | |||
''; | |||
}); | |||
|
There was a problem hiding this comment.
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. :-)
Note that the cabal tests for 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? |
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.) |
I've run the included nixos test and verified it works. |
Ping 👍 am interested in using this as well |
@GrahamcOfBorg test hocker-fetchdocker |
Failure on aarch64-linux (full log) Partial log (click to expand)
|
Failure on x86_64-linux (full log) Partial log (click to expand)
|
Not sure why @GrahamcOfBorg is failing but this works for me. 👍 cc @grahamc |
Thank you @fpletz! |
Is this possibly a Nix 2 vs. Nix 1 problem? |
@grahamc I'm not sure, I will test it tomorrow morning using Nix 2 to be sure. |
Motivation for this change
The stock
dockerTools
innixpkgs
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 becausedockerTools
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 thefetchdocker
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 ofhocker
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 atar
archive for use withdocker load
.Things TODO next, after merging
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)