community.zabbix
community.zabbix copied to clipboard
clarify and test backward compatibility with httpapi
SUMMARY
Clarification on how backward compat for httpapi to zabbix-api can be implemented and guaranteed over the next releases.
See also #557
Current idea plan:
1.) implement a connection switch in ZapiWrapper
With module._socket_path
it's possible to check if httpapi got initialized in any form. This way its possible to even use httpapi if the ansible provided options are not set - we just use the "old" zabbix-api related module options to set the httpapi options.
Then the _zapi property of the wrapper is an instance of ZabbixApiRequest
(httpapi) instead of ZabbixAPI
. As I designed ZabbixApiRequest to be an drop in replacement for ZabbixApi no further changes SHOULD be needed on module side.
2.) implement an new common option connection_type
to all modules
Possible values: auto, zabbix-api, httpapi
Default value: auto
3.) Timed handling of connection_type and fading out of zabbix-api
3.1) For now auto
will default to zabbix-api
and as that in ZabbixAPI usage.
Manuall switching of connection_type: httpapi
will enable every enduser to test the implementation and optimizations. Given all works out like expected in point 1. no other changes have to be made on user side for now.
This way we can declare the new connection as an experimental feature.
I would say we promote this through Ansible Bullhorn at some point in time then.
3.2) After all doc changes and testing implementation we will introduce deprecation warnings for the zabbix-api module options alongside a major version bump and highlighting it an release docs while still keeping the default on zabbix-api.
connection_type: auto
will still prefer zabbix-api
. This way enduser can update to newest version without changes needed. They can transition over to new configs (which should be automagically be recognized by ZapiWrapper) but can still run their old configs alongside. We also introduce deprecation warnings on module side - so calling a module without httpapi will bring up a deprecation notice but it still works.
3.3) After SOME (insert specific duration here (-; ) time/releases we again make a major version bump and switch connection_type: auto
to prefer httpapi. Fallback to zabbix-api is only possible with MANUALL change to zabbix-api. Deprecation notices should be stricter/more alerting somehow.
3.4) Again after a major bump we then remove zabbix-api completely.
I would say we keep connection_type alive to maybe handle future changes the same way.
ISSUE TYPE
- Enhancement
COMPONENT NAME
- connection.httpapi
- httpapi.jsonrpc
I currently can't successfully run mentioned in https://github.com/ansible-collections/community.zabbix/pull/444 code. Silly question how do I provide these variables if I run ansbile-test integration
?
---
# hosts.yml
all:
children:
api:
hosts:
apilocal:
ansible_host: host.docker.internal # the host to connect to ( f.e.: zabbix.mydomain.lan )
ansible_httpapi_port: 80
ansible_httpapi_use_ssl: false
ansible_httpapi_validate_certs: false
ansible_network_os: community.zabbix.jsonrpc
ansible_connection: community.zabbix.httpapi
ansible_user: Admin
ansible_httpapi_pass: zabbix # this should be a vault secret of course or ...
# ansible_httpapi_session_key: slaksdljasdlfj # ... instead of password use a session key (zabbix token)
As a workaround, try to set them in the https://github.com/ansible-collections/community.zabbix/blob/main/tests/integration/targets/setup_zabbix/defaults/main.yml
This test roles is included from every other role. I am not sure if this is a good permanent place though.
Yes, thanks, already tried that and it worked. I am just curious (and did not find anything meaningful on the Internet) how ansible-test "builds" the inventory and against what host (some "testhost"???) it is running all these roles (tests). Drastic change with what we have now is with httpai it will use "ansible_host" as a host to connect to to perform API requests while with zabbix-api we just specify server_url.
I would guess it doesn't build anything and rather defaults to localhost as usually. ansible-test
can be locked inside of a container or run on a host (which we are doing). Found out some --list-targets
option in here https://docs.ansible.com/ansible/latest/dev_guide/testing_integration.html#testing-integration, but have no way to check atm.
Spent some time learning how connection plugins work and I mus say it's very cool stuff.
I have a question about introducing connection_type
variable as proposed above. Why don't we implement backward compatibility following way:
If a user defines
ansible_network_os: community.zabbix.zabbix
ansible_connection: httpapi
Then httpapi code is used by all our modules. Additionally in this case a user can define API token which will be used instead of username/password:
ansible_httpapi_session_key:
auth: 2920b1179da3041e23e60e38b421cd95582fc6b8ae06763e4979580d0fa7c8be
If ansible_network_os
and ansible_connection
are not defined (connection is local) then zabbix-api is in use.
In my opinion this simple approach makes it very easy for users to test new approach. Then with time we can totally remove zabbix-api if needed.
Did you have a look in my implementation? I basically already had it but doesn't have time to finalize it properly with test and so on.
The problem isn't really the httpapi implementation itself but to replace all the pyzabbix wrappers and keeping all back compatible.
https://github.com/rockaut/community.zabbix/tree/httpapi1
Yes of course I am working off your implementation. I am setting _zapi
property to an instance of ZabbixApiRequest
instead of ZabbixAPI
. Some modules just work with this with no changes some modules need to be adapted - this is the next step I'd be working on when we decide how to proceed.
I'll take a look at your httpapi1 branch as I was working off master (it already has httpapi) -(((
Cool. Very cool 😎
Spent some time learning how connection plugins work and I mus say it's very cool stuff.
I have a question about introducing
connection_type
variable as proposed above. Why don't we implement backward compatibility following way:If a user defines
ansible_network_os: community.zabbix.zabbix ansible_connection: httpapi
Then httpapi code is used by all our modules. Additionally in this case a user can define API token which will be used instead of username/password:
ansible_httpapi_session_key: auth: 2920b1179da3041e23e60e38b421cd95582fc6b8ae06763e4979580d0fa7c8be
If
ansible_network_os
andansible_connection
are not defined (connection is local) then zabbix-api is in use.In my opinion this simple approach makes it very easy for users to test new approach. Then with time we can totally remove zabbix-api if needed.
Releasing 2.0.0 is a big milestone for us. Something like that I guess will not be repeated so soon in the future. We may lock ourselves to zabbix-api for a longer period then we think.
Maybe we can word it that zabbix-api is still possible opt-in, but will not be supported and is unmaintained by us. In such case we can also leave a legacy Base classes there. Depends, how much code we are talking about? Calls like zbx.host.get() will not be changed, right?
Maybe we can word it that zabbix-api is still possible opt-in, but will not be supported and is unmaintained by us. In such case we can also leave a legacy Base classes there. Depends, how much code we are talking about? Calls like zbx.host.get() will not be changed, right?
Yes we are not trying to move away from zabbix-api (at least now) we just want to introduce new connection method with full playbooks code backward compatibility. I.e. I am trying to achieve this: if you run your playbook without any changes with our collection 2.0.0 it will still require zabbix-api. If you add this to your inventory:
ansible_network_os: community.zabbix.zabbix
ansible_connection: httpapi
Then httpapi will be used (with possibility to provide token. Is that what we want?
I need an advise on what we should do with documentation and hard requirements for these all modules parameters server_url
, login_user
, login_password
, they are not needed for httpapi.
I propose to remove them from documentation and all modules integration tests but still accept them making them 'optional' this way all current playbooks (that have these parameters) will run with both zabbix-api and httpapi.
Maybe we can word it that zabbix-api is still possible opt-in, but will not be supported and is unmaintained by us. In such case we can also leave a legacy Base classes there. Depends, how much code we are talking about? Calls like zbx.host.get() will not be changed, right?
Yes we are not trying to move away from zabbix-api (at least now) we just want to introduce new connection method with full playbooks code backward compatibility. I.e. I am trying to achieve this: if you run your playbook without any changes with our collection 2.0.0 it will still require zabbix-api. If you add this to your inventory:
ansible_network_os: community.zabbix.zabbix ansible_connection: httpapi
Then httpapi will be used (with possibility to provide token. Is that what we want?
yes this sounds correct
I need an advise on what we should do with documentation and hard requirements for these all modules parameters
server_url
,login_user
,login_password
, they are not needed for httpapi. I propose to remove them from documentation and all modules integration tests but still accept them making them 'optional' this way all current playbooks (that have these parameters) will run with both zabbix-api and httpapi.
change to required=False
and adjust documentation for them that they are required if zabbix-api is present. We should ideally implement a check that controls this whenever modules decide to use zabbix-api and not httpapi.
I need an advise on what we should do with documentation and hard requirements for these all modules parameters
server_url
,login_user
,login_password
, they are not needed for httpapi. I propose to remove them from documentation and all modules integration tests but still accept them making them 'optional' this way all current playbooks (that have these parameters) will run with both zabbix-api and httpapi.change to
required=False
and adjust documentation for them that they are required if zabbix-api is present. We should ideally implement a check that controls this whenever modules decide to use zabbix-api and not httpapi.
If we want to still support zabbix-api, we should have some tests in place I think.
Ok, I reached the point where everything works as expected and ansible-test integration -v --color --diff --python 3.9 --requirements --coverage
runs with no errors in my local dev environment however github fails with very weird message
fatal: [testhost]: FAILED! => {"msg": "the connection plugin 'httpapi' was not found"}
https://github.com/BGmot/community.zabbix/runs/8293665394?check_suite_focus=true
If somebody could take a look I'd appreciate that. (My branch is here https://github.com/BGmot/community.zabbix/tree/bgmot_httpapi did not push it yet to upstream) Meanwhile I will try to dig as well.
Ok we depend on ansible.netcommon collection which in turn depends on ansible.utils. I can copy ansible.netcommon code into our collection not sure whether it is right, I actually got rid of the one created by @rockaut which was almost identical to the one from ansible.netcommon. This is where I am not strong so please advise.
Added ansibl.netcommon installation to test suite https://github.com/BGmot/community.zabbix/commit/9ef854d0784150c83fc9551fc707eb02e7d47acd but surprisingly it did not change anything.
Just so I'm following, is the assumption then that moving to httpapi
you're running plays against the target zabbix host? verses running against localhost and setting server_url
to point to the zabbix host? I ask because I see you're deprecating server_url
but there doesn't seem to be an replacement/alternative in the docs
yes, httpapi is a "connection" plugin so works exactly as for example "ssh" one does - connects to the host from inventory (the play is running against) unless you use delegate_to
or re-define ansible_host
.
I have been using the community.zabbix.zabbix_host_info
module in my plays to determine if the current host is available in zabbix-server. If not it would add the host using the community.zabbix.zabbix_host
It seems that community.zabbix.zabbix_host_info
's module is rewritten to another purpose, because I can not for the life of me use that module anymore to fetch host information.
This isn't a Backwards Compatible module, but functionally a different module (using the same name) without letting people know up front to use something else.