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

Bump to ansible 2.4 #10

Closed
wants to merge 7 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@
# requirements files see:
# https://packaging.python.org/en/latest/requirements.html
install_requires=[
'molecule==1.25.1',
'ansible==2.3.2',
'ansible==2.4',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the Python semantics, this will install 2.4.0.0. Do we want ansible==2.4.6 or ansible<2.4?

'docker',
'docker-py',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you rememebr why this dependency is required? I briefly looked into the state of the opened molecule v2 PRs with Ansible 2.5/2.6 following the discussion in ome/ansible-role-basedeps#5 (comment). However the initial destroy phase failed with ansible/molecule#1384. After removing docker-py from setup.py and re-installing this meta-package, as suggested in the issue and by the output of molecule --debug test, the tests passed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I remember it was a mess due to breaking changes between docker and docker-py, we definitely shouldn't have both. It'll need testing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember what else, but docker-py is certainly critical for docker-compose integration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed docker seems to be a full rewrite of docker-py and both packages certainly incompatible changes.

Looking at the requirements of docker-compose and various issues includes ansible/molecule#1347, ansible/ansible#22993:

  • docker-compose nows declares docker as a dependency instead of docker-py since 2.10.0
  • the Ansible Docker module used to recommend docker-py as a dependency. Recent Ansible versions seem to have included full support for docker and docker scenario guide: docker-py -> docker ansible/ansible#44856 is updating the official doc accordingly.

Switching to docker across the board sounds like the long-term option anyways. For this PR, I wonder whether the decision here is related to the targetted version of Ansible ome/ansible-role-basedeps#5 (comment). If we decide to go for Ansible 2.6, docker should work both for compose and the ansible Docker modules. If we stick to Ansible 2.4, it might be that docker-py is the best choice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following combinations work on travis with idr/deployment IDR/deployment#135

  • ansible==2.5.9 docker-py==1.10.6 molecule==2.18.0
  • ansible==2.6.4 docker-py==1.10.6 molecule==2.18.0
  • ansible==2.6.4 docker==3.5.0 molecule==2.18.0

'docker-compose',
'molecule>=2.4',
'os_client_config',
'shade',
],
)