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: populate the nix database of the container nix store #28561

Merged
merged 2 commits into from
Sep 22, 2017

Conversation

nlewo
Copy link
Member

@nlewo nlewo commented Aug 25, 2017

Currently, the contents closure is copied to the layer but there is no
nix database initialization. If pkgs.nix is added in the contents,
nix-store doesn't work because there is no nix database.

From the contents of the layer, this commit generates and loads the
database in the nix store of the container. This only works if there
is no parent layer that already have a nix store (to support several
nix layers, we would have to merge nix databases of parent layers).

We also add an example to play with the nix store inside the
container. Note it seems more is a missing dependency of the nix
package!

Motivation for this change

Be able to use nix in a container

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
    • Linux
  • 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.

@nlewo
Copy link
Member Author

nlewo commented Aug 25, 2017

cc @lo1tuma @NeQuissimus @globin

@puffnfresh
Copy link
Contributor

This should be optional or outside of dockerTools. We don't use Nix inside of containers and do not want to increase our image sizes with a Nix DB.

@nlewo
Copy link
Member Author

nlewo commented Aug 25, 2017

I use nix inside the container because my container is a hydra slave. It could also be useful to be able to use nix inside the container for debugging purpose.
But I agree this increases the container size and this could be optional (or maybe, we could enable it by default and let the user disable it easily).

@nlewo
Copy link
Member Author

nlewo commented Aug 25, 2017

@puffnfresh I added the option populateNixDb to dockerTools.buildImage and set it to false by default.

@puffnfresh
Copy link
Contributor

@nlewo can you write this as a function which creates a new layer instead of modifying mkPureLayer?

@globin
Copy link
Member

globin commented Aug 25, 2017

I'd definitely prefer making this opt-in

@nlewo
Copy link
Member Author

nlewo commented Aug 25, 2017

@puffnfresh I think it's not trivial to add a layer, and I'm not sure it's a good idea to have a layer with a nix database without the store associated to that database.
Since the current patch doesn't change the actual behavior, but allow us to use nix inside the container, could we discuss your proposition in another PR?

Note also, we currently publish on the docker hub a docker image (nixos/nix) built with a dockerfile based on alpine. With this patch, we are going to generate this image with our tools :)

@nlewo
Copy link
Member Author

nlewo commented Aug 29, 2017

ping

@puffnfresh
Copy link
Contributor

@nlewo isn't this enough?

with import <nixpkgs> { };

let
  nixRegistration = contents: runCommand "nix-registration" {
    buildInputs = [ nixUnstable perl ];
    # For obtaining the closure of `contents'.
    exportReferencesGraph =
      let contentsList = if builtins.isList contents then contents else [ contents ];
      in map (x: [("closure-" + baseNameOf x) x]) contentsList;
    }
    ''
      printRegistration=1 perl ${pkgs.pathsFromGraph} closure-* > $out
    '';
  populateNixStore = args@{ contents, ... }:
    dockerTools.buildImage (args // {
      runAsRoot = dockerTools.shellScript "nix-db.sh" ''
        echo "Generating the nix database..."
        echo "Warning: only the database of the deepest Nix layer is loaded."
        echo "         If you want to use nix commands in the container, it would"
        echo "         be better to only have one layer that contains a nix store."
        # This requires Nix 1.12 or higher
        export NIX_REMOTE=local?root=$PWD
        ${nixUnstable}/bin/nix-store --load-db < ${nixRegistration contents}
      '';
    });
in
populateNixStore {
  name = "populated-example";
  contents = [ nix hello bash coreutils sqlite ];
}

Can we rewrite this PR to just add the above function to dockerTools?

@nlewo
Copy link
Member Author

nlewo commented Aug 30, 2017

@puffnfresh yeah, really nice improvement. Thanks.
I then submitted a new patch with your implementation. I use extraCommand instead of runAsRoot to avoid the need of qemu. I hope that I don't miss something regarding runAsRoot.

Copy link
Contributor

@puffnfresh puffnfresh left a comment

Choose a reason for hiding this comment

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

Even better. Looks good!

@nlewo
Copy link
Member Author

nlewo commented Sep 3, 2017

@globin could you have a look?

@nlewo
Copy link
Member Author

nlewo commented Sep 13, 2017

Maybe @grahamc you could have look?

@grahamc
Copy link
Member

grahamc commented Sep 17, 2017

LGTM!

… store

Currently, the contents closure is copied to the layer but there is no
nix database initialization. If pkgs.nix is added in the contents,
nix-store doesn't work because there is no nix database.

From the contents of the layer, this commit generates and loads the
database in the nix store of the container. This only works if there
is no parent layer that already have a nix store (to support several
nix layers, we would have to merge nix databases of parent layers).

We also add an example to play with the nix store inside the
container. Note it seems `more` is a missing dependency of the nix
package!
The database dump doesn't contain sha and size. This leads to invalid
path in the container. We have to fix the database by using
nix-store.
Note a better way to do this is available in Nix 1.12 (since the
database dump contains all required information).

We also add content output paths in the gcroots since they ca be used
by the container.
@nlewo
Copy link
Member Author

nlewo commented Sep 22, 2017

Could we move forward on this PR ? :)

@NeQuissimus NeQuissimus merged commit 38f65be into NixOS:master Sep 22, 2017
@nlewo
Copy link
Member Author

nlewo commented Sep 22, 2017

Thanks

@nlewo
Copy link
Member Author

nlewo commented Sep 25, 2017

@bjornfor I try to improve this in #29765

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.

None yet

6 participants