Do not set a value for the collectd_version fact if collectd is not y…

[m3t30r]

…et installed

Right now the value of that fact is set to an empty string when collectd is not installed, so on an initial run on a new machine the code in init manifest

$collectd_version_real = pick($::collectd_version, $minimum_version) 

will set $collectd_version_real to an empty string, and as a result, on an initial run the modules that have templates that reference this value will be broken. Not setting this value in the fact will make $collectd_version_real to take the value of $minimum_version which I believe is the intended behaviour. The second run will fix the problem, but it means you must run puppet twice for the initial provisioning.

Another question if should we transform the referenced line in

$collectd_version_real = pick($minimum_version,$::collectd_version) 

so that $collectd_version_real will always have the value of $minimum_version if $minimum_version is defined. This will also provide us a (strange) fix for the problem, but I'm not sure if it's the intended behaviour.

I tested if it works :

[root@vagrant ~]# puppet facts --modulepath /tmp/vagrant-puppet/modules-9be8ec6dcc5d06cd00bef0a0f1c5eaa0/ | grep collectd
[root@vagrant ~]# yum install -y -q collectd
[root@vagrant ~]# puppet facts --modulepath /tmp/vagrant-puppet/modules-9be8ec6dcc5d06cd00bef0a0f1c5eaa0/ | grep collectd
    "collectd_version": "5.5.2.48.g55ef277",
[root@vagrant ~]# cat /tmp/vagrant-puppet/modules-9be8ec6dcc5d06cd00bef0a0f1c5eaa0/collectd/lib/facter/collectd_version.rb 
# Fact: collectd_version
#
# Purpose: Retrieve collectd version if installed
#
# Resolution:
#
# Caveats:  not well tested
#
Facter.add(:collectd_version) do
  setcode do
    if Facter::Util::Resolution.which('collectd')
      collectd_help = Facter::Util::Resolution.exec('collectd -h')
      %r{^collectd ([\w\.]+), http://collectd\.org/}.match(collectd_help)[1]
    end
  end
end

[m3t30r]

I changed the test so it will pass with the new behaviour.

[oranenj]

This looks fine to me, but I don't know if there's a reason the fact has been an empty string before.

[bastelfreak]

Hi @m3t30r. thanks for the PR! Can you please rebase against latest master and take a look at the used email address in your commits? It isn' associated with your github account. You can ping us 24/7 in the IRC channel #voxpupuli if you've any questions.

[bastelfreak]

Ping @m3t30r

[wyardley]

Not sure about the email address, but there are no conflicts, and this looks sensible to me.