grafana / grafana-ansible-collection

grafana.grafana Ansible collection provides modules and roles for managing various resources on Grafana Cloud and roles to manage and deploy Grafana Agent and Grafana
https://docs.ansible.com/ansible/latest/collections/grafana/grafana/index.html#plugins-in-grafana-grafana
GNU General Public License v3.0
128 stars 83 forks source link

Use grafana.ini yaml syntax like grafana kubernetes helm chart #210

Open intermittentnrg opened 4 months ago

intermittentnrg commented 4 months ago

Hello! Long time user of Grafana helm chart here, first time I'm using the ansible role.

The grafana kubernetes helm chart automatically converts yaml to grafana.ini syntax.

This would clean up the grafana.ini.j2 templating code and also support all current and future grafana.ini configuration options. There's several grafana.ini sections that are not supported in this ansible role.

It's quite elegant and easy I think. Example:

roles:
  - name: grafana.grafana.grafana
    vars:
      grafana_ini:
        server:
          root_url: https://intermittent.energy
        auth.anonymous:
          enabled: true
          org_name: Main Org.
          org_role: Viewer
        alerting:
          enabled: false
        log.console:
          format: json
        feature_toggles:
          enable: nestedFolders

generates grafana.ini:

[server]
root_url = https://intermittent.energy

[auth.anonymous]
enabled = true
org_name = Main Org.
org_role = Viewer

[alerting]
enabled = false

[log.console]
format = json

[feature_toggles]
enable = nestedFolders

Needs backwards compatibility, or new breaking changes major version release?

Here is the code used in the grafana helm chart to achieve this, it is uses gotmpl syntax not jinja2: example config: https://github.com/grafana/helm-charts/blob/main/charts/grafana/values.yaml#L772-L1098 templating code: https://github.com/grafana/helm-charts/blob/main/charts/grafana/templates/_config.tpl#L11-L36

Most blocks in grafana.ini are generated with this pattern:

{% for k,v in grafana_xxx.items() %}
{{ k }} = {{ v }}
{%   endfor %}
{% endif %}

These variables can be mapped/reassigned with ["grafana.ini"][xxx] = grafana_xxx to maintain backwards compatibility. Or just give error and prompt user to migrate if they are specified.

As a bonus this makes migrations between helm and ansible fairly trivial.

intermittentnrg commented 4 months ago

Example jinja2 code and playbook to generate grafana.ini as per above:

#!/bin/sh
cat <<EOF > grafana.ini.j2
{% for k, v in grafana_ini.items() %}
{% if v is not mapping %}
{{ k }} = {{ v }}
{% endif %}
{% endfor %}

{% for section, items in grafana_ini.items() %}
{% if items is mapping %}
[{{ section }}]
{% for k,v in items.items() %}
{{ k }} = {{ v }}
{% endfor %}

{% endif %}
{% endfor %}
EOF

cat <<EOF > playbook-template.yaml
- hosts: localhost
  gather_facts: No
  vars:
    grafana_ini:
      app_mode: production
      server:
        root_url: https://intermittent.energy
      log.console:
        format: json
  tasks:
    - template:
        src: grafana.ini.j2
        dest: grafana-template.ini
EOF

ansible-playbook playbook-template.yaml >/dev/null 2>/dev/null
cat grafana-template.ini

Output:

app_mode = production

[server]
root_url = https://intermittent.energy

[log.console]
format = json

helm chart also handle kindIs "invalid" $elemVal to set key =. And kindIs "string" $elemVal to use templating {{ tpl $elemVal $ }}. I'm not sure these are relevant for ansible?

I think above is OK? Next step would be to decide on whether to break or remap current variables to new structure.

ishanjainn commented 4 months ago

cc @gardar

GVengelen commented 4 months ago

I'm familiar with this syntax. It's a nice addition because it's future-compatible. Add it to the existing template, otherwise, it's a breaking change.

gardar commented 4 months ago

I wonder if the community.general.to_ini plugin could be used to convert the yaml to ini instead of using a template?

intermittentnrg commented 4 months ago

Add to existing template is great idea, but old template should be removed eventually. It could render duplicate sections tho. Just make a major release with breaking change and throw errors if old variables are used.

community.general.to_ini sounds good, but does it support settings without section at the top of the .ini file? app_mode and instance_name.

intermittentnrg commented 4 months ago

community.general.to_ini seems to use ConfigParser which requires all settings to belong to a section. https://docs.python.org/3/library/configparser.html#supported-ini-file-structure

Using the same approach as the grafana helm chart is maybe better.

There's an old issue and commit addressing root/sectionless settings:

There was a force_migration=true setting, removed in grafana 11.

intermittentnrg commented 3 months ago

Shall I proceed with a PR?

I have concerns about supporting both simultaneously which could generate duplicate sections, not sure if it's a problem, can explore this further.

IMHO it's cleanest to break compatibility and bump major version. Raise error if old variables are used, migrating should be simple and straightforward.

GVengelen commented 3 months ago

Shall I proceed with a PR?

I have concerns about supporting both simultaneously which could generate duplicate sections, not sure if it's a problem, can explore this further.

IMHO it's cleanest to break compatibility and bump major version. Raise error if old variables are used, migrating should be simple and straightforward.

I agree, go for it. Looking forward to your work.

intermittentnrg commented 3 months ago

I created PR #232 for early feedback, not tested. I kept all entries in README and defaults/main.yaml, but I think most should be deleted as the ansible role should not be documenting grafana.ini unless there's something specific to the role.