@collectd_version not work in Plugin templates

[azhurbilo]

@collectd_version not work in Plugin templates as @ means local scope.
Now all tests works only because of it's hardcoded collectd_version as fact!

https://github.com/azhurbilo/puppet-collectd/blob/master/templates/plugin/cpu.conf.erb#L1

<% if @collectd_version and (scope.function_versioncmp([@collectd_version, '5.5']) >= 0) -%>

Should looks like

<% if scope['collectd::collectd_version'] and (scope.function_versioncmp([scope['collectd::collectd_version'], '5.5']) >= 0) -%>

Or add "collectd_version" variable to manifests/plugin.pp

[ortegaga]

Hi,
I've faced this same issue when realized that collectd was inquiring apache instances every 10s (globally defined in 'collectd.conf') instead of the configured via hiera:

collectd::plugin::apache::interval: '300'

The problem is that the content of the generated config file is:

LoadPlugin apache

but the correct content should be:

<LoadPlugin apache>
  Globals false
  Interval 300
</LoadPlugin>

Following @azhurbilo indications, I've tested both approaches:

• replace all occurrences of @collectd_version with scope['collectd::collectd_version'] in all the templates. This leads to 36 out of 376 tests failed after executing:

bundle exec rake test

• added collectd_version variable to manifests/plugin.pp. Instead, this leads to 23 out of 376 tests failed.

and both "worked for me"™ (tested in versions 5.4.1 and 5.5.0) but no PR possible because of the failed tests. If anyone want to try:
https://github.com/ortegaga/puppet-collectd/tree/fix/377_1
https://github.com/ortegaga/puppet-collectd/tree/fix/377_2

[ortegaga]

Sorry, I didn't notice there is already the PR #374 with the first approach.

[jyaworski]

This seems to have been closed.

[azhurbilo]

@jyaworski, looks like PR #393
didn't solve idempotency issue.
We need reopen this issue

[jyaworski]

collectd_version is now a fact, and should be universally available.

[azhurbilo]

yes, but this fact is available only on second puppet apply (when collectd package installed).
It will be good to get minimum_version until collectd_version fact is nil.

[jyaworski]

#398 #305 and others all point to this problem, and I have an idea for a solution.

Thinking about @azhurbilo's suggestion, I propose we set a conservative $minimum_version on the first run according to the operating system. On the other hand, we can simply say we support collectd 4.10 and onward, which covers almost all operating systems I can think of. Therefore, we can set $minimum_version to be 4.10 as a default, and use it in place of collectd_version if collectd_version is undefined.

Would that solve this?

Tagging @daenney and @igalic

[jyaworski]

Hello:

Look at #412 please.

[jyaworski]

Closing due to #412 fixing this. Please re-open if it doesn't.