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/lxc-container: split logic into a profile and installation image module #164129

Closed
wants to merge 4 commits into from

Conversation

erictapen
Copy link
Member

@erictapen erictapen commented Mar 14, 2022

I found installing NixOS in a LXC container to be quite cumbersome. I expected there to be a module that I can include in my system config to abstract over the LXC specific configuration so I can configure my system like any other machine, as qemu-guest.nix does.

But I found only nixos/modules/virtualisation/lxc-container.nix, which seems to also provide configuration for an installation image, e.g. by setting an empty password and configuring sshd.

So this also addresses #9884.

Description of changes

I introduced the following structure:

  • profiles/lxc-container.nix provides configuration that you always want to have inside a lxc container. This also includes the possibility to generate a tarball from system config, to easily deploy a container with the same configuration. This is also included in hardware-configuration.nix if nixos-generate-config detects that we are running in a lxc container.
  • virtualisation/lxc-image.nix imports profiles/lxc-container.nix and provides additional config for creating the installation images.
  • virtualisation/lxc-container.nix is deprecated and points to virtualisation/lxc-image.nix. Could be removed after the next release.

Also I took the liberty to remove the sys-kernel-debug.mount unit in the profile, as my deployment doesn't complete otherwise. But I have no idea what the unit does.

For testing I ran the following NixOS tests: lxd-image lxd-image-server lxd-nftable lxd
Also I built the containerTarball attribute: nix-build -A containerTarball.x86_64-linux nixos/release.nix

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Member

@mkg20001 mkg20001 left a comment

Choose a reason for hiding this comment

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

I like this a lot. Some minor stuff about where things should go.

nixos/modules/profiles/lxc-container.nix Outdated Show resolved Hide resolved
] ++ templates.files;
};

# Allow the user to login as root without password.
Copy link
Member

Choose a reason for hiding this comment

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

this (until bottom) should either go to the profile or (maybe better) into the hydra image only

Copy link
Member

Choose a reason for hiding this comment

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

wait, profile doesn't make much sense here. best would be to just have this in the hydra image. otherwise it's a "surprise feature" for people that build custom images. (none of those configs are strictly required for the image to work)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hydra image only would mean putting the config here, right? https://github.com/NixOS/nixpkgs/blob/master/nixos/release.nix#L314

Personally I'd say these are good settings to have for an installation image. Also chances are high that people will look into the file when building an install image, at least compared to using the profile. It only does things that are also set in profiles/installation-device.nix, even though that does a lot more not very suited for containers I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@erictapen erictapen Jul 12, 2022

Choose a reason for hiding this comment

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

I relooked at this and agree now, that passwordless login shouldn't be part of the image generation logic. I moved it into its own module nixos/modules/virtualisation/lxc-passwordless-login.nix and included it in the hydra image.

@erictapen
Copy link
Member Author

While working on this PR I repeatedly stumbled over nixos/maintainers/scripts/lxd/lxd-image.nix. I don't really get why its located under nixos/maintainers. It seems unintuitive to me that a hydra job would pull any code from maintainers. But I don't understand the structure enough to propose a better place for the logic.

};

# Add the overrides from lxd distrobuilder
systemd.extraConfig = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thanks.

Any idea what the purpose of the empty LoadCredential= is?

Copy link
Contributor

Choose a reason for hiding this comment

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

An empty directive will delete the directive during override.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok, I understand.

Copy link
Contributor

Choose a reason for hiding this comment

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

This block actually isn't serving its purpose. It was already incorrect, but updating it isn't making a difference.

Aug 27 00:33:34 wire1 systemd[1]: /etc/systemd/system.conf:14: Unknown section 'Service'. Ignoring.

These need to be service configs, added to the services. Distrobuilder either adds it to /etc/systemd/system/service.d/lxc.conf or every .service file in systemd's path. https://github.com/lxc/distrobuilder/blob/f15eec09df7b04f1bede66b0f31354da66748c9a/distrobuilder/main.go#L713-L718

I don't see a way to add it to service.d currently, but am happy to be corrected. The original author actually asked about creating this global service override, but it ended up here incorrectly.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/lxd-distrobuilder-support-for-nixos/21375/1

@mkg20001
Copy link
Member

is this still relevant? this would need an update, since the lxc-container module has changed

@erictapen
Copy link
Member Author

I'm afraid I lost focus on lxc in the last 6 months, as I migrated to systemd containers (via NixOS containers). So I didn't follow the development of the Nixpkgs lxc infrastructure and couldn't tell if this change is still needed.

I'll close for now, as I wouldn't work further on it rn.

@erictapen erictapen closed this Dec 31, 2022
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.

None yet

5 participants