Nautobot Modules

[jvanderaa]

Looking to add a collection to the Ansible Community installation.

The Nautobot collection is used to create, update, and delete Nautobot objects. Nautobot is an Open Source source of truth and network automation platform. These modules are used at the core of workflows to manage objects within the Nautobot project. These modules were forked from the netbox.netbox collection earlier in 2021 and have additional modules added such as a GraphQL action plugin + lookup plugin.

Ansible Galaxy: https://galaxy.ansible.com/networktocode/nautobot
GitHub: https://github.com/nautobot/nautobot-ansible/
Issues Tracker: https://github.com/nautobot/nautobot-ansible/issues
GitHub Handles: @networktocode
Is the collection part of Automation Hub: No
We meet Collection Requirements: Yes

[tadeboro]

Thank you for your submission, @jvanderaa.

The items in the checklist can be in one of the following states:

• Items with a checkmark and no attached comment are OK and need no intervention from your side.
• Items with a checkmark that contain a comment will not block the inclusion process, but by following the advice given in the comment, the collection's quality can be improved.
• Items with no checkmark and no extra details are waiting for evaluation.
• Items with no checkmark that contain a comment need your attention.

Feel free to ask any questions and provide feedback. Thanks in advance for your understanding and patience.

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 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)
  COMMENT: The CoC passed the approval. The full meeting log contains some information on what could be improved, but there is nothing in the CoC that would block the inclusion.
• 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
• 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)
  COMMENT: Modules only support Python >= 3.6 because the underlying Python client supports those Python versions.
• 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: The ansible-test sanity --docker command is still not clean because the ignore files are missing.
• 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
  COMMENT: The CI setup is a bit disorganized, but sanity tests do indeed run on all supported Ansible versions as part of the integration test job.
• all CI tests run against every pull request
• 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)

[jvanderaa]

Getting the latest beta release of ansible-core 2.12 into the pipeline now. I had two updates to make on the sanity for it. I'm just waiting for one approval, which may not be in when you get started. I'll make one more comment at that time.

[jvanderaa]

The PR is now merged in that has a test for ansible-core 2.12 Beta 2 included.

[tadeboro]

I did my final review and as far as I am concerned, the collection is ready for inclusion in Ansible 5.

[jvanderaa]

Thank you @tadeboro for all of the guidance along the way!

[dmsimard]

I did not finish my review since there seem to be some major issues with the licensing in this collection. For example, commit 301e66c8 stripped copyright and licensing information from modules (nautobot/nautobot-ansible@301e66c#diff-1aaebd78d9857e913ef7e74ca7a6448ee8bf3a04a583666f7882e56b378e98a4 is one such module modification). As far as I can tell, this is not legal since it violates the original copyright and license because the new repository seems to be under Apache 2.0 now (again, I am not sure if changing the license here was a valid move).

For posterity, I want to point out that the original licensing and copyright attributions have been restored in nautobot/nautobot-ansible@c34c43a.

[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.

[jvanderaa]

Thanks @tadeboro, with the latest merge of a couple hours ago I believe we have covered all of the items requested.

On the 2.x front, we have released a new release in the 2.x release with the appropriate deprecations. There is also a develop2 branch for any future updates to that code.

On the Ansible testing front, PR #96 added in the Ansible 2.9 setup as mentioned.

I appreciate the help through this, and including this reminder.

[jvanderaa]

@tadeboro were we good on the inclusion?

[dmsimard]

@jvanderaa as discussed in #ansible-network, for posterity: I'm sorry we didn't get back to you. We discussed nautobot's inclusion in a community meeting but unfortunately we didn't manage to get more eyes on the review in time before the deadline for Ansible 5.

The collection can still be included in Ansible 6 if there is interest and gets at least an additional review.

In the meantime the collection can still be used alongside Ansible 5 but you would need to install it from Galaxy.

[jvanderaa]

Let's plan for that inclusion to Ansible 6, if we can't get something into Ansible 5.1 or other dot release.

[jvanderaa]

@dmsimard is this all set for Ansible 6 or is there more that is needed yet?

[jvanderaa]

@dmsimard @tadeboro are we all set to go for Ansible 6?

[jvanderaa]

@gundalow will we be good for the next community version of Ansible to have the modules included?

[Andersson007]

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: Complience with Ansible doc guidelines nautobot/nautobot-ansible#119
• 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)
• 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
• collection repository should not contain any large objects (binaries) comparatively to the current Galaxy tarball size limit of 20 MB. For example, package installers for testing purposes shouldn't be added.
• collection repository should not contain any unnecessary files like, for example, temporary files created by development tools. Temporary files MUST be added to .gitignore.

Tests:

• passed ansible-test sanity
  MUST FIX: please clarify the question Nautobot Modules #28 (comment)
• 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
• all CI tests run against every pull request
• 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)

