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

Collectd plugin fix #935

Closed

Conversation

MichalRebisz
Copy link
Contributor

This is a fix for an issue encountered when trying to use puppet on tripleo deployment with mcelog and snmp_agent plugin.

The deployment would fail pointing at snmp_agent.pp and mcelog.pp. Changed the puppet files and templates to be more in line with the other plugins.

@@ -44,10 +44,7 @@

collectd::plugin { 'snmp_agent':
ensure => $ensure,
content => epp('collectd/plugin/snmp_agent.conf.epp', {
'data' => $data,
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand how this makes a difference. The variable $data and also $table are available in this context. Can you please provide an acceptance test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure ill write some additional tests. As for how this makes a difference im not entirely sure I understand it myself to be honest. As far as I can make sense of it taking into account the error im getting. It looks like when it tries to get the $data out of this context it ends up being null for some reason and although data and table are optional in the template it break. So I tried to work around this by removing this part and getting the variables by a full path in the template. This seems to work correctly both on tripleo and the existing tests on puppet.

@MichalRebisz
Copy link
Contributor Author

After updating to puppet-collectd 12.0 on tripleo this seems no longer to be an issue. I will close this PR since it looks like the issue might have been caused by something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants