Skip to content
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

Revert #711 from dnephin/fix_volumes_on_recreate #863

Merged
merged 1 commit into from Jan 20, 2015

Conversation

dnephin
Copy link

@dnephin dnephin commented Jan 20, 2015

Turns out this needs more testing.

There are some significant differences between docker 1.2 and 1.4.1 that aren't handled properly.

It would be awesome if we could setup wercker to test against at least a couple different versions of dockerd.

…reate"

This reverts commit 55095ef, reversing
changes made to 72095f5.

Signed-off-by: Daniel Nephin <dnephin@yelp.com>
@aanand
Copy link

aanand commented Jan 20, 2015

LGTM

aanand added a commit that referenced this pull request Jan 20, 2015
Revert #711 from dnephin/fix_volumes_on_recreate
@aanand aanand merged commit f57db07 into docker:master Jan 20, 2015
@bfirsh
Copy link

bfirsh commented Jan 26, 2015

@dnephin Wercker is very solidly stuck at a single Docker version, unfortunately.

What we can do is get Fig tested on Docker's Jenkins instance. That'll let us test on Windows and other cool things like that.

@cpuguy83
Copy link
Contributor

@dnephin Please let me know if you need any help on this.
I'm not a pythonist at all, but know the volumes side well and can fix any issues you're seeing on the Docker side of this.

Also would be a great help to test and make sure the Docker 1.5 RC is working!

@dnephin
Copy link
Author

dnephin commented Jan 26, 2015

Thanks @cpuguy83 I'm working on this in #858

I tried to reproduce with docker cli, but was not able to reproduce the error https://gist.github.com/dnephin/e584a7741a514d631632

Next step is to send the requests through mitmproxy to find out why they are different (I believe they should be the same). I was informed the docker cli doesn't support proxy settings until 1.5 anyway, so I'll have to try with 1.5

@cpuguy83
Copy link
Contributor

@dnephin This is probably because the CLI does everything on the create endpoint.

@dnephin
Copy link
Author

dnephin commented Jan 26, 2015

Oh, from the docker-py docs I wasn't aware that was possible, but that makes a lot of sense.

Does it get passed as Binds or Volumes to the remote api? Maybe docker-py is missing that support?

@cpuguy83
Copy link
Contributor

I think @shin- Added this to docker-py recently.
Basically, as of docker 1.2(?) you can pass in anything form HostConfig (read: everything that you were passing in at start) in during create time. https://docs.docker.com/reference/api/docker_remote_api_v1.16/#create-a-container

So it still looks the same, except the start options are nested within the main contianer config as HostConfig. This should look like the docker inspect output.

@dnephin
Copy link
Author

dnephin commented Jan 26, 2015

Ok Binds is missing from docker-py master https://github.com/docker/docker-py/blob/master/docker/client.py#L206

I should have looked at the official API docs instead of just the docker-py source. I'll try that.

@shin-
Copy link

shin- commented Jan 26, 2015

We do support Binds, but you're looking in the wrong place :)

Let me know if I can help!

@dnephin
Copy link
Author

dnephin commented Jan 26, 2015

Ah yes, realized it has to go through host_config.

I believe it's working with host_config ! I need to do some cleanup, and we need to resolve #886 so we can unpin version='1.14', but otherwise this is great progress!

Thanks @cpuguy83 @shin-

yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
…nges

Revert docker#711 from dnephin/fix_volumes_on_recreate
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
@dnephin dnephin deleted the revert_volume_recreate_changes branch August 24, 2015 15:04
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 30, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 30, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Jan 31, 2023
ndeloof pushed a commit to ndeloof/compose that referenced this pull request Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants