$rule_files parameter not respected

[kamusin]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 4.10.9
• Ruby:2.3.6
• Distribution: Ubuntu 16.04
• Module version: 5.0.0

How to reproduce (e.g Puppet code you use)

I declared in the init.pp manifest file the following code, which is pretty straightforward.

class { 'prometheus':
    rule_files        =>  ['/etc/prometheus/testing.rules'],
    scrape_configs    => $scrape_configs,
    extra_options     => $extra_options_real,
    localstorage      => "${data_dir}/prometheus",
    storage_retention => $retention,
  }

after running Puppet on the host, I couldn't see any change related to the parameter $rule_files. Furthermore, the resulting prometheus configuration has set an empty array:

What are you seeing

I see no changes in regards of $rule_files parameters, and looks like this parameter is not in use at all. The resulting yaml configuration shows only:

...
rule_files: []
...

if you read line 194-224:

$extra_rule_files = suffix(prefix(keys($extra_alerts), "${config_dir}/rules/"), '.rules')

  if ! empty($alerts) {
    ::prometheus::alerts { 'alert':
      alerts   => $alerts,
      location => $config_dir,
    }
    $_rule_files = concat(["${config_dir}/alert.rules"], $extra_rule_files)
  }
  else {
    $_rule_files = $extra_rule_files
  }

-> class { '::prometheus::config':
    global_config        => $global_config,
    rule_files           => $_rule_files,
    scrape_configs       => $scrape_configs,
    remote_read_configs  => $remote_read_configs,
    remote_write_configs => $remote_write_configs,
    config_template      => $config_template,
    storage_retention    => $storage_retention,
  }

You can notice that actually the value is calculated from $_rule_files

What behaviour did you expect instead

We are currently using version 3.1.0 in production|staging|dev but we want to upgrade to 5.0.0, however the first step is being able to set the rules parameter since at the moment is loading prometheus without any of our current rules, so I expect to have our rules in place, in a format such as:

rule_files:
- "/etc/prometheus/rules/alerts-service1/*.rules"
- "/etc/prometheus/rules/common/*.rules"

ps: there is another issue in regards of importing rules using that style, see golang/go#11862 for further information, but at the moment it works for us.

Please let me know if you need something from this side.
cheers!

[bastelfreak]

Hi @kamusin, thanks for bringing this up and the detailed explanation. Are you able to provide a patch for this?

[kamusin]

I'm working on a fix @bastelfreak, sorry for the delayed answer by the way.

[kamusin]

For now we are dealing this issue with this branch (https://github.com/kamusin/puppet-prometheus/tree/update-rule-files) since we do have hundreds of rules that need to be migrated (plus testing on the exporters)

the resulting prometheus.yaml configuration now looks like this:

--- /tmp/prometheus.yaml.original	2018-04-13 01:27:06.169501585 -0400
+++ /etc/prometheus/prometheus.yaml	2018-04-13 01:51:52.645730402 -0400
@@ -7,8 +7,9 @@
   scrape_interval: 30s
   scrape_timeout: 15s
 rule_files:
-- "/opt/prometheus/rules/something/*.rules"
-- "/opt/prometheus/rules/common/*.rules"
+- "/etc/prometheus/rules/*.rules"
+- "/etc/prometheus/rules/something/*.rules"
+- "/etc/prometheus/rules/common/*.rules"

[tuxmea]

@kamusin can you rebase? Code is now in server.pp.

[kamusin]

thanks I will try to test this on our dev environment first

[bramblek1]

Any updates @kamusin ? - I believe I have this working - but getting the tests to pass is difficult. @tuxmea the empty array for rule_files in spec/fixtures/files/prometheus*.yaml should be replaced with what is specified in data/defaults.yaml ?

[kamusin]

@bramblek1 sorry for not leaving any updates in a while I just pushed a branch to test version 6.0.6 (we are still on 4.1.1) in our dev environment (this time for real).

[bramblek1]

@kamusin - please take a look at https://github.com/bramblek1/puppet-prometheus/tree/180_rule_files_param

We made similar changes - I think possibly yours does not support the definition of $extra_alerts.

[kamusin]

okay this might require some extra work from my side since the params.pp file it's gone and has broken a few spec tests, and it's also causing some dependency cycles too.

[modules]       # --- Caused by: ---
[modules]       # Puppet::Error:
[modules]       #   Could not find class ::prometheus::params for i-1212121212.us-west-2.compute.internal
[modules]       #   /usr/local/bundle/gems/puppet-5.5.2/lib/puppet/parser/compiler.rb:373:in `block in evaluate_classes'

[bramblek1]

Isn't params gone now ? All the module hiera stuff has gotten rid of it. Have you rebased ? or are you trying to fix the older version?

[bastelfreak]

@bramblek1 correct. We migrated all logic from the params.pp file to hiera.

[kamusin]

@bramblek1 am running your Branch agains't our CI (sorry I assumed that your branch got merged into master which is not the case).

[kamusin]

this is going to take a while (we have a lot of references from prometheus::params in our code.

Finished in 22.75 seconds (files took 3.73 seconds to load)
24 examples, 21 failures

[bramblek1]

@kamusin - I think it would make more sense to test this change in isolation using the CI for this repo, rather than by proxy. If your code is referencing prometheus::params - your issue is that v 6.x is already incompatible. Should I just send this as a PR instead or do you want to rebase yours?

[kamusin]

feel free to send a PR @bramblek1 , yeah I have tons of references in our repository to prometheus::params, unfortunately.