Puppet cleaning templates on each run

[fgallese]

So, I've noticied the following issue:

Scenario:

A host is created using zabbix::agent, which exports a Zabbix_host resource. In this host you specify a default set of basic templates.

Example:

class { 'zabbix::agent':
  server => '192.168.20.11',
  zbx_templates => ['Template OS Linux', 'Template App SSH Service'],
}

You ALSO define some userparameters (one or more) for this host, in order to set some other templates with their configuration.

Example:

zabbix::userparameters { 'facter':
  content   => 'UserParameter=facter_facts,facter --json',
  template => 'Template Facter',
}

On the zabbix server, you define manage_resources to true.

What is happening:

Every time puppet is run on the zabbix server (which collects this resources), the host has all it's userparameter templates unlinked (and cleared) and then linked again.

Notice: /Stage[main]/Zabbix::Resources::Web/Zabbix_host[my.host.fqdn]/templates: templates changed ['Template OS Linux', 'Template App SSH Service', 'Template Facter'] to ['Template OS Linux', 'Template App SSH Service']

What this causes, is that every time puppet runs on the zabbix server, you lose all items and all history of those items.

Why is this happening:

At first, I thougt that the exists check on the Zabbix_host provider wasn't working and the host got created each time, but then I realized that only the templates added by Zabbix_userparameters got unlinked.

Apparently, the problem comes from this piece of code:

def templates=(array)
    should_template_ids = get_templateids(array)

    # Get templates we have to clear. Unlinking only isn't really helpful.
    is_template_ids = zbx.query(
      method: 'host.get',
      params: {
        hostids: @property_hash[:id],
        selectParentTemplates: ['templateid'],
        output: ['host']
      }
    ).first['parentTemplates'].map { |t| t['templateid'].to_i }
    templates_clear = is_template_ids - should_template_ids

    zbx.query(
      method: 'host.update',
      params: {
        hostid: @property_hash[:id],
        templates: transform_to_array_hash('templateid', should_template_ids),
        templates_clear: transform_to_array_hash('templateid', templates_clear)
      }
    )
  end

I added some debugging there and this is the output I got in one run:

-----In templates, should_template_ids:
10001
10102
-----In templates, is_template_ids:
10001
10102
10274
-----In templates, templates_clear:
10274

So, clearly, this is removing the templates that are not present in the Zabbix_host definition, causing the removal of the templates added by Zabbix_userparameters. Then, when the resources Zabbix_userparameters are collected, the templates are linked again.

Proposed solution:

It seems to me, that this function should never remove templates from a host.

Of course, the best scenario would be for it to know about the templates added by Zabbix_userparameters and only removing the extra ones (for example the ones added manually), but I cannot think of a way of doing it.

Therefore, I think the only solution here is for Zabbix_host to never unlink templates, only add the new ones that are missing and should be there. This is the same concept applied with the hosts: if a host is in zabbix but it is not managed by puppet (meaning 'is not exported to puppetdb') then it is NOT removed from zabbix.

What do you guys think about this ? I tryied looking for an issue mentioning this scenario, but I didn't find much.. if I missed something please let me know.

If you agree to this solution, I can provide with a PR that will edit this function in order only add the templates that the Zabbix_host defines and are not there, but never clear any existing linked templates.

[bastelfreak]

@fgallese thanks for reporting this. @baurmatt can you have a look please?

[alexjfisher]

@fgallese Yes, thank you for the very detailed feedback and offer to author a fix.

Like @bastelfreak, I also thought of pinging @baurmatt as he’s been the most involved in this module recently and probably knows it better than anyone else around at this time.

[baurmatt]

But hes on holiday for another week and sadly chilling in the malaysian sun comes first! ;) Sorry if I broke something, will have a look after 10th of March.

[alexjfisher]

@baurmatt Enjoy! Come back rested! ;)

[baurmatt]

So... back from holiday... :)

It seems to me, that this function should never remove templates from a host. ... Therefore, I think the only solution here is for Zabbix_host to never unlink templates, only add the new ones that are missing and should be there.

May I disagree: For me this is valid, as I want to ensure that only the defined templates on zabbix_host are configured for it in Zabbix. If I remove a template from it, I also want it to be removed in Zabbix. Of course this collides with your use-case of zabbix::userparameter/zabbix_userparameters.

I see a couple of options how to fix this:

• zabbix_host gets an additional parameter "templates_purge" (?) which allows to control hot it handles templates
• We create a zabbix_host_template which replaces zabbix_userparameters. zabbix_host_template is a dummy resource which gets later collected and used by zabbix_host. A little bit like concat.

[fgallese]

@baurmatt hope you had a great time !

Thank you for your time checking this.

May I disagree: For me this is valid, as I want to ensure that only the defined templates on zabbix_host are configured for it in Zabbix. If I remove a template from it, I also want it to be removed in Zabbix.

