-
-
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
remove inherit from exporters to not trigger a install of prometheus daemon #187
Conversation
HI @blupman, thanks for the PR. I really like the adjustments to the hiera data and the updated datatypes. However, I'm not sure if (lookup) is the right approach. Shouldn't the main class simply have a parameter like |
Hi @bastelfreak, If the main class is required to be loaded, should we add a This would make a breaking change for the current puppet forge module users, if One other sollution would be to have a sepporate prometheus 'server' sub class I'm not sure of a sollution, to make the prometheus puppet module behave the So thats actualy why i'm not sure on how to proceed. |
I need to do some local testing, but I think it was possible in the past to install an exporter without the prometheus server. I've 4 boxes here that only have the node_exporter. |
I debugged this. Turns out I broke it during a refactoring. Example: https://github.com/voxpupuli/puppet-prometheus/pull/178/files#diff-589781ae49e81337f6a53b370d04c73dR164 |
Thanks for your work! Shall i change my pull request to sollution you approve? (and if yes, how shall i proceed?) |
@blupman I think the best idea is to introduce a Boolean |
I'm going to close this is favour of #194 |
I have run into issue #184 and have removed the inherits from a few exporters to test our theory in no longer automaticaly 'inheriting' the prometheus daemon, upon installing a exporter.
If this is a acceptable sollution, i can expand the merge request to change all the other exporters in the same way.
Thanks,
Niels Jansen
…
Pull Request (PR) description
This Pull Request (PR) fixes the following issues