Skip to content

add deploy_prometheus_exporters role - #959

Open
insatomcat wants to merge 2 commits into
mainfrom
prometheus
Open

add deploy_prometheus_exporters role#959
insatomcat wants to merge 2 commits into
mainfrom
prometheus

Conversation

@insatomcat

Copy link
Copy Markdown
Member

Provide a self-contained role to deploy Prometheus exporters as Podman quadlet units, with per-host exporter selection via inventory variables.

@insatomcat
insatomcat force-pushed the prometheus branch 2 times, most recently from d2ed038 to 25952e2 Compare June 23, 2026 15:49
@insatomcat
insatomcat marked this pull request as draft June 23, 2026 20:53
@insatomcat
insatomcat force-pushed the prometheus branch 4 times, most recently from e5daacf to 7bf65a4 Compare June 24, 2026 08:20
@insatomcat
insatomcat marked this pull request as ready for review June 24, 2026 11:19
Comment thread playbooks/seapath_setup_prometheus_exporters.yaml Outdated
@insatomcat
insatomcat requested a review from dupremathieu June 24, 2026 16:13
@insatomcat
insatomcat force-pushed the prometheus branch 4 times, most recently from 99c7a5e to a638d2e Compare July 3, 2026 15:38
@insatomcat

Copy link
Copy Markdown
Member Author

Hello @dahoat-sprecher, @jaxx0r,
You offered to contribute to Action 3.1 of the roadmap, which includes the Prometheus integration.
This PR proposes what I believe should be the first step: deploying some exporters.
Please don't hesitate to share your feedback and/or approve it if you think it looks good.
Thanks!

@dahoat-sprecher

Copy link
Copy Markdown
Contributor

Hello @insatomcat ,
thanks for the invitation and notification. I am looking forward to having a look as soon as possible. Unfortunately, I won't be able until after my vacation. 😅

@insatomcat
insatomcat force-pushed the prometheus branch 2 times, most recently from 08cfabf to 65289f0 Compare July 18, 2026 06:02
@insatomcat
insatomcat force-pushed the prometheus branch 2 times, most recently from 8704fe1 to 6cc2a18 Compare July 30, 2026 21:11
@dahoat-sprecher

Copy link
Copy Markdown
Contributor

Hello @insatomcat ,

I now had the chance to test this branch and it worked perfectly for me.
However, one thing caught my eye: Did you already plan on how to integrate TLS and authentication?

Best regards,
Daniel

@insatomcat

Copy link
Copy Markdown
Member Author

Hi @dahoat-sprecher,

Thanks a lot for taking the time to test it, glad it worked out of the box for you.

TLS and authentication: good question, and to be honest it is something we still need
to think through properly rather than something already settled. I left it out of this
first step on purpose. There are clear leads though, and the good news is that the
groundwork is already there.

Every exporter deployed by this role can serve its metrics over TLS and verify a
client certificate. For the upstream ones this comes from prometheus/exporter-toolkit,
which gives us a single common configuration format through --web.config.file:

tls_server_config:
  cert_file: /etc/prometheus/exporters/tls/server.crt
  key_file:  /etc/prometheus/exporters/tls/server.key
  client_auth_type: RequireAndVerifyClientCert
  client_ca_file: /etc/prometheus/exporters/tls/ca.crt
  min_version: TLS13

So the likely direction is one such file per host, mounted read-only into the
containers, with the role exposing a single set of variables and taking care of the
small configuration differences between exporters. No reverse proxy, no sidecar, no
extra failure point.

I currently lean towards mutual TLS rather than basic auth for the authentication
part. It keeps any shared secret out of the inventory, revocation goes through the CA,
and it avoids having to generate and vault bcrypt hashes. It is also uniformly
available across the exporters we deploy. That said it is a preference and not a
decision, so I would genuinely like your view on it.

For certificate distribution, this repository already has a pattern worth following:
the syslog_ng_client and vmmgrapi roles take certificate and key paths as
variables and copy them into place. Reusing it would keep things consistent instead of
inventing a mechanism specific to monitoring.

One part will not fit that model at all: the Ceph side, since the mgr prometheus
module is not an exporter-toolkit binary. There the path looks more like
mgr/cephadm/secure_monitoring_stack, which deserves its own discussion.

I would rather keep all of this for a follow-up PR. Working iteratively keeps each PR
small enough to be reviewed and converged on quickly, and in my experience that gets
us to the end result faster than one large change that stalls in review.

So, any other remark on the current state, or do you think we can merge this one and
move the security work to the next step? Feedback very welcome on the design above,
and you are of course more than welcome to take part in it.

@dahoat-sprecher

