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

Linked Templates not updated with xml/json templates #66720

Closed
rockaut opened this issue Jan 23, 2020 · 11 comments · Fixed by #66747
Closed

Linked Templates not updated with xml/json templates #66720

rockaut opened this issue Jan 23, 2020 · 11 comments · Fixed by #66747
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. monitoring Monitoring category python3 support:community This issue/PR relates to code supported by the Ansible community. zabbix Zabbix community

Comments

@rockaut
Copy link
Contributor

rockaut commented Jan 23, 2020

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
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:

<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 ).

<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.

@ansibot
Copy link
Contributor

ansibot commented Jan 23, 2020

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

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jan 23, 2020
@rockaut
Copy link
Contributor Author

rockaut commented Jan 23, 2020

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.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 23, 2020

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:

    # 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.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 23, 2020

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:

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?

@rockaut
Copy link
Contributor Author

rockaut commented Jan 23, 2020

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?

@Akasurde
Copy link
Member

!component =lib/ansible/modules/monitoring/zabbix/zabbix_template.py

@ansibot
Copy link
Contributor

ansibot commented Jan 24, 2020

Files identified in the description:

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

@ansibot
Copy link
Contributor

ansibot commented Jan 24, 2020

@ansibot ansibot added module This issue/PR relates to a module. monitoring Monitoring category support:community This issue/PR relates to code supported by the Ansible community. zabbix Zabbix community and removed needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jan 24, 2020
rockaut added a commit to rockaut/ansible that referenced this issue Jan 24, 2020
fixes ansible#66720

adding a check for versions >= 4.0.16 or >= 4.4.4 to enable deleteMissing for templateLinkage
rockaut added a commit to rockaut/ansible that referenced this issue Jan 24, 2020
New update rule is available from 4.0.16 and 4.4.4 up. Add check for version and enable new update rule.

fixes ansible#66720
@rockaut
Copy link
Contributor Author

rockaut commented Jan 24, 2020

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.

@ansibot ansibot added the has_pr This issue has an associated PR. label Jan 24, 2020
@D3DeFi
Copy link
Contributor

D3DeFi commented Jan 25, 2020

Thanks @rockaut for such a deep analysis of the problem. The thing with importing multiple templates at once is only supported thanks to configuration.import from Zabbix API as you've already found out. The module itself is not very well suited to detect changes on more than one template, that's why I always try to talk about importing multiple templates via XML/JSON files as an experimental feature.

If we would to change to template.update API call we would need to carefully parse XML/JSON by ourselves instead of relaying on the Zabbix itself to load necessary dependencies (template ids, groups ids and so on). To be honest, this module as well as some other zabbix modules are a bit messy and could really use some major overhaul.

Anyways, thanks for providing a PR for this issue! Good job.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 25, 2020

Oh I totally get why it is how it is and I also think it's better than actually parse the xml/json as it would be way to error-prone. That's why I thought it should be handled in Zabbix API ... and now it's already present :D

That said ... I still think it should somehow be mentioned in the readme for the ansible module somehow. Booth things actually, the configuration.import and the multi-object thing. Multi-object isn't only hitting templates but also host as I see it?

I will wrap my thoughts about it next week and maybe contribute something so that we can discuss this aside from this issue/pr.

ansibot pushed a commit that referenced this issue Jan 27, 2020
…plates (#66747)

* enable new update rule to delete missing linked templates

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

* adding fragment file

* Update zabbix_template.py

* Update zabbix_template.py
@ansible ansible locked and limited conversation to collaborators Feb 24, 2020
ruozeng-w pushed a commit to ruozeng-w/ansible that referenced this issue Apr 18, 2020
…plates (ansible#66747)

* enable new update rule to delete missing linked templates

New update rule is available from 4.0.16 and 4.4.4 up. Add check for version and enable new update rule.

fixes ansible#66720

* adding fragment file

* Update zabbix_template.py

* Update zabbix_template.py
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. monitoring Monitoring category python3 support:community This issue/PR relates to code supported by the Ansible community. zabbix Zabbix community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants