nix-community / home-manager

Manage a user environment using Nix [maintainer=@rycee]
https://nix-community.github.io/home-manager/
MIT License
7.15k stars 1.85k forks source link

podman: install package if enabled and create config files #6039

Closed dawidd6 closed 2 days ago

dawidd6 commented 3 weeks ago

Description

Copied some useful options from NixOS podman module and adapted the configuration to home-manager.

Also setting services.podman.enable = true will now add podman to PATH.

Checklist

Maintainer CC

@bamhm182 @n-hass

bamhm182 commented 3 weeks ago

I like it and it looks mostly good. For some reason ubuntu-latest is failing here, but they seem unrelated and I don't seem to be getting them locally. That said, it does appear that the actual output of registries.conf has an extra [registries] section at the top that shouldn't be there.

n-hass commented 3 weeks ago

LGTM. If you wanted to avoid the name clashing with actual resource definitions, you could scope these configuration options under config like services.podman.config.xx

@bamhm182 registries.conf default settings gives this, which should be fine

[registries.block]
registries = []

[registries.insecure]
registries = []

[registries.search]
registries = ["docker.io"]
bamhm182 commented 3 weeks ago

I just had a few spare minutes yesterday, so I opted to clone the repo and run the tests while reviewing it and hadn't had time to dig deep into it. I spun up a new VM to mess with it, and I can't recreate the issue I was seeing in the test, but the test still fails the same way. This is what I am seeing on a few different machines.

$ nix develop --ignore-environment .#podman-configuration
podman-configuration: FAILED
Expected /nix/store/w6qihr88hmn3rd8wck2hra465rqbn36m-nmt-report-podman-configuration/normalized/registries.conf to be same as /nix/store/7sn4505w29lgmkwny52s9nqsm6vf4yp7-configuration-registries-expected.conf but were different:
--- actual
+++ expected
@@ -1,4 +1,3 @@
-[registries]
 [registries.block]
 registries = ["ghcr.io", "gallery.ecr.aws"]

For further reference please introspect /nix/store/w6qihr88hmn3rd8wck2hra465rqbn36m-nmt-report-podman-configuration

Looking into it further, it has to do with using nix develop for the tests as nix-shell --pure tests -A run.podman-configuration appears to work just fine.

dawidd6 commented 3 weeks ago

Hmm. Different nixpkgs revisions? I was using latest master for the evaluation using nix-shell.

bamhm182 commented 2 weeks ago

Just went through and made sure that everything I could find was up to date. Now my test results match those of the automatic pipeline. Sorry for the confusion!

@n-hass, I'm not totally sure I understand what that suggestion is trying to avoid. Where would we run into clashes with the implementation by dawidd6?

n-hass commented 2 weeks ago

Where would we run into clashes with the implementation by dawidd6?

Eg having to name services.podman.containersConf because there is already services.podman.containers. Actual resources vs podman configuration files.

bamhm182 commented 2 weeks ago

Ah, yeah. I see what you're saying. I'm also wondering if this belongs under the podman configuration, or if it ought to be under a services.containers like it is for the main nixpkgs. I know right now there's only podman in HM, but rootless docker is technically a thing and uses the same files.

bamhm182 commented 2 weeks ago

Went ahead and submitted a PR on @dawidd6's fork here. I just put everything under services.podman.config as @n-hass had suggested.

dawidd6 commented 2 weeks ago

Isn't it a little too deep now? services.podman.config.storage.settings, maybe only services.podman.config.storage would suffice?

bamhm182 commented 2 weeks ago

Yeah. Very fair point. Made the change to the PR I made.

EDIT: Just realized you'd already merged. Want to just make the change yourself?

rycee commented 2 days ago

Thanks! Looks good to me in general, just one minor comment.

rycee commented 2 days ago

Thanks! Merged to master now 🙂