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

Document --volume from podman-remote run/create client #9882

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Mar 30, 2021

[NO TESTS NEEDED] This PR is mainly documentation and some code cleanup.

Also cleanup and consolidate handling of other hanlding of podman-remote
hidden options.

Fixes: #9874

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2021
@Luap99
Copy link
Member

Luap99 commented Mar 30, 2021

This is not correct. podman-remote run --volume is supported and should work. It will just not mount the host path on the client instead it will always mount the host path on the server. This should be documented but do not hide --volume from remote.

@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2021

Why would we want to support this?

@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2021

podman-remote main use case in on Macs and Windows, and remote machines, I don't see this as a common behaviour and set's bad expectations.

@Luap99
Copy link
Member

Luap99 commented Mar 30, 2021

Well first there are users who use this, see #8473. Second, how do you provide persistent storage for your containers when you disable the --volume flag. (I know you just hide them, but still documenting that this doesn't work is wrong.)

@edsantiago
Copy link
Member

--volume is kind of special in that it can also refer to named volumes, which (I think?) should be able to work with podman-remote (confirmed: podman-remote volume xxx seems to work). I agree with @Luap99 that the flag shouldn't be marked hidden, but I tentatively disagree about path mounts: I don't think -v /fs:/fs is meaningful in the remote case. (BUt I'm listening, and could be convinced otherwise)

@Luap99
Copy link
Member

Luap99 commented Mar 30, 2021

-v /path:/path is needed for remote. You can have only one storage location with named volumes. When your container needs data from different partitions you cannot use named volumes, also you might want to mount files from the host into the container.
Docker works exactly the same.
The --volume is without a doubt one of the most used flags for run/create. Hiding or removing it will confuse users even more and saying it is not supported is just wrong.

@rhatdan rhatdan changed the title Hide --volume from podman-remote run/create client Document --volume from podman-remote run/create client Mar 30, 2021
@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2021

Beaten into submission.

[NO TESTS NEEDED] This PR is mainly documentation and some code cleanup.

Also cleanup and consolidate handling of other hanlding of podman-remote
hidden options.

Fixes: containers#9874

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2021

I changed the podman-remote help to reflect this information as well.

./bin/podman-remote run --help | grep -- -v,
-v, --volume stringArray Bind mount a volume into the container. Volume src will be on the server machine, not the client

@TomSweeneyRedHat
Copy link
Member

LGTM

@Luap99
Copy link
Member

Luap99 commented Mar 30, 2021

/lgtm
/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2021
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 30, 2021
@rhatdan
Copy link
Member Author

rhatdan commented Mar 30, 2021

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 30, 2021
@openshift-merge-robot openshift-merge-robot merged commit 6189232 into containers:master Mar 30, 2021
@github-actions 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 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman-remote build: -v is a NOP
6 participants