podman-remote cp, varlink API CopyTo &CopyFrom #3568

QiWang19 · 2019-07-12T19:55:11Z

Implemented podman-remote cp adding varlink API CopyTo & CopyFrom
close #4207
Copy inside remote machine

// source file: from remote container
// destination: remote file system
$ bin/podman-remote cp  adc8:/ctrfile local:/home/qiwang/Documents/file

// source file:  from remote machine
// destination:  remote container
$ bin/podman-remote cp local:/home/qiwang/Documents/f.txt adc8:/f26

Copy between local and remote container

// source file: local file system
// destination:  remote container
$ bin/podman-remote cp remote:/home/qiwan/Documents/f.txt adc8:/ctrfile

// source file: from remote container
// destination: local file system
$ bin/podman-remote cp  adc8:/ctrfile remote:/home/qiwan/Documents/file

Varlink API

// Copy file ctrf from containrt de3f to /home/qiwan/Documents/ctrfile
$ sudo varlink call -m unix:/run/podman/io.podman/io.podman.CopyFrom '{"dest": "local", "srcPath": "/ctrf", "srcfMode": 0600, "destPath": "/home/qiwan/Documents/ctrfile", "extract": false, "pause": true, "ctrNameorId": "de3f"}'
{
  "path": [
    "/home/qiwan/Documents/ctrfile"
  ]
}

// Copy and extract /home/qiwan/Documents/f1.tar to container de3f
$ sudo varlink call -m unix:/run/podman/io.podman/io.podman.CopyTo '{"src": "local", "srcPath": "/home/qiwan/Documents/f1.tar", "srcfMode":0600, "destPath": "/archivef", "extract":true, "pause": true, "ctrNameorId": "de3f"}'
{
  "path": "/var/lib/containers/storage/overlay/e451e85006f6215f805b33257b2bea8dad60ab390af758060af0587f345bdfb2/merged/archivef"
}

Signed-off-by: Qi Wang qiwan@redhat.com

openshift-ci-robot · 2019-07-12T19:55:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: QiWang19
To complete the pull request process, please assign mrunalp
You can assign the PR to them by writing /assign @mrunalp in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

QiWang19 · 2019-07-12T20:04:32Z

@jwhonce
Does this satisfy the requirements https://jira.coreos.com/browse/RUN-351?
changed the return value of CopyFrom and CopyTo from file size to file path in order to locate file when podman-remote cp.

pkg/varlinkapi/transfers.go

rh-atomic-bot · 2019-07-15T19:43:40Z

☔ The latest upstream changes (presumably #3531) made this pull request unmergeable. Please resolve the merge conflicts.

jwhonce

A lot of debug and error msg that don't appear to have context when run on a system with multiple users.

I believe this covers the requirements. /cc @baude

pkg/adapter/containers.go

jwhonce · 2019-07-23T21:19:30Z

pkg/adapter/containers_remote.go

+			if strings.HasSuffix(dest, "/") {
+				destPath = fmt.Sprintf("%s/", destPath)
+			}
+			reply, err := iopodman.CopyTo().Send(r.Conn, varlink.Upgrade, location, srcPath, 0600, destPath, extract, pause, ctr.ID())


Shouldn't mode match Src file vs. hard coded?

the source file is from the remote file system this augument is not useful under this situation.

jwhonce · 2019-07-23T21:20:10Z

pkg/adapter/containers_remote.go

+			}
+			reply, err := iopodman.CopyTo().Send(r.Conn, varlink.Upgrade, location, srcPath, 0600, destPath, extract, pause, ctr.ID())
+			if err != nil {
+				return errors.Wrapf(err, "varlink call CopyTo error")


Does err have context to what happened?

jwhonce · 2019-07-23T21:21:09Z

pkg/adapter/containers_remote.go

+			return nil
+		}
+
+		err := r.CopyToContainer(src, dest, srcPath, destPath, extract, pause, ctr)


pkg/adapter/containers_remote.go

jwhonce · 2019-07-23T21:35:56Z

pkg/util/utils.go

+	}
+
+	// untar tempFile under os.TempDir() if the source is a file and not archive
+	tempTarball, err := os.Open(srcFilePath)


Does the tarball carry the modes for the source files and directory path? If so that could clean up the API a bit.

I'm not sure. I'll check. But the mode argument is only useful when using CopyTo() to copy to remote. It is not useful in the CopyFrom or the varlink call

cmd/podman/varlink/io.podman.varlink

rh-atomic-bot · 2019-08-08T22:47:13Z

☔ The latest upstream changes (presumably #3767) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2019-09-30T14:22:02Z

☔ The latest upstream changes (presumably #4001) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2019-10-04T20:28:33Z

☔ The latest upstream changes (presumably #4165) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2019-10-10T14:38:45Z

☔ The latest upstream changes (presumably #4197) made this pull request unmergeable. Please resolve the merge conflicts.

rh-atomic-bot · 2019-10-11T18:57:38Z

☔ The latest upstream changes (presumably #4220) made this pull request unmergeable. Please resolve the merge conflicts.

QiWang19 · 2019-10-14T15:41:31Z

@jwhonce cleaned the reviews and rebased. PTAL?

jwhonce

There is currently a lot of duplication, but knowing we're going to refactor this code to run as the container in the future We can live with it.

jwhonce · 2019-10-15T16:01:44Z

pkg/util/utils.go

 	"github.com/containers/storage/pkg/idtools"
-	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1" // "github.com/opencontainers/image-spec/specs-go/v1"


Why add the comment?

jwhonce · 2019-10-15T16:09:42Z

pkg/adapter/containers.go

+}
+
+// ParsePath parses the input of cp command returns the path and the container
+func (r *LocalRuntime) ParsePath(path string) (*Container, string) {


If you move this code to pkg/util/utils.go you won't need to duplicate it between local and remote.

there will be an import circle cmd/podman imports adapter, adapter imports util, util imports adapter. Because ParsePath needs adapter.Runtime to check the input container. Can I get rid of this circle?

jwhonce · 2019-10-15T16:12:13Z

docs/podman-container.1.md

@@ -17,7 +17,7 @@ The container command allows you to manage containers
 | checkpoint | [podman-container-checkpoint(1)](podman-container-checkpoint.1.md)  | Checkpoints one or more running containers.                  |
 | cleanup    | [podman-container-cleanup(1)](podman-container-cleanup.1.md)    | Cleanup the container's network and mountpoints.                 |
 | commit     | [podman-commit(1)](podman-commit.1.md)              | Create new image based on the changed container.                             |
-| cp         | [podman-cp(1)](podman-cp.1.md)                      | Copy files/folders between a container and the local filesystem.             |
+| cp         | [podman-cp(1)](podman-cp.1.md)                      | Copy files/folders between a container and the local filesystem              |


Why remove the period?

Implemented podman-remote cp adding varlink API CopyTo & CopyFrom Copy inside remote machine ``` // source file: from remote container // destination: remote file system $ bin/podman-remote cp adc8:/ctrfile local:/home/qiwang/Documents/file ``` ``` // source file: from remote container // destination: local file system $ bin/podman-remote cp adc8:/ctrfile remote:/home/qiwan/Documents/file ``` Copy between local and remote container ``` // source file: local file system // destination: remote container $ bin/podman-remote cp remote:/home/qiwan/Documents/f.txt adc8:/ctrfile ``` ``` // source file: from remote machine // destination: remote container $ bin/podman-remote cp local:/home/qiwang/Documents/f.txt adc8:/f26 ``` Varlink API ``` // Copy file ctrf from containrt de3f to /home/qiwan/Documents/ctrfile $ sudo varlink call -m unix:/run/podman/io.podman/io.podman.CopyFrom '{"dest": "local", "srcPath": "/ctrf", "srcfMode": 0600, "destPath": "/home/qiwan/Documents/ctrfile", "extract": false, "pause": true, "ctrNameorId": "de3f"}' { "path": [ "/home/qiwan/Documents/ctrfile" ] } ``` ``` // Copy and extract /home/qiwan/Documents/f1.tar to container de3f $ sudo varlink call -m unix:/run/podman/io.podman/io.podman.CopyTo '{"src": "local", "srcPath": "/home/qiwan/Documents/f1.tar", "srcfMode":0600, "destPath": "/archivef", "extract":true, "pause": true, "ctrNameorId": "de3f"}' { "path": "/var/lib/containers/storage/overlay/e451e85006f6215f805b33257b2bea8dad60ab390af758060af0587f345bdfb2/merged/archivef" } ``` Signed-off-by: Qi Wang <qiwan@redhat.com>

QiWang19 · 2019-10-24T14:59:51Z

There is currently a lot of duplication, but knowing we're going to refactor this code to run as the container in the future We can live with it.

you mean I should refactor my PR now or the whole podman-remote code will be refactored and I need to refactor my PR after that?

rh-atomic-bot · 2019-10-29T20:00:50Z

☔ The latest upstream changes (presumably #4310) made this pull request unmergeable. Please resolve the merge conflicts.

github-actions · 2019-11-29T00:08:49Z

This pull request had no activity for 30 days. In the absence of activity or the "do-not-close" label, the pull request will be automatically closed within 7 days.

openshift-ci-robot · 2019-11-29T00:08:56Z

@QiWang19: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rhatdan · 2019-11-29T10:37:14Z

@QiWang19 Needs a rebase.

github-actions · 2019-12-30T00:04:16Z

This pull request had no activity for 30 days. In the absence of activity or the "do-not-close" label, the pull request will be automatically closed within 7 days.

openshift-ci-robot requested review from jwhonce and umohnani8 July 12, 2019 19:55

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 12, 2019

QiWang19 force-pushed the copyto branch from 2931fc4 to 4de3f1d Compare July 12, 2019 20:17

QiWang19 commented Jul 12, 2019

View reviewed changes

pkg/varlinkapi/transfers.go Outdated Show resolved Hide resolved

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2019

jwhonce requested changes Jul 23, 2019

View reviewed changes

QiWang19 force-pushed the copyto branch from 4de3f1d to 527e661 Compare July 24, 2019 14:33

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2019

QiWang19 force-pushed the copyto branch from 527e661 to 464f337 Compare July 25, 2019 16:33

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 9, 2019

QiWang19 force-pushed the copyto branch from 2b55957 to fb050a7 Compare September 29, 2019 01:18

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2019

QiWang19 force-pushed the copyto branch 2 times, most recently from 1ffe7e5 to 5e4ae2c Compare September 29, 2019 01:40

QiWang19 force-pushed the copyto branch from 5e4ae2c to 09df9e2 Compare October 2, 2019 17:45

QiWang19 force-pushed the copyto branch 3 times, most recently from cbe485d to bc0799e Compare October 9, 2019 19:56

QiWang19 force-pushed the copyto branch 3 times, most recently from a349a6b to 33fb8e8 Compare October 11, 2019 15:52

QiWang19 force-pushed the copyto branch 5 times, most recently from 0e60d46 to 911d8e6 Compare October 14, 2019 14:44

jwhonce reviewed Oct 15, 2019

View reviewed changes

QiWang19 force-pushed the copyto branch from 911d8e6 to c5e7a21 Compare October 15, 2019 17:20

ffromani mentioned this pull request Nov 15, 2019

[WIP] add copy-from command ffromani/pack8s#26

Closed

github-actions bot added the stale-pr label Nov 29, 2019

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2019

rhatdan removed the stale-pr label Nov 29, 2019

github-actions bot added the stale-pr label Dec 30, 2019

vrothberg added the do-not-close label Jan 3, 2020

QiWang19 closed this Feb 5, 2020

QiWang19 deleted the copyto branch June 26, 2020 15:10

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

podman-remote cp, varlink API CopyTo &CopyFrom #3568

podman-remote cp, varlink API CopyTo &CopyFrom #3568

QiWang19 commented Jul 12, 2019 •

edited

Loading

openshift-ci-robot commented Jul 12, 2019

QiWang19 commented Jul 12, 2019

rh-atomic-bot commented Jul 15, 2019

jwhonce left a comment

jwhonce Jul 23, 2019

QiWang19 Oct 14, 2019

jwhonce Jul 23, 2019

QiWang19 Oct 14, 2019

jwhonce Jul 23, 2019

jwhonce Jul 23, 2019

QiWang19 Jul 24, 2019

rh-atomic-bot commented Aug 8, 2019

rh-atomic-bot commented Sep 30, 2019

rh-atomic-bot commented Oct 4, 2019

rh-atomic-bot commented Oct 10, 2019

rh-atomic-bot commented Oct 11, 2019

QiWang19 commented Oct 14, 2019

jwhonce left a comment

jwhonce Oct 15, 2019

jwhonce Oct 15, 2019

QiWang19 Oct 15, 2019

jwhonce Oct 15, 2019

QiWang19 commented Oct 24, 2019

rh-atomic-bot commented Oct 29, 2019

github-actions bot commented Nov 29, 2019

openshift-ci-robot commented Nov 29, 2019

rhatdan commented Nov 29, 2019

github-actions bot commented Dec 30, 2019

podman-remote cp, varlink API CopyTo &CopyFrom #3568

podman-remote cp, varlink API CopyTo &CopyFrom #3568

Conversation

QiWang19 commented Jul 12, 2019 • edited Loading

openshift-ci-robot commented Jul 12, 2019

QiWang19 commented Jul 12, 2019

rh-atomic-bot commented Jul 15, 2019

jwhonce 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

rh-atomic-bot commented Aug 8, 2019

rh-atomic-bot commented Sep 30, 2019

rh-atomic-bot commented Oct 4, 2019

rh-atomic-bot commented Oct 10, 2019

rh-atomic-bot commented Oct 11, 2019

QiWang19 commented Oct 14, 2019

jwhonce 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

QiWang19 commented Oct 24, 2019

rh-atomic-bot commented Oct 29, 2019

github-actions bot commented Nov 29, 2019

openshift-ci-robot commented Nov 29, 2019

rhatdan commented Nov 29, 2019

github-actions bot commented Dec 30, 2019

QiWang19 commented Jul 12, 2019 •

edited

Loading