hpe.nimble collection inclusion in collection index

[ar-india]

Hi,

We have made our code compliant to Ansible-3.0.0. Below are the links.

Ansible Galaxy: https://galaxy.ansible.com/hpe/nimble
GitHub repo: https://github.com/hpe-storage/nimble-ansible-modules
Issue tracker: https://github.com/hpe-storage/nimble-ansible-modules/issues
Redhat Automation Hub: https://cloud.redhat.com/ansible/automation-hub/hpe/nimble?version=1.0.0
github handle Contacts: @ar-india or @datamattsson or [email protected]

[dmsimard]

Hi @ar-india and thanks for your interest !

The deadline for applying for inclusion in Ansible 3.0.0 was January 27th and we will review your application in time for Ansible 4.0.0. Our current proposed schedule expects Ansible 4.0.0 to be released around May 2021.

In the meantime, I encourage you to get familiar with our requirements for collections ahead of time so you can address potential issues ahead of time.

[felixfontein]

I took a quick look at the collection; it looks like several of the requirements are not satisfied. Also running the sanity tests yields a lot of problems that need attention.

Please try to adjust your collection so it satisfies the requirements. Thanks!

[ar-india]

Hi Felix,

Thanks for the review.
I just downloaded the collection from galaxy and ran the command ansible-test sanity . I don't see any error .
my ansible version is as below and folder is /root/.ansible/collections/ansible_collections/hpe/nimble
[root@centos80x64tmp nimble]# ansible --version ansible 2.10.0rc4
I know that i need to update the runtime.yml file under meta folder and might also need to create a changelog. apart from that is there anything else i am missing..

may i know what exactly is the error for you?

Thanks,
Alok

[felixfontein]

@ar-india have you tried running ansible-test sanity --docker -v to make sure that really all tests are run, and not only the ones corresponding to Python versions you have installed locally?

In any case, I'm running ansible-test sanity --docker -v from ansible-core's devel branch, since it has some more checks than the ansible-base 2.10.x one. I get:

ERROR: The 7 sanity test(s) listed below (out of 44) failed. See error output above for details.
compile --python 2.6
compile --python 2.7
compile --python 3.5
import --python 2.6
import --python 2.7
import --python 3.5
validate-modules

The 2.6, 2.7 and 3.5 tests mainly fail because of syntax errors. Adding the correct ignore.txt lines should fix the problem.

validate-modules fails with:

ERROR: Found 46 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/hpe_nimble_access_control_record.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_array.py:0:0: doc-default-does-not-match-spec: Argument 'allow_lower_limits' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_array.py:0:0: doc-default-does-not-match-spec: Argument 'create_pool' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_array.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_chap_user.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_disk.py:0:0: doc-default-does-not-match-spec: Argument 'force' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_disk.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_encryption.py:0:0: doc-default-does-not-match-spec: Argument 'active' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_encryption.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_fc.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_group.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_info.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_initiator_group.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_network.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_partner.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_performance_policy.py:0:0: doc-default-does-not-match-spec: Argument 'dedupe' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_performance_policy.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_pool.py:0:0: doc-default-does-not-match-spec: Argument 'dedupe_all_volumes' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_pool.py:0:0: doc-default-does-not-match-spec: Argument 'force' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_pool.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_protection_schedule.py:0:0: doc-default-does-not-match-spec: Argument 'disable_appsync' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_protection_schedule.py:0:0: doc-default-does-not-match-spec: Argument 'skip_db_consistency_check' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_protection_schedule.py:0:0: doc-default-does-not-match-spec: Argument 'snap_verify' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_protection_schedule.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_protection_template.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_shelf.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_snapshot.py:0:0: doc-default-does-not-match-spec: Argument 'online' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot.py:0:0: doc-default-does-not-match-spec: Argument 'writable' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'allow_writes' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'disable_appsync' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'invoke_on_upstream_partner' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'is_external_trigger' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'skip_db_consistency_check' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'snap_verify' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: doc-default-does-not-match-spec: Argument 'start_online' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_snapshot_collection.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_user.py:0:0: doc-default-does-not-match-spec: Argument 'disabled' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_user.py:0:0: doc-default-does-not-match-spec: Argument 'unlock' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_user.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_user_policy.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_volume.py:0:0: doc-default-does-not-match-spec: Argument 'cache_pinned' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_volume.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module
ERROR: plugins/modules/hpe_nimble_volume_collection.py:0:0: doc-default-does-not-match-spec: Argument 'is_standalone_volcoll' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_volume_collection.py:0:0: doc-default-does-not-match-spec: Argument 'no_reverse' in argument_spec defines default as (None) but documentation defines default as (False)
ERROR: plugins/modules/hpe_nimble_volume_collection.py:0:0: missing-gplv3-license: GPLv3 license header not found in the first 20 lines of the module

