Implement Windows volume/mount support #13908

n1hility · 2022-04-18T13:26:37Z

Based on WSL2 9p support: remaps windows paths to /mnt/ locations for
both podman and Docker API clients.

For example:

podman run -it -v C:\Users\Jason:/blah ubi8-micro sh

Additionally unixified paths sometimes used by Docker clients are supported

podman run -it -v /c/Users/Jason:/blah ubi8-micro sh

This also works with mount paths:

.\docker run -it --mount 'type=bind,source=C:\Users\Jason,destination=/blah' alpine sh

Finally, windows paths can be used on Linux in the WSL session to enable workflows where users are running process under the native Linux prompt yet accessing data on their Win drives.

This code is only active if either the podman-remote client is windows (in which case there is client side path remapping), or if the podman backend is configured as a WSL machine. The latter is necessary since there are parsing ambiguities (see tests) when interpreting potential win paths. While in practice they are uncommon, ideally existing behavior is preserved and the new code paths are only active where needed.

Resolves #13707

Depends on containers/common#1012

Signed-off-by: Jason T. Greene [email protected]

rhatdan · 2022-04-19T20:20:51Z

/approve
LGTM
Need to get changes merged into containers/common first though.

openshift-ci · 2022-04-19T20:20:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n1hility, 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:

~~OWNERS~~ [rhatdan]

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

TomSweeneyRedHat · 2022-04-19T23:38:54Z

LGTM, once c/common is vendored

n1hility · 2022-04-22T00:45:19Z

@rhatdan @TomSweeneyRedHat @Luap99 Here is the updated PR that drops the config updates and instead uses the marker file like discussed on containers/common#1005. However, this update does touch the libpod networking code. To make it easier to review I broke this off in another commit here: 6a44a43

I also did not update the mac code to use the new marker yet (still using the supported config value), i figured it would be better to follow-up immediately with a small update for that than to shove too much in here. Let me know if you prefer me to add it, and I can include that.

pkg/util/utils.go

Luap99 · 2022-04-22T12:55:10Z

/hold PR has merge conflict

Luap99 · 2022-04-22T12:56:09Z

Also my other comments see above are not addressed.

pkg/specgenutil/volumes.go

n1hility · 2022-04-22T13:07:15Z

thanks for the great feedback I will turn this around quickly (and sorry for the missed call - great catch!)

n1hility · 2022-04-22T19:15:52Z

PR Updated to resolve feedback, marker code moved to common here: containers/common#1012

Luap99

Could you create the podman-machine file for the qemu backend as well since we consider the config file option deprecated.

libpod/networking_linux.go

pkg/specgen/volumes.go

n1hility · 2022-04-23T18:03:20Z

Could you create the podman-machine file for the qemu backend as well since we consider the config file option deprecated.

Sure. I was originally planning to do this in a follow-up PR to keep it easier to review, but will go ahead and throw that in.

n1hility · 2022-04-25T18:51:13Z

/unhold

Signed-off-by: Jason T. Greene <[email protected]>

Based on WSL2 9p support: remaps windows paths to /mnt/<drive> locations for both podman and Docker API clients. Signed-off-by: Jason T. Greene <[email protected]>

n1hility · 2022-04-25T18:53:22Z

PR is rebased and updated to latest main of config/common which includes the just merged containers/common#1012

baude · 2022-04-25T20:47:07Z

pkg/specgen/winpath.go

