Alternate module metadata #30

Closed
abadger opened this Issue Aug 12, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@abadger
Member

abadger commented Aug 12, 2016

Proposal: module metadata

Author:Toshio Kuratomi
Date: 2016/08/05

  • Status: Final
  • Proposal type: module management
  • Targeted Release: 2.2/2.3 (Add to the devel branch once stable-2.2 branches)
  • Estimated time to implement: weeks

YAML format and simplified versioning from #28 has been merged into this. This is necessary for implementing #29, Proposal for Managing modules.

Motivation

Currently there are four pieces of metadata that is easily accessible outside of the module:

  • The module's name (contained as the filename minus file extension)
  • The module's category (contained in the directory that the file lives in. Only used for documentation)
  • Whether the module is deprecated (also contained in the module's filename)
  • What interpreter a module is run via (the "shebang" line at the top of the module. No shebang leads to it being treated as a binary module).

As time has progressed we find that this limited amount of metadata is insufficient information for some of the features we want to be able to support for our modules. The file and directory names don't scale to more pieces of metadata and the shebang line format isn't meant for other types of metadata.

Problems

Runtime deficiencies

  • What is "supported"?
    • There's no clear indication to users of whether a module is well supported or not.
    • There's no clear indication to customers of what modules they should expect Ansible is devoting resources to.
    • There's no clear indication to contributors of which modules are in need of a new maintainer.
  • The argument spec on the modules currently contains information about parameters that could be of use to the controller code.
  • We have no way of knowing whether a module can run on a remote platform other than to attempt to run it. The user must read the module's documentation to know if the module requires anything specific on the remote machine.

Developer deficiencies

  • The original core/extras module split was supposed to allow contributors greater access to commit in extras because the core team is currently a bottleneck to reviewing and merging all contributions. There's resistance to opening this up further as not everyone needs to have or should have commit to all modules. We need per-module commit to open this up.
    • With per module commit we can also get rid of the separate repositories for core/extras. Instead, use a single repository for all modules. This solves a number of problems with multiple repos including:
      • tests, action plugins, and module_utils code in a different repo so you need two PRs.
      • syncing of tests between two repos (a PR that updates a module may also need a PR to update tests. There's no way to tell our CI testing that both PRs have to be applied when testing).

Release Engineering deficiencies

  • We're currently shipping all modules with Ansible inside of a single tarball. In the future we want to ship separate tarballs. This requires knowing more about the supported state of a module to know where it belongs.

Solution proposal

The initial metadata proposal is intended to fix the support problems listed above but also be flexible enough that it can be expanded to fix the other problems as well. The 1.0 format does not include argument_spec or platform metadata to solve those two issues but the other mentioned problems are addressed.

Add a ANSIBLE_METADATA global variable to the modules (like current DOCUMENTATION, EXAMPLES, etc). This variable may contain either a python dict or a YAML string:

# Data formatted as a python dict:

ANSIBLE_METADATA = {
    'version': '1.0',
    'supported_by': 'core|community|unmaintained|committer',
    'status': ['stableinterface|preview|deprecated|removed']
}

# Data formatted as a yaml string:
ANSIBLE_METADATA = r"""
    version: "1.0"
    supported_by: "core|community|unmaintained|committer"
    status: ["stableinterface|preview|deprecated|removed"]
"""

Fields in Version 1.0 of the metadata

  • version: An "X.Y" formatted string. X and Y are integers which define the metadata format version. Modules shipped with Ansible are tied to an Ansible release so we will only ship with a single version of the metadata. We'll increment Y if we add fields or legal values to an existing field. We'll increment X if we remove fields or values or change the type or meaning of a field.
  • supported_by: This field records who supports the module. Default value is community. Values are:
    • core: maintained by the ansible core team. Core team will fix bugs, add new features, and review PRs.
    • community: This module is maintained by the community at large. Community is responsible for fixing bugs, adding new features, and reviewing changes.
    • unmaintained: This module currently needs a new community contributor to help maintain it.
    • committer: Committers to the ansible repository are the gatekeepers for this module. They will review PRs from the community before merging but might not generate fixes and code for new features on their own.
  • status: This field records information about the module that is important to the end user. It's a list of strings. The default value is a single element list ["preview"]. The following strings are valid statuses and have the following meanings:
    • stableinterface: This means that the module's parameters are stable. Every effort will be made not to remove parameters or change their meaning. It is not a rating of the module's code quality.
    • preview: This module is a tech preview. This means it may be unstable, the parameters may change, or it may require libraries or web services that are themselves subject to incompatible changes.
    • deprecated: This module is deprecated and will no longer be available in a future release.
    • removed: This module is not present in the release. A stub is kept so that documentation can be built. The documentation helps users port from the removed module to new modules.

Additional

Implementation thoughts

Sample code to parse the metadata:

MODULE = '/srv/ansible/devel/lib/ansible/modules/core/files/copy.py'

import ast
def read_metadata(module):
    data = u''
    mod_ast_tree = ast.parse(open(module).read())
    for child in mod_ast_tree.body:
        if isinstance(child, ast.Assign):
            for target in child.targets:
                if target.id == 'ANSIBLE_METADATA':
                    if isinstance(child.value, ast.Dict):
                        data = ast.literal_eval(child.value)
                        break
    return data
  • Need to add metadata to modules. Having defaults allows us to work with modules that do not have any metadata

Future

Potential future support fields:

  • team: Supported by an organized team that's not Ansible. This needs a use case to determine what value it adds.

Potential future status fields:

None currently planned.

Potential future metadata fields

  • language: specify programming language and version needed to run the module.
  • OS: specify that the program only runs on certain OS's/versions.
  • argument_spec: argument_spec being available to controller code would make no_log for parameters much more robust. Knowing what type a module expects a parameter to be could potentially help us parse values from the playbook better. Combining argument_spec with the documentation of parameters would help us to stop repeating the information in two places.
  • vendor: Extended information about the maintainer of a module when the maintainer is a company
  • X-*: namespace for custom metadata.

Of these, I think only language is close to being ready. The others need more work to provide useful information.

Alternative Implementations

#28 is the main alternative to this implementation. Differences between these are:

  • YAML rather than python dict. A YAML format has been added to this Proposal.
  • Field names: supported_by is equivalent to support. No one really cares about this difference.
  • Supported_by names: contrib is equivalent to core_curated. It felt like core_curated was more descriptive
  • Status names: stable is equivalent to stableinterface. Initial drafts of both proposals used "stable" and everyone agreed that it was misleading. The final draft settles on stableinterface as being the least misleading term.
  • version as simple integer vs X.Y: X.Y is slightly more work to parse for comparisons but it gives us the ability to specify backwards compatible and backwards incompatible changes.
  • Omission of language: It was decided that opening up modules to additional contribution is the main need at this time. Since language isn't quite ready yet, it is best to wait on this.
  • Omission of OS: Same rationale about module contribution applies here. In addition, we don't have a use case for this yet. Better to wait for the use case and then enhance the specification to meet that need.
@abadger

This comment has been minimized.

Show comment
Hide comment
@abadger

abadger Aug 23, 2016

Member

Decisions from today's meeting:

  • Allow either python dict or yaml (bcoca's) format. After ast.parse we'll see if we have a dict or a string. If string, we'll run it through yaml.
  • Version, go with the simpler single version per metadata as per bcoca's Proposal. People writing custom modules will just have to write metadata for the lowest version of ansible that they support and hope that our defaults are okay for everything else.
  • We'll leave off language and OS (from bcoca's proposal) for now. Language will likely happen sometime within 2.3 as we start running into modules that do not work on python3. OS unsure but we'll work on that once we identify the need that it addresses.
  • status field, Must rename stable. For now we're going to rename it to stableinterface. If anyone can bikeshed us a better name that's great. We just need something that avoids the implication of code quality that stable might bring while expressing that the parameters are unlikely to change.
  • Everyone was happy with the proposed defaults of "community" and "preview". We'll use the list of modules generated at the Ansible All-Hands meeting for the initial list of things which do not follow the defaults. (This is a [smaller than ansible-modules-core] list of modules which Ansible has claimed for being gatekeepers for)

@jimi-c or I will write this up as soon as we have time away from working on our 2.2 release features.

Member

abadger commented Aug 23, 2016

Decisions from today's meeting:

  • Allow either python dict or yaml (bcoca's) format. After ast.parse we'll see if we have a dict or a string. If string, we'll run it through yaml.
  • Version, go with the simpler single version per metadata as per bcoca's Proposal. People writing custom modules will just have to write metadata for the lowest version of ansible that they support and hope that our defaults are okay for everything else.
  • We'll leave off language and OS (from bcoca's proposal) for now. Language will likely happen sometime within 2.3 as we start running into modules that do not work on python3. OS unsure but we'll work on that once we identify the need that it addresses.
  • status field, Must rename stable. For now we're going to rename it to stableinterface. If anyone can bikeshed us a better name that's great. We just need something that avoids the implication of code quality that stable might bring while expressing that the parameters are unlikely to change.
  • Everyone was happy with the proposed defaults of "community" and "preview". We'll use the list of modules generated at the Ansible All-Hands meeting for the initial list of things which do not follow the defaults. (This is a [smaller than ansible-modules-core] list of modules which Ansible has claimed for being gatekeepers for)

@jimi-c or I will write this up as soon as we have time away from working on our 2.2 release features.

@abadger

This comment has been minimized.

Show comment
Hide comment
@abadger

abadger Oct 5, 2016

Member

I've written up the changes from the meeting.

Member

abadger commented Oct 5, 2016

I've written up the changes from the meeting.

@abadger abadger referenced this issue Nov 9, 2016

Closed

module metadata #28

@wenottingham wenottingham referenced this issue in ansible/ansible-modules-extras Nov 11, 2016

Closed

Create ansible_tower_organization module #3413

@thaumos

This comment has been minimized.

Show comment
Hide comment
@thaumos

thaumos Dec 7, 2016

Shall we close this since we've implemented it?

thaumos commented Dec 7, 2016

Shall we close this since we've implemented it?

@thaumos

This comment has been minimized.

Show comment
Hide comment
@thaumos

thaumos Dec 7, 2016

closing since implemented, (spoke to @abadger in person ;))

thaumos commented Dec 7, 2016

closing since implemented, (spoke to @abadger in person ;))

@thaumos thaumos closed this Dec 7, 2016

@privateip privateip referenced this issue in ansible/ansible Jan 4, 2017

Merged

Adding ldap_attr module #19286

@abadger abadger added the Agreed label Jan 19, 2017

@gundalow gundalow referenced this issue in ansible/ansible Feb 10, 2017

Merged

Document ANSIBLE_METADATA #21250

@gundalow gundalow added the Completed label Feb 21, 2017

@bmildren bmildren referenced this issue in gaf3/doboto_ansible Mar 7, 2017

Merged

Working with certs 0.5.0 #19

@abadger

This comment has been minimized.

Show comment
Hide comment
@abadger

abadger Mar 14, 2017

Member

note: this has been updated in #54

Member

abadger commented Mar 14, 2017

note: this has been updated in #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment