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

Fix Zabbix 5.4 template imports failing #407

Conversation

href
Copy link
Contributor

@href href commented Jun 24, 2021

Applications are no longer supported, everything is solved through tags now.

Applications are no longer supported, everything is solved through tags now.
@codecov
Copy link

codecov bot commented Jun 24, 2021

Codecov Report

Merging #407 (0d1a5f6) into main (03421c8) will decrease coverage by 2.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #407      +/-   ##
==========================================
- Coverage   78.78%   76.74%   -2.05%     
==========================================
  Files          19       21       +2     
  Lines        2635     2958     +323     
  Branches      665      761      +96     
==========================================
+ Hits         2076     2270     +194     
- Misses        364      452      +88     
- Partials      195      236      +41     
Impacted Files Coverage Δ
...s/community/zabbix/plugins/modules/zabbix_proxy.py 50.00% <0.00%> (-40.17%) ⬇️
...ns/community/zabbix/plugins/modules/zabbix_host.py 83.33% <0.00%> (-0.18%) ⬇️
...community/zabbix/plugins/modules/zabbix_service.py 86.13% <0.00%> (ø)
...unity/zabbix/plugins/modules/zabbix_globalmacro.py 72.03% <0.00%> (ø)
...unity/zabbix/plugins/modules/zabbix_maintenance.py 68.21% <0.00%> (ø)
...ns/community/zabbix/plugins/modules/zabbix_user.py 89.67% <0.00%> (+0.44%) ⬆️
...ity/zabbix/plugins/modules/zabbix_template_info.py 80.95% <0.00%> (+0.95%) ⬆️
...ommunity/zabbix/plugins/modules/zabbix_template.py 77.39% <0.00%> (+1.29%) ⬆️
...s/community/zabbix/plugins/module_utils/helpers.py 75.00% <0.00%> (+2.58%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6061c49...0d1a5f6. Read the comment docs.

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 25, 2021

Thank you for this patch. However, it seems there is also something else broken as updateExisting seems to be ignored for the Zbx 5.4 and module fails idempotency checks.

Currently, we don't have CI enabled for 5.4, but if run locally via docker:

zabbix_version=5.4 docker-compose up -d
ansible-test integration test_zabbix_template

It fails on the following step:

TASK [test_zabbix_template : Import Zabbix template from JSON file with matching values (idempotency check).] **********************************************************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ********_api.Already_Exists: ('Error -32602: Invalid params., Template "ExampleHost" already exists. while sending {"jsonrpc": "2.0", "method": "configuration.import", "params": {"format": "json", "source": "{\\"********_export\\": {\\"date\\": \\"2020-05-29T15:22:11Z\\", \\"templates\\": [{\\"template\\": \\"ExampleHost\\", \\"name\\": \\"ExampleHost\\", \\"description\\": \\"\\", \\"groups\\": [{\\"name\\": \\"Linux servers\\"}, {\\"name\\": \\"Templates\\"}], \\"macros\\": [{\\"macro\\": \\"{$EXAMPLE_MACRO1}\\", \\"value\\": \\"1000\\"}, {\\"macro\\": \\"{$EXAMPLE_MACRO2}\\", \\"value\\": \\"text\\"}], \\"templates\\": [{\\"name\\": \\"FTP Service\\"}, {\\"name\\": \\"Zabbix Proxy\\"}]}], \\"version\\": \\"3.0\\", \\"groups\\": [{\\"name\\": \\"Linux servers\\"}, {\\"name\\": \\"Templates\\"}]}}", "rules": {"discoveryRules": {"createMissing": true, "updateExisting": true, "deleteMissing": true}, "graphs": {"createMissing": true, "updateExisting": true, "deleteMissing": true}, "groups": {"createMissing": true}, "httptests": {"createMissing": true, "updateExisting": true, "deleteMissing": true}, "items": {"createMissing": true, "updateExisting": true, "deleteMissing": true}, "templates": {"createMissing": true, "updateExisting": true}, "templateLinkage": {"createMissing": true, "deleteMissing": true}, "triggers": {"createMissing": true, "updateExisting": true, "deleteMissing": true}, "valueMaps": {"createMissing": true, "updateExisting": true}, "templateDashboards": {"createMissing": true, "updateExisting": true, "deleteMissing": true}}}, "auth": "a8ba01a7bd18a8db7965c2975e68c336", "id": 4}', -32602)

So in order to fix this problem we need to dig even deeper.

Anyway, I think we can merge this patch and solve at least 50% of problems with zbx template rather than having it bugged 100%. Do you agree @sky-joker ?

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 25, 2021

hmm, this seems to work correctly tho when executed multiple times:

from zabbix_api import ZabbixAPI

zbx = ZabbixAPI('http://localhost:8080')
zbx.login('Admin', 'zabbix')

rules = {
    'templates': {
        'createMissing': True,
        'updateExisting': True
    }
}

source = '''<?xml version="1.0" encoding="UTF-8"?>
<zabbix_export>
  <version>5.0</version>
  <templates>
    <template>
      <template>ExampleTemplate</template>
      <groups>
        <group>
          <name>Linux servers</name>
        </group>
      </groups>
    </template>
  </templates>
</zabbix_export>'''

zbx.configuration.import_(dict(format='xml', source=source, rules=rules))

zbx.logout()

@@ -622,6 +622,10 @@ def import_template(self, template_content, template_type='json'):
if LooseVersion(self._zbx_api_version) >= LooseVersion('5.2'):
update_rules["templateDashboards"] = update_rules.pop("templateScreens")

# Zabbix 5.4 no longer supports applications
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @href for the patch!
Could you please add a changelog?

@sky-joker
Copy link
Contributor

@D3DeFi
I agree, please merge after adding a changelog.

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 25, 2021

I was able to verify that attempting to import the same file via Zabbix GUI also results in the same error.

The file is https://github.com/ansible-collections/community.zabbix/blob/main/tests/integration/targets/test_zabbix_template/files/template1_52_higher.json

image

I believe there is nothing wrong with such json, or is it? I think this should be open as a bug for Zabbix

@sky-joker
Copy link
Contributor

@D3DeFi
I tried whether the same error happens, but I didn't confirm in my environment.
I used zabbix container version 5.4.1.

image

I wonder that why doesn't the error happen(environment dependent?).

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 28, 2021

@sky-joker it must be that we are doing something wrong in the zabbix_template. I am running against 5.4.0 (ubuntu-5.4-latest from our docker-compose) file with the following steps:

  1. ansible-test integration test_zabbix_template <- fails
  2. log in manually and attempt to import same template as described in the picture <- fails with template ExampleHost already exists

Other test is:

  1. Log in manually and attempt to import the template
  2. Do it once again -> result is 'No change'

I suspect there was a secret change of how API works (again..). I think this is how it works in the second case (thus not failing) in sort-of pseudocode:

template = zbx.template.get()
cmpres = zbx.configuration.importcompare() if template
zbx.configuration.import() if cmpres.different

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 28, 2021

Now this is very interesting. If I FIRST upload template manually via Import button in Zabbix GUI and then try to print template diff I get:

[]

Buut, if I run ansible-test integration test_zabbix_template and then try to print template diff:

{
  "templates": {
    "added": [
      {
        "after": {
          "template": "ExampleHost",
          "name": "ExampleHost",
          "macros": [
            {
              "macro": "{$EXAMPLE_MACRO1}",
              "value": "1000"
            },
            {
              "macro": "{$EXAMPLE_MACRO2}",
              "value": "text"
            }
          ],
          "templates": [
            {
              "name": "FTP Service"
            },
            {
              "name": "Zabbix Proxy"
            }
          ],
          "uuid": "c1a80829556941e4a1ff307b39130b5a"
        }
      }
    ]
  }
}

This is my current test script:

import json

zbx = ZabbixAPI('http://localhost:8080')
zbx.login('Admin', 'zabbix')


update_rules = {
    'discoveryRules': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'graphs': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'groups': {
        'createMissing': True
    },
    'httptests': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'items': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'templates': {
        'createMissing': True,
        'updateExisting': True
    },
    'templateLinkage': {
        'createMissing': True,
        'deleteMissing': True
    },
    'templateDashboards': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'triggers': {
        'createMissing': True,
        'updateExisting': True,
        'deleteMissing': True
    },
    'valueMaps': {
        'createMissing': True,
        'updateExisting': True
    }
}

with open('tests/integration/targets/test_zabbix_template/files/template1_52_higher.json', 'r') as f:
    source_json = json.load(f)

x = zbx.configuration.importcompare(dict(format='json', source=json.dumps(source_json), rules=update_rules))
print(json.dumps(x, indent=2))

zbx.logout()

Now I am lost :)

@D3DeFi
Copy link
Contributor

D3DeFi commented Jun 28, 2021

Anyway, I will open new issue and merge this. Changelog will be added when I will prepare 1.4.0 release

@D3DeFi D3DeFi merged commit 5e61b1e into ansible-collections:main Jun 28, 2021
@sky-joker
Copy link
Contributor

@D3DeFi

Thanks, for the details.
I confirmed that the same error happens.
Indeed, that's strange behavior.

Anyway, I will open new issue and merge this. Changelog will be added when I will prepare 1.4.0 release

ok, thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants