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

nixos-container: Use new configuration & state directories #87268

Open
wants to merge 5 commits into
base: master
from

Conversation

@adisbladis
Copy link
Member

adisbladis commented May 8, 2020

Motivation for this change

We need to move NixOS containers somewhere else so these don't clash with Podman, Skopeo & other container software in the libpod & cri-o/cri-u/libcontainer ecosystems.

The state directory move is not strictly a requirement but is good for consistency.

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.

Closes #77925

adisbladis added 4 commits May 8, 2020
/etc/containers is also used by Podman, Skopeo & other popular
container tooling so we need to be able to move to another
configuration directory.

The state move is not strictly a requirement but is good for consistency.
We need to move NixOS containers somewhere else so these don't clash
with Podman, Skopeo & other container software in the libpod &
cri-o/cri-u/libcontainer ecosystems.

The state directory move is not strictly a requirement but is good for
consistency.
…rectories
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
@flokli
Copy link
Contributor

flokli commented May 8, 2020

This is only really gonna fix podman etc. for all new NixOS installations.

Existing installations will still stay on /var/lib/containers, so podman will still not work properly.

I assume we don't want to only be able to provide podman for all new installations… Instead of making this conditional on stateVersion, could we ship some "migrate from /var/lib/containers -> /var/lib/nixos-containers code for 1-2 releases (if we can clearly detect nixos-container-generated files?)

@adisbladis
Copy link
Member Author

adisbladis commented May 8, 2020

could we ship some "migrate from /var/lib/containers -> /var/lib/nixos-containers code for 1-2 releases

I thought about it but opted not to because:

  1. Automatic migrations of user data feels scary
  2. They would break rollbacks to previous releases unless we also backported a back-migration

I could add migrations if we are in agreement that it's the correct solution.

@flokli
Copy link
Contributor

flokli commented May 11, 2020

At least the /etc/[nixos-]containers migration should be possible to do independent of stateVersion, and fix #77925, should it?

I agree migrating container root directories in /var/lib/containers is scary, and indeed shouldn't be done automatically (as it contains all data inside these containers too).

But I'm also not a fan of keeping users upgrading from an older NixOS with state in /var/lib/containers indefinitely - we might run into hard-to-debug issues with other container software if things are still left in /var/lib/containers.

I'm not sure what's the best approach there, but feel like the first part (new configuration files) should be less problematic and handled first.

@@ -4,6 +4,10 @@ with lib;

let

configurationPrefix = optionalString (versionAtLeast config.system.stateVersion "20.09") "nixos-";
configurationDirectory = "/etc/${configurationPrefix}containers";
stateDirectory = "/var/lib/${configurationPrefix}containers";

This comment has been minimized.

Copy link
@Ma27

Ma27 May 11, 2020

Member

Hmm, but since we're not supposed to bump the state-version on a system-update, wouldn't this break 20.03 installations when upgrading to 20.09 if there are existing nixos-containers? To fix this, I'd suggest to raise a warning if /var/lib/containers is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.