Needs review: Make the documentation of user namespace options in podman-run clearer#1380
Needs review: Make the documentation of user namespace options in podman-run clearer#1380najamelan wants to merge 2 commits intocontainers:masterfrom
Conversation
This proposes a more comprehensible man page. A number of things have been lost in translation and this should be reviewed: - the former docs from --userns say that it is disabled by default. I suppose that this is the same as --userns:host, but this should be confirmed. It also stated that is would use options like pid=host, which confuses me as pid namespaces are a totally different thing from user namespaces. It also mentions the enabling of --privileged. I think the difference between using --userns:host and not using any user namespace options at all is not clear and maybe not very logical. Also what would be the difference between using --userns:host and using --priveleged alone? - I found the syntax for --gidmap at the bottom of the man page in the examples. In the example it doesn't use '=', eg. podman run `--gidmap 0:30000:2000`. For consistency with the other options I have used '=' for now, but if it is optional, I would remove it everywhere, as less tokens is usually improved readability. For now the inconsistency remains between the options doc and the examples section. - It wasn't very clear to me whether one should hard wrap long lines or not as the contains a mix. - I haven't for now looked at user namespace options on other commands, but that should be done surely before merging. - I didn't know which command to run to generate the groff, so that needs doing still. from issue containers#1374 Signed-off-by: Naja Melan <najamelan@autistici.org> Signed-off-by: Naja Melan <najamelan@autistici.org>
|
Can one of the admins verify this patch?
|
|
bot, add author to whitelist |
|
Docs changes LGTM |
|
bot, retest this please |
docs/podman-run.1.md
Outdated
| **--subgidname**=name | ||
|
|
||
| Name for GID map from the `/etc/subgid` file. Using this flag will run the container with user namespace enabled. This flag conflicts with `--userns` and `--gidmap`. | ||
| Run the container in a new user namespace from the map with 'name' in the `/etc/subgid` file. |
There was a problem hiding this comment.
suggest "from the map" to "using the map"
There was a problem hiding this comment.
ditto other instances.
docs/podman-run.1.md
Outdated
|
|
||
| The following example maps uids 0-2000 in the container to the uids 30000-31999 on the host and gids 0-2000 in the container to the gids 30000-31999 on the host. | ||
| Run the container in a new user namespace with the supplied mapping. This option conflicts with the --userns and --subgidname flags. | ||
| This option can be passed several times to map different ranges. If calling podman run as an unprivileged user, the user needs to have the right to use the mapping. See `man subuid`. |
There was a problem hiding this comment.
man subuid to subuid(5) ditto other spots for subuid and subgid
TomSweeneyRedHat
left a comment
TomSweeneyRedHat
left a comment
There was a problem hiding this comment.
Couple of nits, but very nice change overall. TYVM @najamelan !
Signed-off-by: Naja Melan <najamelan@autistici.org>
|
I made the suggested changes. There's still the question about what |
|
📌 Commit 7d90862 has been approved by |
|
I really propose not merging this before resolving the other questions from the pull request descriptions. I propose:
|
|
@najamelan Lets do that in a different PR. We have a release later today. And getting these documentation fixes in now are good. Then we can continue to clean it up next week. |
|
It's up to you, just know that possibly the documentation for |
|
⌛ Testing commit 7d90862 with merge e6ab690... |
|
💔 Test failed - status-papr |
|
I don't really understand why it's always the FAH28 VMs that blow up on Homu - it should all be running on the same cluster... @rh-atomic-bot retry |
Signed-off-by: Naja Melan <najamelan@autistici.org> Closes: #1380 Approved by: rhatdan
|
☀️ Test successful - status-papr |
Whew! This one started off identical everywhere, but the version in podman-run got fixed in containers#1380, then again in containers#5192, with no corresponding fixes to any of the other man pages. I went with the podman-run version, with a small change in wording. Signed-off-by: Ed Santiago <santiago@redhat.com>
github-actions
bot
added
the
locked - please file new issue/PR
5 participants
This proposes a more comprehensible man page.
A number of things have been lost in translation and this should be reviewed:
podman run --gidmap 0:30000:2000. For consistency with the other options I have used '=' for now, but if it is optional, I would remove it everywhere, as less tokens is usually improved readability. For now the inconsistency remains between the options doc and the examples section.from issue #1374
Signed-off-by: Naja Melan najamelan@autistici.org