-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
new feature - consul_exporter #36
Conversation
pavloos
commented
May 11, 2017
- added consul_exporter
bump |
Hello, Can this PR be looked into please ? |
This looks good to me. @bastelfreak any issues with me merging? |
manifests/consul_exporter.pp
Outdated
} | ||
|
||
$real_download_url = pick($download_url,"${download_url_base}/download/v${version}/${package_name}-${version}.${os}-${arch}.${download_extension}") | ||
validate_bool($purge_config_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @pjfbashton, could you replace those legacy functions with native puppet datatypes?
# [*web_telemetry_path*] | ||
# Path under which to expose metrics. (default "/metrics") | ||
class prometheus::consul_exporter ( | ||
String $arch = $::prometheus::params::arch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
datatypes \o/
@pjfbashton thanks for the datatypes. Can you add at least a little unit test that verifies if the catalog compiles? |
@bastelfreak Oh, those tests failed a while ago, have changed anything ? I think there is a problem with prometheus service - it does not start (at least on Centos 7) on the first run with default settings (just I can try adding unit test although it is new to me, have you got an example I could take a look at? |
One example is at: https://github.com/voxpupuli/puppet-prometheus/blob/master/spec/classes/beanstalkd_exporter_spec.rb I am not really sure why the acceptance tests fail randomly. I wasn't able to reproduce this locally. Sometimes they work, sometimes they don't :( |
Failed again on prometheus service not starting after first run. The same happens for me locally...
|
all green now @bastelfreak |
looks good to me. thank you for the PR. |
new feature - consul_exporter