For the GPLv3 license header you also need ignore.txt entries.

[ar-india]

Hi Felix,
Thanks for the error log. We have done all the required changes and now i am not getting any error locally..
[root@centos80x64tmp nimble]# pwd
/root/.ansible/collections/ansible_collections/hpe/nimble

[root@centos80x64tmp nimble]# ansible-test sanity --docker
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.
Running sanity test 'action-plugin-docs' with Python 3.6
Running sanity test 'ansible-doc' with Python 3.6
Running sanity test 'changelog' with Python 3.6
Running sanity test 'compile' with Python 2.6
Running sanity test 'compile' with Python 2.7
Running sanity test 'compile' with Python 3.5
Running sanity test 'compile' with Python 3.6
Running sanity test 'compile' with Python 3.7
Running sanity test 'compile' with Python 3.8
Running sanity test 'compile' with Python 3.9
Running sanity test 'empty-init' with Python 3.6
Running sanity test 'future-import-boilerplate' with Python 3.6
Running sanity test 'ignores'
Running sanity test 'import' with Python 2.6
Running sanity test 'import' with Python 2.7
Running sanity test 'import' with Python 3.5
Running sanity test 'import' with Python 3.6
Running sanity test 'import' with Python 3.7
Running sanity test 'import' with Python 3.8
Running sanity test 'import' with Python 3.9
Running sanity test 'line-endings' with Python 3.6
Running sanity test 'metaclass-boilerplate' with Python 3.6
Running sanity test 'no-assert' with Python 3.6
Running sanity test 'no-basestring' with Python 3.6
Running sanity test 'no-dict-iteritems' with Python 3.6
Running sanity test 'no-dict-iterkeys' with Python 3.6
Running sanity test 'no-dict-itervalues' with Python 3.6
Running sanity test 'no-get-exception' with Python 3.6
Running sanity test 'no-illegal-filenames' with Python 3.6
Running sanity test 'no-main-display' with Python 3.6
Running sanity test 'no-smart-quotes' with Python 3.6
Running sanity test 'no-unicode-literals' with Python 3.6
Running sanity test 'pep8' with Python 3.6
Running sanity test 'pslint'
Running sanity test 'pylint' with Python 3.6
Running sanity test 'replace-urlopen' with Python 3.6
Running sanity test 'rstcheck' with Python 3.6
Running sanity test 'runtime-metadata' with Python 3.6
Running sanity test 'shebang' with Python 3.6
Running sanity test 'shellcheck'
Running sanity test 'symlinks' with Python 3.6
Running sanity test 'use-argspec-type-path' with Python 3.6
Running sanity test 'use-compat-six' with Python 3.6
Running sanity test 'validate-modules' with Python 3.6
Running sanity test 'yamllint' with Python 3.6
Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg.

---

The latest release has been uploaded to galaxy and github..

Ansible Galaxy: https://galaxy.ansible.com/hpe/nimble
GitHub repo: https://github.com/hpe-storage/nimble-ansible-modules
Issue tracker: https://github.com/hpe-storage/nimble-ansible-modules/issues
Redhat Automation Hub: https://cloud.redhat.com/ansible/automation-hub/hpe/nimble?version=1.0.1 .. still waiting for approval
github handle Contacts: @ar-india or @datamattsson or [email protected]

[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 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
  REQUIRED: i didn't find. It can be a pinned issue describing the release policy, e.g. like in Releasing, Versioning and Deprecation community.general#582
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
  REQUIRED: didn't find
• 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
  REQUIRED: clarifications. I see tag 1.0.0 but on galaxy there's release 1.0.1

Standards and documentation:

• adheres to semantic versioning
  REQUIRED: didn't find such info
• follows licensing rules
  REQUIRED CLARIFICATIONS: Apache-2.0 license must be changed to GPL v3
• follows the Ansible documentation standards and the style guide
  REQUIRE:

remove ANSIBLE_METADATA from all the modules
examples don't use FQCN
core modules in examples like debug must be changed to use FQCNs as well, i.e. ansible.builtin.debug
short_description: line 1 ends with a dot
examples: "- name" of #7 does not start with a capital letter
examples: "- name" of #7 ends with a dot

Possible typos in DOCUMENTATION:
stdin : line [29] : addresss > addresses, address, addressed 
stdin : line [392] : arget > target, largest, get
stdin : line [109] : vollections > collections, collection, collection's 
stdin : line [160] : outputed > outputted, output, outputs

• follows development conventions
  REQUIRED: see what Felix Fontein wrote below
• only uses allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections
  REQUIRED: fix them to use FQCNs
• modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix
  REQUIRED: examples use debug module, must be changed to ansible.builtin.debug
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
  REQUIRED: version_added from 2.9 to 1.0.0 everywhere
• FQCNs is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

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
  REQUIRED: version 1.0.1 is missed

Tests:

• passed ansible-test sanity
  REQUIRED: didn't pass
• 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
  REQUIRED didn't find
• all CI tests run against every pull request
  REQUIRED didn't find
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
  REQUIRED didn't find
• all CI tests run regularly (nightly, or at least once per week)
  REQUIRED didn't find

[ar-india]

thanks @Andersson007 for the review.
regarding point : examples don't use FQCN
can you please point out which example is not using FQCN

[Andersson007]

@ar-india sorry for the late reply. now it's ok

[felixfontein]

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

QUESTION: Python compatibility: is Python 3.6 or later a requirement of the SDK used?

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
  • REQUIRED: this is missing
• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC)
  • REQUIRED: this is missing
• 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
  • REQUIRED: currently we only allow GPLv3+ (and optionally BSD-2-clause for module_utils)
• follows the Ansible documentation standards and the style guide
  • REQUIRED: the value for short_description should not end with a period
• follows development conventions
  • REQUIRED: _info modules must support check_mode
  • REQUIRED: do not use no_log=False everywhere, but only when it is an option that's flagged as a false-positive by the sanity test. The problem with using no_log=False everywhere is that when an option is copy'n'pasted for a new one that's sensitive, it can happen quickly that one forgets to change no_log=False to no_log=True. So no_log=False should only be used sparingly!
• only uses allowed plugin types in the plugins/ directory
• has README.md
• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections
  • REQUIRED: examples do not use FQCN
• modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix
  • REQUIRED: debug in _info module's examples should be ansible.builtin.debug.
• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added
  • REQUIRED: all modules claim to be added in 2.9.0, which is NOT a collection version (there are only 1.0.0 and 1.0.1 releases)
• FQCNs is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

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
  • OPTIONAL: There's no human-readable version of the changelog
  • REQUIRED: The changelog does not contain all versions, for example 1.0.1 is missing.

Tests:

• passed ansible-test sanity
  • REQUIRED: please create a ignore-2.9.txt file, since you claim to support Ansible 2.9
  • REQUIRED: please create a ignore-2.11.txt file, after all Ansible 4.0.0 is based on ansible-core 2.11
  • REQUIRED: multiple validate-modules issues are failing with the 2.11 sanity checks
• 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
  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually? You should really make sure that at least sanity tests (for stable-2.9, stable-2.10, devel resp. soon stable-2.11) run automatically.
• all CI tests run against every pull request
  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?
• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released
  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?
• all CI tests run regularly (nightly, or at least once per week)
  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?

[felixfontein]

A general remark on the repository structure: having the nested subdirectories makes this collection more confusing, since GitHub expects certain files in root, while we expect them to be in the collection root. I would recommend to get rid of the extra directories so that collection root == repository root.

Also it might be a good idea to reorganize the "unit" tests so that they can be run with ansible-test integration (not for CI though :) ).

[ar-india]

@felixfontein thanks for the review. below are the inline comments

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

QUESTION: Python compatibility: is Python 3.6 or later a requirement of the SDK used?
**Yes. 3.6 and above is required**

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

  • REQUIRED: this is missing --- addressed

• have a Code of Conduct (CoC) compatible with the Ansible Code of Conduct (CoC) --- addressed

  • REQUIRED: this is missing

• 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

  • REQUIRED: currently we only allow GPLv3+ (and optionally BSD-2-clause for module_utils)

• follows the Ansible documentation standards and the style guide

  • REQUIRED: the value for short_description should not end with a period.. --- addressed

• follows development conventions

  • REQUIRED: _info modules must support check_mode - --- addressed
  • REQUIRED: do not use no_log=False everywhere, but only when it is an option that's flagged as a false-positive by the sanity test. The problem with using no_log=False everywhere is that when an option is copy'n'pasted for a new one that's sensitive, it can happen quickly that one forgets to change no_log=False to no_log=True. So no_log=False should only be used sparingly! --- addressed

• only uses allowed plugin types in the plugins/ directory

• has README.md

• documentation, examples, and return use FQCNs for M(..), examples, and seealso subsections

regarding the above point of documentation and example use FQCN , can you please point out one place where the issue persist?? . have fixed the debug part to use FQCN. is this what you are referring too?

• REQUIRED: examples do not use FQCN

• modules (or plugins) from ansible-base use ansible.builtin. as a FQCN prefix

  • REQUIRED: debug in _info module's examples should be ansible.builtin.debug. --- addressed

• documentation and return sections use version_added: containing the collection version for which an option, module or plugin was added --- addressed

  • REQUIRED: all modules claim to be added in 2.9.0, which is NOT a collection version (there are only 1.0.0 and 1.0.1 releases)

• FQCNs is used for extends_documentation_fragment:, unless you are referring to doc_fragments from ansible-base

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

• OPTIONAL: There's no human-readable version of the changelog -- --- addressed
• REQUIRED: The changelog does not contain all versions, for example 1.0.1 is missing. --- addressed

Tests:

• passed ansible-test sanity

  • REQUIRED: please create a ignore-2.9.txt file, since you claim to support Ansible 2.9 --- --- addressed
  • REQUIRED: please create a ignore-2.11.txt file, after all Ansible 4.0.0 is based on ansible-core 2.11 -- --- addressed
  • REQUIRED: multiple validate-modules issues are failing with the 2.11 sanity checks

• 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

  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually? You should really make sure that at least sanity tests (for stable-2.9, stable-2.10, devel resp. soon stable-2.11) run automatically.
    As of now we run sanity test manually. will work on making sure sanity test runs automatically.

• all CI tests run against every pull request

  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?

YES. we run our Unit test manually. The modules has some internal lab dependency and hence QA team runs the the test manually on each PR and release.

• all CI tests run against a commit that releases the collection; if they don't pass, the collection won't be released

  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?

YES. we run our Unit test manually. The modules has some internal lab dependency and hence QA team runs the the test manually on each PR and release.

• all CI tests run regularly (nightly, or at least once per week)

  • REQUIREMENT: there does not seem to be any CI, 'unit' test playbooks are run manually?

[jamescassell]

when we had ansible/ansible modules, we didn't want the vendor name in the module name, only the product name, so hpe_nimble_fc would have been instad nimble_fc. This is still a good practice, and I'd recommend dropping the hpe_ prefix from the module names.

[felixfontein]

To change this, rename the modules, add symlinks for the old names, and add meta/runtime.yml entries so that Ansible 2.10+ users are informed that the old names are deprecated. They can then be removed in a future major release.

[ar-india]

Thanks @felixfontein @jamescassell

Do we have any existing examples where this has been done. I am little confused about how to add the symlinks. adding information to runtime.yml is fine but where exactly do i need to make the change for symlink.

Thanks,
Alok

[tadeboro]

Hi @ar-india!

If you need an example of a backward-compatible change with a symlink, you can look at openshift/community.okd@037f8b1.

But what you need to do is:

1. Rename the module (for example, git mv hpe_nimble_fc.py nimble_fc.py).
2. Add symlink with the old name linking to the moved file (ln -s nimble_fc.py hpe_nimble_fc.py).

I hope this helps.

That being said, I would say that the renaming of the modules is a nice-to-have thing and not a strict requirement. So I will not block the inclusion into the Ansible 4 package if the modules do not get renamed. Other MUST fix things in reviews do need to be fixed before the collection can be included in Ansible 4 package.

[tadeboro]

Thank you for your submission, @ar-india.

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.

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

---

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)
• 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
  Comment: Most of the modules are missing return value documentation.
• follows development conventions
  Comment: There are a few places where the code duplicates ansible parameter validation (for example, https://github.com/hpe-storage/nimble-ansible-modules/blob/32e82465848c523f5e40143f99f41adb7e48947c/ansible_collection/hpe/nimble/plugins/modules/hpe_nimble_array.py#L415 and https://github.com/hpe-storage/nimble-ansible-modules/blob/32e82465848c523f5e40143f99f41adb7e48947c/ansible_collection/hpe/nimble/plugins/modules/hpe_nimble_protection_schedule.py#L450)
• 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

Tests:

• passed ansible-test sanity
• 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)

[ar-india]

@tadeboro
Thanks for the review.
we have now created a github workflow action https://github.com/hpe-storage/nimble-ansible-modules/actions
and CI is now running for every commit.

regarding changelog not having 1.1.0 ==> i am yet to upload and create the new version on galaxy .. we are running some regression test and once i get a green signal from QA will do the same

[tadeboro]

I noticed that you only run tests on stable 2.10 and 2.11 branches while your metadata states that you also support Ansible 2.9. This needs to be synced up before inclusion.

[ar-india]

ok.. have added stable-2.9 too also now..

[ar-india]

https://github.com/hpe-storage/nimble-ansible-modules/actions/runs/756405478

[tadeboro]

Thank you, @ar-india, for your perseverance. Once the 1.1.0 hits Ansible Galaxy, this collection is ready for inclusion as far as I am concerned. Congratulations!

[felixfontein]

Regarding licenses: we updated the inclusion rules, the Apache 2.0 license is now allowed for modules and module_utils. However, it is curently not allowed for anything else, neither for doc_fragments nor for non-code files. @ar-india @datamattsson would it be possible to adjust the licenses for the collection accordingly? If not, we will have to wait for a decision from RedHat legal, which could mean that this collection misses the deadline for Ansible 4.

[datamattsson]

Thanks for the guidance. We've raised a PR to change the repo to GPL-3.0. The modules will remain Apache-2.0.

• Mutate repo licenses hpe-storage/nimble-ansible-modules#39

Please let us know any other immediate blockers.

[felixfontein]

I think once hpe-storage/nimble-ansible-modules#40 is merged, most issues are fixed. It's easier to verify that once that PR is merged though :)

Some things that are missing though:

1. While the CHANGELOG.rst file in that PR covers versions 1.0.1 and 1.0.2, the changelos/changelog.yaml does not.
2. I cannot find tags for 1.0.1 and 1.0.2.
3. I cannot find information on releasing and versioning (hpe.nimble collection inclusion in collection index #13 (reply in thread) claims it is addressed, by I can't find where).

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

[ar-india]

@tadeboro @felixfontein and others.

Thanks everyone for reviewing our ansible modules and providing your review comments. All the comments have been addressed in our new release v1.1.0. and is up and ready for review.

here are the details

Ansible Galaxy: https://galaxy.ansible.com/hpe/nimble
GitHub repo: https://github.com/hpe-storage/nimble-ansible-modules
Issue tracker: https://github.com/hpe-storage/nimble-ansible-modules/issues
github handle Contacts: @ar-india or @datamattsson or [email protected]

[felixfontein]

I've taken another look, it looks good now! There's one tiny detail I noticed: since the doc fragment contains check_mode is not supported. while the _info module's notes contain This module supports check_mode, the generated docs' note sections for that module now says:

NOTES:
      * This module supports `check_mode'.
      * check_mode is not supported.

This is a bit confusing :)

I would recommend to move the notes: from the docs fragment into every module (except the _info module). (This also allows you to start implementing check_mode for a non-_info module at any point, since then only the notes: in that module need to be adjusted.)

[ar-india]

addressed in new version . pls check

[felixfontein]

Looks good 👍

[Andersson007]

@tadeboro @felixfontein I have no idea for what ./changelogs/.plugin-cache.yaml is used but it contains version_added: 2.9.0. Is it OK?

[felixfontein]

That file can actually be deleted. It is a small cache created by antsibull-changelog so that if you run it multiple times in a row, it doesn't has to collect infos on all modules and plugins every time. (That isn't really necessary for smaller collections as that process is reasonable fast, but for collecting information on all modules in ansible/ansible it used to be very important ;) )

It looks like when updating the changelog, a newer version of this file was not checked in. @ar-india @datamattsson I would recommend to simply delete it (and add it to .gitignore), since that information will only get stale if you forget to check newer versions in and is not used by anything.

[ar-india]

@felixfontein @Andersson007
thanks for the comments.
have taken care of both the comments and have uploaded the new version 1.1.2. please take a look

[ar-india]

please let us know if there is any additional work pending for getting this collection included in coll index. I believe ones it is approved it will be picked up automatically right?

[felixfontein]

This collection has been prepared for inclusion in Ansible 4 in this PR: ansible-community/ansible-build-data#66

[tadeboro]

The hpe.nimble Ansible Collection was accepted into the Ansible community package. The relevant part of the community meeting starts here.

Congratulations!