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.

Sign up for GitHub

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

Jump to bottom

nixos-containers: add docs about nested containers #85746

Merged
merged 1 commit into from Apr 22, 2020
Merged

nixos-containers: add docs about nested containers #85746

merged 1 commit into from Apr 22, 2020

Conversation

iblech
Copy link
Contributor

@iblech iblech commented Apr 22, 2020

Motivation for this change

Recently I started experimenting with our lightweight containers and was very impressed by how well they work. I'm grateful to all who have worked to bring this feature to light.

I was also pleasantly surprised to notice that nested containers work just fine. However, I fell into a small trap, which this pull request hopefully removes by improving the documentation:

By default, support for containers is disabled for machines which are themselves containers to a host. This default is readily apparent from the source (default = !config.boot.isContainer) but not from the generated documentation (which simply lists Default: true). This pull request adds two short sentences to the documentation, hopefully clarifying the situation.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@Mic92 Mic92 merged commit 6eb1020 into NixOS:master Apr 22, 2020
@ajs124
Copy link
Member

ajs124 commented Apr 22, 2020

This default is readily apparent from the source (default = !config.boot.isContainer) but not from the generated documentation (which simply lists Default: true).

Why not just set defaultText?

Also, I'm like 80% sure this broke the nixos-manual-combined build.

@iblech
Copy link
Contributor Author

iblech commented Apr 22, 2020

Why not just set defaultText?

Oh, I did not know about defaultText! Thank you for pointing that out. :-)

Also, I'm like 80% sure this broke the nixos-manual-combined build.

How so? I cannot reproduce this error. (I'm using

nix-build ./nixos --arg configuration '{ boot.isContainer = true; documentation.nixos.enable = true; documentation.enable = true; }' -A config.system.build

but am not completely sure that this test is sufficient.)

@ajs124
Copy link
Member

ajs124 commented Apr 22, 2020

#85788 fixed it.

@iblech
Copy link
Contributor Author

iblech commented Apr 22, 2020

Ah, wow, this community is quick! Thank you for your patience and sorry for my slip.

@sorki sorki mentioned this pull request Feb 24, 2021
Merged
10 tasks
sorki added a commit to sorki/nixpkgs that referenced this pull request Mar 4, 2021
@sorki
nixos/nixos-containers: default boot.enableContainers to true
fc2fa3c 
Related to NixOS#85746 which addresses documentation issue,
digging deeper for a reason why this was disabled
was simply because it wasn't working which is not the case anymore.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@iblech @ajs124 @Mic92