[CI:DOCS] Man pages: refactor common options: --volume #15706

edsantiago · 2022-09-08T20:53:13Z

This one is a nightmare, because --volume has been edited
in four different files throughout the years (five if you
count podman-build, which I am not including in this PR).
Those edits have not always been done in sync.

The list of options was reordered 2022-06-28 by Giuseppe in #14734,
but only in podman-create and -run (not in podman-pod-*). No
explanation of why, but I'll assume he knew what he was doing,
and have accepted that for the reference copy.

There was also a big edit in #8519.

The "Propagation property...bind mounted" sentence first appeared
in pod-clone, in #14299 by cdoern, with no obvious source of where
it came from. I choose to include it in the reference copy.

The "copy" option seems to work in pod-create, so I'm including
it in the reference copy. Someone please yell loudly if this is
not the case.

The "disables SELinux separation for containers used in the build",
no idea, changed that to just "for the container/pod"

The "advanced users / overlay / upperdir / workdir" paragraph
makes zero sense to me, but hey, I assume it applies to all
the commands, so I put it in the reference copy.

Finally, there's still a mishmash of backticks, asterisks, underscores,
and even quotation marks. Someone is gonna have to perform major
cleanup on this one day, but at least it'll be in only one place.

Signed-off-by: Ed Santiago [email protected]

None

openshift-ci · 2022-09-08T20:53:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [edsantiago]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

edsantiago

I've made comments on what I think are the riskiest changes. There are tons more changes, but those are mostly backticks asterisks underscore type stuff. I think.

This is really impossible to review. My only suggestion is, ignore the man pages, read ONLY the new volume.md file, and think about whether every single word in it applies to each of the four man pages.

TIA.

edsantiago · 2022-09-08T20:53:54Z

docs/source/markdown/options/volume.md

+* **rw**|**ro**
+* **z**|**Z**
+* [**O**]
+* [**U**]
+* [**no**]**copy**
+* [**no**]**dev**
+* [**no**]**exec**
+* [**no**]**suid**
+* [**r**]**bind**
+* [**r**]**shared**|[**r**]**slave**|[**r**]**private**[**r**]**unbindable**


The list of options was reordered 2022-06-28 by Giuseppe in #14734,
but only in podman-create and -run (not in podman-pod-*). No
explanation of why, but I'll assume he knew what he was doing,
and have accepted that for the reference copy.

edsantiago · 2022-09-08T20:56:25Z

docs/source/markdown/options/volume.md

+Propagation property can be specified only for bind mounted volumes and not for
+internal volumes or named volumes. For mount propagation to work the source mount


The "Propagation property...bind mounted" sentence first appeared
in pod-clone, in #14299 by cdoern, with no obvious source of where
it came from. I choose to include it in the reference copy.

I believe this is correct for all.

edsantiago · 2022-09-08T20:57:20Z

docs/source/markdown/options/volume.md

+* **z**|**Z**
+* [**O**]
+* [**U**]
+* [**no**]**copy**


The "copy" option was not present in the pod-* man pages, but it
seems to work in pod-create, so I'm including
it in the reference copy. Someone please yell loudly if this is
not the case.

Yes this should work.

edsantiago · 2022-09-08T20:57:58Z

docs/source/markdown/options/volume.md

+Note: Do not relabel system files and directories. Relabeling system content
+might cause other confined services on your machine to fail.  For these types
+of containers we recommend disabling SELinux separation.  The option
+**--security-opt label=disable** disables SELinux separation for the <<container|pod>>.


This used to say "...for containers used in the build". Maybe a leftover from podman-build? Anyhow, I ditched that.

Good, this is correct.

edsantiago · 2022-09-08T20:59:18Z

docs/source/markdown/options/volume.md

+For advanced users, the **overlay** option also supports custom non-volatile
+**upperdir** and **workdir** for the overlay mount. Custom **upperdir** and
+**workdir** can be fully managed by the users themselves, and Podman will not
+remove it on lifecycle completion.
+Example **:O,upperdir=/some/upper,workdir=/some/work**


This paragraph was present only in podman-run. I chose to include it here, in the reference copy, so now it will show up in all those man pages. If that is wrong, someone please yell.

Should be the same for all.

edsantiago · 2022-09-08T21:02:16Z

docs/source/markdown/options/volume.md

+For example if a user wanted to volume mount their entire home directory into a
+<<container|pod>>, they need to disable SELinux separation.
+
+	   $ podman <<subcommand>> --security-opt label=disable -v $HOME:/home/user fedora touch /home/user/file


And actually this entire paragraph (starting with "Note: Do not relabel") was present only in the podman-create and -run man pages, not in the pod-* ones. I choose to include it now in all. If this is wrong, please yell.

Oh, and, the <<subcommand>> on pod pages will show podman create/run, not podman pod create/run. I need to fix that, but it won't be today.

It should be everywhere.

rhatdan · 2022-09-09T10:29:39Z

LGTM
@giuseppe @mheon PTAL

rhatdan · 2022-09-09T13:04:04Z

@containers/podman-maintainers PTAL

mheon · 2022-09-09T13:14:02Z

