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

Unable to build container with init script #155839

Closed
jtojnar opened this issue Jan 20, 2022 · 9 comments · Fixed by #155962
Closed

Unable to build container with init script #155839

jtojnar opened this issue Jan 20, 2022 · 9 comments · Fixed by #155962
Labels
0.kind: regression Something that worked before working no longer 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS

Comments

@jtojnar
Copy link
Member

jtojnar commented Jan 20, 2022

Describe the bug

After updating from b2737d4 to d5dae65, my config no longer builds:

error: Cannot merge definitions of `system.build.installBootLoader'. Definition values:
       - In `/nix/store/1q3bx2vjp4afq1wglhxgva9rhs69k8ri-source/nixos/modules/virtualisation/container-config.nix': "/nix/store/176gh50y24c0lx2bnnmsvf9wazb73php-coreutils-9.0/bin/true"
       - In `/nix/store/1q3bx2vjp4afq1wglhxgva9rhs69k8ri-source/nixos/modules/system/boot/loader/init-script/init-script.nix': <derivation /nix/store/65w7c71fli7vh0gnqa11272jrc6bqjxj-init-script-builder.sh.drv>

Those two modules appear to by enabled by the copy of the vpsadminos module I am importing but weirdly, neither of those seem to have changed:

https://github.com/NixOS/nixpkgs/blob/3cbb02087b4c2dfc3be664d7152cec51765e53ef/nixos/modules/virtualisation/container-config.nix
https://github.com/NixOS/nixpkgs/blob/3cbb02087b4c2dfc3be664d7152cec51765e53ef/nixos/modules/system/boot/loader/init-script/init-script.nix

Steps To Reproduce

Run nixos-rebuild build-vm -I nixpkgs=$PWD -I nixos-config=<(echo '{ ... }: { boot.isContainer = true; boot.loader.initScript.enable = true; }')

@jtojnar jtojnar added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 0.kind: regression Something that worked before working no longer labels Jan 20, 2022
@jtojnar
Copy link
Member Author

jtojnar commented Jan 20, 2022

Bisecting points to 4014fb6 cc @roberth

@jtojnar
Copy link
Member Author

jtojnar commented Jan 20, 2022

Maybe this should be mkDefault?

system.build.installBootLoader = "${pkgs.coreutils}/bin/true";

@roberth
Copy link
Member

roberth commented Jan 20, 2022

This is great! Those were always in conflict, but one of them won, by luck of the imports order. You really don't want to have anything important depend on that, so I'm glad this was caught, although I wish the experience of catching it would be nicer.

Maybe this should be mkDefault?

That line does feel like a default.

@roberth
Copy link
Member

roberth commented Jan 20, 2022

Hold on, it seems to be the other way around. That line should be a mkForce instead, to disable boot loader stuff for containers.

@roberth
Copy link
Member

roberth commented Jan 20, 2022

It's kind of the wrong way around, imports-ing stuff you don't need and then disabling it, but it's the best we can do without RFC22 / Minimal module list.

I've opened #148456 and #153211 to work towards those goals with minimal impact on regular NixOS.

@jtojnar
Copy link
Member Author

jtojnar commented Jan 20, 2022

Hold on, it seems to be the other way around. That line should be a mkForce instead, to disable boot loader stuff for containers.

I would assume VPSAdminOS wants the boot loader even for containers if they are explicitly enabling the init scripts. cc @aither64

@aither64
Copy link
Contributor

I don't fully understand all these changes, but yes, I would say that boot.loader.initScript (and system.build.installBootLoader) is needed when NixOS is in a container. I mean a container run e.g. by LXC, which is what we do on vpsAdminOS. We don't know what is inside the container and depend on it having /sbin/init. I think mkDefault on installBootLoader would work.

@roberth
Copy link
Member

roberth commented Jan 20, 2022

I thought of the wrong kind of container. #155839 (comment) lgtm.

jtojnar added a commit that referenced this issue Jan 20, 2022
LXC containers like those used by VPSAdminOS might want to install a bootloader
so passing `true` to `system.build.installBootLoader` without any priority specified,
causes a conflict for such systems with the recent `system.build` changes:

4014fb6

Fixes: #155839
@jtojnar
Copy link
Member Author

jtojnar commented Jan 20, 2022

Opened #155962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: regression Something that worked before working no longer 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants