Add podman machine cp subcommand
#25331
Conversation
jakecorrenti
force-pushed
the
machine-cp
branch
from
0e3d563 to
18304ce
Compare
jakecorrenti
force-pushed
the
machine-cp
branch
2 times, most recently
from
914a438 to
55cf202
Compare
jakecorrenti
force-pushed
the
machine-cp
branch
from
55cf202 to
076256f
Compare
jakecorrenti
force-pushed
the
machine-cp
branch
3 times, most recently
from
eaf21be to
0ed0ba3
Compare
|
one question i guess else LGTM |
jakecorrenti
force-pushed
the
machine-cp
branch
from
0ed0ba3 to
eed8e8f
Compare
jakecorrenti
force-pushed
the
machine-cp
branch
3 times, most recently
from
602102f to
b7b6dc7
Compare
| } | ||
|
|
||
| // Passing an absolute windows path of the format <volume>:\<path> will cause | ||
| // `copy.ParseSourceAndDestination` to think the volume is a Machine. Check |
There was a problem hiding this comment.
just thinking about it does podman cp not have the exact same issue, i.e. should this fix be moved into ParseSourceAndDestination()?
There was a problem hiding this comment.
Wouldn't that result in the same issue mentioned below having to do with single name containers?
cmd/podman/machine/cp.go
Outdated
| if specgen.IsHostWinPath(args[0]) { | ||
| srcMachine = "" | ||
| srcPath = args[0] | ||
| } | ||
|
|
||
| if specgen.IsHostWinPath(args[1]) { | ||
| destMachine = "" | ||
| destPath = args[1] | ||
| } |
There was a problem hiding this comment.
Also isn't this breaking for machine names with one letter? i.e. I think it runs into the same issue as shown in #25323
We do allow a machine called C so that can be a problem although super unlikely that anyone would be using single letter machine names hopefully so I am fine to ignore that part for now. Though adding this issue in a comment here might help future readers.
jakecorrenti
force-pushed
the
machine-cp
branch
2 times, most recently
from
f99e5fe to
a69cc67
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jakecorrenti, Luap99 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 |
openshift-ci
bot
added
the
approved
| directory = "foo-dir" | ||
| directoryPath = filepath.Join(GinkgoT().TempDir(), directory) | ||
|
|
||
| fileInDirectory = "bar.txt" |
There was a problem hiding this comment.
For some reason tests are failing during cleanup for me on Windows because this file cannot be removed. The error message is "The process cannot access the file because it is being used by another process.". This can be specific to my machine though.
There was a problem hiding this comment.
I'm relying on the CI for my windows machine, but I'll keep an eye on it.
jakecorrenti
force-pushed
the
machine-cp
branch
from
a69cc67 to
4fec981
Compare
jakecorrenti
force-pushed
the
machine-cp
branch
2 times, most recently
from
b775a29 to
9e12f5c
Compare
Add a new `podman machine cp` subcommand to allow users to copy files or directories between a running Podman Machine and their host. Tests cover the following cases: - Copy a file from the host machine to the VM - Copy a directory from the host machine to the VM - Copy a file from the VM to the host machine - Copy a directory from the VM to the host machine - Copy a file to a directory - Copy a directory to a file Signed-off-by: Jake Correnti <[email protected]>
jakecorrenti
force-pushed
the
machine-cp
branch
from
9e12f5c to
42fb942
Compare
|
/lgtm |
Does this PR introduce a user-facing change?
Adds a new
podman machine cpsubcommand. This will allow users to copy files or directories between a running Podman Machine and their host.