prometheus-community / ansible

Ansible Collection for Prometheus
https://prometheus-community.github.io/ansible/
Apache License 2.0
396 stars 133 forks source link

feat: Add prometheus prometheus_provisioning_synced support #453

Open sergiocharpineljr opened 2 weeks ago

sergiocharpineljr commented 2 weeks ago

Add support for prometheus_provisioning_synced (fixes https://github.com/prometheus-community/ansible/issues/147).

github-actions[bot] commented 2 weeks ago

Docs Build πŸ“

Thank you for contribution!✨

The docs for this PR have been published here: https://prometheus-community.github.io/ansible/pr/453

You can compare to the docs for the main branch here: https://prometheus-community.github.io/ansible/branch/main

The docsite for this PR is also available for download as an artifact from this run: https://github.com/prometheus-community/ansible/actions/runs/11702477098

File changes:

Click to see the diff comparison. **NOTE:** only file modifications are shown here. New and deleted files are excluded. See the file list and check the published docs to see those files. ```diff diff --git a/home/runner/work/ansible/ansible/docsbuild/base/prometheus_role.html b/home/runner/work/ansible/ansible/docsbuild/head/prometheus_role.html index da4a0c4..9cb4b55 100644 --- a/home/runner/work/ansible/ansible/docsbuild/base/prometheus_role.html +++ b/home/runner/work/ansible/ansible/docsbuild/head/prometheus_role.html @@ -312,27 +312,39 @@ To check whether it is installed, run
+

prometheus_provisioning_synced

+

boolean

+
+

Should the provisioning of alert rule files be kept synced. If true, previous provisioned files will be removed if not referenced anymore.

+

Choices:

+
    +
  • false ← (default)

  • +
  • true

  • +
+
+ +

prometheus_read_only_dirs

list / elements=string

Additional paths that Prometheus is allowed to read (useful for SSL certs outside of the config directory)

-
+

prometheus_remote_read

list / elements=string

Remote read. It is compatible with the official configuration

-
+

prometheus_remote_write

list / elements=string

Remote write. Compatible with the official configuration

-
+

prometheus_scrape_config_files

list / elements=string

@@ -341,7 +353,7 @@ To check whether it is installed, run Default: ["prometheus/scrape_configs/*.yml", "prometheus/scrape_configs/*.json"]

-
+

prometheus_scrape_configs

list / elements=dictionary

@@ -349,7 +361,7 @@ To check whether it is installed, run Default: [{"job_name": "prometheus", "metrics_path": "{{ prometheus_metrics_path }}", "static_configs": [{"targets": ["{{ ansible_fqdn | default(ansible_host) | default('localhost') }}:9090"]}]}, {"file_sd_configs": [{"files": ["{{ prometheus_config_dir }}/file_sd/node.yml"]}], "job_name": "node"}]

-
+

prometheus_static_targets_files

list / elements=string

@@ -357,7 +369,7 @@ To check whether it is installed, run Default: ["prometheus/targets/*.yml", "prometheus/targets/*.json"]

-
+

prometheus_stop_timeout

string

@@ -365,7 +377,7 @@ To check whether it is installed, run Default: "600s"

-
+

prometheus_storage_retention

string

@@ -373,7 +385,7 @@ To check whether it is installed, run Default: "30d"

-
+

prometheus_storage_retention_size

string

@@ -383,7 +395,7 @@ To check whether it is installed, run Default: "0"

-
+

prometheus_system_group

string

@@ -391,7 +403,7 @@ To check whether it is installed, run Default: "prometheus"

-
+

prometheus_system_user

string

@@ -399,14 +411,14 @@ To check whether it is installed, run Default: "prometheus"

-
+

prometheus_targets

dictionary

Targets which will be scraped.

-
+

prometheus_version

string

@@ -415,21 +427,21 @@ To check whether it is installed, run Default: "2.54.1"

-
+

prometheus_web_config

dictionary

A Prometheus web config yaml for configuring TLS and auth.

-
+

prometheus_web_external_url

string

External address on which prometheus is available. Useful when behind reverse proxy. Ex. `http://example.org/prometheus`

-
+

prometheus_web_listen_address

string

```
sergiocharpineljr commented 2 weeks ago

I tested this locally but didn't include any tests. If you feel like we should, please let me know and give me some guidance on the best approach to follow.

sergiocharpineljr commented 2 weeks ago

Thanks. I wonder if it might be cleaner to simply switch the Copy custom alerting rule files task over to the ansible.posix.synchronize module with delete: true to solve this. Also generally not a fan of using the json_query filter unless absolutely necessary.

I've followed the suggestion in https://github.com/prometheus-community/ansible/issues/147 to base this on Grafana's code: https://github.com/grafana/grafana-ansible-collection/blob/84d6af3b1c3991d6c560a8fe9c09d8fc1811f520/roles/grafana/tasks/dashboards.yml#L136-L142

The ansible.posix.synchronize might be tricky because of the ansible_managed.yml file we have in the same dir. So not sure how to make this idempotent.

json_query was used because of the default filter. Doing __rules_managed_copied.dest directly causes issues when prometheus_alert_rules = [] and the __rules_managed_copied var is not created.

zhan9san commented 2 weeks ago

hi @sergiocharpineljr

ansible.posix.synchronize can work as well.

FYI

---
- name: Synchronize directories using ansible.posix.synchronize
  hosts: localhost
  tasks:
    - name: Synchronize foo/rules to bar/rules with delete option
      ansible.posix.synchronize:
        src: ~/tmp/foo/rules/
        dest: ~/tmp/bar/rules/
        delete: yes
        recursive: yes
        rsync_opts:
          - "--exclude='ansible_managed.yml'"
jack@v:~/tmp$ ansible-playbook a.yaml
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [Synchronize directories using ansible.posix.synchronize] **********************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] **************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Synchronize foo/rules to bar/rules with delete option] ************************************************************************************************************************************************************************************************************************
ok: [localhost]

PLAY RECAP **************************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

jack@v:~/tmp$ tree foo/rules/ bar/rules/
foo/rules/
└── aaa.yml
bar/rules/
β”œβ”€β”€ aaa.yml
└── ansible_managed.yml

2 directories, 3 files
sergiocharpineljr commented 2 weeks ago

@zhan9san @gardar Ok, I can try the synchronize approach. Should we leave it as optional, keeping the current method as default (or setting delete: false) or do you think we should always force copy instead?

zhan9san commented 2 weeks ago

I prefer using synchronize with delete: true as default.

@gardar Your thought?

gardar commented 2 weeks ago

I prefer using synchronize with delete: true as default.

@gardar Your thought?

I agree; ideally, the roles should strictly manage how Prometheus is configured. However, I understand that’s not always the case, so having it as an optional feature could be useful.

Another concern I have is that using the synchronize module adds rsync as a dependency to the role, and rsync isn’t always installed by default.