docker module: fix regressions introduced by f38186c and 80aca4b

[zfil]

Currently docker facts are broken since 80aca4b

1/ Fix in commit f38186c introduced a regression when removing a container:

failed: [localhost] => {"changed": true, "failed": true}
msg: Docker API Error: no such id:   ef7b1bafa9950645ca...

manager.get_inspect_containers() is invoked on a non existing container

2/ facts are not returned with consistent information depending what state is requested to the module

This pull request propose a fix to return consistent information and don't fail when removing containers.

[zfil]

Fixes issue #1863

[zfil]

@abadger do I need to add something in this pull request ?

[gregdek]

Sorry @zfil for the triage delay.

@cove @joshuaconner @softzilla @smashwilson Please review this PR to make sure it adheres to the following guidelines:

http://docs.ansible.com/developing_modules.html#module-checklist

If it passes these guidelines, and if you believe it’s a good PR otherwise, please add a comment with "shipit" in the text, and we will flag it for inclusion.

If it fails these guidelines, please add a comment with "needs_revision" in the text, along with the needed changes, and we will flag it for revision.

[zfil]

@gregdek any way to speed this a bit ? This is a quite trivial pull request ...

[zfil]

@abadger
BTW if this PR is accepted and merge #1457 and #1879 should be closed.

I'm using this for a couple of month without issues ...

[ThomasSteinbach]

I would be so glad to see any solution that would fix the broken facts of 80aca4b. Since January this year I am struggeling around with patching my and others Ansible installation with a Playbook that will apply my fix 035afa2 to their the installation. However it will copy the docker.py, which is meanwhile outdated.

[zfil]

@ThomasSteinbach yeah I don't understand why this is taking soo long.

@gregdek the people you requested to review this seems not interested. Could something be done here ?

[smashwilson]

Ah, this looks perfect.

@zfil: My apologies for the delay. 🙇 My life went just a bit nuts over the past few months, and I'm no longer using this module day-to-day, both of which combined to make me a rather poor maintainer as of late.

[abadger]

Thanks @zfil for the code! I'll go ahead and close #1457, #1863, and #1879
Thanks @smashwilson for the review and I hope life is a bit less hectic for you these days!

We do seem to be short on people who are using this module and want to review code changes to it... @zfil if you're interested let us know if you want to be added to the owners list. If anyone else does not want to be cc'd on these docker tickets to review changes to the module anymore, you can let us know that too and we can remove you. (or you guys can submit PRs -- we keep track of who we should CC via the author: field in the module DOCUMENTATION string.)

[abadger]

Merged

Hi!

This has been merged in, and will also be included in the next major release.

If you or anyone else has any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

• https://groups.google.com/forum/#!forum/ansible-project - for user questions, tips, and tricks
• https://groups.google.com/forum/#!forum/ansible-devel - for strategy, future planning, and questions about writing code

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular
issue is resolved.

Thank you!

[ThomasSteinbach]

Hi @abadger
you can put me on the owners list too, if you'd like to. I use the docker module very often and would like to give sth. back. I committed the pull request #1457

Kind Regards

[abadger]

@ThomasSteinbach Thank! I've added you to the list.

[zfil]

Thanks @abadger !
You can put me on the owner list as well.

[abadger]

Done. Thanks guys.