Repository: ansible/ansible
Pull request: zabbix_template - enable new update rule to delete missing linked templates
##### SUMMARY
New update rule is available from 4.0.16 and 4.4.4 up. Add check for version and enable new update rule.

fixes #66720

##### ISSUE TYPE
- Feature Pull Request

##### COMPONENT NAME
zabbix_template

##### ADDITIONAL INFORMATION
Using new update rule set to true, linked templates getting unlinked if removed from xml/json files.

Problem statement:
Linked Templates not updated with xml/json templates
##### SUMMARY
If you import a template with a xml/json file the linked templates of a templates ( so the child templates not the root ones ) aren't processed if you want to remove linked templates.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
zabbix_templates

##### ANSIBLE VERSION
```shell
ansible 2.9.2
  config file = /mnt/d/Code/gitlab/zabbix-servers/ansible/ansible.cfg
  configured module search path = ['/home/fism/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /mnt/d/Code/gitlab/zabbix-servers/venv/lib/python3.6/site-packages/ansible
  executable location = /mnt/d/Code/gitlab/zabbix-servers/venv/bin/ansible
  python version = 3.6.9 (default, Nov  7 2019, 10:44:02) [GCC 8.3.0]
```

##### OS / ENVIRONMENT
Zabbix 4.4.4 on RHEL7

##### STEPS TO REPRODUCE
Export a template (no matter if with Zabbix itself or ansible). Change the corresponding linked template section of a template and reimport it.

Exported with Zabbix:
```yaml
<zabbix_export>
    <version>4.4</version>
    <date>2020-01-23T13:25:32Z</date>
    <groups>
        <group>
            <name>Templates</name>
        </group>
    </groups>
    <templates>
        <template>
            <template>fisma</template>
            <name>fisma</name>
            <templates>
                <template>
                    <name>fismb</name>
                </template>
            </templates>
            <groups>
                <group>
                    <name>Templates</name>
                </group>
            </groups>
        </template>
    </templates>
</zabbix_export>
```

Change to this with removing all linked templates ( fismb in above example ).
```yaml
<zabbix_export>
    <version>4.4</version>
    <date>2020-01-23T13:25:32Z</date>
    <groups>
        <group>
            <name>Templates</name>
        </group>
    </groups>
    <templates>
        <template>
            <template>fisma</template>
            <name>fisma</name>
            <groups>
                <group>
                    <name>Templates</name>
                </group>
            </groups>
        </template>
    </templates>
</zabbix_export>
```

##### EXPECTED RESULTS
fismb should be unlinked from fisma.


##### ACTUAL RESULTS
fismb is still linked with fisma.

Hints from issue discussion:
Files identified in the description:
None

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Also:
- fisma with linked fismb, fismc
if you want to remove only fismb it doesn't change. But Ansible is reporting the "changed": true state.

Linking on creation of a new template is working.
Actually I can't really wrap my head around that propblem. Maybe @sky-joker @D3DeFi can comment on it.

As i see it, the linked templates are read out in the ``check_template_changed`` function but at nowhere other. Also there are variables like link_template_ids mentioned often in code but they aren't the linked templates/child templates but the template id under the root templates nodes.

Also I don't know how this currently could work with multiple templates in one file as those nodes aren't iterated over.

If you look at zabbix_template.py line 680:

```python
    # Identify template names for IDs retrieval
    # Template names are expected to reside in ['zabbix_export']['templates'][*]['template'] for both data types
    template_content, template_type = None, None
    if template_json is not None:
        template_type = 'json'
        template_content = template_json
        json_parsed = template.load_json_template(template_content)
        template_names = list(t['template'] for t in json_parsed['zabbix_export']['templates'])

    elif template_xml is not None:
        template_type = 'xml'
        template_content = template_xml
        xml_parsed = template.load_xml_template(template_content)
        template_names = list(t.find('template').text for t in list(xml_parsed.find('templates')))

    else:
        template_names = [template_name]
```
Here only the content and the names of the templates are "read" out but things like macros not.

What I don't get: the whole thing is then "just" imported in function ``import_template`` as is. So no parsing or anything should be impacting the actual changes. All that might influence this things are the update_rules. But here nothing can be changed.
Now I get it ... but it's still cumbersome.

