New Collection: grafana.grafana

[ishanjainn]

An Ansible collection to configure and manage resources on Grafana Cloud. This differs from the community.grafana collections in that it is can be used for managing resources on Grafana Cloud only and not on the self-hosted versions of Grafana.

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

[Andersson007]

hello @ishanjainn thanks for submitting the collection!
I'll publish this request in the next Bullhorn. The collection will get reviewed as soon as there are volunteers.

[ishanjainn]

Hi @Andersson007, Any update for this? Thanks in advance

[Andersson007]

@ishanjainn hi, there are no volunteers so far, I publish a list of all the collections waiting for reviews every week. If someone appears, they'll copy the collection checklists to this discussion and will start the review. You'll see it.

[mbialon]

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.

Note for reviewers: If you don't know how to check any of the points below, please ask maintainers of the collection you're reviewing or a Steering Committee member for clarifications in comments of corresponding inclusion discussion.

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
  MUST FIX: The repository is missing tags for collection releases.

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: Shouldn't sanity tests be run against Python 2.6 or greater?
• 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:

Note for reviewers: If you don't know how to check the points below, please ask maintainers of the collection you're reviewing how you can do it.

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

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

[ishanjainn]

Thanks @mbialon for the review. I have added tag 1.0.2 to the repository and will continue to add tags going forward.

Regarding Python 2.6, The sanity tests for Ansible 2.9 does cover Python 2.6 as you can see here

[mbialon]

@ishanjainn Thank you for pointing out the place to verify Python 2.6 usage. This concludes my review and I think the collection looks great.

[Andersson007]

@mbialon thanks for the contribution!

@ishanjainn one community review has been done, so at least one @ansible-collections/steering-committee review will still be required. I hope someone will appear soon. Thanks

[ishanjainn]

Hey @Andersson007, I know this has been mentioned in Bullhorn but yet to get a review, Is there any other option to get this more visibility for a review?

[Andersson007]

@ishanjainn hey, we've leveraged only Bullhorn to announce inclusion requests so far.
I can post it on Matrix also, but it's a text chat, so I'm not sure it will be effective. If you have other ideas, please let me know.
If there are no reviewers, I'll try to find time to conduct the first round of review myself next week but anyway the second review will be needed.
Thanks for pinging

[ishanjainn]

Got it, Thanks!

[dmsimard]

o/

Just for the sake of context and broader picture, there is a discussion here about the potential of merging or collaboration between community.grafana and grafana.grafana: ansible-collections/community.grafana#278

[Andersson007]

@dmsimard thanks for letting us know!

@ishanjainn so the plan is, roughly, to move content from community.grafana to grafana.grafana and then to deprecate the former, right?

[ishanjainn]

Hey @Andersson007 , Yes thats correct

[Andersson007]

hey @ishanjainn , ok, thanks for the feedback, then my comment about reviewing grafana.grafana myself this week is relevant

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

Note for reviewers: If you don't know how to check any of the points below, please ask maintainers of the collection you're reviewing or a Steering Committee member for clarifications in comments of corresponding inclusion discussion.

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
  MUST FIX: There's a mismatch between tags (there's only one in the repo) and releases on Galaxy.
  COMMENT: How the process works? Would you like to enable autopublishing by Zuul (when Zuul sees a new tag, it'll upload in on Galaxy)? If yes, please take a look at https://github.com/ansible/zuul-config/#enable-the-github-application

Standards and documentation:

• adheres to semantic versioning
• follows licensing rules
• follows the Ansible documentation standards and the style guide
  RETURN block:
  MUST FIX: Descriptions don't end with a period in all the modules.
  SHOULD FIX: Would be nice to have samples (if they are not huge)
  DOCUMENTATION block:
  MUST FIX: Add notes: section with - Supports C(check_mode). or - Does not support C(check_mode). to every module. If all the modules support check mode, you can create and load a doc fragment. IMPORTANT: before fixing this, please take a look at the next bullet point.
  MUST FIX: Use proper markup for arg options (recheck in all the modules), examples
  a. "- Continue matching subsequent sibling nodes if set to True." should be changed to C(true).
  b. "For example, "<stack_slug>.grafana.net"." -> ".. C(<stack_slug>.grafana.net)"
  c. "Will be set to https://<stack_slug>.grafana.net." -> ".. C(https://<stack_slug>.grafana.net)."
  d. Use true/false instead of True/False for consistency with other docs, see the community decision
• follows development conventions; as well as these other requirements:
  MUST FIX: I see supports_check_mode=True but i don't see it actually works. Maybe I missed something? If the modules just ignore it, this statement can mislead users.
  If the true check mode is impossible to implement, this should be made safe for users and mentioned in the documentation.
  SHOULD FIX: HAS_REQUEST is not used. You can use something like:

  if not HAS_REQUESTS:
      module.fail_json(msg=missing_required_lib('requests'))
  

  • 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
  COMMENT: i see changelog.yml and CHANGELOG.rst look different, do you follow https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_release_without_branches.html when releasing?
• 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. Temporary files should be added to .gitignore.

Tests:

Note for reviewers: If you don't know how to check the points below, please ask maintainers of the collection you're reviewing how you can do it.

• In most cases, CI is set up via GitHub Actions.
• Check .yml files in the .github/workflows directory. There must be at least the sanity section under jobs containing the ansible-test sanity command running against all supported ansible-core versions that must be also listed there, for example, - stable-2.11, - stable-2.12, - stable-2.13.
• Check workflow runs by clicking the Actions tab in the repository's page - you're interested in Scheduled runs, runs against release commits and runs against pull requests.
• If there are no workflows in the Actions tab, ask the collection maintainers how CI is implemented.

• passed ansible-test sanity
• if test/sanity/ignore*.txt exists, it MUST 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: you can remove stable-2.9, -2.10 from the test matrices as they are EOL (2.11 will get EOL soon too)
• all CI tests MUST run against every pull request
• all CI tests MUST run regularly (nightly, or at least once per week)
• sanity tests MUST run against a commit that releases the collection; if they don't pass, the collection won't be released
  COMMENT: How the release process work? I don't see any related runs.

[Andersson007]

@ishanjainn I've finished the first round of my review. The collection generally looks good.
I've found a couple of things to fix/clarify.
Please take a look at the MUST FIXes, SHOULD FIXes and COMMETs above and ping me again when you fix it/if you have any questions.
Thanks

[ishanjainn]

Thanks @Andersson007 for the Review. These changes have been incorporated in Version 1.0.3

MUST FIX: There's a mismatch between tags (there's only one in the repo) and releases on Galaxy.

I have added tags to the specific versions that were missing, Thanks for noting this

COMMENT: How the process works? Would you like to enable autopublishing by Zuul (when Zuul sees a new tag, it'll upload in on Galaxy)? If yes, please take a look at https://github.com/ansible/zuul-config/#enable-the-github-application

Have added a release workflow now which will create the Release on Github. Uploading to Galaxy keeping it manual for now. Let me known if that process needs to be in the README.

follows the Ansible documentation standards and the style guide

I have made changes to incorporate these, Thanks for bringing this to attention

MUST FIX: I see supports_check_mode=True but i don't see it actually works. Maybe I missed something? If the modules just ignore it, this statement can mislead users.

Yes that was incorrect, Have removed it now

SHOULD FIX: HAS_REQUEST is not used. You can use something like

Implemented this for each module, Thanks

COMMENT: i see changelog.yml and CHANGELOG.rst look different, do you follow https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_release_without_branches.html when releasing?

Have fixed it now, thanks

COMMENT: How the release process work? I don't see any related runs.

Have added a release workflow as I mentioned, It will be the way for releasing new version going further

[Andersson007]

@ishanjainn thanks for the rapid feedback!

I have added tags to the specific versions that were missing, Thanks for noting this

Marked as done, thanks

Have added a release workflow now which will create the Release on Github. Uploading to Galaxy keeping it manual for now. Let me known if that process needs to be in the README.

It'd be imo great to have it in README available for future maintainers

I have made changes to incorporate these, Thanks for bringing this to attention

All the other stuff is marked as done, thanks (I would recommend using module.fail_json(msg=missing_required_lib('requests')) format)

LGTM for inclusion

[Andersson007]

@ansible-collections/steering-committee there are one community and one SC review done. Could anyone just take a look and approve the inclusion/add comments?

[acozine]

+1 for inclusion from me. It's great to see the review feedback addressed so quickly and thoroughly. Thanks @mbialon @Andersson007 @ishanjainn.

[ishanjainn]

Thanks @acozine

[felixfontein]

While browsing through the modules, I noticed that most of them are not idempotent. They either do a change, or fail. That looks wrong to me, and I cannot imagine that the API does not allow to query information to do idempotency checks (which should also allow to implement check mode). This is the case for example for:

• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/folder.py
• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/datasource.py
• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/dashboard.py
• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/cloud_plugin.py
• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/alert_notification_policy.py

Two modules also seem to have bugs, as in some cases they do not seem to return a tuple, but None:

• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/cloud_stack.py#L162 (if nothing in the loop matches)
• https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/alert_contact_point.py#L163 (if nothing in the loop matches)

When failing because of missing requirements, modules should use missing_required_lib to compose the message for module.fail_json(). This isn't done in at least one case: https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/alert_notification_policy.py#L211

[ishanjainn]

Hey, Quick update have added idempotency to all modules except datasources and dashboards modules as We are not yet sure if we want to change the way how the datasource and dashboard configs are taken as input from the playbook. But this way we agree, implementing idempotency for them also get tough to implement. Till then we will add a note for users

[ishanjainn]

Really appreciate the feedback here @felixfontein ,
As I understand From the requirements checklist standpoint, All are completed Wanted to take thoughts on this @Andersson007 @acozine aswell

[felixfontein]

@ishanjainn I don't see any mention in the module documenation (https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/datasource.py#L11, https://github.com/grafana/grafana-ansible-collection/blob/main/plugins/modules/dashboard.py#L11) that these modules are not idempotent. When do you plan to add that note?

[ishanjainn]

Have added it as a note to both now, Thanks

[felixfontein]

Thanks, looks good now.

[Andersson007]

As there are 2 reviews and approvals (+1 from me), I've created a topic ansible-community/community-topics#163.
If no objections, the collection will get included on 2022-12-06 and will appear in the package in the next Ansible minor release after that.

@ishanjainn if there are any not released changes you made based on our feedback here, could you please release the collection?
Thanks

[ishanjainn]

Thanks @Andersson007 , All feedback and comments received have been already released.

[Andersson007]

@ishanjainn cool then, thanks for the info

[Andersson007]

Included ansible-community/ansible-build-data#184, is already available in Ansible 7.1, thanks everyone!

[ishanjainn]

Thanks!

[ishanjainn]

Hey @felixfontein, Is there a process after the inclusion has been done? I failed to find a documentation if there is, Can you guide me to it?

Context: I see all collections with their repo under https://github.com/ansible-collections org. So wanted to check if we have to also move the repo under this org. Having the grafana.grafana under this org would remove the CLA issue we are having while merging community.grafana and grafana.grafana so just wanted to check. Thanks in advance

[gotmax23]

I'm fine with moving grafana.grafana to the ansible-collections org. I asked in the #ansible-community room¹ if anyone from the Ansible Community team at Red Hat is able to coordinate with you on that. You might have to wait until after the holidays.

Footnotes

1. BTW, I'd suggest joining the Ansible Community Working Group chat room if you haven't already :). It's #community:ansible.com on Matrix and #ansible-community on Libera. ↩

[samccann]

This was mentioned in another issue but just in case - community members are starting to disappear for their year-end breaks so folks may not get back to this before the new year.

[ishanjainn]

Thanks @gotmax23 @samccann, and after holidays is completely fine.

[Andersson007]

@ishanjainn if you want to move it, feel free to transfer the repo, NP (i don't remember how it works, i think someone with admin permissions in ansible-collections should accept it or something, we could try)

[ishanjainn]

Got it, Just wanted to see if its an option if the CLA stuff doesn't work out completely.