Docker modules: improve documentation on docker vs. docker-py Python package requirements #42457

felixfontein · 2018-07-07T09:06:08Z

SUMMARY

The original (semi-)official Python module for working with docker was called docker-py, which exists up to version 1.10.6. It has been renamed/superseeded to/by the docker module, which starts with version 2.0.0.

This (and the fact that this is not mentioned on either of the pypi pages; though such a note was apparently added, but later got lost again) leads to a lot of confusion, which can be seen for example in #42162 and #42258. A lot of people seem to believe since the requirements and error messages only mention python-py, that the modules do not work with the Python docker module (which is wrong, except if this is caused by a bug -- but that should not be fixed by adjusting the requirements). On the other hand, python-py is still required for Python 2.6, since Python 2.6 support was dropped before the first release of docker.

This PR tries to clean up the documentation by adding a disclaimer in the requirements section of every module mentioning both docker and docker-py (recommending docker if Python 2.6 is not required), and updates the error messages in the common code in module_utils/docker_common.py to also mention docker and not just docker-py.

Superseeds #42370 and #42394. Fixes #42258.
CC @samdoran, @kassiansun.

I would appreciate it if some of the docker module maintainers could take a look at this in more detail. Thanks.

ISSUE TYPE

Docs Pull Request

COMPONENT NAME

lib/ansible/module_utils/docker_common.py
docker_container
docker_image
docker_image_facts
docker_login
docker_network
docker_secret
docker_service
docker_swarm
docker_volume

ANSIBLE VERSION

2.6.1

ansibot · 2018-07-07T09:07:58Z

cc @agronholm @chouseknecht @cove @dusdanig @joshuaconner @keitwb @olsaki @softzilla @tbouvet @zfil
click here for bot help

felixfontein · 2018-07-07T09:13:17Z

