Add validation to config changes #122
Conversation
costela
force-pushed
the
config_verify
branch
2 times, most recently
from
255b121
to
377b264
Compare
|
@costela Many thanks for the PR. Please check the travis error: |
|
@tuxmea yeah, I'm on it. Had a last minute change of heart on the last patch, and it didn't go very smoothly 😉 |
costela
force-pushed
the
config_verify
branch
3 times, most recently
from
bec0510
to
2a8b1b1
Compare
|
@tuxmea done |
|
Please note there's a small breaking change in the last patch: the |
| class prometheus::install | ||
| { | ||
| class prometheus::install ( | ||
| Boolean $purge_config_dir = true, |
| @@ -78,4 +82,11 @@ | |||
| system => true, | |||
| }) | |||
| } | |||
| file { $prometheus::config_dir: | |||
There was a problem hiding this comment.
why moving this from config to install class?
There was a problem hiding this comment.
as I mentioned in a comment above: this was necessary to add the missing dependency between the config file and the alerts file without creating a dependency cycle. Now both depend on the directory, which gets created first during install.
The config file needs the alerts file, because the newly introduced validation with promtool chokes if it can't find the named alerts file.
That's also the reason why I removed the conditional creation of the alerts file and create it always (even if empty). Otherwise the dependency would have to be conditional on the alert file's creation and the prometheus.yaml.erb template would also have to be changed to not always reference a possibly non-existent alerts file.
In summary: it just felt like the path of least resistance 😉
This is needed to break a dependency cycle involving alert.rules and the main config. Minor breaking change: if anyone used the purge argument directly to prometheus::config (instead of the prometheus class itself), the call will have to be adjusted.
Add validation to config changes
This PR adds a symlink to promtool in the same way a symlink to prometheus is currently added to /usr/local/bin.
Than it uses File's
validate_cmdto call promtool and avoid replacing a configuration file with a broken version, both for the central config and for the alert rules.Finally, also fix the first error thrown by the config check: we create the alerts file even if no alerts were provided. The empty file won't trigger a syntax error and also doesn't bother prometheus itself.
Fixes #31