-
Notifications
You must be signed in to change notification settings - Fork 308
Feature provisioning dashboards #121
Feature provisioning dashboards #121
Conversation
…t. And add missing tags.
…already exists for datasources.
Any updates on this? I could use it, and it looks like the build is failing because of a simple linting failure due to trailing empty lines. |
Sure, I fixed it. Do you know how to relaunch the review ? |
Ok the review launches automatically. The travis build got a problem with rsync. I don't know why. Should I go on using rsyng module ? I use it to because it's a flexible way to maintain an exact copy of directory content for the provisioned grafana dashboards. |
5cc73ea
to
e45dde2
Compare
CI is using docker containers with minimal tool set which doesn't include rsync which in turn is needed by |
…ote about rsync need in README.
tasks/dashboards.yml
Outdated
when: grafana_use_provisioning | ||
register: synchronize_result | ||
|
||
- name: Set privileges on provisioned dashboards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this task and next one are only executed when previous one was changed, then it might be better to use those as handlers instead of using when: synchronize_result.changed
and registering variable. In the end that's what handlers are for. I don't know why this wasn't showing up in CI, but when I run it locally I immediately got linter errors:
[ANSIBLE0016] Tasks that run when changed should likely be handlers
/home/paulfantom/cloudalchemy/ansible-grafana/tasks/dashboards.yml:174
Task/Handler: Set privileges on provisioned dashboards
[ANSIBLE0016] Tasks that run when changed should likely be handlers
/home/paulfantom/cloudalchemy/ansible-grafana/tasks/dashboards.yml:183
Task/Handler: Set privileges on provisioned dashboards directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes I see. I never used handlers for this kind of situation. I'll do that immediatly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can notify multiple handlers from one task like:
- name: doing sth
command: echo "OK"
notify:
- do thing A
- do thing B
This is almost ready to be merged, just this one ANSIBLE0016 issue. After fixing that I will merge this and create a new release 😄 |
Well done! Thank you very much for this! 🎉 |
Ouf ! 😄 |
* Fix grafana provisioning dir scaned on remote host rathen on localhost. And add missing tags. * Add parameter and tasks to use grafana dashboard provsioning like it already exists for datasources. * Fix yaml lint, and remove unused commented task. * Add a rsync install task when using grafana_provisioning. * Fix dashboard permissions for idempotency. * Move rsync installation to molecule default prepare playbook. Add a note about rsync need in README. * Fix molecule default prepare playbook by removing bad condition. * Move rsync requirement to requirements section. * Add "provisioned dashboards changed" handler. * Replace when "provisioned dashboards changed" tasks by a handler notification. * Fix lint: remove trailing spaces [minor] release
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Add dashboard provisioning like it is laready done for datasources.
Add a parameter grafana_provisioning_synced to allow previously provisioned dashboard removal.
Add missing tags to the main task.