Re-requesting an Existing Pull Request

[sky-joker]

SUMMARY

The following are existing pull requests for the Zabbix module in ansible upstream that I have now confirmed.

• Fix error in zabbix_mediatype.py ansible/ansible#68333
• Update zabbix_host.py ansible/ansible#66860
• zabbix_mediatype parameter renamed in Zabbix 4.4 and onwards #64604 ansible/ansible#64607
• zabbix_screen.py: host search using multiple group ansible/ansible#64428
• Adding support for macros {$ANSIBLE_HOST} and {$ANSIBLE_PORT}, provid… ansible/ansible#31933
• New zabbix_user_group module ansible/ansible#58051
• zabbix_import module to import zabbix configuration from file ansible/ansible#43311

If @D3DeFi agrees to #33 (comment), let's re-ask for the above Pull Request.
If you agree, would you like to request all the above PR again?

[D3DeFi]

@sky-joker I agree. If you want, I can iterate through them tomorrow, request each to be reopened here and close them in ansible/ansible.

[sky-joker]

Thank you @D3DeFi

I agree. If you want, I can iterate through them tomorrow, request each to be reopened here and close them in ansible/ansible.

Could you please handle this for me?

[D3DeFi]

I believe that everything I've had tagged as a todo is now closed in ansible/ansible and present here.

Added a few PRs to your list as well.

[sky-joker]

Thank you @D3DeFi for your cooperation!
I checked the box for closed ones.

[D3DeFi]

Np, but I would still leave this issue open until we are sure that each of those PRs has been reopened here.

[D3DeFi]

ansible/ansible#64428 merged to the collection as #68

[D3DeFi]

both ansible/ansible#64607 and ansible/ansible#68333 deprecated by ansible/ansible#67693 and #82

[D3DeFi]

@sky-joker I was looking at ansible/ansible#66860. In the first commit, the patch started as something different and actually introduced something, but after 11 commits later (that the patch consists of) the actual changes (logical) are quite minimal.

Basically this:

if inventory_mode:
    if LooseVersion(self._zbx_api_version) <= LooseVersion('4.4.0'):
        if host['inventory']:
            if int(host['inventory']['inventory_mode']) != self.inventory_mode_numeric(inventory_mode):
                return True
        elif inventory_mode != 'disabled':
            return True
    else:
        if int(host['inventory_mode']) != self.inventory_mode_numeric(inventory_mode):
            return True

Versus this (exact same thing with additional comment, changed positions of if statements and version bump from 4.4.0 to 4.4.1):

if inventory_mode:
    if host['inventory']:
        # The way to read the inventory_mode is different on versions older than 4.4.1
        if LooseVersion(self._zbx_api_version) < LooseVersion('4.4.1'):
            if int(host['inventory']['inventory_mode']) != self.inventory_mode_numeric(inventory_mode):
                return True
        else:
            if int(host['inventory_mode']) != self.inventory_mode_numeric(inventory_mode):
                return True
    elif inventory_mode != 'disabled':
        return True

I don't see any meaning in reordering of if statements, neither do we miss on comment. As for the version bump 4.4.0 -> 4.4.1 should I commit that? Is someone even using 4.4.1 anymore?

[sky-joker]

Thank you @D3DeFi for looking at ansible/ansible#66860.
hm…This is a tough one...

The Zabbix 4.4 will no longer be supported after July.
https://www.zabbix.com/life_cycle_and_release_policy

Because of that, I wonder we don’t need the support of zabbix_host module.
However, I think Zabbix 4.4.1 should also be considered if zabbix_host will be supporting Zabbix 4.4.
Because the return JSON structure has changed from 4.4.1.
ansible/ansible#66860 (comment)

[D3DeFi]

I wanted to commit that, but I've just spotted another difference in the snippets above (comparison character):
if LooseVersion(self._zbx_api_version) <= LooseVersion('4.4.0'):
vs:
if LooseVersion(self._zbx_api_version) < LooseVersion('4.4.1'):

