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

Updated docker-py to 0.6.0 to fix issues with directories and symlinks #659

Closed
wants to merge 1 commit into from

Conversation

petercv
Copy link

@petercv petercv commented Nov 19, 2014

By updating docker-py to version 0.6.0 an issue is resolved where symlinks and empty directories were not properly passed to docker when building an image. See #651. This issue resulted in images that were different from images built with the default docker client.

I successfully ran all the tests.

Output:

Ran 156 tests in 206.877s

OK

@thaJeztah
Copy link
Member

You forgot to "sign off" your commit, see https://github.com/docker/fig/blob/master/CONTRIBUTING.md

You don't have to create a new pull request to fix this, just amend your commit and force push, that should automatically update the pull request.

@petercv
Copy link
Author

petercv commented Nov 19, 2014

Sorry about that, fixed now.

@thaJeztah
Copy link
Member

Hmm seems that added extra commits instead of replacing the existing one. I'm not very good at git (sorry) perhaps someone else is able to help out fixing it. 😄

@petercv petercv force-pushed the master branch 3 times, most recently from bd27d1c to 0b76274 Compare November 19, 2014 20:17
@petercv
Copy link
Author

petercv commented Nov 19, 2014

Think I fixed it now with some help of a friend and StackOverflow. :)

@thaJeztah
Copy link
Member

Yeah! All green 😃

Signed-off-by: Peter Verhage <peter@egeniq.com>
@petercv
Copy link
Author

petercv commented Nov 24, 2014

@thaJeztah I updated the pull request last Friday to also update the version number in setup.py (although during building of fig you get a warning on the requirements in setup.py that they already have been satisfied). Now the tests fail, but there is no description on why they failed. Do you know more?

@bfirsh
Copy link

bfirsh commented Nov 24, 2014

It's because Wercker is running Docker 1.2.0, which I think we should still support.

@petercv
Copy link
Author

petercv commented Nov 25, 2014

@bfirsh sure? Just read an article that 1.3.2 has just been released which includes 2 fixes for major security vulnerabilities, so anyone should upgrade anyway.

@dnephin
Copy link

dnephin commented Dec 3, 2014

I'd like to get this into the next release if it's ready.

We should be able to fix the build with an environment variable in the wercker config setting the docker api version for docker-py.

It looks like it doesn't support an environment variable yet, but it should be easy enough to add that support to fig/cli/docker_client.py, something like:

docker_api_version = os.environ.get('DOCKER_API_VERSION', docker.client.DEFAULT_DOCKER_API_VERSION)
...
Client(..., version=docker_api_version)

I'm not exactly sure how to set an environment variable in wercker.yml, but it should be possible, or we can have it set as part of script/test.

I think with these changes you should be able to get the tests passing.

@bfirsh
Copy link

bfirsh commented Dec 9, 2014

I think we can pretty safely use version 1.14 of the remote API by default.

@bfirsh
Copy link

bfirsh commented Dec 9, 2014

Here we go: #718

@bfirsh
Copy link

bfirsh commented Dec 9, 2014

Fixed in #718. Thanks @petercv!

@bfirsh bfirsh closed this Dec 9, 2014
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.

4 participants