Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Add command ignite cp #550

Merged
merged 10 commits into from
Apr 13, 2020
Merged

Add command ignite cp #550

merged 10 commits into from
Apr 13, 2020

Conversation

darkowlzz
Copy link
Contributor

This implements cp command on top of #495 .

Implements bidirectional copy from host to VM and VM to host using sftp.
The copy command syntax is similar to docker cp with source and
destination that can have VM reference name or ID separated by a
filepath using ":". VM reference in copy source means to copy from VM to
host and VM reference in copy destination means to copy from host to VM.

Example usage:

$ ignite cp localfile.txt my-vm:remotefile.txt
$ ignite cp my-vm:remotefile.txt localfile.txt

File permissions and owners are also applied to the copied files.
Symlinks are followed and the destination files are copied.

Fixes #419

@darkowlzz darkowlzz requested a review from twelho as a code owner March 2, 2020 15:52
@stealthybox
Copy link
Contributor

After review on Monday dev call:

  • add e2e tests for to-vm and from-vm to existing file (use exec command to cat file in vm and compare)
  • look into cp file vm:dir/ and cp vm:file hostdir/
    (the target directory could be fixed in a different PR if it's complicated)

Thanks for building this :)

@darkowlzz
Copy link
Contributor Author

darkowlzz commented Mar 4, 2020

Added a fix for the case when the destination is a directory:

  • If the source is a file and destination is a directory, the file is placed inside the destination directory.
  • If the source is a directory and destination is also a directory with different name, the source is copied as a subdirectory in the destination directory.

Also added a fix for the case when the source is a directory and destination is a file of the same name, copying returns an error.

FATA[0000] failed to copy files from VM to host: cannot overwrite non-directory "rdir/qdir"(Host) with directory "/root/qdir"(VM)

Added e2e test for host to VM and VM to host copying of files and symlinks.
Couldn't figure out how to compare host and VM directory content in a good way.

@darkowlzz
Copy link
Contributor Author

Rebased.

@darkowlzz
Copy link
Contributor Author

Rebased and added e2e tests for copying directory.

@darkowlzz
Copy link
Contributor Author

Rebased.

@darkowlzz
Copy link
Contributor Author

e2e test failure seems to be because the VM run failed.
Tried running the same tests locally and they work well. Need to re-run the CI build.

@luxas luxas added this to the v0.7.0 milestone Apr 10, 2020
@stealthybox
Copy link
Contributor

@luxas and I think this looks really good from a code standpoint.

I was able to replicate the CI failures locally on my first try, so I think there may be an issue with the tests, or maybe a dependency change is causing a regression.

Are your tests perhaps depending on existing vm's on your host?

I did rebase on master as well.

git rebase master
make ignite bin/amd64/Dockerfile
make e2e-nobuild
sudo IGNITE_E2E_HOME=/home/stealthybox/Repos/ignite \
        /home/stealthybox/.local/usr/local/go/bin/go test \
        ./e2e/. -v -mod=vendor \
        -count 1 \
        -run Test
[sudo] password for stealthybox: 
=== RUN   TestCopyFileFromHostToVM
=== RUN   TestCopyFileFromHostToVM/file_with_content
    TestCopyFileFromHostToVM/file_with_content: cp_test.go:38: assertion failed: error is not nil: exit status 1: vm run: 
        ["/home/stealthybox/Repos/ignite/bin/ignite" "run" "--name=e2e_test_copy_to_vm_file_with_content" "weaveworks/ignite-ubuntu" "--ssh"]
        time="2020-04-10T09:57:20-06:00" level=info msg="Created VM with ID \"5f6d480830927c76\" and name \"e2e_test_copy_to_vm_file_with_content\""
        time="2020-04-10T09:57:20-06:00" level=info msg="Networking is handled by \"cni\""
        time="2020-04-10T09:57:20-06:00" level=info msg="Started Firecracker VM \"5f6d480830927c76\" in a container with ID \"ignite-5f6d480830927c76\""
        time="2020-04-10T09:57:31-06:00" level=fatal msg="dial tcp 127.0.0.1:22: connect: connection refused"
        
=== RUN   TestCopyFileFromHostToVM/empty_file
--- FAIL: TestCopyFileFromHostToVM (19.59s)
    --- FAIL: TestCopyFileFromHostToVM/file_with_content (13.84s)
    --- PASS: TestCopyFileFromHostToVM/empty_file (5.75s)
=== RUN   TestCopySymlinkedFileFromHostToVM
--- PASS: TestCopySymlinkedFileFromHostToVM (5.30s)
=== RUN   TestCopyFileFromVMToHost
    TestCopyFileFromVMToHost: cp_test.go:162: assertion failed: error is not nil: exit status 1: vm run: 
        ["/home/stealthybox/Repos/ignite/bin/ignite" "run" "--name=e2e_test_copy_file_from_vm_to_host" "weaveworks/ignite-ubuntu" "--ssh"]
        time="2020-04-10T09:57:44-06:00" level=info msg="Created VM with ID \"3a3f22e264b36552\" and name \"e2e_test_copy_file_from_vm_to_host\""
        time="2020-04-10T09:57:45-06:00" level=info msg="Networking is handled by \"cni\""
        time="2020-04-10T09:57:45-06:00" level=info msg="Started Firecracker VM \"3a3f22e264b36552\" in a container with ID \"ignite-3a3f22e264b36552\""
        time="2020-04-10T09:57:56-06:00" level=fatal msg="dial tcp 127.0.0.1:22: connect: connection refused"

@darkowlzz
Copy link
Contributor Author

darkowlzz commented Apr 11, 2020

Rebased again.

Thanks for trying to reproduce the problem. I'm not sure why I'm still not able to reproduce it. Ran many times, individually, multiple times and along with all the other tests. The test is independent and doesn't depend on any existing VM. The error seems to be related to ssh connection to the created VMs.

@stealthybox
Copy link
Contributor

Please rebase on master which now includes #581 :)
Thanks for your help debugging

Greg Pauloski and others added 10 commits April 13, 2020 15:31
Implement bidirectional copy from host to VM and VM to host using sftp.
The copy command syntax is similar to `docker cp` with source and
destination that can have VM reference name or ID separated by a
filepath using ":". VM reference in copy source means to copy from VM to
host and VM reference in copy destination means to copy from host to VM.

File permissions and owners are also applied to the copied files.
Symlinks are followed and the destination files are copied.
When source is a file and destination is a directory, append the base
file name of source to the destination path, moving the source into
destination directory.
When source is a directory and destination is also a directory with a
different name, copy the source to destination as a subdirectory. If the
destination directory has a file with the same name as the source, fail
to copy. Directory should not overwrite an existing file.
@stealthybox
Copy link
Contributor

We've dealt with the SSH E2E test flake.
We're now just down to the 1 snapshot issue.

I've run these e2e's on my machine several times, and I think we can safely merge this.

@stealthybox stealthybox merged commit fd8cd29 into weaveworks:master Apr 13, 2020
@darkowlzz darkowlzz deleted the cp branch July 19, 2020 12:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copy files from host to VM, while the VM is in running state
3 participants