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

zabbix_template - enable new update rule to delete missing linked templates #66747

Merged
merged 4 commits into from Jan 27, 2020

Conversation

rockaut
Copy link
Contributor

@rockaut rockaut commented Jan 24, 2020

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.

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
@ansibot
Copy link
Contributor

ansibot commented Jan 24, 2020

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. monitoring Monitoring category needs_triage Needs a first human triage before being processed. small_patch support:community This issue/PR relates to code supported by the Ansible community. zabbix Zabbix community labels Jan 24, 2020
@rockaut rockaut changed the title enable new update rule to delete missing linked templates zabbix_template - enable new update rule to delete missing linked templates Jan 24, 2020
@ansibot
Copy link
Contributor

ansibot commented Jan 24, 2020

The test ansible-test sanity --test pylint [explain] failed with 4 errors:

lib/ansible/modules/monitoring/zabbix/zabbix_template.py:608:15: bad-whitespace: No space allowed after bracket             if ( LooseVersion(api_version).version[:3] >= LooseVersion('4.0.16').version ) or \                ^
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:608:89: bad-whitespace: No space allowed before bracket             if ( LooseVersion(api_version).version[:3] >= LooseVersion('4.0.16').version ) or \                                                                                          ^
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:609:16: bad-whitespace: No space allowed after bracket                 ( LooseVersion(api_version).version[:3] >= LooseVersion('4.4.4').version ):                 ^
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:609:89: bad-whitespace: No space allowed before bracket                 ( LooseVersion(api_version).version[:3] >= LooseVersion('4.4.4').version ):                                                                                          ^

The test ansible-test sanity --test pep8 [explain] failed with 6 errors:

lib/ansible/modules/monitoring/zabbix/zabbix_template.py:608:17: E201: whitespace after '('
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:608:89: E202: whitespace before ')'
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:609:17: E125: continuation line with same indent as next logical line
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:609:18: E201: whitespace after '('
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:609:89: E202: whitespace before ')'
lib/ansible/modules/monitoring/zabbix/zabbix_template.py:610:16: E111: indentation is not a multiple of four

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jan 24, 2020
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 24, 2020
@rockaut
Copy link
Contributor Author

rockaut commented Jan 24, 2020

Is there a way to rerun the shippable tests?

@Akasurde
Copy link
Member

@D3DeFi @sky-joker Could you please review this?

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. labels Jan 24, 2020
@sky-joker
Copy link
Contributor

Thank you @rockaut for reporting and patch the issues!

Is there a way to rerun the shippable tests?

You need to close and open this PR.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 25, 2020

I updated the version checks. It's not pretty or efficient but also not too shabby imo.
I splitted the 4.0 and 4.4 version check on intention for readability to not get a long if-and-or clause.

Can someone check it against a 4.2 branch. I don't have one at hand.

@D3DeFi
Copy link
Contributor

D3DeFi commented Jan 25, 2020

I updated the version checks. It's not pretty or efficient but also not too shabby imo.
I splitted the 4.0 and 4.4 version check on intention for readability to not get a long if-and-or clause.

Can someone check it against a 4.2 branch. I don't have one at hand.

works for me, thanks

shipit

@rockaut
Copy link
Contributor Author

rockaut commented Jan 25, 2020

Why is this one check (#60) always "unstable"? I don't get it. It says something about mysql ...

{
"changed": false,
"msg": "Unable to start service mysql: Job for mysql.service failed because the control process exited with error code.\nSee \"systemctl status mysql.service\" and \"journalctl -xe\" for details.\n"
}

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jan 25, 2020
@D3DeFi
Copy link
Contributor

D3DeFi commented Jan 27, 2020

Why is this one check (#60) always "unstable"? I don't get it. It says something about mysql ...

{
"changed": false,
"msg": "Unable to start service mysql: Job for mysql.service failed because the control process exited with error code.\nSee \"systemctl status mysql.service\" and \"journalctl -xe\" for details.\n"
}

If I am not mistaken, integration testing workflow should go like this (note that every integration role has its own generated play):

  1. roles not important for us right now
  2. test/integration/targets/setup_mysql_db (as prereq for roles below)
  3. test/integration/targets/mysql_*
  4. test/integration/targets/setup_mysql_db/handlers/main.yml (cleanup after setup_mysql_db role)
  5. roles not important for us right now
  6. test/integration/targets/setup_mysql_db (as prereq for roles below)
  7. test/integration/targets/setup_zabbix
  8. test/integration/targets/zabbix_*

Problem probably resides at the step 4. I would start searching there as there may be something that should be cleaned up after mysql daemon, but currently is not, preventing second installation of mysql on the same container from starting correctly.

Shame on me as I've met this error more than once, but never really tried to dive deeper into it as close & reopen of PR was quicker atm for me, but shortsighted. Lets see If I can come up with something for it today and open a PR for mysql maintainers.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 27, 2020

Thank you @D3DeFi ... ping me if I have something to do.

@D3DeFi
Copy link
Contributor

D3DeFi commented Jan 27, 2020

@rockaut please try to close & reopen this PR until CI build passes. No need for this PR to wait for fixes meant for setup_mysql_db.

@rockaut
Copy link
Contributor Author

rockaut commented Jan 27, 2020

temporary close for pipeline fixes

@rockaut rockaut closed this Jan 27, 2020
@rockaut
Copy link
Contributor Author

rockaut commented Jan 27, 2020

reopen for pipeline build

@rockaut rockaut reopened this Jan 27, 2020
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 27, 2020
Copy link
Contributor

@D3DeFi D3DeFi left a comment

Choose a reason for hiding this comment

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

shipit

@sky-joker
Copy link
Contributor

Looks good to me :)
Thank you @rockaut!

shipit

@ansibot ansibot added automerge This PR was automatically merged by ansibot. shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Jan 27, 2020
@ansibot ansibot merged commit 055cf91 into ansible:devel Jan 27, 2020
@ansible ansible locked and limited conversation to collaborators Feb 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 automerge This PR was automatically merged by ansibot. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. monitoring Monitoring category shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. zabbix Zabbix community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linked Templates not updated with xml/json templates
5 participants