podman: Added build, image, and volume quadlet support

[bamhm182]

Description

• Added support for build, image, and volume quadlets
• Resolved test failures due to podman 5.3.0 upgrade
• Replaced several instances of pkgs.podman with services.podman.package

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.

Maintainer CC

@n-hass @bamhm182

[bamhm182]

Cursory testing appears to have everything functioning as intended, but submitting PR to get more eyes on it to try to break things. Will also do more in depth testing when I get a moment.

Also looking for a bit of feedback on:

• The way I am finding container resources is a little hairy. Is there a better way to do this?

Other notes:

• There doesn't appear to be a way to tag images, so our standard approach of using nix.home-manager.managed=true to find our images doesn't really work. I don't think it's that big of a problem as there is hopefully periodic pruning of images. The images that come from builds are tagged and managed appropriately.

I just realized I forgot to add in the integration tests for the build quadlet. I am also expecting the container dependencies for build quadlets won't be quite right. Also looks like the docs are not building properly. Will look into all this in a few hours. (EDIT: Resolved.)

[bamhm182]

After playing around with this more, I am wondering if we should add an option to services.podman.volumes.* to stop the activation script from nuking the volume automatically when removed? I am thinking like a preserve option which adds a tag like homemanager.preserve=true that would have the activation script log that it has been queued for deletion, but that flag is set and won't be deleted. I think this is the only quadlet that needs something like this as images can be rebuilt, contains can be reset, but people will be upset if their data gets clobbered.

The current functionality when setting services.podman.containers.mysleep.volumes = [ "somevol:/tmp"] prior to this PR is that a volume named "somevol" will be created automatically if it doesn't exist, and it will not be deleted when everything is removed from the nix config, which makes me think preserving the volumes would be the default option.

Alternatively, there could be an option like services.podman.deleteHmVolumes, which is set to false by default and would result in volumes not being looked at then false, and managed like everything else when true.

[n-hass]

still haven’t had time to have a proper play around and review, but I would go with your first option - add a persistent/preserve option to volumes that defaults true, set a label nix.home-manager.preserve = true, and adjust the activation script to respect it

[bamhm182]

All good. Should be good to go with that change.

[bamhm182]

Ran into a couple snags with the Builds quadlets. Fixed an issue where it should be referencing localhost\homemanager\bld instead of homemanager\bld. Easy enough.

One issue that I'm having a bit of a harder time with is that the Pull option appears to be breaking the ability to build an image because it appears podman is understanding the value set for --pull as the context directory. The quadlet builds the following line:

$ podman build --pull always --tls-verify --tag homemanager/coder --label nix.home-manager.managed=true --file /nix/store/v9hfyvdv0lr86w3xdkrix30g5dppr6vg-coder-containerfile
Error: context must be a directory: "./newer"

It works when executed with an =, like this:

$ podman build --pull=always ...

I am thinking that I may remove the pull attribute and expect users who wish to set it to do builds."bld".extraPodmanArgs = [ "--pull=newer" ]; if they want it set.

[rycee]

Can use optionals. Something like

Suggested change

	WantedBy = (if buildDef.autoStart then [
	"default.target"
	"multi-user.target"
	] else
	[ ]);
	WantedBy = optionals buildDef.autoStart [
	"default.target"
	"multi-user.target"
	];

ought to work.

[rycee]

Can change types.submodule { to types.submodule ({name, ...}: { and then change this like so:

Suggested change

	type = with types; nullOr str;
	default = null;
	type = with types; nullOr str;
	default = "Service for build ${name}";
	defaultText = "Service for build \${name}";

[rycee]

The parentheses are unnecessary

Suggested change

	Description = (if (builtins.isString buildDef.description) then
	buildDef.description
	else
	"Service for build ${name}");
	Description = if isString buildDef.description then
	buildDef.description
	else
	"Service for build ${name}";

but I think the better solution is to make description have type str and default the value to "Service for build ${name}". See my other comment.

[bamhm182]

I went ahead and made the changes in your other comment, then set this as Description = buildDef.description;. If it is null, then it is automatically dropped. Much simpler!

[rycee]

Suggested change

	"Whether to start the build on boot. (Requires user lingering)";
	"Whether to start the build on boot. Requires user lingering.";

[rycee]

Not sure why the podman module ended up using this home.file format, seems to me that it would be possible to simply use xdg.configFile.

Suggested change

	home.file."${config.xdg.configHome}/podman/images.manifest".text =
	xdg.configFile."podman/images.manifest".text =

[rycee]

Suggested change

	ImageTag = ([ "homemanager/${name}" ] ++ buildDef.tags);
	Label = (buildDef.labels // { "nix.home-manager.managed" = true; });
	ImageTag = [ "homemanager/${name}" ] ++ buildDef.tags;
	Label = buildDef.labels // { "nix.home-manager.managed" = true; };

[rycee]

Suggested change

	buildConfig = (podman-lib.deepMerge {
	buildConfig = podman-lib.deepMerge {

[rycee]

Perhaps this could be simplified to

Suggested change

	buildSection = if (builtins.hasAttr "Build" extraConfig) then
	extraConfig.Build
	else
	{ };
	imageTags = if (builtins.hasAttr "ImageTag" buildSection) then
	buildSection.ImageTag
	else
	[ ];
	imageTags = (extraConfig.Build or {}).ImageTag or [];

[rycee]

Does this tag really have to be in position 0? Perhaps something like

Suggested change

	containsRequiredTag = (if builtins.length imageTags > 0 then
	builtins.elemAt imageTags 0
	else
	"") == "homemanager/${quadletName}";
	in if (containsRequiredTag || builtins.length imageTags == 0) then
	[ ]
	else [{
	assertion = false;
	message = ''
	In '${quadletName}' config. Build.ImageTag: '[ "${
	builtins.concatStringsSep ''" "'' imageTags
	}" ]' does not have 'homemanager/${quadletName}' at index 0.'';
	}];
	containsRequiredTag = builtins.elem "homemanager/${quadletName}" imageTags;
	imageTagsStr = concatMapStringsSep " " toString imageTags;
	in [{
	assertion = imageTags == [ ] || containsRequiredTag;
	message = ''
	In '${quadletName}' config. Build.ImageTag: '[ ${imageTagsStr} ]' does not contain 'homemanager/${quadletName}'.'';
	}];

would work?

[bamhm182]

The reason that I had it forced to position 0 was so that you could easily discern which ones were from Home Manager and which ones were built outside of Home Manager. That said, it probably isn't really that important and this code is a lot more straight forward. Will make the swap.

[rycee]

Thanks! Looks quite impressive. I've added a few mostly stylistic comments. I cannot comment on the functionality since I don't use the module myself.

[bamhm182]

Thanks for all of your feedback @rycee. Incorporating it into a new commit now. I went ahead and took the suggestions that you made towards build.nix and applied them to all the new quadlets. As they do not affect the functionality of the old quadlets, I have left them there for now. I have one more PR I want to make to get pod and kube quadlets in here, then I believe this code base is in need of a refactor just to make sure that everything is standardized across all files and it's all easy to read and maintain. I will incorporate these suggestions (and a handful of others I have noticed) through the rest of the files into that refactor PR.

As for as the functionality goes, I have been converting all my containers from the way I used to manage them into this module. Have yet to notice any significant problems that haven't already been addressed.