aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorColin Wilk <colin@wilk.cx>2026-07-21 19:10:19 +0200
committerColin Wilk <colin@wilk.cx>2026-07-21 19:11:08 +0200
commit0055971afd5524f0df00fc8d2203a80f828885f7 (patch)
tree23019217d6a692aa83a5b09ede12bb3ebbc9da69
parentd9b5c59a50ac9967dc365f2c8586869d6ef5d9bd (diff)
downloadansible-role-prometheus-target-0055971afd5524f0df00fc8d2203a80f828885f7.tar.gz
ansible-role-prometheus-target-0055971afd5524f0df00fc8d2203a80f828885f7.zip
fix: prevent lost YAML targets during parallel writes
The YAML strategy performed a separate read-modify-write operation for every managed host. Because these tasks were delegated to the same Prometheus host, parallel Ansible forks could read the same original file before any fork wrote its update. Each fork then generated YAML containing only its own target. Although the copy module atomically replaced the file, it did not make the full read-modify-write sequence atomic. The last fork to write therefore overwrote targets added by earlier forks. Collect exporter updates from all hosts through hostvars, process them in one run_once operation, and write each distinct target file once. This preserves every target without requiring users to set serial. Add parallel coverage for multiple target files and label groups, and reset the fixtures on every convergence so stale files cannot hide the race.
-rw-r--r--molecule/default/converge.yml15
-rw-r--r--molecule/default/tests/test_yaml_strategy.py17
-rw-r--r--tasks/main.yml7
-rw-r--r--tasks/strategy_yaml.yml150
4 files changed, 133 insertions, 56 deletions
diff --git a/molecule/default/converge.yml b/molecule/default/converge.yml
index 4e379da..f882c44 100644
--- a/molecule/default/converge.yml
+++ b/molecule/default/converge.yml
@@ -185,16 +185,19 @@
pre_tasks:
- name: Create yaml parallel target
ansible.builtin.copy:
- dest: /opt/yaml_parallel.yml
+ dest: "{{ item }}"
mode: "0644"
content: |
- labels:
job: node
targets:
- existing:9100
- force: false
+ force: true
delegate_to: "{{ prometheus_target_host }}"
run_once: true
+ loop:
+ - /opt/yaml_parallel.yml
+ - /opt/yaml_parallel_second.yml
vars:
prometheus_target_host: prometheus
@@ -207,6 +210,14 @@
host: "{{ inventory_hostname }}:9100"
labels:
job: node
+ - path: /opt/yaml_parallel_second.yml
+ host: "{{ inventory_hostname }}:9200"
+ labels:
+ job: node
+ - path: /opt/yaml_parallel.yml
+ host: "{{ inventory_hostname }}:9300"
+ labels:
+ job: alternate
################################################################################
################################################################################
diff --git a/molecule/default/tests/test_yaml_strategy.py b/molecule/default/tests/test_yaml_strategy.py
index 89dbdeb..7ba834e 100644
--- a/molecule/default/tests/test_yaml_strategy.py
+++ b/molecule/default/tests/test_yaml_strategy.py
@@ -44,6 +44,22 @@ def test_yaml_parallel_writes_keep_all_hosts_in_same_group(host):
"application4:9100",
]
+ assert get_group_by_labels(groups, {"job": "alternate"})["targets"] == [
+ "application:9300",
+ "application2:9300",
+ "application3:9300",
+ "application4:9300",
+ ]
+
+ second_groups = read_yaml_file(host, "/opt/yaml_parallel_second.yml")
+ assert get_group_by_labels(second_groups, {"job": "node"})["targets"] == [
+ "existing:9100",
+ "application:9200",
+ "application2:9200",
+ "application3:9200",
+ "application4:9200",
+ ]
+
def test_yaml_user_edited_file_is_reparsed_and_keeps_expected_semantics(host):
groups = read_yaml_file(host, "/opt/yaml_user_edited.yml")
@@ -70,6 +86,7 @@ def test_yaml_output_remains_parseable_after_all_operations(host):
for path in [
"/opt/yaml_bootstrap.yml",
"/opt/yaml_parallel.yml",
+ "/opt/yaml_parallel_second.yml",
"/opt/yaml_user_edited.yml",
"/opt/yaml_move_remove_last.yml",
"/opt/yaml_branch_matrix.yml",
diff --git a/tasks/main.yml b/tasks/main.yml
index 10f54f7..ecffd79 100644
--- a/tasks/main.yml
+++ b/tasks/main.yml
@@ -1,11 +1,16 @@
---
-- name: Process all exporters
+- name: Process YAML exporters
+ ansible.builtin.include_tasks: strategy_yaml.yml
+ when: prometheus_target_strategy == "yaml"
+
+- name: Process all lineinfile exporters
ansible.builtin.include_tasks:
file: process_exporter.yml
loop: "{{ prometheus_target_exporter +
([] if prometheus_target_skip_default_exporters else prometheus_target_default_exporters) }}"
loop_control:
loop_var: item
+ when: prometheus_target_strategy != "yaml"
- name: Run handlers
ansible.builtin.include_tasks:
diff --git a/tasks/strategy_yaml.yml b/tasks/strategy_yaml.yml
index 8a8e38b..24cd6c8 100644
--- a/tasks/strategy_yaml.yml
+++ b/tasks/strategy_yaml.yml
@@ -1,71 +1,115 @@
---
-- name: Check if target file exists
- ansible.builtin.stat:
- path: "{{ _target_path }}"
- register: _target_file_stat
- delegate_to: "{{ prometheus_target_host }}"
+- name: Prepare YAML exporter updates
+ ansible.builtin.set_fact:
+ _prometheus_target_yaml_updates: >-
+ {%- set ns = namespace(updates=[]) -%}
+ {%- set exporters = prometheus_target_exporter +
+ ([] if prometheus_target_skip_default_exporters else prometheus_target_default_exporters) -%}
+ {%- for exporter in exporters -%}
+ {%- set defaults = prometheus_target_exporter_defaults[exporter.id] | default({})
+ if exporter.id is defined else {} -%}
+ {%- set path =
+ (exporter.path_prefix if exporter.path_prefix is defined else
+ (defaults.path_prefix | default(prometheus_target_exporter_target_prefix))) ~
+ ((exporter.path if exporter.path is defined else defaults.path) | mandatory) -%}
+ {%- set host = (exporter.host if exporter.host is defined else defaults.host) | mandatory -%}
+ {%- set labels = (defaults.labels | default({})) | combine(exporter.labels | default({})) -%}
+ {%- set _ = ns.updates.append({'path': path, 'host': host, 'labels': labels}) -%}
+ {%- endfor -%}
+ {{ ns.updates }}
+
+# Every play host prepares its values above. Applying all updates from one host
+# prevents parallel Ansible forks from overwriting each other's YAML changes.
+- name: Aggregate YAML exporter updates
+ ansible.builtin.set_fact:
+ _prometheus_target_yaml_updates_all: >-
+ {%- set ns = namespace(updates=[]) -%}
+ {%- for target_host in ansible_play_batch -%}
+ {%- for update in hostvars[target_host]._prometheus_target_yaml_updates -%}
+ {%- set _ = ns.updates.append(update) -%}
+ {%- endfor -%}
+ {%- endfor -%}
+ {{ ns.updates }}
+ run_once: true
-- name: Read target file
+- name: Collect YAML target paths
+ ansible.builtin.set_fact:
+ _prometheus_target_yaml_paths: >-
+ {{ _prometheus_target_yaml_updates_all | map(attribute='path') | unique | list }}
+ run_once: true
+
+- name: Read YAML target files
ansible.builtin.slurp:
- src: "{{ _target_path }}"
- register: _target_file_content
+ src: "{{ _yaml_path }}"
delegate_to: "{{ prometheus_target_host }}"
- when: _target_file_stat.stat.exists
+ loop: "{{ _prometheus_target_yaml_paths }}"
+ loop_control:
+ loop_var: _yaml_path
+ register: _yaml_target_files
+ failed_when: false
+ run_once: true
-- name: Parse YAML and add host to matching target group
+# Build each destination completely in memory and write it once. Besides
+# avoiding lost updates, this keeps updates to different files independent.
+- name: Build updated YAML target files
ansible.builtin.set_fact:
- _updated_targets: >-
- {%- set targets = (_target_file_content.content | b64decode | from_yaml) if _target_file_stat.stat.exists else [] -%}
- {%- set targets = targets if targets is not none else [] -%}
- {%- set ns = namespace(found=false, result=[], match=true) -%}
- {%- for group in targets -%}
- {%- set group_labels = group.labels | default({}) -%}
- {%- set ns.match = true -%}
- {%- if _target_labels | length != group_labels | length -%}
- {%- set ns.match = false -%}
- {%- else -%}
- {%- for key, value in _target_labels.items() -%}
- {%- if group_labels.get(key) != value -%}
- {%- set ns.match = false -%}
+ _prometheus_target_yaml_files: >-
+ {%- set output = namespace(files=[]) -%}
+ {%- for file in _yaml_target_files.results -%}
+ {%- set state = namespace(groups=(file.content | b64decode | from_yaml)
+ if file.content is defined else []) -%}
+ {%- set state.groups = state.groups if state.groups is not none else [] -%}
+ {%- for update in _prometheus_target_yaml_updates_all if update.path == file._yaml_path -%}
+ {%- set next = namespace(found=false, groups=[]) -%}
+ {%- for group in state.groups -%}
+ {%- set group_labels = group.labels | default({}) -%}
+ {%- if group_labels == update.labels -%}
+ {%- if update.host not in group.targets -%}
+ {%- set _ = group.targets.append(update.host) -%}
+ {%- endif -%}
+ {%- set next.found = true -%}
+ {%- set _ = next.groups.append(group) -%}
+ {%- else -%}
+ {%- set filtered = group.targets | reject('equalto', update.host) | list -%}
+ {%- if filtered | length > 0 -%}
+ {%- set replacement = {'targets': filtered} -%}
+ {%- if group_labels | length > 0 -%}
+ {%- set _ = replacement.update({'labels': group_labels}) -%}
+ {%- endif -%}
+ {%- set _ = next.groups.append(replacement) -%}
+ {%- endif -%}
{%- endif -%}
{%- endfor -%}
- {%- endif -%}
- {%- if ns.match -%}
- {%- if _target_host not in group.targets -%}
- {%- set _ = group.targets.append(_target_host) -%}
- {%- endif -%}
- {%- set ns.found = true -%}
- {%- set _ = ns.result.append(group) -%}
- {%- else -%}
- {%- set filtered_targets = group.targets | reject("equalto", _target_host) | list -%}
- {%- if filtered_targets | length > 0 -%}
- {%- if group_labels | length > 0 -%}
- {%- set _ = ns.result.append({"labels": group_labels, "targets": filtered_targets}) -%}
- {%- else -%}
- {%- set _ = ns.result.append({"targets": filtered_targets}) -%}
+ {%- if not next.found -%}
+ {%- set new_group = {'targets': [update.host]} -%}
+ {%- if update.labels | length > 0 -%}
+ {%- set _ = new_group.update({'labels': update.labels}) -%}
{%- endif -%}
+ {%- set _ = next.groups.append(new_group) -%}
{%- endif -%}
- {%- endif -%}
+ {%- set state.groups = next.groups -%}
+ {%- endfor -%}
+ {%- set _ = output.files.append({'path': file._yaml_path, 'groups': state.groups}) -%}
{%- endfor -%}
- {%- if not ns.found -%}
- {%- if _target_labels | length > 0 -%}
- {%- set new_group = {"labels": _target_labels, "targets": [_target_host]} -%}
- {%- else -%}
- {%- set new_group = {"targets": [_target_host]} -%}
- {%- endif -%}
- {%- set _ = ns.result.append(new_group) -%}
- {%- endif -%}
- {{ ns.result }}
+ {{ output.files }}
+ run_once: true
-- name: Write updated target file
+- name: Write updated YAML target files
ansible.builtin.copy:
- content: "{{ _updated_targets | to_nice_yaml(indent=2, width=1337) }}"
- dest: "{{ _target_path }}"
+ content: "{{ _yaml_file.groups | to_nice_yaml(indent=2, width=1337) }}"
+ dest: "{{ _yaml_file.path }}"
mode: "0644"
delegate_to: "{{ prometheus_target_host }}"
become: true
- register: _yaml_result
+ loop: "{{ _prometheus_target_yaml_files }}"
+ loop_control:
+ loop_var: _yaml_file
+ label: "{{ _yaml_file.path }}"
+ register: _yaml_results
+ run_once: true
-- name: Track changes
+- name: Track aggregated YAML changes on each host
ansible.builtin.set_fact:
- changed: "{{ changed | default(false) or _yaml_result.changed }}"
+ changed: >-
+ {{ hostvars[ansible_play_batch | first]._yaml_results.results |
+ selectattr('changed') | list | length > 0 }}