community.grafana
community.grafana copied to clipboard
Replace GHA boilerplate with the use of an Action
This patch replaces manually maintained copy-paste of ansible-test
invocations with the use of a reusable GitHub Action1 that wraps
it allowing to only keep the matrix on the project side allowing the
reusable bits to be maintained separately and be shared across the
community.
This has been earlier implemented for the community.digitalocean
collection2 among others.
SUMMARY
See above.
ISSUE TYPE
- Maintenance Pull Request
- Testing Pull Request
COMPONENT NAME
.github/workflows/ansible-test.yml
ADDITIONAL INFORMATION
This is also on the way of being included into the default collection skeleton at https://github.com/ansible/ansible/pull/74901/files#diff-096f7bb125bbac5751a11cfa3f7682b8d7c7c8cfffb0feddd10fb78e7346950d.
@webknjaz based on latest news I guess we have to update the matrix to have tests on devel no more using Python 3.8
Am I right ?
Am I right ?
Fair enough.
Codecov Report
Merging #266 (2e5fda7) into main (33fa3de) will decrease coverage by
25.45%. The diff coverage isn/a.
@@ Coverage Diff @@
## main #266 +/- ##
===========================================
- Coverage 69.45% 44.00% -25.46%
===========================================
Files 16 16
Lines 1709 1709
Branches 320 320
===========================================
- Hits 1187 752 -435
- Misses 385 937 +552
+ Partials 137 20 -117
| Flag | Coverage Δ | |
|---|---|---|
| integration | ? |
|
| sanity | 22.81% <ø> (ø) |
|
| units | 73.54% <ø> (+0.13%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| plugins/modules/grafana_datasource.py | 14.60% <0.00%> (-60.68%) |
:arrow_down: |
| plugins/modules/grafana_organization.py | 33.80% <0.00%> (-50.71%) |
:arrow_down: |
| plugins/modules/grafana_folder.py | 28.28% <0.00%> (-47.48%) |
:arrow_down: |
| plugins/modules/grafana_user.py | 51.61% <0.00%> (-39.79%) |
:arrow_down: |
| plugins/modules/grafana_notification_channel.py | 18.04% <0.00%> (-39.52%) |
:arrow_down: |
| plugins/modules/grafana_dashboard.py | 14.11% <0.00%> (-37.65%) |
:arrow_down: |
| plugins/modules/grafana_team.py | 65.06% <0.00%> (-21.24%) |
:arrow_down: |
| .../modules/grafana/grafana_team/test_grafana_team.py | 97.42% <0.00%> (+0.42%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 33fa3de...2e5fda7. Read the comment docs.
I'm happy with this. Maintainers welcome to merge once they are happy.
It's not ready yet. I've faced issues with integration tests.
@rrey I've found that parametrization of the grafana is a bit weird. Have you thought of remaking it somehow to make it flexible? FWIW I was thinking about reverting the integration test changes for now, merging only units+sanity and trying to solve the integration problem in follow-ups. WDYT?
@rrey I've found that parametrization of the grafana is a bit weird. Have you thought of remaking it somehow to make it flexible?
@webknjaz Can you elaborate ? Maybe it is the use of Github Actions services that bothers you ?
If yes, It is actually super useful because it is a simple way to have Grafana running and test against it. It allows to avoid anything related to Grafana installation. I had a problem with ansible-test when I setup the CI early in the Collection creation because this mechanism is not compatible with ansible-test running in docker (the github service and the ansible-test container won't be sharing network).
That is why the integration tests are not using the --docker option for integration tests.
FWIW I was thinking about reverting the integration test changes for now, merging only units+sanity and trying to solve the integration problem in follow-ups. WDYT?
Sounds good !
@webknjaz Can you elaborate ? Maybe it is the use of Github Actions services that bothers you ?
If yes, It is actually super useful because it is a simple way to have Grafana running and test against it. It allows to avoid anything related to Grafana installation. I had a problem with ansible-test when I setup the CI early in the Collection creation because this mechanism is not compatible with ansible-test running in docker (the github service and the ansible-test container won't be sharing network).
That is why the integration tests are not using the --docker option for integration tests.
So basically, the containers that ansible-test spawns end up being on a different docker network than the ones that the GitHub Actions Runner starts. It's visible at https://github.com/ansible-collections/community.grafana/runs/7510549704?check_suite_focus=true#step:2:14.
And because of that, the tests can't communicate with the Grafana instance. I was considering improving the action to auto-detect that network (it's name is not static across runs) and adding a --docker-network param to the invocation.
Another solution (likely better) is integrating the Grafana service invocation into the tests. This would allow running the tests locally the same way. Although, I'm not sure how to organize this best — ansible-test in ansible-core has a number of built-in "cloud service providers/stubs" for testing the Core itself but I don't think that we have any way for the collections to implement their own "providers" for integration testing. Also, at the moment, ansible-test doesn't have a parametrization mechanism.
Maybe, it could be implemented through run.sh reading a grafana version value from a file on disk and GHA modifying that file prior to calling the action.
But for now, go ahead and merge #270. I'll keep thinking of how to properly instrument this on the action's side. No timeline promises, though.
I was considering improving the action to auto-detect that network (it's name is not static across runs)
https://docs.github.com/en/actions/learn-github-actions/contexts#job-context documents that there's ${{ job.services.<service_id>.network }} so maybe autodetection isn't really needed.
Cool ! Happy to test anything here if you add a feature in ansible-test
New attempt: https://github.com/ansible-collections/community.grafana/pull/276.
v1.9.0 implemented auto-exposing the container network that GH creates in workflows: https://github.com/ansible-community/ansible-test-gh-action/discussions/37.