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

Support for Zabbix Agent 2 through agent parameters #782

Closed sgnl05 closed 3 years ago

sgnl05 commented 3 years ago

Pull Request (PR) description

This PR adds support for using Zabbix Agent 2, without adding much complexity to the module.

Users wanting to use Zabbix Agent 2 will have to set the following parameters in Hiera:

zabbix::agent::agent_configfile_path: '/etc/zabbix/zabbix_agent2.conf'
zabbix::agent::include_dir: '/etc/zabbix/zabbix_agent2.d'
zabbix::agent::pidfile: '/var/run/zabbix/zabbix_agentd2.pid'
zabbix::agent::servicename: 'zabbix-agent2'
zabbix::agent::zabbix_package_agent: 'zabbix-agent2'

This Pull Request (PR) fixes the following issues

Fixes #692

kenyon commented 3 years ago

Could you use EPP instead of ERB for the new templates? We prefer to use EPP.

sgnl05 commented 3 years ago

Sure! I'll start working on them tomorrow.

root-expert commented 3 years ago

Hello, thanks for your contribution!

Can you add a section in the readme on how to configure agent2 both by class parameters and with hiera?

Also we have to consider adding acceptance tests for agent v2.

sgnl05 commented 3 years ago

I think I've done what I can now. Some tests are failing due to cleanup of init scripts for unsupported OS versions. We should probably just remove those tests, but I'm always struggling with tests and I guess I'll just have to ask for some help here.

root-expert commented 3 years ago

My only concern is that a global switch may be better for the end user.

Maybe a boolean paramater use_agent_v2 will automatically set the correct default values (e.g package name, service name) and install v2 package. That may increase the module complexity though :(

Another thing is that, if a user has zabbix agent v1 already installed and wants to switch to v2, the module should take care of purging v1 package (and vice-versa).

Thoughts?

sgnl05 commented 3 years ago

I think in general that a module that installs a package, sets a config file and manages the service should have parameters that are configurable so that alternative packages/config templates/service names can be used. It would give the module the flexibility needed to use Zabbix Agent 2, without increasing complexity.

This PR became way to complex, and I'm unable to fix the tests that fail because of it.

I'd like to close the PR, and submit a new one that only focuses on the package name, config template and the service name,