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

added scpi command for copying images #3182

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@marconi
Copy link

marconi commented Mar 11, 2016

This adds copy image (scpi) command for copying images from one machine to another. Helpful if you have huge image that pulling it again will take long or if you don't have internet access.

This is my first PR, any feedback would be appreciated. :)

added scpi command for copying images
Signed-off-by: Marconi Moreto Jr. <me@marconijr.com>

@marconi marconi force-pushed the marconi:scpi branch from 4e324d7 to 09e25b6 Mar 11, 2016

@GordonTheTurtle GordonTheTurtle removed the dco/no label Mar 11, 2016

@nathanleclaire

This comment has been minimized.

Copy link
Contributor

nathanleclaire commented Mar 11, 2016

Very interesting. I guess this would allow for things like:

$ docker $(docker-machine config swarmnode-0) pull me/image
$ docker-machine scpi swarmnode-0:me/image swarmnode-1
$ docker-machine scpi swarmnode-0:me/image swarmnode-2

?

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 12, 2016

@nathanleclaire yep, sorry forgot to add an example. The syntax is basically:

docker-machine scpi [machine]:[image][:tag] [machine]

The tag is optional, if omitted, defaults to latest.

return host, image, nil
}

func generateSaveFilename(srcImage string) string {

This comment has been minimized.

@nathanleclaire

nathanleclaire Mar 12, 2016

Contributor

Could you make a test for this method as well please?

This comment has been minimized.

@marconi

func getScpiHostAndImage(hostAndPath string, isSource bool, api libmachine.API) (*host.Host, string, error) {
var (
hostName = hostAndPath

This comment has been minimized.

@nathanleclaire

nathanleclaire Mar 12, 2016

Contributor

If this is going to be set to different value later on, why not leave off this initial assignment ?

This comment has been minimized.

@marconi

marconi Mar 12, 2016

Author

Because it acts as default value when this function is called for destination where it doesn't need parsing since destination only contains machine name directly, unlike source where it has machine and image name.

@nathanleclaire

This comment has been minimized.

Copy link
Contributor

nathanleclaire commented Mar 12, 2016

Cool use case -- I'm definitely interested. Why not use the Docker client API that machine has (this is how we manage swarm containers) directly instead of running docker commands over SSH?

added test for generateSaveFilename
Signed-off-by: Marconi Moreto Jr. <me@marconijr.com>

@marconi marconi force-pushed the marconi:scpi branch from b8d0b24 to 13b342c Mar 12, 2016

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 12, 2016

@nathanleclaire ah right, I think I can make that work. Will keep you posted.

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 12, 2016

@nathanleclaire I made some progress and I thought it even became simpler, let me know what you think. ;)

refactored to use local docker client
Signed-off-by: Marconi Moreto Jr. <me@marconijr.com>

@marconi marconi force-pushed the marconi:scpi branch from b410698 to ebd24e1 Mar 12, 2016

@GordonTheTurtle

This comment has been minimized.

Copy link

GordonTheTurtle commented Mar 12, 2016

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "scpi" git@github.com:marconi/machine.git somewhere
$ cd somewhere
$ git rebase -i HEAD~4
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

@marconi marconi force-pushed the marconi:scpi branch from 060cd19 to f1e331d Mar 12, 2016

@GordonTheTurtle

This comment has been minimized.

Copy link

GordonTheTurtle commented Mar 12, 2016

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "scpi" git@github.com:marconi/machine.git somewhere
$ cd somewhere
$ git rebase -i HEAD~4
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

1 similar comment
@GordonTheTurtle

This comment has been minimized.

Copy link

GordonTheTurtle commented Mar 12, 2016

Please sign your commits following these rules:
https://github.com/docker/docker/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "scpi" git@github.com:marconi/machine.git somewhere
$ cd somewhere
$ git rebase -i HEAD~4
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Ammending updates the existing PR. You DO NOT need to open a new one.

cross-platform tmp path
Signed-off-by: Marconi Moreto Jr. <me@marconijr.com>

@marconi marconi force-pushed the marconi:scpi branch from f1e331d to 1ad0199 Mar 12, 2016

@nathanleclaire

This comment has been minimized.

Copy link
Contributor

nathanleclaire commented Mar 14, 2016

Hm, I think I have not been clear in communicating what I mean.

I mean that instead of shelling out to docker and docker-machine commands, it should use the built-in DockerClient from mcndockerclient module: https://github.com/docker/machine/blob/master/libmachine/mcndockerclient/docker_client.go#L11

Example usage in Swarm code here: https://github.com/docker/machine/blob/master/libmachine/provision/configure_swarm.go#L110

Can you update it to use that please?

Likewise, it should probably invoke scp directly. I'd suggest trying to use the existing docker-machine scp-related methods.

Thanks!

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 15, 2016

@nathanleclaire ah, I see. Sure will update, I was actually initially looking for something like it but wasn't sure how libmachine worked.

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 15, 2016

Only see one problem, the DockerClient doesn't have support for saving image. But the engine-api's client does, I saw some discussion with dockerclient moving to engine-api so its prolly not worth it adding an image save endpoint for it esp. since machine has this as well.

@marconi

This comment has been minimized.

Copy link
Author

marconi commented Mar 15, 2016

Closing this for now.

@marconi marconi closed this Mar 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment