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

Feature/multiple rules files #166

Merged
merged 9 commits into from
Feb 26, 2018

Conversation

zipkid
Copy link
Member

@zipkid zipkid commented Feb 23, 2018

Allow the use of multiple alert rules files.

@@ -158,6 +158,7 @@
Array $remote_read_configs = $::prometheus::params::remote_read_configs,
Array $remote_write_configs = $::prometheus::params::remote_write_configs,
$alerts = $::prometheus::params::alerts,
$extra_alerts = {},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a datatype for this new parmeter and documentation in the header.

@@ -11,29 +11,32 @@
# [*alertfile_name*]
# Filename to use when storing the alerts file
#
class prometheus::alerts (
String $location,
define prometheus::alerts (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please write a spec file with unit tests for this new defined resource?

Variant[Array,Hash] $alerts,
String $alertfile_name = $prometheus::params::alertfile_name,
) inherits prometheus::params {
String $location = "$::prometheus::config_dir/rules",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please adjust the documenation in the header

@bastelfreak
Copy link
Member

Hi @zipkid , thanks for the PR. I added some inline comments.

@zipkid zipkid force-pushed the feature/multiple_rules_files branch from e241bb1 to 0446853 Compare February 26, 2018 09:52
@zipkid
Copy link
Member Author

zipkid commented Feb 26, 2018

@bastelfreak Comments handled and spec test added.

) inherits prometheus::params {
if ( versioncmp($::prometheus::version, '2.0.0') < 0 ){
file { "${location}/${alertfile_name}":
String $location = "${::prometheus::config_dir}/rules",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datatypes \o/

@bastelfreak
Copy link
Member

This looks good to me. @tuxmea can you review this as well?

@tuxmea
Copy link
Member

tuxmea commented Feb 26, 2018

Needs a major version bump as interface changes from class to define.

@zipkid
Copy link
Member Author

zipkid commented Feb 26, 2018

I am willing to bump the version but unsure about the actions / steps to do it....

@bastelfreak
Copy link
Member

@zipkid, don't worry. We will deal with this in our release workflow.

@bastelfreak bastelfreak merged commit 668d7d8 into voxpupuli:master Feb 26, 2018
@zipkid zipkid deleted the feature/multiple_rules_files branch February 27, 2018 06:30
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants