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

Updated zabbix_host module to support Zabbix 5.0 #51

Merged
merged 2 commits into from
May 19, 2020
Merged

Updated zabbix_host module to support Zabbix 5.0 #51

merged 2 commits into from
May 19, 2020

Conversation

D3DeFi
Copy link
Contributor

@D3DeFi D3DeFi commented May 18, 2020

SUMMARY

Zabbix 5.0 support for zabbix_host module.

Game breaking changes were:

  • hostinterface.get now returns details: [] for each interface type, even when not needed
  • macro.get now returns type: 0 for text macros and new type type: 1 is for secret macros
  • secret macros return from API without value property so there is no way to check if user wants to
    change the value of macro. We can either always change it or require user to change something alongside it like description. I went with the later to not break idempotency of the module.

Fixes #46

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/modules/zabbix_host.py

ADDITIONAL INFORMATION

I've finally added options for interfaces into module.argument_spec. The downside is that ansible now prints warnings when type is an integer value as it is converted into a string:

  interfaces:
    - type: 1
       main: 1
...omitted...

This was added in a good faith by me in the past (interfaces[0]['type'] = 'agent'). Users shouldn't need to bother with API details and it would make sense to change every numerical value to either bool or string in the future, but it cannot be done without breaking backwards compatibility.

Just for example - we would have interfaces[0]['main'] = True instead of interfaces[0]['main'] = 1. I don't know if this makes sense and if we want to pursue this in the future or if I should convert interfaces[0]['type'] back to an integer. zabbix_action is using this logic and doesn't use numerical values anywhere where it doesn't make sense.

SNMP details are required, Zabbix documentation is broken - https://support.zabbix.com/browse/ZBX-17719

There is an additional issue, where details should be a dictionary and is accepted by an API as such. When details is empty, API returns empty list - details: [] instead, hence the conversion in the code to empty dict.

@D3DeFi D3DeFi added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request module The issue or pull request is related to Zabbix module Zabbix 5 support labels May 18, 2020
@D3DeFi D3DeFi requested a review from sky-joker May 18, 2020 08:43
@D3DeFi
Copy link
Contributor Author

D3DeFi commented May 18, 2020

Failing integration tests for zabbix_proxy on 5.0 as expected. Other than that, looks like it might work.

@toerb @JohannesKreuzer if you can give this changes some tests on your side, it would be very much appreciated.

@toerb
Copy link

toerb commented May 18, 2020

@D3DeFi looks good! New hosts are created properly and already existing one are not changed if there is no need for it. This solves the problem described in #46.

@sky-joker
Copy link
Contributor

Excellent!!
Thank you @D3DeFi for this patch :)

LGTM

Failing integration tests for zabbix_proxy on 5.0 as expected. Other than that, looks like it might work.

I agree with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request module The issue or pull request is related to Zabbix module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

host.create with SNMP interface not working with Zabbix 5.0
3 participants