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

docker: update to Nix 1.11.14 #1562

Merged
merged 1 commit into from Sep 11, 2017
Merged

docker: update to Nix 1.11.14 #1562

merged 1 commit into from Sep 11, 2017

Conversation

peti
Copy link
Member

@peti peti commented Sep 8, 2017

  • Use the latest Nix version 1.11.14.

  • Attempts to download the Nix installation tarball from http://nixos.org
    redirect to https these days, which wget doesn't support unless OpenSSL is
    available.

  • Use addgroup and adduser commands to create the Nix build users.

  • Split image generation into multiple steps instead so that intermediate
    images can be cached.

  • Link the Nix profile script into /etc/profile.d, where it's run
    automatically.

Use the command "docker build -t nix . && docker run -it --rm nix sh -"
to build and run the Nix docker container.

@Anton-Latukha
Copy link

Anton-Latukha commented Sep 8, 2017

Image split is appreciated by me. (no, I changed my mind)
Also using link to /etc/profile.d/ is a more proper approach.

RUN wget -O- https://nixos.org/releases/nix/nix-1.11.14/nix-1.11.14-x86_64-linux.tar.bz2 | bzcat - | tar xf -
RUN addgroup -g 30000 -S nixbld
RUN for i in $(seq 1 30); do \
adduser -S -D -h /var/empty -g "Nix build user $i" -u $((30000 + $i)) -G nixbld nixbld$i ; \
Copy link

Choose a reason for hiding this comment

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

This will work as is, because we know initial conditions and have i in 1..30.
If we talk ideal clean code:

  1. $((30000 + $i)) -> $((30000 + i))
    Variables inside arithmetic functions proper not to expand.
    Like here: https://bash.cyberciti.biz/guide/Perform_arithmetic_operations
    Shell processes them right, by itself inside (( )).
    And commanding to shell to expand variable as a string, before doing operations, can lead to error.

    i='1+2'
    echo $((30000 / $i))
    30002
  2. nixbld$i -> nixbld"$i"

@msegado
Copy link

msegado commented Sep 11, 2017

FYI, each of those RUN commands adds an immutable layer to the filesystem... the last two commands for example ("Clean up files that we no longer need") will not make the image any smaller since they can't change the layers before them, and in fact they'll make it slightly larger since they add two layers themselves. More info on this: https://beenje.github.io/blog/posts/dockerfile-anti-patterns-and-best-practices/

Given that most of those RUN commands should be fast (there's not much saved by caching the result of, e.g., addgroup -g 30000 -S nixbld) and that the caches would be invalidated any time you changed the version in the first command anyway, I expect it makes sense to keep them all into a single command as in the original Dockerfile.

- Use the latest Nix version 1.11.14.

- Attempts to download the Nix installation tarball from http://nixos.org
  redirect to https these days, which wget doesn't support unless OpenSSL is
  available.

- Use addgroup and adduser commands to create the Nix build users.

- Link the Nix profile script into /etc/profile.d, where it's run
  automatically.

- Dropped installation of bash and tar. Neither tool is essential for running
  Nix.

Use the command "docker build -t nix . && docker run -it --rm nix sh -"
to build and run the Nix docker container.
@peti
Copy link
Member Author

peti commented Sep 11, 2017

@msegado, thank you very much for pointing that out. I no idea that every RUN execution creates a new dependency that cannot be removed. I assumed that the final image would stand on its own -- but clearly it does not. I've adoped the PR accordingly.

@edolstra edolstra merged commit 359ede1 into NixOS:master Sep 11, 2017
@domenkozar
Copy link
Member

Thanks!

peti added a commit to basvandijk/nix-workshop that referenced this pull request Sep 25, 2017
Don't run every single command in a separate RUN environment. Creating dozens
of intermediate docker images (which cannot be garbage collected) makes no
sense. See NixOS/nix#1562 (comment) for
further details.
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

5 participants