+	"github.com/pkg/errors"
+)
+
+func isHostWinPath(path string) bool {


possible to get unittests on these functions to avoid simple regressions?

@baude these are tested via specgenutil_test which I originally had to do for API access reasons. Although, I think those reasons may no longer apply. I can see if I can move them to specgen

@baude ah yes, it creates an import cycle that was the other reason they had to be in the other package.

rhatdan · 2022-04-26T12:33:57Z

/lgtm

n1hility force-pushed the win-mounts branch 3 times, most recently from 7f393f4 to 3b9e0d2 Compare April 18, 2022 14:18

n1hility changed the title ~~Implements Windows volume/mount support~~ Implement Windows volume/mount support Apr 19, 2022

rhatdan changed the title ~~Implement Windows volume/mount support~~ [WIP] Implement Windows volume/mount support Apr 19, 2022

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 19, 2022

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

n1hility force-pushed the win-mounts branch from 3b9e0d2 to 278bf48 Compare April 21, 2022 22:48

n1hility changed the title ~~[WIP] Implement Windows volume/mount support~~ Implement Windows volume/mount support Apr 22, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2022

Luap99 reviewed Apr 22, 2022

View reviewed changes

pkg/util/utils.go Outdated Show resolved Hide resolved

openshift-ci bot assigned rhatdan Apr 22, 2022

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

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 22, 2022

Luap99 reviewed Apr 22, 2022

View reviewed changes

pkg/specgenutil/volumes.go Outdated Show resolved Hide resolved

n1hility force-pushed the win-mounts branch from 278bf48 to 8a866e3 Compare April 22, 2022 18:09

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2022

n1hility changed the title ~~Implement Windows volume/mount support~~ [WIP] Implement Windows volume/mount support Apr 22, 2022

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2022

n1hility force-pushed the win-mounts branch from 8a866e3 to 388f9ba Compare April 22, 2022 18:14

n1hility mentioned this pull request Apr 22, 2022

Deprecate MachineEnabled and replace with API containers/common#1012

Merged

Luap99 reviewed Apr 23, 2022

View reviewed changes

libpod/networking_linux.go Outdated Show resolved Hide resolved

pkg/specgen/volumes.go Show resolved Hide resolved

n1hility force-pushed the win-mounts branch 2 times, most recently from ec69d4a to fcde02f Compare April 25, 2022 18:49

n1hility changed the title ~~[WIP] Implement Windows volume/mount support~~ Implement Windows volume/mount support Apr 25, 2022

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 25, 2022

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022

n1hility added 2 commits April 25, 2022 13:52

Update to use new common machine API

3b6ffcd

Signed-off-by: Jason T. Greene <[email protected]>

Implements Windows volume/mount support

b0d36f6

Based on WSL2 9p support: remaps windows paths to /mnt/<drive> locations for both podman and Docker API clients. Signed-off-by: Jason T. Greene <[email protected]>

n1hility force-pushed the win-mounts branch from fcde02f to b0d36f6 Compare April 25, 2022 18:52

baude reviewed Apr 25, 2022

View reviewed changes

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

openshift-merge-robot merged commit ace6672 into containers:main Apr 26, 2022

edsantiago mentioned this pull request Apr 26, 2022

vendor in containers/(common,buildah,storage) #14005

Merged

This was referenced May 10, 2022

CI flake: secret file is not leaked into image #13417

Closed

Need timeout bump in containerignore-filtering-embedded-etc test #14184

Closed

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 21, 2023

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Windows volume/mount support #13908

Implement Windows volume/mount support #13908

n1hility commented Apr 18, 2022 •

edited

Loading

rhatdan commented Apr 19, 2022

openshift-ci bot commented Apr 19, 2022

TomSweeneyRedHat commented Apr 19, 2022

n1hility commented Apr 22, 2022

Luap99 commented Apr 22, 2022

Luap99 commented Apr 22, 2022

n1hility commented Apr 22, 2022

n1hility commented Apr 22, 2022

Luap99 left a comment

n1hility commented Apr 23, 2022

n1hility commented Apr 25, 2022

n1hility commented Apr 25, 2022

baude Apr 25, 2022

n1hility Apr 25, 2022

n1hility Apr 25, 2022

rhatdan commented Apr 26, 2022

Implement Windows volume/mount support #13908

Implement Windows volume/mount support #13908

Conversation

n1hility commented Apr 18, 2022 • edited Loading

rhatdan commented Apr 19, 2022

openshift-ci bot commented Apr 19, 2022

TomSweeneyRedHat commented Apr 19, 2022

n1hility commented Apr 22, 2022

Luap99 commented Apr 22, 2022

Luap99 commented Apr 22, 2022

n1hility commented Apr 22, 2022

n1hility commented Apr 22, 2022

Luap99 left a comment

Choose a reason for hiding this comment

n1hility commented Apr 23, 2022

n1hility commented Apr 25, 2022

n1hility commented Apr 25, 2022

baude Apr 25, 2022

Choose a reason for hiding this comment

n1hility Apr 25, 2022

Choose a reason for hiding this comment

n1hility Apr 25, 2022

Choose a reason for hiding this comment

rhatdan commented Apr 26, 2022

n1hility commented Apr 18, 2022 •

edited

Loading