Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

180 rule files param #241

Merged
merged 6 commits into from
Jul 30, 2018
Merged

Conversation

bramblek1
Copy link

Pull Request (PR) description

Fixes an issue where prometheus::rule_files param was never used to populate configuration as it was not included when combining extra_alerts and alerts params.

This Pull Request (PR) fixes the following issues

Fixes #180

…g a list of rule files from data in prometheus::alerts and prometheus::extra_alerts
…metheus1 and 2 .

I am not sure if prometheus1 is even covered, and confused that what looks like variable interpolation is going right through to File[/etc/prometheus/prometheus.yaml] as literal
@bramblek1
Copy link
Author

I think I have mistaken what belongs in the prometheus.yaml fixtures. Does this get expanded with defaults? I suspect not.

@bastelfreak
Copy link
Member

@bramblek1 the fixtures represent an actual config from a running system. Puppet won't expand any variables in it.

@bramblek1
Copy link
Author

Thanks @bastelfreak - I think I have run across a different issue.
bundle exec rake test works for me locally. I see the beaker tests failing in travis-ci with

Error: /Stage[main]/Prometheus::Config/File[prometheus.yaml]/ensure: change from 'absent' to 'present' failed: Execution of '/usr/local/bin/promtool check-config /etc/prometheus/prometheus.yaml20180730-1400-tcodz7' returned 1: Checking /etc/prometheus/prometheus.yaml20180730-1400-tcodz7 FAILED: "/etc/prometheus/$prometheus::{config_dir}/$prometheus::{alertfile_name}.rules" does not point to an existing file

Since this error seems to come from promtool check , that string must be ending up in prometheus.yaml as a literal, which is what confused me previously.

This seems to be coming from the module hiera data/defaults.yaml - where prometheus::rule_files is using puppet DSL type syntax, not hiera lookup.

prometheus::rule_files:
  - "$prometheus::{config_dir}/$prometheus::{alertfile_name}.rules"

Since server.pp is handling the addition of alert.rules if alerts is non-empty , I think this default should be an empty list.

@bastelfreak bastelfreak added bug Something isn't working and removed tests-fail labels Jul 30, 2018
@bastelfreak
Copy link
Member

thanks for fixing this @bramblek1 !

@bastelfreak bastelfreak merged commit f4fb0c6 into voxpupuli:master Jul 30, 2018
@kamusin
Copy link

kamusin commented Jul 31, 2018

Thank you so much @bramblek1!

cegeka-jenkins pushed a commit to cegeka/puppet-prometheus that referenced this pull request Aug 28, 2019
Rovanion pushed a commit to Rovanion/puppet-prometheus that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$rule_files parameter not respected
3 participants