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.

Sign up for GitHub

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

Jump to bottom

Add compatibility for docker-py version 3 #36973

Merged
merged 1 commit into from Mar 6, 2018
Merged

Add compatibility for docker-py version 3 #36973

merged 1 commit into from Mar 6, 2018

Conversation

skylerbunny
Copy link
Contributor

SUMMARY

Adds decision trees in the container, image, and network docker modules to account for Docker SDK for Python changes made in version 3. Effectively adds Ansible support for the docker_ modules for Python 3.6 since Docker SDK 3 adds that support.

The SDK changes being fixed for comptatibility are specifically outlined at https://docker-py.readthedocs.io/en/stable/change-log.html#breaking-changes . I cannot guarantee I've fixed every incompatibility, but I'm fairly sure I've found at least "most" of them - and certainly this addresses more than current devel, which is "none of them". I tested creating images, pulling an image, pushing an image to a tar file, and running a container with and without detaching it, using (docker-py, docker < 3, and docker >= 3).

Fixes #35612 .

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/module_utils/docker_common.py
lib/ansible/modules/cloud/docker/docker_container.py
lib/ansible/modules/cloud/docker/docker_image.py
lib/ansible/modules/cloud/docker/docker_network.py

ANSIBLE VERSION

devel
(This may want to be backported to at least 2.5, which was separated from devel at about the same time that docker SDK 3 came out.)

ADDITIONAL INFORMATION

Credit to @sivel for early clues as to where to apply module fixes. I took those, modified, and applied more of them.

@ansibot
Copy link
Contributor

ansibot commented Mar 4, 2018

cc @chouseknecht @cove @dusdanig @jctanner @joshuaconner @kassiansun @keitwb @olsaki @softzilla @zfil
click here for bot help

@ansibot ansibot added bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. docker module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 4, 2018
@ansibot
Copy link
Contributor

ansibot commented Mar 4, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/modules/cloud/docker/docker_container.py:1002:0: trailing-whitespace Trailing whitespace

The test ansible-test sanity --test pep8 [explain] failed with 12 errors:

lib/ansible/modules/cloud/docker/docker_container.py:696:9: E201 whitespace after '('
lib/ansible/modules/cloud/docker/docker_container.py:696:44: E202 whitespace before ')'
lib/ansible/modules/cloud/docker/docker_container.py:995:13: E201 whitespace after '('
lib/ansible/modules/cloud/docker/docker_container.py:995:48: E202 whitespace before ')'
lib/ansible/modules/cloud/docker/docker_container.py:999:27: E203 whitespace before ':'
lib/ansible/modules/cloud/docker/docker_container.py:1002:66: W291 trailing whitespace
lib/ansible/modules/cloud/docker/docker_container.py:2154:9: E201 whitespace after '('
lib/ansible/modules/cloud/docker/docker_container.py:2154:15: E201 whitespace after '('
lib/ansible/modules/cloud/docker/docker_container.py:2154:50: E202 whitespace before ')'
lib/ansible/modules/cloud/docker/docker_container.py:2154:52: E202 whitespace before ')'
lib/ansible/modules/cloud/docker/docker_image.py:250:9: E201 whitespace after '('
lib/ansible/modules/cloud/docker/docker_image.py:250:44: E202 whitespace before ')'

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 4, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Mar 5, 2018
@skylerbunny skylerbunny changed the title Add comptability for docker-py version 3 Add compatibility for docker-py version 3 Mar 5, 2018
@sivel sivel requested a review from chouseknecht March 5, 2018 15:25
@skylerbunny
Copy link
Contributor Author

(Squashed two commits down to one and rebased against current devel. Apologies for any confusion. I think I did it cleanly.)

@sivel sivel mentioned this pull request Mar 5, 2018
Closed
@chouseknecht chouseknecht merged commit d984afa into ansible:devel Mar 6, 2018
chouseknecht pushed a commit to chouseknecht/ansible that referenced this pull request Mar 6, 2018
@skylerbunny
Add compatibility for docker-py version 3 (ansible#36973)
d23558b
@chouseknecht chouseknecht mentioned this pull request Mar 6, 2018
Merged
chouseknecht pushed a commit that referenced this pull request Mar 6, 2018
Add compatibility for docker-py version 3 (#36973) (#37071)
c2cc6e4
abadger pushed a commit that referenced this pull request Mar 6, 2018
@skylerbunny @abadger
Add compatibility for docker-py version 3 (#36973)
d6f150c 
(cherry picked from commit d984afa)
@ju2wheels
Copy link

What versions will this be available in?

@skylerbunny
Copy link
Contributor Author

It's in devel (aka 2.6) and was also backported to 2.5. This PR covered 2.6, and #37071 covers 2.5.

@skylerbunny skylerbunny deleted the docker-py-3 branch March 6, 2018 16:05
@abadger
Copy link
Contributor

abadger commented Mar 6, 2018

I've cherry-picked it to stable-2.4 as well. So the next releases to have it should be 2.5.0 and 2.4.4

@abadger abadger moved this from TODO to Done in Python 3 compatibility Mar 20, 2018
@abadger abadger mentioned this pull request Mar 29, 2018
Closed
@sivel sivel mentioned this pull request Mar 29, 2018
Closed
@skylerbunny skylerbunny mentioned this pull request Jun 22, 2018
Merged
@felixfontein felixfontein mentioned this pull request Oct 21, 2018
Closed
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chouseknecht chouseknecht chouseknecht approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. docker module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. python3 support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
No open projects
Python 3 compatibility
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

docker_image module TypeError: build() got an unexpected keyword argument 'stream'
6 participants
@skylerbunny @ansibot @ju2wheels @abadger @chouseknecht @bcoca