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

docker modules: make sure everything works with older docker-py versions #55258

Merged
merged 9 commits into from Apr 17, 2019

Conversation

Projects
None yet
4 participants
@felixfontein
Copy link
Contributor

commented Apr 14, 2019

SUMMARY

I've been going through the tests for all modules (except docker_stack, which doesn't use docker-py) with older docker-py versions to see if there are problems.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

docker_container
docker_container_info
docker_host_info
docker_network
docker_node
docker_node_info
docker_prune
docker_swarm
docker_swarm_info
docker_swarm_service
docker_swarm_service_info

@ansibot ansibot added the module label Apr 14, 2019

@felixfontein felixfontein changed the title [WIP] docker modules: make sure everything works with older docker-py versions docker modules: make sure everything works with older docker-py versions Apr 14, 2019

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Apr 14, 2019

Everything looks fine now (from docker-py 1.8.0 on; I've tested all major versions, and 1.10.6, 2.0.2, 3.7.2). The docker_swarm tests had a lot of trouble on my machine, but that's kind of to be expected...

ready_for_review

@ansibot ansibot added community_review and removed WIP labels Apr 14, 2019

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Apr 16, 2019

@DBendit @WojciechowskiPiotr @akshay196 @danihodovic @dariko @hannseman @jwitko @kassiansun @tbouvet @WojciechowskiPiotr reviews are very welcome :) It would be nice to get this merged soon so it will definitely be in Ansible 2.8.0.

@@ -245,15 +245,18 @@ def get_docker_items_list(self, docker_object=None, filters=None, verbose=False)
header_images = ['Id', 'RepoTags', 'Created', 'Size']
header_networks = ['Id', 'Driver', 'Name', 'Scope']

filter_arg = dict()
if filters:
filter_arg['filters'] = filters

This comment has been minimized.

Copy link
@WojciechowskiPiotr

WojciechowskiPiotr Apr 16, 2019

Contributor

This change is purely for backward compatibility with docker-py? If yes what was wrong with the original approach?

This comment has been minimized.

Copy link
@felixfontein

felixfontein Apr 17, 2019

Author Contributor

client.networks() only got support for the filters argument in docker-py 2.0.2; apparently it was forgotten to add it until then (the API probably supported it since the beginning, or at least much earlier). I could have also put this into the networks branch only (and leave the others as-is).

@ansibot ansibot removed the needs_triage label Apr 16, 2019

@@ -259,7 +259,12 @@ def get_service_inspect(self, service_id, skip_missing=False):
Single service information structure
"""
try:
service_info = self.inspect_service(service=service_id)
service_info = self.inspect_service(service_id)
except NotFound as exc:

This comment has been minimized.

Copy link
@hannseman

hannseman Apr 17, 2019

Contributor

Shouldn't the if exc.status_code == 404: ... be removed in the expect-block below if we are now using NotFound?

https://github.com/docker/docker-py/blob/master/docker/errors.py#L13

This comment has been minimized.

Copy link
@felixfontein

felixfontein Apr 17, 2019

Author Contributor

I've just removed it.

@hannseman

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

Looks like an unrelated failure on linux/fedora29/4:

Failed to download packages: Cannot download Packages/c/container-selinux-2.94-1.git1e99f1d.fc29.noarch.rpm: All mirrors were tried

https://app.shippable.com/github/ansible/ansible/runs/119627/95/tests

@hannseman
Copy link
Contributor

left a comment

shipit

@ansibot ansibot added shipit and removed needs_revision labels Apr 17, 2019

@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

shipit

@ansibot ansibot added the automerge label Apr 17, 2019

@ansibot ansibot merged commit 12d26ec into ansible:devel Apr 17, 2019

1 check passed

Shippable Run 119635 status is SUCCESS.
Details
@felixfontein

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2019

@WojciechowskiPiotr @hannseman thank you very much for reviewing this!

@felixfontein felixfontein deleted the felixfontein:docker-old-docker-py-versions branch Apr 17, 2019

felixfontein added a commit to felixfontein/ansible that referenced this pull request Apr 17, 2019

docker modules: make sure everything works with older docker-py versi…
…ons (ansible#55258)

* General test improvements.

* Adjust tests to older docker-py versions.

* docker_swarm_server_info: work around problems with older docker-py versions

* Bump minimal docker-py version for options network_filters and disk_usage.

* More general test improvements.

* Correct usage of docker_image.

* Put files into output directory.

* Speed up test.

* Remove old check.

(cherry picked from commit 12d26ec)

ruimoreira added a commit to ruimoreira/ansible that referenced this pull request Apr 22, 2019

docker modules: make sure everything works with older docker-py versi…
…ons (ansible#55258)

* General test improvements.

* Adjust tests to older docker-py versions.

* docker_swarm_server_info: work around problems with older docker-py versions

* Bump minimal docker-py version for options network_filters and disk_usage.

* More general test improvements.

* Correct usage of docker_image.

* Put files into output directory.

* Speed up test.

* Remove old check.

abadger added a commit that referenced this pull request Apr 23, 2019

docker modules: make sure everything works with older docker-py versi…
…ons (#55258)

* General test improvements.

* Adjust tests to older docker-py versions.

* docker_swarm_server_info: work around problems with older docker-py versions

* Bump minimal docker-py version for options network_filters and disk_usage.

* More general test improvements.

* Correct usage of docker_image.

* Put files into output directory.

* Speed up test.

* Remove old check.

(cherry picked from commit 12d26ec)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.