voxpupuli / puppet-zabbix

Puppet module for creating and maintaining zabbix components with puppet.
https://forge.puppet.com/puppet/zabbix
Apache License 2.0
80 stars 228 forks source link

The zabbix_template_host doesn't appear to be idempotent #360

Open Sher-Chowdhury opened 7 years ago

Sher-Chowdhury commented 7 years ago

Hello,

In my puppet code I've written:

  $zabbix_templates.each |String $zabbixtemplate| {
    zabbix_template_host { "${zabbixtemplate}@${fqdn}" :
      ensure      => 'present',
      zabbix_user => $zabbix_username,
      zabbix_pass => $zabbix_password,
      zabbix_url  => $zabbix_url,
    }
  }

The above code works, However it doesn't appear to be idempotent. That is, the template linking occurs during the initial puppet run, however on successive puppet runs, puppet claims that the link has been created over and over again.

bastelfreak commented 7 years ago

@Sher-Chowdhury thanks for rising up this issue!


@alexjfisher can you take a look at this?

alexjfisher commented 7 years ago

@Sher-Chowdhury Hi! I'm not sure how much help I can be. I've not used this type and don't have a zabbix system to hand anymore to test against. In the provider code, exists? must be returning false where it should be returning true. Could you try adding some debug to confirm the hostid and template_id methods are returning sensible values (and not just nil for example).

@bastelfreak It was zabbix_template I spent time enhancing, not zabbix_template_host.

Sher-Chowdhury commented 7 years ago

Hi @alexjfisher - Hope you're doing ok. That's what I was thinking too. It looks like this line doesn't appear to work:

https://github.com/voxpupuli/puppet-zabbix/blob/master/lib/puppet/provider/zabbix_template_host/ruby.rb#L38

This should be returning a boolean value. The following should output an array:

 zbx.templates.get_ids_by_host(hostids: [hostid])

Then the next part:

zbx.templates.get_ids_by_host(hostids: [hostid]).include?(template_id.to_s)

should test whether a particular value is in the array and return a boolean.

I wrote a simple script that would test this:

$ cat zabbix_template_link_checker.rb
require "zabbixapi"

zbx = ZabbixApi.connect(
  :url => 'https://xxxxxx/api_jsonrpc.php',
  :user => 'xxxxxx',
  :password => 'xxxxxxx'
)

hostid = 10634

array_of_template_ids_attached_to_host = zbx.templates.get_ids_by_host(hostids: [hostid])

puts 'Valid list of templates that are currently linked to host, 10634:'
puts array_of_template_ids_attached_to_host

puts 'Test that returns: true'

template_id = 10296
result = zbx.templates.get_ids_by_host(hostids: [hostid]).include?(template_id.to_s)
puts result

puts 'Test that returns: false'

template_id = 1000000000
result = zbx.templates.get_ids_by_host(hostids: [hostid]).include?(template_id.to_s)
puts result

puts 'end of testing'

exit

When I run this script, I get:

$ ruby zabbix_template_link_checker.rb
Valid list of templates that are currently linked to host, 10634:
10105
10211
10243
10296
10307
10317
Test that returns: true
true
Test that returns: false
false
end of testing

So at first sight it looks like this line doesn't work, but when I actually test it, then I find that it does work!

However I'll also try enabling debug, as you have suggested in case it sheds more light on this issue. Thanks Alex.

petems commented 7 years ago

Hello all, so I did a bit of digging on this.

The original issue was that the Host Template didn't exist on the master. So, when @Sher-Chowdhury renamed the template, it was added in and was idempotent afterward.

So the idempotency was a red herring: It's actually an issue that the template name didn't exist, and the API didn't return an error for a non-existent template ID was tried to be added: The API response returns true regardless:

[2] pry(#<Puppet::Type::Zabbix_template_host::ProviderRuby>)> zbx.templates.get_id(host: 'dont-find-me')
=> nil
[3] pry(#<Puppet::Type::Zabbix_template_host::ProviderRuby>)> zbx.hosts.get_id(host: 'dont-find-me')
=> nil
[4] pry(#<Puppet::Type::Zabbix_template_host::ProviderRuby>)> zbx.templates.mass_add(hosts_id: [nil],templates_id: [nil])
=> true
[1] pry(#<ZabbixApi::Templates>)> exit

This is actually part of a larger issue with the Zabbix API handling malformed JSON incorrectly: https://support.zabbix.com/browse/ZBX-11906 https://support.zabbix.com/browse/ZBX-3783

In the meantime, a patch like this should make things better:

def template_id
    zbx = connect
    raise Puppet::Error, "Zabbix API could not find Template ID from template_name #{template_name}" if zbx.templates.get_id(host: template_name).nil?
    @template_id ||= zbx.templates.get_id(host: template_name)
  end

  def hostid
    zbx = connect
    raise Puppet::Error, "Zabbix API could not find Host ID from hostname #{hostname}" if zbx.templates.get_id(host: hostname).nil?
    @hostid ||= zbx.hosts.get_id(host: hostname)
  end

PR incoming