There are two ways to do this in the Zabbix API: with template.update and with configuration.import.

If you use template.update - which i thought where the case for ansible - all would work as I was expecting it. If you look at https://www.zabbix.com/documentation/4.4/manual/api/reference/template/update there is this statement for templates:

```text
Templates to replace the currently linked templates. Templates that are not passed are only unlinked.
```

So if you want to unlink a template from another template just don't pass it.

The current Ansible implementation for importing xml/json file is using configuration.import instead. And here this whole process doesn't work as describe. In fact if you use the import function of the zabbix frontend it also doesn't work this way.

What do you others think of this?
I opened a Zabbix issue too ( https://support.zabbix.com/browse/ZBX-17215 ) - I think this might be a issue for the Zabbix team as the API should work consistently here. That said I'm not shure if it shouldn't be covered in the Ansible module too somehow. At least it should be mentioned in the docs?
!component =lib/ansible/modules/monitoring/zabbix/zabbix_template.py
Files identified in the description:
* [`lib/ansible/modules/monitoring/zabbix/zabbix_template.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/monitoring/zabbix/zabbix_template.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @logan2211 @rubentsirunyan @sookido
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
Tested with Zabbix 4.4.4 against the new options metioned in Zabbix Issue. Works now as expected.

Added PR for new API update rule.

Commit message: Update zabbix_template.py

Patch to review:
diff --git a/changelogs/fragments/66747-zabbix_template-newupdaterule-deletemissinglinkedtemplate.yml b/changelogs/fragments/66747-zabbix_template-newupdaterule-deletemissinglinkedtemplate.yml
new file mode 100644
index 00000000000000..f2fb054c927cba
--- /dev/null
+++ b/changelogs/fragments/66747-zabbix_template-newupdaterule-deletemissinglinkedtemplate.yml
@@ -0,0 +1,3 @@
+minor_changes:
+  - zabbix_template - adding new update rule templateLinkage.deleteMissing for newer zabbix versions (https://github.com/ansible/ansible/pull/66747).
+  
diff --git a/lib/ansible/modules/monitoring/zabbix/zabbix_template.py b/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
index 59446455076cd5..4a446712a081dc 100644
--- a/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
+++ b/lib/ansible/modules/monitoring/zabbix/zabbix_template.py
@@ -605,6 +605,10 @@ def import_template(self, template_content, template_type='json'):
             if LooseVersion(api_version).version[:2] <= LooseVersion('3.2').version:
                 update_rules['applications']['updateExisting'] = True
 
+            if (LooseVersion(api_version).version[:3] >= LooseVersion('4.0.16').version) or \
+               (LooseVersion(api_version).version[:3] >= LooseVersion('4.4.4').version):
+                update_rules['templateLinkage']['deleteMissing'] = True
+
             import_data = {'format': template_type, 'source': template_content, 'rules': update_rules}
             self._zapi.configuration.import_(import_data)
         except ZabbixAPIException as e:


Code review comments:
diff_hunk: @@ -605,6 +605,10 @@ def import_template(self, template_content, template_type='json'):
             if LooseVersion(api_version).version[:2] <= LooseVersion('3.2').version:
                 update_rules['applications']['updateExisting'] = True
 
+            if (LooseVersion(api_version).version[:3] >= LooseVersion('4.0.16').version) or \
path: lib/ansible/modules/monitoring/zabbix/zabbix_template.py
text: @user2:
With this condition expression, the following version will be matched and an error will occur.

* 4.4.0
* 4.4.1
* 4.4.2
* 4.4.3

There must be conditions expression to exclude the above versions.
For example, the conditions to narrow down the range are as follows.

```python
            if LooseVersion('4.0.16').version <= LooseVersion(api_version).version[:3] <= LooseVersion('4.0.17').version or \
               LooseVersion(api_version).version[:3] >= LooseVersion('4.4.4').version:
```

But, in this case, `4.0.17` will be dynamic when a new minor version is released.
@user1, What do you think?

@user1:
Hmm, what about using `4.1` instead of `4.0.17`? I think it is really improbable that 4.0 branch will be getting that much updates.

@author:
Hm... I see. I think best would be to check if it's in the 4.0 branch first and then if it's >= .16.
the >= 4.4.4 should be sufficient anyways.