Copy link
Copy Markdown
Contributor

Yes, I fully agree on the usage of mTLS, I am only afraid of the complexity of handling the certificates and will look into the existing approaches.

Two items might deserve a second look which I try to comment directly.

Exec=--path.rootfs=/host --collector.textfile.directory=/host/var/lib/prometheus/node_exporter --web.listen-address={{ deploy_prometheus_exporters_listen_address }}:9100 --collector.systemd

[Service]
Restart=unless-stopped

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, this value should be always. The default value might be Restart=no on invalid values.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. unless-stopped is a Podman/Docker restart policy, not a systemd one: systemd only accepts no/on-success/on-failure/on-abnormal/on-watchdog/on-abort/always, and Quadlet copies the [Service] section verbatim into the generated unit. I confirmed it on one of our lab machines that already runs these exporters:

/etc/containers/systemd/node-exporter.container:  Restart=unless-stopped
systemctl show -p Restart node-exporter:          Restart=no
systemctl show -p Restart podman-exporter:        Restart=no

So the effective policy really was Restart=no, exactly as you suspected.

Fixed to Restart=always + RestartSec=5. While at it I aligned the other templates: ha_cluster_exporter.container.j2 and podman-exporter.container.j2 had no [Service] section at all, so they were on Restart=no too. All five exporters now share the same policy, and the molecule verify asserts it so a regression gets caught.

After redeploying the fixed role on that machine, systemctl kill --signal=SIGKILL node-exporter gives NRestarts=1 and the exporter is back up and scrapable a few seconds later.

ansible.builtin.systemd:
name: podman.socket
enabled: true
state: started

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, the service is only started if not already running. Therefore, changes on the template are not picked up until an explicit restart of the service or the machine (same for "Enable and start prometheus exporter services on #Line 61.

Claude claimed there is a workaround to combine looped items with handlers:

# tasks/main.yml
- name: Deploy prometheus exporter container units
  ansible.builtin.template:
    src: "{{ item }}.container.j2"
    dest: "/etc/containers/systemd/{{ item }}.container"
    ...
  loop: "{{ deploy_prometheus_exporters_exporters }}"
  register: deploy_prometheus_exporters_unit_files
  notify:
    - Reload systemd after exporter container unit changes
    - Restart changed exporter services

# handlers/main.yml
- name: Reload systemd after exporter container unit changes
  ansible.builtin.systemd:
    daemon_reload: true

- name: Restart changed exporter services
  ansible.builtin.systemd:
    name: "{{ item }}"
    state: restarted
  loop: "{{ deploy_prometheus_exporters_unit_files.results | selectattr('changed') | map(attribute='item') | list }}"
  when: deploy_prometheus_exporters_manage_services | bool

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, and this is a real bug: state: started is a no-op on an already running service, so a template change was written to disk and the unit regenerated, but the container kept running with the old configuration until a manual restart or a reboot. The role was not convergent.

I applied essentially your snippet: the template task is now registered and notifies both handlers, and a new Restart changed exporter services handler restarts only the units whose file actually changed.

Two details worth noting on top of it:

  • handlers run in declaration order, not in notification order, so the daemon-reload stays declared first in handlers/main.yml (there is a comment saying so now);
  • I moved the podman.socket task before the flush_handlers, and dropped the when on meta: flush_handlers. The manage_services condition now lives on the restart handler itself, which is where it belongs, and a daemon-reload on its own is harmless.

I also had a look at the enabled: true sitting next to it, since Quadlet units live in /run/systemd/generator/ and systemd normally refuses to enable a generated unit. It turns out to be a silent no-op rather than a failure: systemctl is-enabled returns generated with rc 0, and Ansible's systemd module only calls systemctl enable for enabled-runtime/indirect/alias, so it never runs. Since it can never do anything on a Quadlet unit, and reading it suggests the role handles boot activation when the [Install] section does, I dropped it from the exporter task. The one on podman.socket stays, that one is a real unit.

Tested on a lab machine that was already running the exporters. First run: both .container files change, the reload and restart handlers fire, and the effective policy goes from Restart=no to Restart=always with the services restarted, no reboot involved. Second run: changed=0 and no handler, so the role stays idempotent.

Provide a self-contained role to deploy Prometheus exporters as Podman
quadlet units, with per-host exporter selection via inventory variables.

Yocto and SLES are not supported yet.

Signed-off-by: Florent Carli <florent.carli@rte-france.com>
Configure the built-in Ceph mgr prometheus module after HEALTH_OK,
including per-daemon server_addr binding on each host's admin IP.

Signed-off-by: Florent Carli <florent.carli@rte-france.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants