-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Fix volumes on recreate #711
Conversation
This is a great idea. Needs a rebase though. :( |
acd1449
to
456e3e6
Compare
I need to stop adding tests at the bottom of the file so I don't have to rebase everytime. Rebased |
cc @aanand |
I suppose this could use some integration tests showing that it solves the issue. |
456e3e6
to
abf880e
Compare
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
b2efda5
to
d273947
Compare
Ok, this is rebased, and I've added an integration test (which fails on master). |
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
d273947
to
7eb476e
Compare
Oh right, I have to actually start the container in older versions of docker. This test was passing for docker 1.4.1. Pushed another update to get the test working for docker 1.2.0 |
LGTM |
Hmm, tests are failing for me against Docker 1.4.1. |
Hmm, I didn't actually test it on 1.4.1 after making the change for 1.2.
|
I've not reviewed this code but... there exists a bug in 1.4.1 where on start Docker will always try to re-apply everything in volumes-from. If the volumes-from container no longer exists, it will error out. This may be related. |
@aanand tests on master are working for me with docker 1.4.1 |
I can reproduce it on master on a fresh Digital Ocean machine with Docker 1.4.1 - here's the relevant bit of
|
@aanand I was able to reproduce the error on another host I believe the issue is related to this TODO I left (service.py#L297) The intermediate container has the correct volumes, but the final container is being passed the config which creates a new volume first (before the correct volume is applied during I'll submit another PR to address the 1.4.1 issue. |
Revert #711 from dnephin/fix_volumes_on_recreate
Fix volumes on recreate Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
…nges Revert docker#711 from dnephin/fix_volumes_on_recreate Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
Resolves: #447 (and probably some other related issues)
Instead of using
volumes_from
on the intermediate container, inspect the volumes, and copy them over the to intermediate container and from the intermediate to the final container.