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

Add missing parameters for podman container quadlet #847

Merged
merged 9 commits into from
Oct 6, 2024

Conversation

tompre
Copy link
Contributor

@tompre tompre commented Sep 22, 2024

As noticed in #844 the group_add parameter should be mapped to GroupAdd in the .container file.
I noticed that several other parameters are missing or not mapped correctly.

Missing parameters that should be mapped (podman version that introduced them):

  • cgroups -> CgroupsMode (latest -> most probably v5.3.0)

Parameters that should be mapped to .container file syntax

  • etc_hosts -> podman_args, should be AddHost (latest -> most probably v5.3.0)
  • log_opt -> podman_args, could be LogOpt (v5.2.0)
  • network_aliases -> podman_args, should be NetworkAlias (v5.2.0)
  • stop_signal -> podman_args, should be StopSignal (v5.2.0)
  • group_add -> podman_args, should be GroupAdd (v5.1.0)

Parameters that should be mapped to podman_args:

  • cpus
  • platform

There are also available parameters that can not be set using the collection but might be useful.
They are out of scope for this pull request, but some would be really useful such as AutoUpdate.
Here is a list for documentation:

  • Autoupdate
  • Mask
  • SeccompProfile
  • SecurityLabelDisable
  • SecurityLabelFileType
  • SecurityLabelLevel
  • SecurityLabelNested
  • SecurityLabelType
  • Unmask

Resolves #844
This reverts commit 4f24ece from pull request #827

@tompre tompre changed the title WIP: Add missing parameters for podman container quadlet Add missing parameters for podman container quadlet Sep 22, 2024
@sshnaidm
Copy link
Member

@tompre thanks for this great work! Are you going to do log opt also or it's fine to review/merge as is?

@tompre
Copy link
Contributor Author

tompre commented Sep 26, 2024

@sshnaidm I somehow thought that LogOpt would only capture the path mapping. This is not the case, so I added LogOpt too. There is one commit per parameter. If you want I can squash them together for the merge.

@tompre
Copy link
Contributor Author

tompre commented Sep 26, 2024

I did some more research and added the podman versions when the parameters where introduced.
What is your approach to installed podman versions?
Should I open another Issue/Patch for the two params that are not jet part of a release?

I had an issue with using .pod files on some distro as the podman version did not support them.

@sshnaidm
Copy link
Member

@tompre I do check a podman version in a few places like here, running a command. But I try to use it only when it make difference in a logic. In general I rely on user to run command with supported parameters for their Podman version. I can't track all Podman version changes, there are too much 😄
I think it will be a period when options will be more supported in Quadlet, so most of them will go there from PodmanArgs, and in this period we'll see a lot of inconsistency. So I'd leave it to user and just configure as latest released version does. Hopefully there are no much users of "middle" (ex 5.2.1, 5.2.3 etc) versions who will be affected.

@sshnaidm
Copy link
Member

sshnaidm commented Oct 3, 2024

@tompre I'm inclined to merge it in current state and then to add not released yet parameters when Podman releases them. Any objections?

@tompre
Copy link
Contributor Author

tompre commented Oct 6, 2024

Sounds good. I had an idea for automatically finding out when features where added.
I would like to open an new issue/pull request for that when I have more time.

@sshnaidm
Copy link
Member

sshnaidm commented Oct 6, 2024

Sounds good. I had an idea for automatically finding out when features where added. I would like to open an new issue/pull request for that when I have more time.

That's a great idea, please go for it!

@sshnaidm sshnaidm merged commit 8daec72 into containers:master Oct 6, 2024
20 checks passed
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.

quadlet module ignores the groups parameter when generating container unit file
2 participants