docs/source/markdown/options/volume.md

+
+Create a bind mount. If `-v /HOST-DIR:/CONTAINER-DIR` is specified, Podman
+bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Podman
+container. Similarly, `-v SOURCE-VOLUME:/CONTAINER-DIR` will mount the volume


mount the named volume from the host into the container

Does Buildah support named volumes?

Ah, no podman build manpage changes in here, OK. Ignore my earlier question.

Thanks. Just to make ultra-sure, here is what the new diff looks like:

Create a bind mount. If `-v /HOST-DIR:/CONTAINER-DIR` is specified, Podman bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Podman -container. Similarly, `-v SOURCE-VOLUME:/CONTAINER-DIR` will mount the volume -in the host to the container. If no such named volume exists, Podman will +container. Similarly, `-v SOURCE-VOLUME:/CONTAINER-DIR` will mount the named +volume from the host into the container. If no such named volume exists, Podman will create one. (Note ....

mheon · 2022-09-09T13:16:02Z

docs/source/markdown/options/volume.md

+The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume
+will be mounted into the container at this directory.
+
+Volumes may specify a source as well, as either a directory on the host


This feels like it should be combined with the first paragraph

Sorry to be really dense, but this is not my area of expertise. Could you give me a precise diff for me to apply, or defer this for a (much-needed) future man page cleanup? This option has taken a lot out of me, and I'm feeling nervous enough about it as it is. I don't trust my judgment enough to make this change on my own.

Remove the first sentence, and graft the remainder of the paragraph onto the end of the first paragraph.

This one is a nightmare, because --volume has been edited in four different files throughout the years (five if you count podman-build, which I am not including in this PR). Those edits have not always been done in sync. The list of options was reordered 2022-06-28 by Giuseppe in containers#14734, but only in podman-create and -run (not in podman-pod-*). No explanation of why, but I'll assume he knew what he was doing, and have accepted that for the reference copy. There was also a big edit in containers#8519. The "Propagation property...bind mounted" sentence first appeared in pod-clone, in containers#14299 by cdoern, with no obvious source of where it came from. I choose to include it in the reference copy. The "**copy**" option seems to work in pod-create, so I'm including it in the reference copy. Someone please yell loudly if this is not the case. The "disables SELinux separation for containers used in the build", no idea, changed that to just "for the container/pod" The "advanced users / overlay / upperdir / workdir" paragraph makes zero sense to me, but hey, I assume it applies to all the commands, so I put it in the reference copy. Finally, there's still a mishmash of backticks, asterisks, underscores, and even quotation marks. Someone is gonna have to perform major cleanup on this one day, but at least it'll be in only one place. Signed-off-by: Ed Santiago <[email protected]>

edsantiago · 2022-09-09T14:21:36Z

Re-pushed with @mheon's fixed wording and with a new <<fullsubcommand>> for the command example.

rhatdan · 2022-09-09T15:13:37Z

/lgtm

openshift-ci bot added the release-note-none label Sep 8, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2022

edsantiago commented Sep 8, 2022

View reviewed changes

mheon reviewed Sep 9, 2022

View reviewed changes

edsantiago force-pushed the docs_dedup_volume branch from 4fa1fa3 to 3a9a7dc Compare September 9, 2022 14:20

openshift-ci bot assigned rhatdan Sep 9, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2022

openshift-merge-robot merged commit 94864cb into containers:main Sep 9, 2022

edsantiago deleted the docs_dedup_volume branch September 9, 2022 15:29

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI:DOCS] Man pages: refactor common options: --volume #15706

[CI:DOCS] Man pages: refactor common options: --volume #15706

edsantiago commented Sep 8, 2022

openshift-ci bot commented Sep 8, 2022

edsantiago left a comment

edsantiago Sep 8, 2022

edsantiago Sep 8, 2022

rhatdan Sep 9, 2022

edsantiago Sep 8, 2022

rhatdan Sep 9, 2022

edsantiago Sep 8, 2022

rhatdan Sep 9, 2022

edsantiago Sep 8, 2022

rhatdan Sep 9, 2022

edsantiago Sep 8, 2022

rhatdan Sep 9, 2022

rhatdan commented Sep 9, 2022

rhatdan commented Sep 9, 2022

mheon Sep 9, 2022

mheon Sep 9, 2022

mheon Sep 9, 2022

edsantiago Sep 9, 2022

mheon Sep 9, 2022

mheon Sep 9, 2022

edsantiago Sep 9, 2022

mheon Sep 9, 2022

edsantiago commented Sep 9, 2022

rhatdan commented Sep 9, 2022

		Propagation property can be specified only for bind mounted volumes and not for
		internal volumes or named volumes. For mount propagation to work the source mount

[CI:DOCS] Man pages: refactor common options: --volume #15706

[CI:DOCS] Man pages: refactor common options: --volume #15706

Conversation

edsantiago commented Sep 8, 2022

openshift-ci bot commented Sep 8, 2022

edsantiago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Sep 9, 2022

rhatdan commented Sep 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edsantiago commented Sep 9, 2022

rhatdan commented Sep 9, 2022