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

Add support for collections as dependencies #2609

Merged
merged 6 commits into from
Mar 29, 2020

Conversation

greg-hellings
Copy link
Contributor

@greg-hellings greg-hellings commented Mar 13, 2020

Enhance the ansible_galaxy dependency to understand collections options
in addition to roles directly.

PR Type

  • Bugfix Pull Request

@greg-hellings
Copy link
Contributor Author

Addresses #2466

@greg-hellings
Copy link
Contributor Author

This branch fails in my local environment, but it fails in the same way that master fails for me. So hopefully it won't fail in Zuul.

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Can you explain me why you transformed ansible_galaxy.py into a module? I see a very big number of changes while my impression was that this was more about enabling collections support to the already existing galaxy module.

@ssbarnea ssbarnea self-assigned this Mar 16, 2020
@greg-hellings
Copy link
Contributor Author

@ssbarnea Install of collection dependencies requires a completely separate invocation (ansible-galaxy collection install) from installing just roles (ansible-galaxy install). The command line argument long forms are different (--requirements-file --collections-path vs --role-file --roles-path). And, while the requirement files can be combined into a single one with the new syntax, they still require separate calls and passing the older syntax file into the new call results in a failure (i.e. a file that's just a list of roles will cause a failure if passed to ansible-galaxy collection install).

Taken all together, I tried to squeeze the changes into the existing file, but it would have required quite a bit of hackery that was very fragile. ansible_galaxy.py was not really set up to have two separate calls with close-but-not-identical structures. Nearly every method had a if installing_collection: ... else: ... block in it. options, default_options, bake, setup, etc all needed to be aware of both versions. It made it easier to just split the functionality between a base class and specific implementations.

@greg-hellings
Copy link
Contributor Author

Just would like to prod again for reviews on this. It's blocking my efforts to create collections that depend on other collections because I can't use Molecule to test.

@ssbarnea ssbarnea added the gate label Mar 26, 2020
@ssbarnea
Copy link
Member

I did a brief review and minimal manual testing but due to its size and lack of other reviews, I am going to take a leap-of-faith and let it slip in. I hope I will not regret.

@greg-hellings, a big thanks for taking care of this!

@greg-hellings
Copy link
Contributor Author

@ssbarnea - I'm not sure how to parse those failures of the gate job. The error is ModuleNotFoundError: No module named 'ansible.module_utils.docker', which doesn't seem to be something related to the change I've introduced. I've merged latest master with my branch locally and the error doesn't occur here, so I'm pushing that merge commit to this PR to see if it passes the gate job.

@ansible-zuul ansible-zuul bot removed the gate label Mar 26, 2020
@greg-hellings
Copy link
Contributor Author

Currently tests are failing on Python 2.7 because you can't pip install the necessary openstacksdk module there:

$ pip install "openstacksdk>=0.15.0"
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. A future version of pip will drop support for Python 2.7. More details about Python 2 support in pip, can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support
Collecting openstacksdk>=0.15.0
  Using cached openstacksdk-0.44.0-py2.py3-none-any.whl (1.3 MB)
Collecting keystoneauth1>=3.18.0
  Using cached keystoneauth1-3.18.0-py2.py3-none-any.whl (310 kB)
Collecting cryptography>=2.1
  Using cached cryptography-2.8-cp27-cp27mu-manylinux2010_x86_64.whl (2.3 MB)
Collecting os-service-types>=1.7.0
  Using cached os_service_types-1.7.0-py2.py3-none-any.whl (24 kB)
Collecting requestsexceptions>=1.2.0
  Using cached requestsexceptions-1.4.0-py2.py3-none-any.whl (3.8 kB)
Collecting decorator>=4.4.1
  Using cached decorator-4.4.2-py2.py3-none-any.whl (9.2 kB)
Processing /home/ghelling/.cache/pip/wheels/a7/c1/ea/cf5bd31012e735dc1dfea3131a2d5eae7978b251083d6247bd/PyYAML-5.3.1-cp27-cp27mu-linux_x86_64.whl
Collecting jsonpatch!=1.20,>=1.16
  Using cached jsonpatch-1.25-py2.py3-none-any.whl (11 kB)
Collecting iso8601>=0.1.11
  Using cached iso8601-0.1.12-py2.py3-none-any.whl (12 kB)
Processing /home/ghelling/.cache/pip/wheels/b4/ad/c4/1faa9b2da61b12b7c3821cda298be230c457ee740bd8658cb2/dogpile.cache-0.9.0-py2.py3-none-any.whl
Collecting netifaces>=0.10.4
  Using cached netifaces-0.10.9-cp27-cp27mu-manylinux1_x86_64.whl (31 kB)
Collecting appdirs>=1.3.0
  Using cached appdirs-1.4.3-py2.py3-none-any.whl (12 kB)
Collecting six>=1.10.0
  Using cached six-1.14.0-py2.py3-none-any.whl (10 kB)
Collecting ipaddress>=1.0.17; python_version < "3.3"
  Using cached ipaddress-1.0.23-py2.py3-none-any.whl (18 kB)
ERROR: Could not find a version that satisfies the requirement futurist>=2.1.0 (from openstacksdk>=0.15.0) (from versions: 0.1.0, 0.1.1, 0.1.2, 0.2.0, 0.3.0, 0.4.0, 0.5.0, 0.6.0, 0.7.0, 0.8.0, 0.9.0, 0.10.0, 0.11.0, 0.12.0, 0.13.0, 0.14.0, 0.15.0, 0.16.0, 0.17.0, 0.18.0, 0.19.0, 0.20.0, 0.21.0, 0.21.1, 0.22.0, 0.23.0, 1.0.0, 1.1.0, 1.2.0, 1.3.0, 1.3.1, 1.3.2, 1.4.0, 1.5.0, 1.6.0, 1.7.0, 1.8.0, 1.8.1, 1.9.0, 1.10.0)
ERROR: No matching distribution found for futurist>=2.1.0 (from openstacksdk>=0.15.0)

This is causing Python 2.7 tests to fail. It looks like this failure is new as of openstacksdk 0.44 and limiting the install to >=0.15.0,<0.44.0 succeeds.

@greg-hellings
Copy link
Contributor Author

OK, the Python 2.7 issue is addressed, now it's seeing this error:

2020-03-27 04:31:57.042334 | TASK [ensure-tox : Output tox version]
2020-03-27 04:31:56.807477 | centos-7 | Traceback (most recent call last):
2020-03-27 04:31:56.807700 | centos-7 |   File "/usr/bin/tox", line 7, in <module>
2020-03-27 04:31:56.807884 | centos-7 |     from tox import cmdline
2020-03-27 04:31:56.808106 | centos-7 |   File "/usr/lib/python2.7/site-packages/tox/__init__.py", line 9, in <module>
2020-03-27 04:31:56.808224 | centos-7 |     import pluggy
2020-03-27 04:31:56.808454 | centos-7 |   File "/usr/lib/python2.7/site-packages/pluggy/__init__.py", line 16, in <module>
2020-03-27 04:31:56.808658 | centos-7 |     from .manager import PluginManager, PluginValidationError
2020-03-27 04:31:56.808931 | centos-7 |   File "/usr/lib/python2.7/site-packages/pluggy/manager.py", line 11, in <module>
2020-03-27 04:31:56.809075 | centos-7 |     import importlib_metadata
2020-03-27 04:31:56.809309 | centos-7 |   File "/usr/lib/python2.7/site-packages/importlib_metadata/__init__.py", line 16, in <module>
2020-03-27 04:31:56.809486 | centos-7 |     from ._compat import (
2020-03-27 04:31:56.809725 | centos-7 |   File "/usr/lib/python2.7/site-packages/importlib_metadata/_compat.py", line 20, in <module>
2020-03-27 04:31:56.809957 | centos-7 |     from backports.configparser import ConfigParser
2020-03-27 04:31:56.810166 | centos-7 | ImportError: No module named configparser
2020-03-27 04:31:57.628220 | centos-7 | ERROR
2020-03-27 04:31:57.628439 | centos-7 | {
2020-03-27 04:31:57.628514 | centos-7 |   "delta": "0:00:00.064515",
2020-03-27 04:31:57.628576 | centos-7 |   "end": "2020-03-27 04:31:56.815348",
2020-03-27 04:31:57.628637 | centos-7 |   "msg": "non-zero return code",
2020-03-27 04:31:57.628697 | centos-7 |   "rc": 1,
2020-03-27 04:31:57.628783 | centos-7 |   "start": "2020-03-27 04:31:56.750833"
2020-03-27 04:31:57.628909 | centos-7 | }

This is beyond my ken and outside of my expertise.

@ssbarnea
Copy link
Member

ssbarnea commented Mar 27, 2020 via email

@ssbarnea
Copy link
Member

@greg-hellings Please rebase it. Sadly by the time I sent it to the gate, the gate was broken due to py27 issues. Now that we removed py27, it should go in easily.

Enhance the ansible_galaxy dependency to understand collections options
in addition to roles directly.
When the current role is part of a collection, set the collection
search path for ANSIBLE to the parent collection, so that any playbooks
referencing collections within the same namespace can do their job
appropriately
@greg-hellings
Copy link
Contributor Author

Rebased. First round of tests are passed.

@ssbarnea ssbarnea added the gate label Mar 29, 2020
@ansible-zuul ansible-zuul bot merged commit 8205b8e into ansible:master Mar 29, 2020
greg-hellings added a commit to greg-hellings/molecule that referenced this pull request Apr 6, 2020
Fixes a significant typo in ansible#2609, and adds a test to be sure the error
does not creep back in
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants