docs: add new containers-config(5) man page#817
Conversation
github-actions
Bot
added
storage
|
@Luap99 the tests are probably #816 (comment) . |
|
@Luap99 (I’m not sure I can get to this today.) Can you have a LLM proofread tiny typos, please? |
|
That's probably nitpick, but standardizing on "drop-in" or "drop in" would be good. Currently, we use them randomly it seems. I would vote for "drop-in". |
| Admin overrides: | ||
|
|
||
| - `%ProgramData%\containers\<name>.conf` | ||
| - `%ProgramData%\containers\<name>.conf.d\` |
There was a problem hiding this comment.
Technically the code also searches in .rootful.conf.d and .rootless.conf.d on Windows, right?
I think we should keep this undocumented, but I still wanted to point this out during review, because it might be interesting to someone with more knowledge of the implications.
There was a problem hiding this comment.
I think we likely should fix that in the code, practically there is no such things as an uid on windows. On windows os.Getuid() returns -1 which currently in parse.go it uses the rootful path (uid > 0)
I guess if windows is hard coded to rootful then it might be best to just not support it at all? Or at least not document it?
There was a problem hiding this comment.
Not documenting is a sane thing to do imho. Removing support for rootful and rootless would be cleaner for sure, but I don't think it needs to be part of this PR. This one should focus just on man-page.
|
@Luap99 , thanks for the patch. The new man-page looks great. I tried to correct few typos and grammar issues and pointed out few places where the documentation does not match the code. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks! LGTM overall.
All comments are non-blocking, feel free to merge as is if that helps the timing.
| - `$XDG_CONFIG_HOME/containers/containers.conf.d/50-myconf.conf` | ||
| - `/usr/share/containers/containers.conf.d/99-conf.conf` | ||
|
|
||
| ## Example 2 |
There was a problem hiding this comment.
Non-blocking: I’m not sure we need both examples, don’t they basically say the same thing? Also using a pretty different form of the two examples is a bit surprising.
I’m mildly in favor of example 2, with the “empty file to silence a built-in default” technique as a useful demonstration.
There was a problem hiding this comment.
I have left both, the first one is quicker to follow for users who understand the concept already.
| - `%APPDATA%\containers\<name>.conf.d\` | ||
|
|
||
|
|
||
| # Load order |
There was a problem hiding this comment.
Non-blocking: consider placing this section before the lists. Otherwise the user might need to scroll over a few pages, including the rarely-relevant FreeBSD / Windows sections, to understand how this fits together.
There was a problem hiding this comment.
I left this, I couldn’t think of a good phrasing.
|
|
||
| # SEE ALSO | ||
| containers-storage.conf(5), containers-policy.json(5), containers-registries.conf(5), tmpfiles.d(5) | ||
| **[containers-config(5)](containers-config.5.md)**, containers-storage.conf(5), containers-policy.json(5), containers-registries.conf(5), tmpfiles.d(5) |
There was a problem hiding this comment.
(Non-blocking: The extra bold on the one item seems a bit unusual here…
[https://man7.org/linux/man-pages/man7/man-pages.7.html says .BR name (section), although we ~never do that IIRC.]
If the goal is to follow Podman’s docs/MANPAGE_SYNTAX.md, fine, but then please update the whole line.
Applies to other pages as well.
)
There was a problem hiding this comment.
It turns out Podman has specific code to strip the links when converting to man pages, and those links otherwise pollute the man page text.
I have dropped the links to .md files entirely for now; eventually we could consider inheriting / sharing that Markdown preprocessing step.
There was a problem hiding this comment.
uh I should have looked at this before...
mhh, I kinda like links because well I can use them when one looks at markdown in github but sure lets make sure the man page must look right first and foremost.
There was a problem hiding this comment.
I have filed #827 to track the possible improvement.
| __/etc/containers/containers.rootless.d/\*.conf__ and __/etc/containers/containers.rootless.d/\$UID/\*.conf__. The UID is the user's uid which podman runs under so it can be used to specify a certain config for only a single user without having to put the config into the user's home directory. | ||
| For user specific configuration it reads __\$XDG_CONFIG_HOME/containers/containers.conf__ and | ||
| __\$XDG_CONFIG_HOME/containers/containers.conf.d/\*.conf__ files. When `$XDG_CONFIG_HOME` is not set it falls back to using `$HOME/.config` instead. | ||
| By default, the configuration is read from `$XDG_CONFIG_HOME/containers/containers.conf` (or from `$HOME/.config/containers/containers.conf` if `$XDG_CONFIG_HOME` is unset), if it exists; otherwise from `/etc/containers/containers.conf`; otherwise from `/usr/share/containers/containers.conf`. |
There was a problem hiding this comment.
Should this also discuss the CONTAINERS… and …OVERRIDE environment variables, or would that be better in individual pages?
There was a problem hiding this comment.
I left this as is, unsure how much we want to commit to the environment variables.
| In addition to containers.conf, drop-in files using the same format from the following directories are also read: | ||
| - `$XDG_CONFIG_HOME/containers/containers.conf.d` (or from `$HOME/.config/containers/containers.conf.d` if `$XDG_CONFIG_HOME` is unset) | ||
| - `/etc/containers/containers.conf.d` | ||
| - `/etc/containers/containers.rootful.conf.d` (only when running as uid 0) |
There was a problem hiding this comment.
(Absolutely non-blocking, this is fine as is, no action necessary: I’m a bit ambivalent about including the full lists in the individual pages. On the other hand it’s easier to refer to lists without reading another page, OTOH this loses the FreeBSD / Windows nuance, and it makes the centralized man page feel perhaps a bit unnecessary.)
Add a general man page which should describe the new config file loading order with some examples. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Remove outdated descriptions, there is no need to list storage.conf paths here and also we do not need to duplicate the paths we list twice. Instead just have one list of the paths and link to containers-config(5). Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
Sure, updated. @jankaluza PTAL. |
| This document describes the loading order of the **[containers.conf](containers.conf.5.md)**, | ||
| **[storage.conf](../../storage/docs/containers-storage.conf.5.md)** and | ||
| **[registries.conf](../../image/docs/containers-registries.conf.5.md)**. |
There was a problem hiding this comment.
all these link changes are part of the "Add a standard EXAMPLES section", is that right? I assume they should be in different one.
There was a problem hiding this comment.
I messed something up, I had a separate commit for the removal of links.
|
LGTM |
4 participants
Add a general man page which should describe the new config file loading order with some examples.