Skip to content
This repository was archived by the owner on Feb 1, 2021. It is now read-only.

Add support save images#739

Closed
jimmyxian wants to merge 2 commits intodocker-archive:masterfrom
jimmyxian:add-support-images-save
Closed

Add support save images#739
jimmyxian wants to merge 2 commits intodocker-archive:masterfrom
jimmyxian:add-support-images-save

Conversation

@jimmyxian
Copy link
Copy Markdown
Contributor

Add support save images

jimmyxian added 2 commits May 6, 2015 04:14
Signed-off-by: Xian Chaobo <xianchaobo@huawei.com>
Signed-off-by: Xian Chaobo <xianchaobo@huawei.com>
@jimmyxian jimmyxian changed the title Add support save images DO NOT MERGE- integration : Add support save images May 7, 2015
@jimmyxian jimmyxian force-pushed the add-support-images-save branch 2 times, most recently from 7d108ad to 7ed9937 Compare May 7, 2015 08:42
@jimmyxian jimmyxian changed the title DO NOT MERGE- integration : Add support save images Add support save images May 7, 2015
@jimmyxian
Copy link
Copy Markdown
Contributor Author

ping @aluzzardi @vieux @abronan

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, in order to make sure that save actually looks up at different machines I would probably do something like:

start_docker_with_busybox 2
docker -H ${HOSTS[0]} tag busybox test1
docker -H ${HOSTS[0]} tag busybox test2
docker_swarm save test1 test2

This makes sure save is looking at both hosts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, does not support multi images on multiple nodes. :)

@aluzzardi
Copy link
Copy Markdown
Contributor

@vieux You are way more familiar than me with what's that supposed to do, could you take a look?

What kind of things should be tested? Maybe a load afterwards on a third (clean host, without even busybox, just start_docker), run images on it and make sure both test1 and test2 are there?

api/handlers.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we do not need this bFoundEngine.
Just check with foundEngineAddr == "" as Golang alreay inits it to an empty string.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@aluzzardi
Copy link
Copy Markdown
Contributor

Regarding my previous comment on saving images which are on different machines:

1/ If it's something we want to support, we should add a test
2/ If it's something we don't want to support, we should add a test anyway and verify that it fails properly

@vieux
Copy link
Copy Markdown
Contributor

vieux commented May 7, 2015

@jimmyxian @aluzzardi I say we don't support save on multi machine (at least for now) so we it would be nice to add a proper error and a test.

@vieux
Copy link
Copy Markdown
Contributor

vieux commented May 7, 2015

Also, can we then rmi busybox and testimage; load the tar file; and do a images again ?

@jimmyxian
Copy link
Copy Markdown
Contributor Author

@chanwit @aluzzardi @vieux Yeah. I will update by the comments.

@jimmyxian jimmyxian force-pushed the add-support-images-save branch from 6a79416 to 7a33022 Compare May 8, 2015 02:44
@jimmyxian
Copy link
Copy Markdown
Contributor Author

ping @aluzzardi @vieux @chanwit Modified by the comments, check again, thks

@aluzzardi aluzzardi mentioned this pull request May 8, 2015
@jimmyxian jimmyxian closed this May 12, 2015
@jimmyxian jimmyxian deleted the add-support-images-save branch May 12, 2015 12:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants