From 9b5bd4094fa415d2c95b09d8e5af039843a79a1d Mon Sep 17 00:00:00 2001 From: Eike Frost Date: Tue, 28 Nov 2017 15:37:50 +0100 Subject: [PATCH] zabbix_host: fix various idempotency problems (#33138) * reorder interfaces handling for force=no, making sure it works when no interfaces are specified in the module parameters when no interfaces are specified on update, use existing interfaces obtained from API. check whether visible_name is set in check_all_properties; if not set as module parameter, no comparison is necessary. Check if description is set as module parameter before comparing as well * link_templates need the same treatment * add inventory update checks and simplify update procedure * make specifying proxy optional on update (keeping it as is when not specified), as well * pep8 fixes * add tls_*-checks for updates and make tls_*-options actually optional --- .../modules/monitoring/zabbix/zabbix_host.py | 185 +++++++++++------- 1 file changed, 110 insertions(+), 75 deletions(-) diff --git a/lib/ansible/modules/monitoring/zabbix/zabbix_host.py b/lib/ansible/modules/monitoring/zabbix/zabbix_host.py index daa0b3dd20e..195d7f28f0e 100644 --- a/lib/ansible/modules/monitoring/zabbix/zabbix_host.py +++ b/lib/ansible/modules/monitoring/zabbix/zabbix_host.py @@ -274,13 +274,13 @@ class Host(object): parameters['proxy_hostid'] = proxy_id if visible_name: parameters['name'] = visible_name - if tls_psk_identity: + if tls_psk_identity is not None: parameters['tls_psk_identity'] = tls_psk_identity - if tls_psk: + if tls_psk is not None: parameters['tls_psk'] = tls_psk - if tls_issuer: + if tls_issuer is not None: parameters['tls_issuer'] = tls_issuer - if tls_subject: + if tls_subject is not None: parameters['tls_subject'] = tls_subject if description: parameters['description'] = description @@ -353,7 +353,7 @@ class Host(object): # get host by host name def get_host_by_host_name(self, host_name): - host_list = self._zapi.host.get({'output': 'extend', 'filter': {'host': [host_name]}}) + host_list = self._zapi.host.get({'output': 'extend', 'selectInventory': 'extend', 'filter': {'host': [host_name]}}) if len(host_list) < 1: self._module.fail_json(msg="Host not found: %s" % host_name) else: @@ -429,7 +429,9 @@ class Host(object): # check all the properties before link or clear template def check_all_properties(self, host_id, host_groups, status, interfaces, template_ids, - exist_interfaces, host, proxy_id, visible_name, description, host_name): + exist_interfaces, host, proxy_id, visible_name, description, host_name, + inventory_mode, inventory_zabbix, tls_accept, tls_psk_identity, tls_psk, + tls_issuer, tls_subject, tls_connect): # get the existing host's groups exist_host_groups = self.get_host_groups_by_host_id(host_id) if set(host_groups) != set(exist_host_groups): @@ -453,14 +455,51 @@ class Host(object): return True # Check whether the visible_name has changed; Zabbix defaults to the technical hostname if not set. - if host['name'] != visible_name and host['name'] != host_name: - return True - - # The Zabbbix API returns an empty description as an empty string - if description is None: - description = '' - if host['description'] != description: - return True + if visible_name: + if host['name'] != visible_name and host['name'] != host_name: + return True + + # Only compare description if it is given as a module parameter + if description: + if host['description'] != description: + return True + + if inventory_mode: + if host['inventory']: + if int(host['inventory']['inventory_mode']) != self.inventory_mode_numeric(inventory_mode): + return True + elif inventory_mode != 'disabled': + return True + + if inventory_zabbix: + proposed_inventory = copy.deepcopy(host['inventory']) + proposed_inventory.update(inventory_zabbix) + if proposed_inventory != host['inventory']: + return True + + if tls_accept is not None: + if int(host['tls_accept']) != tls_accept: + return True + + if tls_psk_identity is not None: + if host['tls_psk_identity'] != tls_psk_identity: + return True + + if tls_psk is not None: + if host['tls_psk'] != tls_psk: + return True + + if tls_issuer is not None: + if host['tls_issuer'] != tls_issuer: + return True + + if tls_subject is not None: + if host['tls_subject'] != tls_subject: + return True + + if tls_connect is not None: + if int(host['tls_connect']) != tls_connect: + return True return False @@ -479,13 +518,13 @@ class Host(object): templates_clear_list = list(templates_clear) request_str = {'hostid': host_id, 'templates': template_id_list, 'templates_clear': templates_clear_list, 'tls_connect': tls_connect, 'tls_accept': tls_accept} - if tls_psk_identity: + if tls_psk_identity is not None: request_str['tls_psk_identity'] = tls_psk_identity - if tls_psk: + if tls_psk is not None: request_str['tls_psk'] = tls_psk - if tls_issuer: + if tls_issuer is not None: request_str['tls_issuer'] = tls_issuer - if tls_subject: + if tls_subject is not None: request_str['tls_subject'] = tls_subject try: if self._module.check_mode: @@ -494,6 +533,15 @@ class Host(object): except Exception as e: self._module.fail_json(msg="Failed to link template to host: %s" % e) + def inventory_mode_numeric(self, inventory_mode): + if inventory_mode == "automatic": + return int(1) + elif inventory_mode == "manual": + return int(0) + elif inventory_mode == "disabled": + return int(-1) + return inventory_mode + # Update the host inventory_mode def update_inventory_mode(self, host_id, inventory_mode): @@ -501,12 +549,7 @@ class Host(object): if not inventory_mode: return - if inventory_mode == "automatic": - inventory_mode = int(1) - elif inventory_mode == "manual": - inventory_mode = int(0) - elif inventory_mode == "disabled": - inventory_mode = int(-1) + inventory_mode = self.inventory_mode_numeric(inventory_mode) # watch for - https://support.zabbix.com/browse/ZBX-6033 request_str = {'hostid': host_id, 'inventory_mode': inventory_mode} @@ -621,20 +664,24 @@ def main(): if interface['type'] == 1: ip = interface['ip'] + # Use proxy specified, or set to 0 + if proxy: + proxy_id = host.get_proxyid_by_proxy_name(proxy) + else: + proxy_id = 0 + # check if host exist is_host_exist = host.is_host_exist(host_name) if is_host_exist: - # Use proxy specified, or set to None when updating host - if proxy: - proxy_id = host.get_proxyid_by_proxy_name(proxy) - else: - proxy_id = 0 - # get host id by host name zabbix_host_obj = host.get_host_by_host_name(host_name) host_id = zabbix_host_obj['hostid'] + # If proxy is not specified as a module parameter, use the existing setting + if proxy is None: + proxy_id = zabbix_host_obj['proxy_hostid'] + if state == "absent": # remove host host.delete_host(host_id, host_name) @@ -646,14 +693,18 @@ def main(): host_groups = host.get_host_groups_by_host_id(host_id) group_ids = host.get_group_ids_by_group_names(host_groups) - if not force: - # get existing groups, interfaces and templates and merge them with ones provided as an argument - # we do not want to overwrite anything if force: no is explicitly used, we just want to add new ones - for group_id in host.get_group_ids_by_group_names(host.get_host_groups_by_host_id(host_id)): - if group_id not in group_ids: - group_ids.append(group_id) + # get existing host's interfaces + exist_interfaces = host._zapi.hostinterface.get({'output': 'extend', 'hostids': host_id}) + + # if no interfaces were specified with the module, start with an empty list + if not interfaces: + interfaces = [] - for interface in host._zapi.hostinterface.get({'output': 'extend', 'hostids': host_id}): + # When force=no is specified, append existing interfaces to interfaces to update. When + # no interfaces have been specified, copy existing interfaces as specified from the API. + # Do the same with templates and host groups. + if not force or not interfaces: + for interface in copy.deepcopy(exist_interfaces): # remove values not used during hostinterface.add/update calls for key in interface.keys(): if key in ['interfaceid', 'hostid', 'bulk']: @@ -666,54 +717,38 @@ def main(): if interface not in interfaces: interfaces.append(interface) + if not force or link_templates is None: template_ids = list(set(template_ids + host.get_host_templates_by_host_id(host_id))) - # get exist host's interfaces - exist_interfaces = host._zapi.hostinterface.get({'output': 'extend', 'hostids': host_id}) - exist_interfaces_copy = copy.deepcopy(exist_interfaces) + if not force: + for group_id in host.get_group_ids_by_group_names(host.get_host_groups_by_host_id(host_id)): + if group_id not in group_ids: + group_ids.append(group_id) # update host - interfaces_len = len(interfaces) if interfaces else 0 - - if len(exist_interfaces) > interfaces_len: - if host.check_all_properties(host_id, host_groups, status, interfaces, template_ids, - exist_interfaces, zabbix_host_obj, proxy_id, visible_name, description, host_name): - host.link_or_clear_template(host_id, template_ids, tls_connect, tls_accept, tls_psk_identity, - tls_psk, tls_issuer, tls_subject) - host.update_host(host_name, group_ids, status, host_id, - interfaces, exist_interfaces, proxy_id, visible_name, description, tls_connect, tls_accept, - tls_psk_identity, tls_psk, tls_issuer, tls_subject) - module.exit_json(changed=True, - result="Successfully update host %s (%s) and linked with template '%s'" - % (host_name, ip, link_templates)) - else: - module.exit_json(changed=False) + if host.check_all_properties(host_id, host_groups, status, interfaces, template_ids, + exist_interfaces, zabbix_host_obj, proxy_id, visible_name, + description, host_name, inventory_mode, inventory_zabbix, + tls_accept, tls_psk_identity, tls_psk, tls_issuer, tls_subject, tls_connect): + host.link_or_clear_template(host_id, template_ids, tls_connect, tls_accept, tls_psk_identity, + tls_psk, tls_issuer, tls_subject) + host.update_host(host_name, group_ids, status, host_id, + interfaces, exist_interfaces, proxy_id, visible_name, description, tls_connect, tls_accept, + tls_psk_identity, tls_psk, tls_issuer, tls_subject) + host.update_inventory_mode(host_id, inventory_mode) + host.update_inventory_zabbix(host_id, inventory_zabbix) + + module.exit_json(changed=True, + result="Successfully update host %s (%s) and linked with template '%s'" + % (host_name, ip, link_templates)) else: - if host.check_all_properties(host_id, host_groups, status, interfaces, template_ids, - exist_interfaces_copy, zabbix_host_obj, proxy_id, visible_name, description, host_name): - host.update_host(host_name, group_ids, status, host_id, interfaces, exist_interfaces, proxy_id, - visible_name, description, tls_connect, tls_accept, tls_psk_identity, tls_psk, tls_issuer, - tls_subject) - host.link_or_clear_template(host_id, template_ids, tls_connect, tls_accept, tls_psk_identity, - tls_psk, tls_issuer, tls_subject) - host.update_inventory_mode(host_id, inventory_mode) - host.update_inventory_zabbix(host_id, inventory_zabbix) - module.exit_json(changed=True, - result="Successfully update host %s (%s) and linked with template '%s'" - % (host_name, ip, link_templates)) - else: - module.exit_json(changed=False) + module.exit_json(changed=False) + else: if state == "absent": # the host is already deleted. module.exit_json(changed=False) - # Use proxy specified, or set to 0 when adding new host - if proxy: - proxy_id = host.get_proxyid_by_proxy_name(proxy) - else: - proxy_id = 0 - if not group_ids: module.fail_json(msg="Specify at least one group for creating host '%s'." % host_name)