Skip to content

Conversation

@tieong
Copy link

@tieong tieong commented May 31, 2025

Hey there,

First time I do a Nix related PR, so would appreciate pointers to do things better.

This PR allow us to create privileged and unprivileged LXC containers, the only thing we are missing here is apparmor support which should not be too hard to enable.

I dug a little into this and we just need to wrap the lxc package so that it can finds apparmor_parser, also it does hardcode stuff such as /usr/bin/lxc-start so we need to patch that as well, and the newuidmap/newgidmap binaries

INFO     idmap_utils - ../src/lxc/idmap_utils.c:lxc_map_ids:165 - newuidmap binary is missing
INFO     idmap_utils - ../src/lxc/idmap_utils.c:lxc_map_ids:171 - newgidmap binary is missing
DEBUG    idmap_utils - ../src/lxc/idmap_utils.c:lxc_map_ids:186 - No newuidmap and newgidmap binary found. Trying to write directly with euid 0

Then it just needs more docs and examples on how to set this, the rest of the pain points I had are in the commit messages.

But it's mostly about how we handle lxc includes, the lxc package and run_command sed problems.

tieong added 14 commits May 31, 2025 22:44
pve-lxc-syscalld: Call pve-lxc-syscalld package

pve-lxc-syscalld: Add package and fix execstart
We already propagate diff and ip binaries via propagatedBuildInputs
but this does not seems to works everywhere.

Take the iface_set_master subroutine in pve-common/src/PVE/Network.pm

I thought it would know how to find ip but it doesn't for some
reason. Is it because it is in an eval block and the context is
somewhat different?
Without these it cannot untar archives
This was taken straight out of the nixpkgs definition, there is
probably a better way than doing this.

This was done because there were hardcoded stuff in the lxc package
which I needed to replace and it was easier to refer to this package
from pve-container.

I only made 2 modifications compared to the nixpkgs all in the
postInstall section:

I add this at the end, because it was complaining about missing coreutils:

    sed -i $out/libexec/lxc/lxc-containers \
      -e "s|touch|${coreutils}/bin/touch|" \
      -e "s|rm|${coreutils}/bin/rm|"

And I removed this line:

     substituteInPlace $out/share/lxc/config/common.conf --replace-fail "$out/share" "/run/current-system/sw/share"

/run/current-system/sw/share was empty, not sure how it's supposed to
be populated.
To use proper types, for example null wasn't getting rendered in the
final service file, needed to add it between quotes.
Added these services because that's how it's done in a vanilla proxmox
install, but I think pve-container service being presents are enough
IIRC, we could try removing the wants lxc.

Also instead of defining these here, maybe we could leverage the
builtin virtualisation.lxc? I did not do so because it was
quicker/easier with these services present directly.
LXC config file that gets generated needs to include a lot of stuff.

First is LXCFS so that containers actually know what are their true
uptime for example not the host.

Second seccomp rules

And the rest are common config, note that ideally it also wants distro
specific files, these are not packaged for nix yet.

https://github.com/lxc/lxc-templates/tree/main/config
@tieong tieong mentioned this pull request May 31, 2025
@tieong
Copy link
Author

tieong commented May 31, 2025

Also this might be a good opportunity to decide how we handle container templates download and gpg keys, so that we can use the pveam download/list/available command and download from the gui.

@JulienMalka
Copy link
Member

Ideally, given that this is a big piece of functionality, and given that I do not personally use this functionality, to make sure that this doesn't break with updates without anyone realizing, we need a good coverage of it with NixOS tests.

@tieong
Copy link
Author

tieong commented Jun 12, 2025

Will take a look this weekend hopefully

tieong added 8 commits June 27, 2025 19:56
The patch is necessary because gpgv complains if the keyring is in
armor format instead of binary.

Latest version of sequoia do not allow to produce binary output so
this will be needed in the future as well.
This should already be replaced via a sed, furthermore substituteAll
has been deprecated and throw an error now.
@tieong
Copy link
Author

tieong commented Jun 27, 2025

Not really sure how to solve those pesky collisions problems when I upgrade pve-storage to 9.0.1:

erl> error: collision between `/nix/store/ax4zxx7c84dkc3q4hma6y8cw5f1yd56f-pve-storage-9.0.1/bin/pvesm' and `/nix/store/llhl9w7l5cf4afys1pcggsy0x1ajl3hi-pve-storage-9.0.1/bin/pvesm'

Would appreciate pointers.

@JulienMalka
Copy link
Member

I am currently very busy with a deadline, but will review and help you merge this PR after july 18.

@tieong
Copy link
Author

tieong commented Jul 7, 2025

Investigated a bit more this weekend.

Collisions are caused by the pve-container that needs pve-storage, and pkgs such as pve-ha-manager or proxmox-ve depends on pve-container and pve-storage hence the collisions.

I tried to played with the lib.lowPrio/hiPrio but it didn't work

@max-zelinski
Copy link

any updates on getting this merged by chance?

@NotChristianGarcia
Copy link

NotChristianGarcia commented Nov 30, 2025

I can utilize this branch to some success in container creation in vm. Is it expected for this PR to go through or waiting for 9.1 change or something? 9.1 has some snazzy image -> lxc imports now that many will be interested in. Haven't tested that feature, only premade lxc.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants