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/nixos-containers: inherit {base,extra}Modules #349163

Closed

Conversation

SomeoneSerge
Copy link
Contributor

@SomeoneSerge SomeoneSerge commented Oct 16, 2024

No idea if this generally makes sense, but I've tried something of the sort with microvm.nix and it didn't seem to fall apart.
I'd expect the change to be "breaking".

CC @roberth

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@roberth
Copy link
Member

roberth commented Oct 17, 2024

I'm pretty sure it's supposed to be a completely independent NixOS, also considering a completely different version can be loaded with the nixpkgs option.
That's just what users expect from a container, to be unaffected by host configuration.

I imagine this would be useful for testing "what if I'd already upstreamed my module", so perhaps it could be an explicit opt-in? Something like containers.<name>.inheritNixOS = true;? And then have an assertion that nixpkgs is unset.

@SomeoneSerge
Copy link
Contributor Author

That's just what users expect from a container, to be unaffected by host configuration.

Well, baseModules clearly aren't "host configuration", so the question is are extraModules? Just looking at the code that's already there, I'd expect one to draw the line at modules.

Put another way, I observe that it is a recurrent need for me, and I'm not doing anything special, to inject generic/host-agnostic modules into nixosSystem such that they'd propagate into containers and microVMs, same as they propagate into specialisations with inheritParent = false. If extraModules aren't the right knob, then maybe we should invent one.

@SomeoneSerge
Copy link
Contributor Author

Something like containers..inheritNixOS = true;? And then have an assertion that nixpkgs is unset.

Ok, if I'm being naive I should say let's just propagate extraModules in specialArgs separately and use only them...

@roberth
Copy link
Member

roberth commented Oct 17, 2024

Well, baseModules clearly aren't "host configuration",

I meant host configuration to be anything that affects system.build.toplevel (modulo any declarative containers referenced by it).

As a NixOS user I can pass a different set of baseModules and theoretically make it behave like NixBSD.
However, even without exaggerating, keep in mind that users will expect containers to work the same wherever they're installed; not just between hosts managed by the same user.

Do you really want to make NixOS containers decidedly less reproducible? This affects the "image" part of the container after all; not just runtime stuff.

let's just propagate extraModules in specialArgs separately and use only them...

The same reasoning applies.

You are free to add similar wiring by hand, and it would also be ok to make it easier by adding a flag like I proposed to propagate those modules automatically, but it must be opt-in and per container.
Doing anything more implicit will be a problem for users, I can already see them complaining "So NixOS containers are less reproducible than Docker images, so why are we even doing this?"

@SomeoneSerge
Copy link
Contributor Author

Do you really want to make NixOS containers decidedly less reproducible? This affects the "image" part of the container after all; not just runtime stuff.

I don't see how they're getting any less reproducible; baseModules and extraModules are an input to the containers just like to normal nixosConfigurations: you pin them and they're reproducible. The difference is, when you manage in one place a collection of N containers and they share some declarations, do you repeat imports = [ a b c d ... z ] N times, or do you inject them at a level above

it must be opt-in and per container. Doing anything more implicit will be a problem for users

Ah yes sure, the config.nixpkgs argument was already convincing

@roberth
Copy link
Member

roberth commented Oct 18, 2024

do you repeat imports = [ a b c d ... z ] N times, or do you inject them at a level above

Preferably the first. That way it's clear what goes into the container. I guess the word I was looking for was portability; being able to reproduce it as part of a different system, which in this case is merely a different host.

If it's explicitly imported, it's easy to move the container to different host (with different baseModules) and get the same container toplevel.

@SomeoneSerge
Copy link
Contributor Author

Opening a draft, I was wondering if the proposed behaviour would read as surprising, unintuitive or wrong to someone previously familiar with the module. That is now clear enough. The old behaviour should not be changed under the same name. At the most, a new behaviour under a new name may be considered.

The documentation for the old behaviour is to be updated: that both {base,extra}Modules are ignored by design (this is a surprising behaviour at least to me)

I'm still uncertain as to the purpose of extraModules, at the point they seem like an ad hoc hack for specialisations.

Preferably the first. That way it's clear what goes into the container. I guess the word I was looking for was portability; being able to reproduce it as part of a different system, which in this case is merely a different host.

Having slept on it, I maybe see better where you're coming from and am reconsidering my motivations. The lens I've been looking through, the use-case, is that the artifact to be kept portable is not a set of individual containers, but the collection of containers as a whole. The entrypoint would produce the collection, not a single container. The individual containers might move between machines defined in a single monorepo, which all share the same "extra modules". The machines and containers might be populated using something like readDir to hide the friction, which is helped by having the augmented definition of nixosSystem.

@roberth
Copy link
Member

roberth commented Oct 18, 2024

Hey, thanks for opening this PR, and I'm glad we aligned on this.

extraModules is somewhat of a mystery to me too; all I know is it defaults to getEnv "NIXOS_EXTRA_MODULE_PATH" which is very ad hoc and something I kind of wish didn't exist but also didn't seem to do much harm.
This makes me consider that we should perhaps just deprecate and remove it.

@roberth
Copy link
Member

roberth commented Oct 18, 2024

I've opened a PR to eventually remove the environment variable, but the use case "pretend NixOS has an extra/different module" also comes up in the test framework (implemented with baseModules instead), so I'm leaning toward documenting it for that purpose.

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

Successfully merging this pull request may close these issues.

2 participants