New collection review: equinix.metal

[detiber]

We've created a new collection (equinix.metal) to replace the existing community.general.packet_* modules to go along with the rebranding of Packet to Equinix Metal.

Collection on Galaxy (version 1.4.0)
Collection Source code: https://github.com/equinix/ansible-collection-metal
Collection issue tracker: https://github.com/equinix/ansible-collection-metal/issues
Collection maintainers: @detiber @displague

[detiber]

The current release of the collection on Galaxy now includes all of the modules that we wish to support going forward (community.general.packet_volume, and community.general.packet_volume_attachment being the notable exceptions, where we are not interested in porting over new versions, but should likely keep the old versions around for existing users).

As part of the migration we should also likely update the documentation here: https://docs.ansible.com/ansible/latest/scenario_guides/guide_packet.html as well.

[displague]

I created equinix/ansible-collection-metal#29 for this guide, though I hope through this discussion we can find a direct approach.

Perhaps unrelated, the old Packet docs and guides are quite out of date and include dead-links to the Packet wiki:
https://docs.ansible.com/ansible/latest/collections/community/general/packet_ip_subnet_module.html

It would be nice to update those links while deprecating the old resources docs.

[gundalow]

This is great,
If you follow the process here https://docs.ansible.com/ansible/devel/dev_guide/developing_collections.html#migrating-ansible-content-to-a-different-collection we can remove/deprecate the modules from community.general and get people using your new collection.

Please shout out here once the PRs are created, or if you need any help.

[felixfontein]

@detiber are the modules in the collection (packet_device, packet_ip_subnet, packet_project, packet_sshkey) backwards compatible to the versions in community.general? (I.e. can we redirect for c.g 3.0.0 assuming that equinix.metal is included, or do we need to deprecate and refer to the modules from equinix.metal as alternatives?)

[Andersson007]

@detiber ^^

[displague]

packet_volume and packet_volume_attachment will not have an Equinix Metal replacement. The other modules are present here, I'm not sure if the interfaces for those specific modules are backwards compatible (@detiber).

[tadeboro]

Just a reminder: collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26 (full schedule). This inclusion request still has some open issues that need to get addressed by then.

[tadeboro]

Last reminder for inclusion submission

Collections that want to be part of the Ansible 4.0.0 package need to get approval by 2021-04-26, which is just one week away (full schedule). Make sure you submit your collection for the final review a few days before the deadline (reviews take a non-trivial amount of time) and that your collection does meet all criteria listed in the requirements document.

[Andersson007]

@detiber @displague thanks for fixing the issues! When the collection is ready for final review, please put a separate comment at the bottom.

[tadeboro]

The equinix.metal Ansible Collection was not accepted into the Ansible community package. The relevant part of the community meeting starts here.

The next opportunity for inclusion will be in about six months for Ansible 5.0.0.

[vielmetti]

Hi @tadeboro - thinking ahead a little, is there a firm schedule published yet for Ansible 5.0.0?

[tadeboro]

The 5.0.0 schedule is published at https://docs.ansible.com/ansible/devel/roadmap/COLLECTIONS_5.html We already started reviewing candidates for 5.0.0. The current status is available at ansible-community/community-topics#32.

[felixfontein]

Ansible Collections Checklist (short version)

For details about the following points, refer to the Collection Requirements.

Every comment should say whether the reviewer expects it to be addressed, or whether it's optional.

Public availability and communication:

• published on Ansible Galaxy with version 1.0.0 or later
• has a policy of releasing, versioning and deprecation announced to contributors and users in some way
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
• have a publicly available issue tracker that does not require a paid level of service to create an account or view issues
• have a public SCM repository, releases of the collection are tagged in this repository

Standards and documentation:

• adheres to semantic versioning
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  SHOULD FIX Descriptions (except short_description) should always contain complete sentences. Descriptions I saw in the collection often omit the period.
  MUST FIX Examples often contain invalid tasks, like (device_ids is wrongly indented):

  - name: Gather information about a particular device using ID
    hosts: localhost
    tasks:
      - equinix.metal.device_info:
        device_ids:
          - 173d7f11-f7b9-433e-ac40-f1571a38037a
  

  (this happens in a lot of _info modules with ids).
• follows development conventions; as well as these other requirements:
  • modules that only gather information are named <something>_info
  • modules that return ansible_facts are named <something>_facts and do not return non-facts
  • other modules must not allow querying information using specific state option values, or similar mechanisms (like state=get or state=query). These features should be moved to <something>_info or <something>_fact modules.
  • check_mode is supported in all _info and _facts modules
• supports Python 2.6 or greater and Python 3.5 or greater. If it does not, read the full guidelines to see if you qualify for an exception and document the unsupported Python versions in the collection README.md and in every module and plugin (or in doc fragments)
  Modules require Pyhton 2.7 or greater, as documented in README.
  MUST FIX The module docuentation does not mention that 2.7 or later is required.
• only uses the allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return sections use FQCNs for the M(..) format macros when referring to modules
• modules (or plugins) from ansible-core use ansible.builtin. as a FQCN prefix
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
• FQCNs are used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-core

Collection management:

• galaxy.yml having tags field set
• collection dependencies must have a lower bound on the version which is at least 1.0.0, and are all part of the ansible package
• meta/runtime.yml defines the minimal version of Ansible which the collection works with
• has changelog, preferably with changelogs/changelog.yaml

Tests:

• passed ansible-test sanity
  SHOULD FIX Running ansible-test sanity --docker -v from current ansible-core 2.12 development version lists some pylint errors:

  ERROR: Found 3 pylint issue(s) which need to be resolved:
  ERROR: plugins/modules/device.py:359:15: use-a-generator: Use a generator instead 'any(a['public'] and a['address_family'] == ip_v and a['address'] for a in addr_list)'
  ERROR: plugins/modules/device.py:363:15: use-a-generator: Use a generator instead 'all(has_public_ip(d.ip_addresses, ip_v) for d in ds)'
  ERROR: plugins/modules/sshkey.py:145:19: use-a-generator: Use a generator instead 'all(select_dict[f] == getattr(k, f) for f in select_dict)'
  

• if test/sanity/ignore*.txt exists, it does not contain error codes listed here
• has CI tests up and running against each of the "major versions" of ansible-base/ansible-core that the collection supports
  MUST FIX Tests are run against Ansible 2.9, ansible-base 2.10 and the latest devel version, but not against ansible-core 2.11 (stable-2.11 branch in ansible/ansible).
• all CI tests run against every pull request
  REQUEST FOR CLARIFICATIONS Integration tests do not seem to run against Pull Requests. Is that intentional?
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
• all CI tests run regularly (nightly, or at least once per week)

[tadeboro]

There are less than two weeks left for collections to get ready for inclusion in Ansible 5 (see roadmap for more details). This candidate still has unresolved issues and is not ready for inclusion right now.

[mariolenz]

This inclusion request looks stale. Additionally, galaxy says it's deprecated. There's also a PR to document deprecation, but for whatever reason it hasn't been merged yet: equinix/ansible-collection-metal#62

I'm not sure what to do with this. Move it to another category like Invalid or Suspended, or just close it?

[felixfontein]

I'm for closing it. Especially considering that the collection is marked as deprecated.

[displague]

The deprecation PR has been merged.

Looking at some of the references above, I'm not sure if additional steps were missed or needed per https://docs.ansible.com/ansible/devel/dev_guide/developing_collections_migrating.html given the current state of the deprecated metal collection and the equinix collection successor.

[displague]

The Equinix collection was renamed and refactored to expand scope to cover more Equinix APIs than Metal. It is now at: - https://github.com/equinix-labs/ansible-collection-equinix - https://galaxy.ansible.com/ui/repo/published/equinix/cloud/

[displague]

Over the past few years the team building this collection has changed shape. It's no longer clear to the current maintainers what the purpose of the discussion.

• Inclusion in Galaxy (which we already have)
• Collection review (what does this unlock? or does it just offer best-practice community review of the published collection?)

[Andersson007]

@displague thanks for the feedback, I'll close the request for now. See https://docs.ansible.com/ansible/latest/dev_guide/developing_collections_path.html , there are some links