[Andersson007]

@jvanderaa thanks for submitting the collection! If you fix the sanity, i don't see any other obstacles for inclusion of the collection

[jvanderaa]

We will check in on the sanity tests. I know we had some particular setup around getting the docs published appropriately to RTD.

[Andersson007]

when running sanity tests locally they stumbled upon the following:

Running sanity test "shebang"
ERROR: Found 61 shebang issue(s) which need to be resolved:
ERROR: docs/plugins/aggregate_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/cable_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/circuit_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/circuit_termination_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/circuit_type_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/cluster_group_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/cluster_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/cluster_type_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/console_port_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/console_port_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/console_server_port_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/console_server_port_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_bay_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_bay_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_interface_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_interface_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_role_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/device_type_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/front_port_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/front_port_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/gql_inventory_inventory.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/index.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/inventory_inventory.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/inventory_item_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/ip_address_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/ipam_role_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/lookup_graphql_lookup.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/lookup_lookup.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/manufacturer_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/nautobot_server_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/platform_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_feed_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_outlet_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_outlet_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_panel_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_port_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/power_port_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/prefix_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/provider_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/query_graphql_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rack_group_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rack_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rack_role_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rear_port_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rear_port_template_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/region_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/rir_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/route_target_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/service_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/site_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/status_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/tag_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/tenant_group_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/tenant_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/virtual_chassis_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/virtual_machine_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/vlan_group_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/vlan_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/vm_interface_module.rst:0:0: file without shebang should not be executable
ERROR: docs/plugins/vrf_module.rst:0:0: file without shebang should not be executable

Is it needed that rst files are executable?

[FragmentedPacket]

3.3.1 has been released to address this: https://galaxy.ansible.com/networktocode/nautobot

Is this the last remaining item for inclusion?

[Andersson007]

@FragmentedPacket cool, i'll update my review and will create a topic for inclusion, thanks!

[felixfontein]

While glancing over this collection, I found a couple of issues:

• follows the Ansible documentation standards and the style guide
  SHOULD FIX: I found multiple instances of:

      query_params:
        description:
          - This can be used to override the specified values in ALLOWED_QUERY_PARAMS that is defined
          - in plugins/module_utils/utils.py and provides control to users on what may make
          - an object unique in their environment.

Here a single sentence is spread over three paragraphs (!). This should be a single paragraph.

• has changelog, preferably with changelogs/changelog.yaml
  SHOULD FIX: three instances of namespace: networktocode.nautobot in changelogs/changelog.yaml should be changed to namespace: ''
• passed ansible-test sanity
  MUST FIX: ansible-test sanity --docker does not pass with ansible-core 2.12. I suspect it also does not pass with ansible-core 2.11, and it won't pass with ansible-core 2.13 / devel branch either.

[Andersson007]

@jvanderaa i've run sanity tests against devel, stable-2.13, stable-2.12, stable-2.11 and they've failed in all of the cases.
Could you please fix the collection to make it passing the tests?

[FragmentedPacket]

Is there a way for it to only run on the plugins/ folder?

[kristianheljas]

Although the docs do not state it explicitly, you can specify a folder as well: ansible-test sanity plugins/

I will create a PR to the docs.

[Andersson007]

as @felixfontein spotted

it has to pass for the whole collection (as published on galaxy), otherwise we cannot automate sanity tests

[felixfontein]

And another one: by restricting it to plugins/, you are missing validation of changelog fragments (if you use them), of meta/runtime.yml, and potentially other things.