While I understand and agree with your point, I think that then there must be some warning in the documentation mentioning that any change to this (including manual changes made with the Zabbix Web GUI) will be removed and purged on next puppet run (when managed resources set to true). If it's already mentioned please ignore me, I just didn't find it.

This is important because unlinking the templates will purge all history of the metrics it contains.. I imagine someone manually linking themplates using the web interface and then losing all history on a puppet run.. it can be a bad day.

More over, this is also happening with hostgroups, you are purging all hostgroups not defined on the Zabbix_host resource. This should at least be optional.

In my use case, I use the HostMetaData setting in order to properly assign groups to hosts with Auto Registration Rules in Zabbix, but puppet is removing them.

All things considered, I think the easiest solution is to add some kind of "strict => true/false" setting to be used when "manage_resources" is set to true.

[baurmatt]

I think that then there must be some warning in the documentation mentioning that any change to this (including manual changes made with the Zabbix Web GUI) will be removed and purged on next puppet run (when managed resources set to true)

I think that's a good idea.

More over, this is also happening with hostgroups, you are purging all hostgroups not defined on the Zabbix_host resource. This should at least be optional.

In my use case, I use the HostMetaData setting in order to properly assign groups to hosts with Auto Registration Rules in Zabbix, but puppet is removing them.

All things considered, I think the easiest solution is to add some kind of "strict => true/false" setting to be used when "manage_resources" is set to true.

I'm not sure if we should do this. The reason for this is: If you decide to manage your things (not limited to this module) with Puppet, there shouldn't be other (Auto Registration Rule, people doing things manually, ...) stuff manipulating the same parameters. This will also lead to unpredictable behaviors. That's why I'm voting to implemented this:

We create a zabbix_host_template which replaces zabbix_userparameters. zabbix_host_template is a dummy resource which gets later collected and used by zabbix_host. A little bit like concat.

That would ensure that zabbix_userparameters works again with zabbix_host.

[fgallese]

Fair enough, I do like that approach, but I still think it should include a purge option, at least for hostgroups.

Think of my scenario: I use Zabbix's own feature of auto registration rules, where a host is put inside a group based on the MetaData. In this scenario, the hostgroups are not delcared in the Zabbix_host resourse, so they are purged on each Puppet run. I'd like to override this behaviour.

Another solution would be to be able to declare a Zabbix_hostgroup in puppet and then collect it in the server, concatenating this with the hostgroups defined in the Zabbix_host, just like you proposed with the templates. If this was possible, I'd remove my autoregistration rules in favour of this configuration in puppet.

The ultimate goal I am trying to achieve here is to be able to set templates and hostgroups to a host outside of the Zabbix_host definition, this way you can have a monitoring.pp definition inside your custom puppet modules which takes care of setting up the monitoring for that particular module, while also making sure the host in zabbix is gonna have the correct hostgroup and templates.

What's your opinion on this @baurmatt ?

[baurmatt]

I think we're doing something quite similar as you do, at least for the templates part (hostgroups will probably implemented the same at some point).

Preamble: As our setup contains about 3000 hosts, a concat like collector isn't really possible because the performance gets worse with every host. For example: Imagine every host has 5 templates and 3 host groups. That would sum up to about 24000 resources in the catalog only for host/template/hostgroup. Looping through this amount of resources is really expensive especially if you have to do it 3000 times.

That's the reason we're doing the following:

Every profile contains a monitoring/trending.pp with the following snippet

sys11zabbix::agent::template { 'mytemplate': }

This will just place a file in a directory:

  file { "/var/lib/syseleven/trending.templates/${template_name}":
    ensure  => file,
    owner   => 'root',
    group   => 'root',
    mode    => '0400',
    content => '# Managed by Puppet',
  }

The next puppet run will than collect those files with a fact and ship it to the Puppetserver:

Facter.add(:sys11zabbix_templates) do
  setcode do
    begin
      names = Dir.glob('/var/lib/syseleven/trending.templates/*')
    rescue StandardError
      names = []
    end
    names = names.map { |x| File.basename(x) }
    names.sort
  end
end

That fact is than used in Puppet to configure the zabbix_host object accordingly:

class { 'zabbix::agent':
  zbx_templates => $::sys11zabbix_templates,
}

This can easily extended for hostgroups, is a pure Puppet solution and doesn't depend on the concat-ish solution i proposed above. Depending on the size of your setup, this might be the best solution.

[mergwyn]

I’ve come across this issue now and would like to have a try at solving it if we can agree a solution. My use case is relatively simple:

• Where I need specific support within a profile I use zabbix_template_host to link necessary templates to hosts
• If I have manage_resources set when I setup the agent, then as per this issue all templates get unlinked before the list of templates specified in the zabbix::agent call are added.

My preference is to implement a simple purge_templates flag, defaulting to true to manage this specific behaviour in zabbix::agent.

I see that PR700 has already implemented something similar as part of a broader scope and I could base the solution around commit 55e2727 in that PR.