Include ibm.spectrum_virtualize collection

[Shilpi-J]

Intent is to include ibm.spectrum_virtualize collections in Ansible community release. Please validate and provide your feedback for the same.
Ansible Galaxy: https://galaxy.ansible.com/ibm/spectrum_virtualize
GitHub: https://github.com/ansible-collections/ibm.spectrum_virtualize
Issues Tracker: https://github.com/ansible-collections/ibm.spectrum_virtualize/issues
GitHub Handles:
Is the collection part of Automation Hub: Yes
We meet Collection Requirements: Yes

[gundalow]

https://github.com/ansible-collections/ibm.spectrum_virtualize/blob/develop/meta/runtime.yml#L2 says the collection supports Ansible 2.9 (and higher), though CI testing doesn't include 2.9 https://github.com/ansible-collections/ibm.spectrum_virtualize/blob/develop/.github/workflows/main.yml#L15-L18

[Shilpi-J]

Thanks John, we are working on this.

[Shilpi-J]

We have included sanity checks against 2.9 as well in CI

[Shilpi-J]

Please validate and share your feedback.

[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)
  RECOMMENDED: put the link in README
• 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
  MUST FIX: Prepare the collection for inclusion in Ansible ibm.spectrum_virtualize#52
• 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
  COMMENT: See Include ibm.spectrum_virtualize collection #35 (comment)
• 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)
• according to the requirements, collections to be included in the package must have unit or integration tests
  MUST FIX: Add unit or integration tests

[Andersson007]

@Shilpi-J i've finished the first round of review. See the above and ansible-collections/ibm.spectrum_virtualize#52.
For me, the major issue is the last must fix, i.e. absence of unit or integration tests.

[Shilpi-J]

@Andersson007 The collection does have unit tests.https://github.com/ansible-collections/ibm.spectrum_virtualize/tree/develop/tests/unit/plugins/modules
It would be great if you can let me know what else is expected here?

[gundalow]

Appears that Unit tests aren't currently been run, please copy https://github.com/ansible-collections/collection_template/blob/main/.github/workflows/ansible-test.yml#L81-L134 into your GitHub Action.

[Andersson007]

@Shilpi-J I wasn't clear, sorry, as John spotted they should be included in the GHA workflow

[Shilpi-J]

Thanks @Andersson007
I got the point, will add them in the workflow.

[Andersson007]

@Shilpi-J what do you think of simplifying the examples, i.e. instead of

 92 - name: Using Spectrum Virtualize collection to create an empty host cluster
 93   hosts: localhost
 94   collections:
 95     - ibm.spectrum_virtualize
 96   gather_facts: no
 97   connection: local
 98   tasks:
 99     - name: Define a new host cluster
100       ibm_svc_hostcluster:
101         clustername: "{{clustername}}"
102         domain: "{{domain}}"
103         username: "{{username}}"
104         password: "{{password}}"
105         log_path: /tmp/playbook.debug
106         name: hostcluster0
107         state: present
108         ownershipgroup: group1

something like

 99 - name: Define a new host cluster
100   ibm.spectrum_virtualize.ibm_svc_hostcluster:
101     clustername: "{{clustername}}"
102     domain: "{{domain}}"
103     username: "{{username}}"
104     password: "{{password}}"
105     log_path: /tmp/playbook.debug
106     name: hostcluster0
107     state: present
108     ownershipgroup: group1

[Shilpi-J]

@Andersson007 Yes we can do that..

[Shilpi-J]

@Andersson007 We have incorporated your suggestions in our collection. The new release is out with all these changes.
Please validate and share your feedback.
Thanks

[Andersson007]

@Shilpi-J thanks! I'll try to take a look as soon as possible.

[Andersson007]

@Shilpi-J the collection LGTM now, thanks for fixing things, I'll create a corresponding community topic and will ask steering committee folks to conduct the second necessary review. Thanks!

[Andersson007]

ansible-community/community-topics#85

[Shilpi-J]

Thanks @Andersson007

[cybette]

Review by @cybette [done]

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
• 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
• public plugins, roles and playbooks do not use files outside of meta/, plugins/, roles/, and playbooks/

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
• 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)

[Shilpi-J]

@cybette @Andersson007 Is the review complete? Do you see anything which is pending from our side? Our collection new release 1.9.0 is out. Let me know if our collection is ready for the inclusion.
Thanks

[cybette]

Hi @Shilpi-J , apologies for the delay as I have been held up by PTO and some personal obligations. I will have the review completed within the next week. Thanks for your patience and understanding!

[Shilpi-J]

Thanks @cybette , waiting for your feedback.

[cybette]

LGTM! Once again sorry for taking so long and thanks for the great cooperation and contribution!

[Shilpi-J]

Thanks Carol.
@Andersson007 @cybette Since both the reviews are complete, what would be the next step? How soon can we expect our collection to be included?

[Andersson007]

@Shilpi-J I would expect that it'll happen within ~2 weeks since now

[Shilpi-J]

Thanks @Andersson007

[gundalow]

Approved, thanks all

[Shilpi-J]

Thanks @gundalow

[Andersson007]

@Shilpi-J ansible-community/ansible-build-data#148 has been merged.
The collection will appear in the package in the next 6.X minor release (which happens every 3 weeks).
Thanks everyone!

[Shilpi-J]

This is great news ! Thanks @Andersson007