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_host.present: many fixes #58604

Merged
merged 8 commits into from
Feb 22, 2021

Conversation

piterpunk
Copy link

@piterpunk piterpunk commented Oct 1, 2020

What does this PR do?

This PR fixes allows zabbix_host.present to:

  • Create a new host in Zabbix with a "Visible Name" configured, as was stated in state documentation.
  • Compare and update an existing host
  • Really update the existing interfaces instead trying removing and recreating them all.
  • Added the mandatory parameters to create a SNMP hostinterface in Zabbix 5.0
  • Now processes additional host properties like "description" or "inventory_mode".

What issues does this PR fix or reference?

Fixes #58602
Fixes #58603
Fixes #53838
Fixes #58620

Previous Behavior

Host's "Visible Name" never was configured because zabbix_host.present didn't pass the parameter visible_name to zabbix.create_host. Other additional host properties were also ignored.

Host comparison and update fails because non existent key "bulk" and a failure in inventory check.

The update_interfaces didn't really update them, it try to removes and recreates them. Usually this strategy fails, as interfaces with linked items can't be removed. Also, no SNMP interface can be created in Zabbix 5.0

New Behavior

Host's "Visible Name" is configured and additional host properties are processed. Host comparison and update runs Ok.

The update_interfaces now try to match the interface types and update them in place, then create the new ones and remove the rest.

SNMP interfaces can be created and updated seamlessly.

Merge requirements satisfied?

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@piterpunk piterpunk requested a review from a team as a code owner October 1, 2020 06:37
@piterpunk piterpunk requested review from garethgreenaway and removed request for a team October 1, 2020 06:37
@ghost ghost requested a review from twangboy October 1, 2020 06:37
@piterpunk piterpunk changed the title zabbix_host.present: many fixes [WIP]: zabbix_host.present: many fixes Oct 2, 2020
@piterpunk
Copy link
Author

Converted to Work-In-Progress, I will include the fix for #53838 to keep the state 100% Ok with the documentation.

@piterpunk piterpunk force-pushed the zabbix_host_present_many_fixes branch from 6e522fa to 4a691b8 Compare October 3, 2020 05:07
@piterpunk piterpunk force-pushed the zabbix_host_present_many_fixes branch from 4a691b8 to 29b5cd3 Compare October 9, 2020 05:39
@piterpunk piterpunk changed the title [WIP]: zabbix_host.present: many fixes zabbix_host.present: many fixes Oct 9, 2020
@piterpunk piterpunk force-pushed the zabbix_host_present_many_fixes branch from 4c4b982 to 9f43e5b Compare December 14, 2020 05:03
@piterpunk
Copy link
Author

I fixed the last failing tests today and will be great to have this approved before began to test and adjust the other Zabbix related states for Zabbix 5+

@sagetherage sagetherage added the Aluminium Release Post Mg and Pre Si label Dec 15, 2020
salt/states/zabbix_host.py Outdated Show resolved Hide resolved
piterpunk added 7 commits February 11, 2021 01:03
- Added visible_name to be pass to zabbix module. It was on
  documentation but the argument was not sent to underlying module.
- Added checks before "pop" the elements "bulk" and "details" from
  hostinterfaces_get's response. Without that, the interface comparison
  didn't works with Zabbix >= 5.0
- Fixed the "inventory" comparison. It failed when there isn't a
  current and a new inventory.
- Major rewrite of the update_interfaces routine.
- Adequation to the new lint, black, isort, requirements.
- Now the zabbix_host.present accepts all host properties allowing to
  change the inventory mode or add a description to the created host.
- Pass the "details" mandatory parameter to create SNMP hostinterfaces.
  The lack of this parameter prevents the host creation when any of its
  interfaces are SNMP.
@piterpunk
Copy link
Author

@sagetherage to me it's important to have this PR merged before the code-freeze, I am using it at other projects and give maintenance to out-of-tree code isn't good.

@sagetherage
Copy link
Contributor

@piterpunk we have it marked for Aluminium and code freeze is moved by a week and this is looking good! it needs to be rebased with master, but currently passing all tests - hurray!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aluminium Release Post Mg and Pre Si Tests-Passed
Projects
None yet
6 participants