(Once #40839 is merged, there's one error message in there which should also be updated to mention docker as well. CC @resmo)

agronholm · 2018-07-07T09:26:58Z

lib/ansible/modules/cloud/docker/docker_image_facts.py

@@ -36,6 +36,12 @@
 requirements:
  - "python >= 2.6"
  - "docker-py >= 1.7.0"
+  - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+     module has been superseeded by L(docker,https://pypi.org/project/docker/)


Typo: superseeded -> superseded

Thanks for noticing! I just fixed all of them.

agronholm · 2018-07-07T09:27:05Z

lib/ansible/modules/cloud/docker/docker_login.py

@@ -77,6 +77,12 @@
 requirements:
    - "python >= 2.6"
    - "docker-py >= 1.7.0"
+    - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+       module has been superseeded by L(docker,https://pypi.org/project/docker/)


agronholm · 2018-07-07T09:27:13Z

lib/ansible/modules/cloud/docker/docker_network.py

@@ -96,6 +96,12 @@
 requirements:
    - "python >= 2.6"
    - "docker-py >= 1.7.0"
+    - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+       module has been superseeded by L(docker,https://pypi.org/project/docker/)


agronholm · 2018-07-07T09:27:20Z

lib/ansible/modules/cloud/docker/docker_secret.py

@@ -60,6 +60,10 @@

 requirements:
  - "docker-py >= 2.1.0"
+  - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+     module has been superseeded by L(docker,https://pypi.org/project/docker/)


agronholm · 2018-07-07T09:27:32Z

lib/ansible/modules/cloud/docker/docker_service.py

@@ -143,6 +143,13 @@

 requirements:
    - "python >= 2.6"
+    - "docker-py >= 1.8.0"
+    - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+       module has been superseeded by L(docker,https://pypi.org/project/docker/)


Here as well.

agronholm · 2018-07-07T09:27:38Z

lib/ansible/modules/cloud/docker/docker_swarm.py

@@ -136,6 +136,13 @@
    - docker
 requirements:
    - python >= 2.7
+    - docker-py >= 1.7.0
+    - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+       module has been superseeded by L(docker,https://pypi.org/project/docker/)


agronholm · 2018-07-07T09:27:46Z

lib/ansible/modules/cloud/docker/docker_volume.py

@@ -68,6 +68,12 @@
 requirements:
    - "python >= 2.6"
    - "docker-py >= 1.10.0"
+    - "Please note that the L(docker-py,https://pypi.org/project/docker-py/) Python
+       module has been superseeded by L(docker,https://pypi.org/project/docker/)


And finally here too.

kassiansun · 2018-07-07T10:25:45Z

lib/ansible/modules/cloud/docker/docker_swarm.py

@@ -136,6 +136,13 @@
    - docker
 requirements:
    - python >= 2.7
+    - docker-py >= 1.7.0


I think docker_swarm is for docker's built-in swarm mode, docker-py should have a higher version number?

Per http://docker-py.readthedocs.io/en/stable/change-log.html, it should be >= 1.10.0

Hmm, good question; the code also uses the log_driver parameter of create_swarm_spec, which has (according to the change log) only introduced in 2.6.0; I'll bump it up to that for now. @tbouvet: do you know more details?

kassiansun · 2018-07-07T10:29:49Z

lib/ansible/modules/cloud/docker/docker_volume.py

@@ -68,6 +68,12 @@
 requirements:
    - "python >= 2.6"
    - "docker-py >= 1.10.0"


Per change log, this should be 1.5.0, but align this with MIN_DOCKER_VERSION = 1.7.0, it should not be listed explictly here.

Actually this is document, keep this and align to 1.7.0.

It uses the labels parameter for the create_volume call, so according to https://docker-py.readthedocs.io/en/stable/change-log.html it requires 1.10.0.

kassiansun · 2018-07-07T10:31:50Z

lib/ansible/modules/cloud/docker/docker_service.py

@@ -143,6 +143,13 @@

 requirements:
    - "python >= 2.6"
+    - "docker-py >= 1.8.0"


Does docker_service needs an explicitly dependency on docker-py version, or just implicitly inherits docker-compose's requirement?

It should implicitly inherit docker-compose's requirements. My idea was to list it nonetheless (so it's listed for all docker_* modules), but I can remove it if you prefer.

Yes, leaving a deprecation comment about docker-py and docker everywhere is boring, but useful for newbies.

kassiansun · 2018-07-07T10:35:04Z

lib/ansible/modules/cloud/docker/docker_container.py

@@ -96,7 +96,7 @@
    description:
      - Path to a file containing environment variables I(FOO=BAR).
      - If variable also present in C(env), then C(env) value will override.
-      - Requires docker >= 2.3.0.
+      - Requires docker-py >= 1.4.0.


Align with MIN_DOCKER_VERSION

I removed it, since the general requirements for the module already specify docker-py >= 1.7.0.

kassiansun · 2018-07-07T15:26:35Z

There're too many commits, could you please rebase them? It will make the commit history cleaner after getting merged.

felixfontein · 2018-07-07T15:46:36Z

I've squashed them. This would have happen during merge anyway, but this way the commit message is nicer ;-)

Akasurde · 2018-07-07T19:07:46Z

@felixfontein Does this resolves - #38833 ?

felixfontein · 2018-07-07T20:59:48Z

@Akasurde I don't think so. This PR is (so far) only about the dependency on the docker / docker-py Python modules, and not on the Docker daemon/server/API version required.

kassiansun · 2018-07-08T00:40:42Z

shipit

…bout docker vs. docker-py.

felixfontein · 2018-07-09T05:52:46Z

Now that #40839 has been merged, I rebased against devel and adjusted the new messages there according to the rest of this PR.

felixfontein · 2018-07-12T05:48:53Z

Everyone, thanks for your reviews, and thanks for handling this! :)

felixfontein · 2018-07-15T08:00:42Z

I will create a backport PR for stable-2.6 as soon as #42533 is merged.

…package requirements (ansible#42457) * Make sure all docker-py/docker requirements are listed, and clarify about docker vs. docker-py. * Adjusting changes made in ansible#40839.

…package requirements (#42457) * Make sure all docker-py/docker requirements are listed, and clarify about docker vs. docker-py. * Adjusting changes made in #40839.

felixfontein mentioned this pull request Jul 7, 2018

Update docker pipy version to 2.3 #42394

Merged

agronholm suggested changes Jul 7, 2018

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. labels Jul 7, 2018

kassiansun suggested changes Jul 7, 2018

View reviewed changes

kassiansun approved these changes Jul 7, 2018

View reviewed changes

felixfontein force-pushed the docker-improve-requirement-docs branch from 744ed7d to ceb2d6f Compare July 7, 2018 15:45

tao12345666333 approved these changes Jul 8, 2018

View reviewed changes

jctanner added this to the 2.7.0 milestone Jul 8, 2018

felixfontein added 2 commits July 9, 2018 07:49

Make sure all docker-py/docker requirements are listed, and clarify a…

607b365

…bout docker vs. docker-py.

Adjusting changes made in ansible#40839.

d316a76

felixfontein force-pushed the docker-improve-requirement-docs branch from ceb2d6f to d316a76 Compare July 9, 2018 05:51

samdoran merged commit 765a513 into ansible:devel Jul 11, 2018

felixfontein deleted the docker-improve-requirement-docs branch July 12, 2018 05:48

felixfontein mentioned this pull request Jul 12, 2018

[2.6] Fix docker container init check requirements #42533

Merged

felixfontein mentioned this pull request Jul 16, 2018

docker_container: create_host_config() got an unexpected keyword argument 'init' #42162

Closed

felixfontein mentioned this pull request Jul 17, 2018

[2.6] Docker modules: improve documentation on docker vs. docker-py Python package requirements #42877

Merged

felixfontein mentioned this pull request Jul 25, 2018

docker modules: improve documentation on docker / docker-py conflict #43238

Merged

skylerbunny mentioned this pull request Aug 14, 2018

docker_container fails while pulling new image #29512

Closed

felixfontein mentioned this pull request Aug 29, 2018

docker scenario guide: docker-py -> docker #44856

Merged

ansible locked and limited conversation to collaborators Jul 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docker modules: improve documentation on docker vs. docker-py Python package requirements #42457

Docker modules: improve documentation on docker vs. docker-py Python package requirements #42457

felixfontein commented Jul 7, 2018

ansibot commented Jul 7, 2018

felixfontein commented Jul 7, 2018

agronholm Jul 7, 2018

felixfontein Jul 7, 2018

agronholm Jul 7, 2018

agronholm Jul 7, 2018

agronholm Jul 7, 2018

agronholm Jul 7, 2018

agronholm Jul 7, 2018

agronholm Jul 7, 2018

kassiansun Jul 7, 2018

kassiansun Jul 7, 2018

felixfontein Jul 7, 2018

kassiansun Jul 7, 2018

kassiansun Jul 7, 2018

felixfontein Jul 7, 2018

kassiansun Jul 7, 2018

felixfontein Jul 7, 2018

kassiansun Jul 7, 2018

kassiansun Jul 7, 2018

felixfontein Jul 7, 2018

kassiansun commented Jul 7, 2018

felixfontein commented Jul 7, 2018

Akasurde commented Jul 7, 2018

felixfontein commented Jul 7, 2018

kassiansun commented Jul 8, 2018

felixfontein commented Jul 9, 2018

felixfontein commented Jul 12, 2018

felixfontein commented Jul 15, 2018

Docker modules: improve documentation on docker vs. docker-py Python package requirements #42457

Docker modules: improve documentation on docker vs. docker-py Python package requirements #42457

Conversation

felixfontein commented Jul 7, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Jul 7, 2018

felixfontein commented Jul 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kassiansun commented Jul 7, 2018

felixfontein commented Jul 7, 2018

Akasurde commented Jul 7, 2018

felixfontein commented Jul 7, 2018

kassiansun commented Jul 8, 2018

felixfontein commented Jul 9, 2018

felixfontein commented Jul 12, 2018

felixfontein commented Jul 15, 2018