So I think that we already support 4.4.1 properly. Judging from the source code you have linked in the previous conversation 4.4.0 vs 4.4.1 this further confirms that. Thanks! :)

Next on my radar would be ansible/ansible#58051. Do you have something planned for it or can I work towards porting it here?

[sky-joker]

Next on my radar would be ansible/ansible#58051. Do you have something planned for it or can I work towards porting it here?

I don’t have a plan, so would you create this patch and PR if possible?
If you busy, I’ll start to work on this patch creation :)

[D3DeFi]

Next on my radar would be ansible/ansible#58051. Do you have something planned for it or can I work towards porting it here?

I don’t have a plan, so would you create this patch and PR if possible?
If you busy, I’ll start to work on this patch creation :)

No worries. I will try some cherry-pick magic to see if we can preserve original author contributions.

[JiffsMaverick]

@D3DeFi @sky-joker Hello! Is it possible to backport support for version 5.0 of Zabbix to ansible 2.9? You fixed these bugs in 2.10 and it was awesome, thanks. But we have been unable to work normally with Zabbix 5.0 on ansible 2.9 for several months due to this problem.

PS: it seems that for some reason, this bugfix (ansible/ansible@7b2cfda and ansible/ansible#65392) did not get into the 2.9 branch.

[D3DeFi]

@JiffsMaverick boi, 9 thumbsups after 6 hours? Why noone requested this sooner :( With sooo much happening around collections these days (well since march) we've completely forgot about backporting to old ansible. I am sorry.

For 5.0 zero you would require even #51 and that may be considered a feature request, I don't know if it would be accepted, but the one above mentioned should get through. I will try to find some time during weekend hopefully.

Btw putting this version of zabbix_host into library/ folder should work as a workaround. We can dig for other modules in the commit as well if you need those too.

[D3DeFi]

Backports open in #71288, #71289 and #71290. I am pretty positive that first one will be accepted. Not sure about the rest of them.

@JiffsMaverick do you please have a possibility to test ansible/ansible#71289 within your environment and comment on the (back)port PR that it is working for you? But in order for this one to work you also need ansible/ansible#71288

[JiffsMaverick]

@D3DeFi Thank you for fast fix! Changes from #71288 and #71289 works!

[D3DeFi]

Seems that ansible/ansible#71289 won't be merged, but with ansible/ansible#71288 it seems to no longer fail on Zabbix 5.0 (even though you will not be able to manage details for SNMP interfaces. So next minor release of 2.9.x should have this fixed.

[derekpurdy]

Btw putting this version of zabbix_host into library/ folder should work as a workaround. We can dig for other modules in the commit as well if you need those too.

I am having the same issue and I'm just confused on how to use this workaround. Where do you place that file?

edit: I grabbed the latest tar from https://releases.ansible.com/ansible/ansible-latest.tar.gz and placed zabbix_host.py into /usr/lib/python3/dist-packages/ansible/modules/monitoring/zabbix and it appears to work.

[D3DeFi]

@derekpurdy if you have ansible 2.9 failing with KeyError on zabbix 5.0 then you should be fine with just updating to ansible 2.9.13 as some minor bugfixes were included in the latest release.

Btw putting this version of zabbix_host into library/ folder should work as a workaround. We can dig for other modules in the commit as well if you need those too.

I am having the same issue and I'm just confused on how to use this workaround. Where do you place that file?

This is somewhat ugly hack used for official modules if you need to override them. More information can be found in this documentation.

To sum it up. You just create library folder inside your project directory and download ansible module to it. Then if you run ansible with verbose argument you should be able to see that it used local module instead of the one shipped with it.

You just need to remember the reason why you did it an remove locally stored module once the fix is available in some release to prevent the module not being updated in the future.

Btw, since ansible 2.9.10 you should by able to also install collections as a whole. This is probably the best course of action as ansible 2.10 will ship collections by default and relevant fixes and features will be backported to 2.9 less and less.

[D3DeFi]

ansible/ansible#43311 - cannot apply any cherry-pick magic and original author not responding so this will not be moved to the collection.
ansible/ansible#31933 - same