ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
92 stars 19 forks source link

NWO: All version numbers should be tagged (automatically or manually) #178

Closed felixfontein closed 4 years ago

felixfontein commented 4 years ago

Proposal: All version numbers should be tagged

Author: Felix fontein <@felixfontein>

Date: 2020-05-21

Motivation

In the NWO, both ansible-base (ansible.builtin) and collections have version numbers. If a feature is added, deprecated, or removed, it should be clear in which version of which collection this happen(s/ed).

Example 1: module test in collection foo.bar uses files doc fragment. Option attributes (from files) says version_added: 2.3, while foo.bar has version 1.0.0. Users reading this in the docs are confused.

Example 2: doc fragment in module_utils in foo.bar deprecates an existing option baz, to be removed in version 2.0.0 of foo.bar. The module stuff from foo.fancy uses the doc fragment and module_utils from foo.bar. When a user runs foo.facy.stuff with this option, a deprecation warning This option will be removed in version 2.0.0 will be printed. Since foo.fancy has version 2.5.1, the user is confused.

Problems

Make it clear to users in documentation and deprecation warnings which collection is talked about. The user does not know that option attributes of foo.bar.test comes from ansible.builtin, or that the option in foo.fancy.stuff is deprecated because it comes from the foo.bar collection.

A more detailled writeup: https://github.com/ansible/community/wiki/Version-numbers-for-documentation-deprecation-removal-in-the-collection-world

Solution proposal

Testing (optional)

ansible-test can validate some of these:

  1. In validate-modules, it can ensure that version numbers use the correct format if they appear in the documentation or in the module argument spec. I.e. it can check whether the tag is there, and when the tag is there, make sure that collection versions follow the semantic versioning version number definition, and for Ansible versions that they are parsable by StrictVersion.
  2. A pylint plugin can check all module.deprecate() and display.deprecation() calls, and ensure that they add the correct tags and that the verison number is correct (assuming that the version is passed as a literal in the call).

Documentation (optional)

This requires updating the dev guide, so that people know how to tag version numbers, and it should be announced in ansible/community#346 and ansible-collections/overview#45 so that module/plugin and collection maintainers know about this.

Anything else?

Q: Why not label all version numbers (i.e. also the ones appearing in documentation and doc fragments)? A: For two reasons:

  1. Most version numbers which are used are version_added. Deprecations do not happen very often. So the task that happens most (adding version_added) is not complicated unnecessarily, while the task that is made more complicated doesn't happen very often.
  2. I would prefer all versions to be tagged automatically, but unfortunately for deprecation done by code this is not feasible. So let's automatically tag as much as possible.

Q: Isn't this a lot of work (to tag all version numbers in code)? A: I think it is not that much work:

  1. Deprecation doesn't happen that often.
  2. Currently existing deprecations outside ansible-base have to be adjusted anyway, since these currently in most cases still use Ansible version numbers (due to migration). If this proposal is through before everyone updated these version numbers, it will be almost no extra work.
  3. I'm volunteering to do the work for community.general and community.network. That's probably the largest batch. :)

Q: Isn't this making forking collections (copying them and releasing them under a new name) more complicated? A: Not much: right now you already have to adjust many places where the collection name is imprinted, and usually a global search-and-replace with some manual post-processing (verify all changes) does the job fine. Since deprecations don't happen too often, this should not increase the amount of work by much.

felixfontein commented 4 years ago

I've updated the PR (ansible/ansible#69680) to also require that removed_at_date and deprecated_aliaes.date are tagged (these are also generated by code). These date-based deprecations have been added in ansible/ansible#68177.

felixfontein commented 4 years ago

ansible/ansible#69680 has been merged, so this now works.

AlanCoding commented 4 years ago

Was the intention for the version_added to allow for specifying the collection? For example:

    ask_inventory:
      description:
        - Prompt user for inventory on launch.
      type: bool
      version_added: "ansible.builtin:2.9"

The collection didn't introduce the option, it was carried over from Ansible 2.9. Likewise, if someone did fork a collection, the version numbers would be wrong unless the new collection backdated version or something, which would be meaningless anyway.

Right now this gets the invalid semantic version error, saying it got awx.awx:ansible.builtin:2.9, so it appends the collection name onto the front.

felixfontein commented 4 years ago

No, that's not intended. version_added always refers to the collection it is part of. In this case, version_added should not be there, because the option was there when the module was introduced to the collection.

(During migration all version_added's were removed anyway, unfortunately even the ones with 2.10 as the value, which IMO should be there so the ACD docs will be more helpful.)

felixfontein commented 4 years ago

ansible/ansible#69926 has been merged, which changes the implementation away from tagged version numbes/dates to extra fields for the collection name.

felixfontein commented 4 years ago

@AlanCoding with the current implementation (now in 2.10 beta 1, so it shouldn't change again) you can explicitly specify the collection name. You might need to add ignore.txt entries in some cases, though.

felixfontein commented 4 years ago

Closing this proposal, since a better system has been now implemented.