Change to the if !defined puppetlabs-apache 'pattern'

[RobLuckePuppet]

I'm new to puppet, so forgive me if this is a misunderstanding. Is the documentation for the module

collectd::plugin::write_graphite::carbon {'my_graphite':
  graphitehost   => 'dev2-mono-metrics',
  graphiteport   => 2003,
  graphiteprefix => 'collectd.',
  protocol       => 'tcp',
}

on the web documentation for the module missing a "class" designator? Before I added it, the configuration was not written and I was able to break the "puppet agent -t" command. I was just blithely cutting and pasting from the documentation. (-;]>

The error message was very confusing, so it took a while to track things down:

# puppet agent -t
Info: Using configured environment 'production'
Info: Retrieving pluginfacts
Info: Retrieving plugin
Info: Loading facts
Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Class[Collectd] is already declared; cannot redeclare at /etc/puppetlabs/code/environments/production/manifests/collectd.pp:3 at /etc/puppetlabs/code/environments/production/manifests/collectd.pp:3:1 on node dev2-mono-metrics.us-west-2.compute.internal
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

My definition triggering the error is the example above. At the same time, /var/log/messages shows:

Jun 15 22:09:26 dev2-mono-metrics puppet-agent[5491]: Could not retrieve catalog; skipping run
Jun 15 22:09:30 dev2-mono-metrics puppet-agent[5558]: Using configured environment 'production'
Jun 15 22:09:30 dev2-mono-metrics puppet-agent[5558]: Retrieving pluginfacts
Jun 15 22:09:30 dev2-mono-metrics puppet-agent[5558]: Retrieving plugin
Jun 15 22:09:31 dev2-mono-metrics puppet-agent[5558]: Loading facts
Jun 15 22:09:32 dev2-mono-metrics puppet-agent[5558]: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Class[Collectd] is already declared; cannot redeclare at /etc/puppetlabs/code/environments/production/manifests/collectd.pp:3 at /etc/puppetlabs/code/environments/production/manifests/collectd.pp:3:1 on node dev2-mono-metrics.us-west-2.compute.internal
Jun 15 22:09:32 dev2-mono-metrics puppet-agent[5558]: Not using cache on failed catalog
Jun 15 22:09:32 dev2-mono-metrics puppet-agent[5558]: Could not retrieve catalog; skipping run

[bzed]

de1d0ca95c3a4e5f09ea29843219abde2e277db9 is the reason for that - including ::collectd everywhere is not such a good idea. Especially as it breaks the way to use the module as described in README.md...

[bzed]

@jyaworski any ideas on that? at least a documentation update is necessary...

[crimpy88]

@RobLuckePuppet - You have probably solved this one by now. Without seeing the full contents of /etc/puppetlabs/code/environments/production/manifests/collectd.pp it is a little difficult, but will attempt to answer it anyhow.

Going with the assumption that the file looks/looked somewhat like this...

class collectd {

  collectd::plugin::write_graphite::carbon {'my_graphite':
    graphitehost   => 'dev2-mono-metrics',
    graphiteport   => 2003,
    graphiteprefix => 'collectd.',
    protocol       => 'tcp',
  }

}

The issue that you will run into here is that you are re-declaring the collectd class at the head of your file.

What you want it to look like instead (swap profiles to something useful for you):

class profiles::collectd {
  include ::collectd

  ::collectd::plugin::write_graphite::carbon {'my_graphite':
    graphitehost   => 'dev2-mono-metrics',
    graphiteport   => 2003,
    graphiteprefix => 'collectd.',
    protocol       => 'tcp',
  }

}

Please add more context if that has not solved it for you.

[bzed]

@crimpy88 thats still broken way beyond what I can think, it basically makes it imposible to use collectd in some larger setup. Here is a smallish example for you:

foo/bar1.pm:

class foo::bar1 {
include ::collectd::plugin::ntpd
}

foo/bar2.pm:

class foo::bar2 {
include ::collectd::plugin::irq
}

foo/fuzz.pm:

class fuzz {
  class { 'collectd' :
  }
}

foo.pm:

class foo {
  include ::foo::fuzz
  include ::foo::bar1
  include ::foo::bar2
}

and things will break badly.

The case of having all definitions in one file is a rare one, probably nobody in largish seups would use that.

[crimpy88]

@bzed - I must be missing something, as I just created the files just as you have laid out and it installed as expected.

# pwd
/etc/collectd.d
# ls -la
total 24
drwxr-x---.   2 root root  4096 Sep 23 23:40 .
drwxr-xr-x. 105 root root 12288 Sep 23 20:25 ..
-rw-r-----.   1 root root   117 Sep 23 23:40 10-irq.conf
-rw-r-----.   1 root root   173 Sep 23 23:40 10-ntpd.conf

[bzed]

Which puppet version are you using? and does it work if you run it several times? and/or can you pass different params to the collectd class?

[crimpy88]

v3.8.7 Just ran it 5x in a row without issue. Yes, I am able to pass different params to it.

[alexjfisher]

@bzed You can declare a class using include-like statements multiple times, but if you declare it using the resource-like way, you can't (or at least you've got to do this only once and before any other includes of the same class). https://docs.puppet.com/puppet/latest/reference/lang_classes.html#include-like-vs-resource-like

So actually, your example in https://github.com/voxpupuli/puppet-collectd/issues/520#issuecomment-249191717 should work, but wouldn't if you reordered the includes in foo eg

class foo {  
  include ::foo::bar1
  include ::foo::bar2
  include ::foo::fuzz #eek this won't work as collectd class has already been included.
}

Change fuzz to ..

class fuzz {
  include ::collectd
}

and it should be fine. If you need to provide parameters to collectd, you can use hiera instead of the resource-like declaration. Does this help?

[bzed]

No, because I can't use hiera there. Too much stuff that needs to be fetched/generated/calculated.

[bzed]

(I can't even guarantee the inclusion order as I'd have to change like most of our puppet modules do be calle din some specific order. It worked perfectly fine before - also using 'include collectd' at the places where it is used now doesn't make sense as the collectd class might not even be evaluated at that time - you'd have to use 'require' - and doing so is even more... crap.)

[ffrank]

Perhaps we should switch the whole module to this, er, "pattern".

[miksercz]

I second removing the includes from all classes and instead going the way puppetlabs-apache went, as @ffrank suggested.

We just upgraded to the newest version of collectd and going through our collectd plugin calls, adding require => Class['collectd'] on all of them isn't fun :)