Added zabbix_action module #49189
Added zabbix_action module #49189
Conversation
ansibot
added
ci_verified
ansibot
removed
the
ci_verified
|
@Akint @D3DeFi @abulimov @akomic @cove @dj-wasabi @eikef @harrisongu @logan2211 @RedWhiteMiko @sookido As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
ansibot
added
community_review
|
does not look too bad, I would label it shippable, @D3DeFi any comments? (it actually looks very good, good job!) |
ansibot
removed
the
needs_triage
|
Agree @resmo , very nice and readable addition. I was scrolling through it earlier today and wanted to test it with multiple zabbix-server versions, but haven't had that much time to actually do it. I can test it tomorrow or we can ship it right away if you want. @rubentsirunyan thank you for your awesome contribution! |
|
waiting for your feedback after testing |
There was a problem hiding this comment.
I am usually testing changes against all current Zabbix-server LTS releases 2.2, 3.0, 4.0. There is, however, no rule established, which versions should be supported in ansible zabbix modules.
Note that I've basically just tested cases documented in EXAMPLES.
This module doesn't work with 2.2 release and requires explicitly setting esc_period to 60 instead of default 1h to work on 3.0 release. (See my comments below).
Works as intended for 3.4 and 4.0, except a single idempotency problem I am mentioning later in my comments.
I would say there are these options:
- code support for older versions of zabbix (2.2 end of limited support - 31 August, 2019; 3.0 end of full support - 28 February, 2019)
- explicitly specify, that this module requires Zabbix-server 3.4 or higher (should go somewhere to
requirements: inDOCUMENTATIONvariable? @resmo) - change default values from string representations (e.g.
1h) to integers (e.g.60) for options likeesc_periodand specify that this module requires Zabbix-server 3.0 or higher to run and Zabbix-server 3.4 to use ACK & recovery operations on actions (this should go to documentation under each module argument afaik - see my comments below)
IMO we can go with the 2nd option and lock this module only for 3.4+ releases. Idempotency fix can be delivered later via PR as I don't think that is something groundbreaking for now. What do you think guys?
@rubentsirunyan with merging of this PR you're becoming module maintainer so I am interested in your opinion in this as well :)
|
Sorry for verbosity, I got carried away a bit |
|
@D3DeFi Thanks for the detailed review. I would go with the third option (+ delivering idempotency fix later via PR). I'll do the necessary changes and update the PR today. |
|
@D3DeFi resolved all the issues including the idempotency issue. |
|
Thank you for fixing it on such a short notice :) |
|
shipit |
|
Merging... Thanks @rubentsirunyan, good job! And thanks @D3DeFi for the review. |
SUMMARY
Implements zabbix_action module to manipulate Zabbix action objects on Zabbix server via API.
ISSUE TYPE
COMPONENT NAME
zabbix_action
ADDITIONAL INFORMATION