I suggest adding new items to Ansible community package collection inclusion requirements.
Justification: this change doesn’t imply a lot of work but it’d make the EE users life much easier.
Please take a look at the pull request.
2 Likes
I personally am not interested in making this a requirement. The requirements.txt and bindep files’ syntax is sometimes too limited to fully express a collections’ dependencies. For larger collections, such as community.general or community.network, it may be impractical or at least not useful (nobody is going to use every single plugin in the package) to express the dependencies for every single plugin in the package if one place. Additionally, execution environments are orthogonal to the ansible package. If collections want to release their own EEs, they’re welcome to do that, but I don’t think we should require them to have EE configuration or the requirements files to enable that. I would suggest including stub bindep or requirements.txt files in the collection-template if you want to encourage collection maintainers to include them.
4 Likes
In particular for community.general, this is very inpractical and probably even outright dangerous, as it would add so many different dependencies that a) the chance of conflicts gets pretty high, and b) the EE will be really huge.
This totally makes sense for special-purpose collections, where these requirements can also be compiled a lot easier.
Finally, another problem is that when building an EE, you cannot decide which collection’s requirements to skip or ignore. If you get a conflict, you have to remove a collection from the EE to resolve (if picking specific versions of the collections doesn’t help). As long as this isn’t possible, forcing collections to declare their dependencies for EEs potentially makes life harder for EE builders.
(Disclaimer: I did add EE config files to all collections I maintain, except community.general, for the above reasons. We - as in c.g maintainers - discussed this once, see Execution environment support for community.general · Issue #4512 · ansible-collections/community.general · GitHub. Especially for collections such as c.g, solutions as mentioned in Create guidelines for collection python requirements · Issue #224 · ansible-community/community-topics · GitHub would likely be more useful.)
4 Likes
In any case: if we decide to require collections in Ansible to provide EE configs, we should have an exception at least for c.general and c.network.
(And another piece of interest: Make EE a proper EE with dependencies installed by felixfontein · Pull Request #22 · ansible-community/images · GitHub - especially more at the end of the discussion in there.)
2 Likes
- @gotmax23 i like the idea to put the files to collection_template, thanks! please take a look EE: add bindep.txt and requirements.txt by Andersson007 · Pull Request #66 · ansible-collections/collection_template · GitHub
- @felixfontein yep, there definitely should be exception for c.general and c.network and i don’t think it’s a frequent case, feels like rather an exception in the collection world
- anyway, I see your concerns, how about changing
MUSTtoSHOULDin the statements? I understand that it’s better not to pollute the requirements with unnecessary stuff but I think it’d be a good trade-off between a couple of new lines and the convenience it brings to EE users. What do you think? - there can be tree options in the future vote:
- Leave it with MUST (with exception for community.general and .network)
- Use SHOULD instead
- Not include the statements at all
1 Like
I think SHOULD instead of MUST is a good start. Especially for collection maintainers who themselves don’t use EEs creating proper requirement files can be tricky.
One problem is also that if you add a requirement to it, you cannot easily remove it without a new major release anymore (this happened to the docker-compose Python package dependency in community.docker - the result was that community.docker got kicked out of the devtools team’s EE since that requirement conflicted with other requirements).
2 Likes
I still feel a bit hesitant about having this here when the data is in no way used by the ansible package build process itself and EEs are a separate thing, but I guess I’d be okay with a SHOULD.
2 Likes
One example why simply having requirements.txt in the collection root without an EE configuration file can be outright dangerous: https://github.com/nutanix/nutanix.ansible/blob/main/requirements.txt - from the dependencies listed there you can see that these are obviously not collection dependencies, but for testing, building, or doing something else with the collection. But since that file is in the collection root, and meta/ contains no EE config, ansible-builder uses this file as the collection dependencies, even though that’s obviously not what the collection maintainers intended.
4 Likes
That reminds me of python projects where one finds requirements.txt for the runtime reqs and requirements-test.txt or requirements-dev.txt for the obvious purpose(s).
And that leads me to think that even if we store the EE-related file in a different directory we might still need to have one requirement file for the EE runtime and a different one for the testing (of the EE itself).
@felixfontein should that prompt a change in ansible-builder not to assume that behaviour as a default?
1 Like
@russoz I think it’s next to impossible to change ansible-builder’s behavior, since a lot of folks depend on it and probably also internalized that it works that way. I think it’s better to set up collection_template that you cannot accidentally fall in this trap.
2 Likes
all the points are fair, changed the PR, PTAL
I am inclined to prefer MUST to SHOULD and to implement this into linter as soon is approved.
That change should improve how collection maintainers are keeping track of dependencies and how these are compiled later inside execution environments.
Yes, we have few exceptions, but these are VERY FEW!
Having an accurate requirements and bindep file in a collection makes the consumers life much easier. Whether it’s building an EE or simply a virtual env specific to an ansible project having these files in place IMO is a necessity.
I also think test and dev requirements should be captured in seperate files.
For community.general and community.network I think these files should also exist. If it is not possible to resolve conflicts within these collections, I consider that a bug, if only because it is then impossible to run the entirety of the tests for the collection with a single run.
+1 for having these files templated out
+1 for making it an inclusion requirement
+1 for open issues in repos where it is not possible today
+1 for support in ansible-lint to help
If a collection has no external deps (python or system), Should empty files exist? Or does missing files imply no deps? (I prefer empty files)
-brad
1 Like
For the record, I stand by this statement. I understand that people want to push EEs, but I’ve yet to hear a compelling reason as to why this should be a MUST requirement that specifically relates to the ansible Python package. If ansible-lint wants to require this, that’s its own prerogative.
2 Likes
Good point. As far as I understand EEs, they’re meant to be minimal and just include the required collections. On the other hand, the ansible community package is meant to be “batteries included” with a lot of (hopefully) useful collections. They kind of contradict each other.
That said, I think even SHOULD would be too much. The ansible community package doesn’t need this EE stuff, so why make it a requirement (or a recommendation)?
1 Like
Using the requirements and bindep file for an EE build is just one use case, I can think of several more:
- Documents the requirements for a given collection in a familiar way
- Helps with the build of a python virtual environment capable of using a collection
- Used within a CI pipeline to test ansible content using a collection and it’s requirements
I get that these may not be directly related to “all-in-one” ansible package, but every collection within the package is also available as a standalone collection on galaxy, so it should be safe to assume all the collections within the ansible package should be easily consumed and used outside the package.
While having this as an inclusion requirement may not directly improve the experience of an ansible package user today, it would ensure all collections that make up the package are normalized with regard to their python and system requirements in a common fashion.
1 Like
That was my initial thought, but I was aiming for a compromise approach with SHOULD.
I’ll note that we already require collections to document requirements within plugin DOCUMENTATION, but that’s
Those three usecases are definitely valid, even if there’s other ways to address them. Thank you for bringing them up! I’m mainly hung up on the lack of direct benefit to users of the ansible package.
Regarding the last two points, we already strugle with collection maintainers running the required tests, so I’m not inclined to add a requirement for how they have to run them. Also, there’s the issue with runtime vs. testing dependencies. There should be a separation of concerns there, and asking maintainers to put a requirements.txt for test dependencies in the repo root doesn’t address that and doesn’t help with EEs which only need runtime dependencies.
1 Like
I strongly disagree here. Especially for community.general, the set of requirements would be rather large and heavily increase EE size without providing much benefit, since most users only use a handful (or maybe a few handful) of modules/plugins from community.general. (Also note that most modules can also be run on localhost, thus you should also include dependencies for ~600 modules next to the dependencies for ~120 plugins.)
Because of that, and the numerous potential conflicts of these dependencies, I find it a very bad idea to add EE dependencies to community.general (and probably also community.network - that one might be less problematic since its more homogenous).
A system which allows to specifically install the requirements needed for a certain set of plugins/modules (or providing these as a list, to allow installing them in an EE) would be a lot more helpful, both for EE users and other Ansible users. But that requires tooling that does not yet exist.
1 Like
FTR, we discussed this a bit in Create guidelines for collection python requirements · Issue #224 · ansible-community/community-topics · GitHub.
A little late to the party but I also don’t think that these should be community package inclusion requirements, and this goes double for files in the root of the collection. I did add some files to meta/ in my collection (https://github.com/ansible-collections/community.hashi_vault/tree/main/meta) based on a PR opened by an end-user: https://github.com/ansible-collections/community.hashi_vault/pull/105
But I don’t do much in the way of testing it either.
My collection is rather small with few requirements. For something like c.g, it may be impossible to include all requirements for all content without any conflicts.
An execution environment creator needs to know which components of a collection it needs, and which requirements are needed for those.
I would be in favor of a SHOULD for meta/ EE files, but would not be in favor of a SHOULD for files in the root, and not in favor of MUST for any of these.
2 Likes