Add official place for plugin utils in collections: plugins/plugin_utils

[felixfontein]

Proposal: Add official place for plugin utils in collections

Author: Felix Fontein <@felixfontein>

Date: 2020-11-03

• Status: New
• Proposal type: core/collection design
• Targeted release: 2.11 or 2.12 (for ansible-test support, should work with all collection-supporting versions of ansible/ansible-base right away)
• Estimated time to implement: 0 days for designating place, unknown time for adding ansible-test support

Motivation

Right now, shared code for modules and plugins can be put into plugins/module_utils. There are some sanity checks making sure that this code only imports code from Ansible that can be used by modules. If something is put in there which is explicitly for use by plugins only, and which does for example from ansible.utils.display import Display, one has to disable several failing tests:

plugins/module_utils/utility_not_for_modules.py import-2.6!skip
plugins/module_utils/utility_not_for_modules.py import-2.7!skip
plugins/module_utils/utility_not_for_modules.py import-3.5!skip
plugins/module_utils/utility_not_for_modules.py import-3.6!skip
plugins/module_utils/utility_not_for_modules.py import-3.7!skip
plugins/module_utils/utility_not_for_modules.py import-3.8!skip
plugins/module_utils/utility_not_for_modules.py import-3.9!skip
plugins/module_utils/utility_not_for_modules.py pylint:ansible-bad-module-import

(This is for ignore-2.10.txt and ignore-2.11.txt, it's one less line for ignore-2.9.txt.)

Right now, to avoid such things, collection authors can essentially place such plugin utilities anywhere they want inside the collection (as long as they are not in plugins/modules and plugins/module_utils). The collection loader allows all places.

Problems

There is no standard place for non-module utilities.

This makes it harder to implement import analysis for plugins (ansible-test sanity), since right now there are no restrictions on what plugins can/should do.

Solution proposal

I propose to designate plugins/plugin_utils as the natural place where to put utilities which are used by plugins, but not by modules.

Testing (optional)

Right now this does not require testing. Just having an official (and documented!) place for plugin utilities is already a good thing.

Documentation (optional)

This should definitely be documented, so that collection authors know about the place. It should also be announced in ansible-collections/overview#45.

[bcoca]

I would still keep it in module_utils as plugins already consume a few things from that location and it has become a place for common code for many things other than modules (to_text is a pervasive example).

Most of the sanity tests ensure that 'common code' does not depend on code outside of what is designated as 'common code' and I believe that should still be the case even for 'common plugin code'.

[ganeshrn]

The common directory to share code across plugins will certainly help for scenarios where the plugins execute on the control node (applicable for networking modules implemented as action plugin). Recently we ended up adding a file within top-level directory under collection plugins folder to solve the sanity issues for shared code in module utils. Reference https://github.com/ansible-collections/ansible.utils/blob/main/plugins/validate/_base.py

Another example https://github.com/ansible-collections/ansible.utils/blob/main/tests/sanity/ignore-2.11.txt

[felixfontein]

@bcoca that's totally fine for code which only needs stuff from other module_utils (like to_text), but for code that requires access to things that cannot be accessed from modules / module_utils, like Display, module_utils is a bad place. Mixing everything in module_utils makes it harder for people to re-use stuff, since for every bit they have to check whether they can really use it, or whether it (indirectly) depends on something from Ansible that they can't.

@ganeshrn thanks for the example! I heard that several collections are doing that, but haven't seen it yet. Putting everything into one common publicly known place is definitely better, just think of what would happen if Ansible suddenly adds a validate plugin type :)

[ganeshrn]

@felixfontein Based on how the loader is implemented for validate sub-plugin https://github.com/ansible-collections/ansible.utils/blob/main/plugins/module_utils/validate/base.py#L43 combined with FQCN I don't see that as an issue :)

[felixfontein]

@ganeshrn ok, I guess I should have said ansible-base, not Ansible. I guess it won't happen in this case, but for 3rd party collections introducing their own directory names, it could easily happen...

[mattclay]

The issue here, at least from a testing perspective, is how test tools know which rules to enforce on which files. When ansible-test runs it enforces different rules for module code (modules, module_utils) and non-module code (all other plugin types). As the requirements for module and non-module code continue to diverge further, trying to use module_utils for both becomes increasingly problematic:

1. Rules for controller-only code can't be enforced on module_utils even when code there may be controller-only.
2. Rules for module-only code are enforced on controller-only code, requiring sanity test ignore entries.

There is also an additional scenario which is a mix of the two (which a separate plugin_utils would not help with): module code which is only supported running on the controller (anything depending on ansible-connection for example). Some module tests still apply (validate-modules) while others do not (compile on python 2.6).

[felixfontein]

There's a statement by the core team on this made in last Tuesday's meeting (https://meetbot.fedoraproject.org/ansible-meeting/2020-11-10/ansible_core_public_irc_meeting.2020-11-10-19.00.html), see 19:53:43 to 19:56:53 in https://meetbot.fedoraproject.org/ansible-meeting/2020-11-10/ansible_core_public_irc_meeting.2020-11-10-19.00.log.html

[felixfontein]

This has essentially been implemented: https://github.com/ansible-collections/overview/blob/main/collection_requirements.rst#modules--plugins

Also ansible-test's import sanity test checks plugin_utils (ansible/ansible#73599).