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

Userparameter fix for #1361 #1363

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

Japje
Copy link
Contributor

@Japje Japje commented Aug 7, 2024

SUMMARY

Userparameters uses include_dir which is a list since a few patches back. This breaks the placement of userparameter configs.

This fixes #1361

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

zabbix_agent

ADDITIONAL INFORMATION
TASK [community.zabbix.zabbix_agent : Install user-defined userparameters]
failed: [zabbix.example.com] (item={'name': 'nf_conntrack'}) => {"ansible_loop_var": "item", "changed": false, "checksum": "7006c624038e572cf4f6be5ffc02120cccbdf623", "item": {"name": "nf_conntrack"}, "msg": "Destination directory ['/etc/zabbix/zabbix_agent2.d', '/etc/zabbix/zabbix_agent2.d/plugins.d'] does not exist"}
failed: [zabbix.example.com] (item={'name': 'iptables'}) => {"ansible_loop_var": "item", "changed": false, "checksum": "a9e014f8469a37318d75f1dbcff55e0345402da4", "item": {"name": "iptables"}, "msg": "Destination directory ['/etc/zabbix/zabbix_agent2.d', '/etc/zabbix/zabbix_agent2.d/plugins.d'] does not exist"}

after

TASK [community.zabbix.zabbix_agent : Install user-defined userparameters] 
changed: [zabbix.example.com] => (item={'name': 'nf_conntrack'})
changed: [zabbix.example.com] => (item={'name': 'iptables'})

@Thulium-Drake
Copy link
Contributor

We should make a test for this, I wrote #1335 to address the issues with Zabbix Agent2's default ootb include structure (that requires 2 include statements to fully work), but I completely missed it with the unit tests..

@@ -29,7 +29,7 @@
- name: Install user-defined userparameters
ansible.builtin.template:
src: "{{ zabbix_agent_userparameters_templates_src }}/{{ item.name }}.j2"
dest: "{{ zabbix_agent_include_dir }}/userparameter_{{ item.name }}.conf"
dest: "{{ zabbix_agent_include_dir | first }}/userparameter_{{ item.name }}.conf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK I like what you did here, I would just make one ask. Given what @AlexPykavy did in #1366 that handles both a string and a list, if you can do the same thing here so we can be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated my PR to support this.

@Japje
Copy link
Contributor Author

Japje commented Aug 9, 2024

We should make a test for this, I wrote #1335 to address the issues with Zabbix Agent2's default ootb include structure (that requires 2 include statements to fully work), but I completely missed it with the unit tests..

I have no experience writing tests, so i cant help out on this.

@pyrodie18 pyrodie18 merged commit f6686c6 into ansible-collections:main Aug 9, 2024
102 checks passed
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.

Default value of zabbix_agent_include_dir for zabbix_agent2 breaks userparameter task
3 participants