add postgres exporter #236
Conversation
manifests/postgres_exporter.pp
Outdated
| String $download_extension, | ||
| String $download_url_base, | ||
| Array[String] $extra_groups, | ||
| String $group, |
There was a problem hiding this comment.
can you update the String datatype to String[1] on all locations where it makes sense? (It enforces a minimum string length of 1).
There was a problem hiding this comment.
I've added quite a few [1]'s to where it makes sense. also some minor cleanups, in de comments
| end | ||
|
|
||
| describe 'install correct binary' do | ||
| it { is_expected.to contain_file('/usr/local/bin/postgres_exporter').with('target' => '/opt/postgres_exporter-0.4.6.linux-amd64/postgres_exporter') } |
There was a problem hiding this comment.
please also add something like this:
it { is_expected.to compile.with_all_deps }
There was a problem hiding this comment.
added, a few tests.
I used redis_exporter as a starting point, which is about the only spec test that doesn't do this, shall i create an other pull request enhanching the redis_exporter?
bastelfreak
added
enhancement
|
hey @blupman, thanks for the PR! I did some little inline comments, but all in all this looks good. Travis is currently having some issues, but I hope that will be fixed soon. |
data/defaults.yaml
Outdated
| @@ -253,6 +253,18 @@ prometheus::blackbox_exporter::package_name: 'blackbox_exporter' | |||
| prometheus::blackbox_exporter::modules: {} | |||
| prometheus::blackbox_exporter::config_file: '/etc/blackbox-exporter.yaml' | |||
| prometheus::blackbox_exporter::version: '0.7.0' | |||
| prometheus::postgres_exporter::data_source_uri: 'host=/var/run/postgresql/ sslmode=disable' | |||
| prometheus::postgres_exporter::postgres_pass: 'postgres_exporter_password' | |||
| prometheus::postgres_exporter::postgres_user: 'postgres_exporter_username' | |||
There was a problem hiding this comment.
should postgres_user and postgres_pass default to undef? Might makes it more obvious to users that they need to change it.
There was a problem hiding this comment.
agreed,
i had to add the postgres_pass and postgres_pass to the spec test, to make the daemon start
manifests/postgres_exporter.pp
Outdated
| class prometheus::postgres_exporter ( | ||
| String[1] $download_extension, | ||
| String[1] $download_url_base, | ||
| Array[String] $extra_groups, |
There was a problem hiding this comment.
Can you please update this to Array[String[1]]? That will enforce a minimum string length of 1.
manifests/postgres_exporter.pp
Outdated
| Boolean $purge_config_dir = true, | ||
| Boolean $restart_on_change = true, | ||
| Boolean $service_enable = true, | ||
| String $service_ensure = 'running', |
There was a problem hiding this comment.
for all the other String datatypes here: Please ensure a minimal String length of 1.
There was a problem hiding this comment.
now its done for all non optional strings.
|
@blupman can you recheck again please? I made some more inline comments. |
|
@blupman can you take a look at the failing spec tests? |
|
i should work now, i found some time to make the non acceptance
spectests work localy, now invest a bit of time in makine acceptance
tests work locally
…On 30/07/2018 12:58, Tim Meusel wrote:
@blupman <https://github.com/blupman> can you take a look at the
failing spec tests?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#236 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEtG4g6v3CzNrPvPCQKl2paL89sFWs6Jks5uLua8gaJpZM4VTFU2>.
|
|
Thanks! |
| # Service startup scripts style (e.g. rc, upstart or systemd) | ||
| # | ||
| # [*install_method*] | ||
| # Installation method: url or package (only url is supported currently) |
There was a problem hiding this comment.
only url is supported currently. Is that true?
There was a problem hiding this comment.
I'm not aware of a packaged version of postgres_exporter, if it exsist, its not tested.
* add postgres exporter * spec fixes * spec fixes * default to no env vars, and extra input checks * add postgres exporter * add pointer to postgres_exporter module * stricter data typing, spectest enhanchements * tuning spec tests * change spec tests * fix rubocop. * update to follow suggestions of review * add a comma, spec fixing.. * set required params in acceptance test.
* add postgres exporter * spec fixes * spec fixes * default to no env vars, and extra input checks * add postgres exporter * add pointer to postgres_exporter module * stricter data typing, spectest enhanchements * tuning spec tests * change spec tests * fix rubocop. * update to follow suggestions of review * add a comma, spec fixing.. * set required params in acceptance test.
Pull Request (PR) description
I have added a postgres_exporter module, the module is installing https://github.com/wrouesnel/postgres_exporter
This Pull Request (PR) fixes the following issues