[ZBXNEXT-219] Will be nice to implement additional option to XML import: If trigger is missing in the XML file, remove it from the Zabbix database. Created: 2010 Feb 03  Updated: 2014 Dec 10  Resolved: 2014 Aug 01

Status: Closed
Project: ZABBIX FEATURE REQUESTS
Component/s: API (A), Frontend (F)
Affects Version/s: 2.0.0
Fix Version/s: 2.3.3

Type: New Feature Request Priority: Major
Reporter: Igor Danoshaites (Inactive) Assignee: Unassigned
Resolution: Fixed Votes: 6
Labels: import, xml
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by ZBXNEXT-1317 deleteMissing API configuration.import Closed
is duplicated by ZBXNEXT-978 New configuration option: Update exis... Open
is duplicated by ZBXNEXT-2240 Add "deleteNonexisting" rule for all ... Closed

 Description   

Will be nice to implement additional option to XML import functionality: If trigger is missing in the XML file, remove it from the Zabbix database.



 Comments   
Comment by richlv [ 2010 Feb 03 ]

maybe useful to implement that as an additional option for all elements - if missing, remove ?
so it would work for hosts, items, triggers, template linkage, applications, host macros...

Comment by Thomas Spengler [ 2010 May 11 ]

would be a nice option

Comment by Igor Danoshaites (Inactive) [ 2010 May 11 ]

Thomas, please note that you can ALSO vote for this issue.
In this case if this issue will have sufficient number of votes it could be implemented more quickly.

Comment by Alexander Vladishev [ 2014 May 27 ]

Specification: https://www.zabbix.org/wiki/Docs/specs/ZBXNEXT-219

Comment by Ivo Kurzemnieks [ 2014 Jun 18 ]

(1) Translation string changes:

  • Added new strings:
    • Create new
    • Delete missing
    • Delete all elements that are not present in the XML file?
    • Cannot find item "%1$s" on "%2$s" used in trigger prototype "%3$s" of discovery rule "%4$s" on "%5$s".
    • Cannot find item "%1$s" on "%2$s" used in graph prototype "%3$s" of discovery rule "%4$s" on "%5$s".
    • Cannot find item "%1$s" on "%2$s" used in graph "%3$s".
    • Cannot find item "%1$s" on "%2$s" used in trigger "%3$s".
  • Removed strings:
    • Add missing
    • Cannot find host or template "%1$s" used in graph "%2$s".

jelisejev CLOSED.

Comment by Ivo Kurzemnieks [ 2014 Jun 18 ]

RESOLVED in svn://svn.zabbix.com/branches/dev/ZBXNEXT-219

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 03 ]

(2) I've exported a host and tried to import it back with all of the delete checkboxes enabled. I received an error:

No permissions to referred object or it does not exist! [conf.import.php:182 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->deleteMissingItems() → CItem->delete() → CApiService::exception() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/api/classes/CItem.php:630]

iivs It was trying to delete webitems. RESOLVED in r47148

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 03 ]

(3) I've exported the host and removed all graphs from the XML. When I import it back, the graphs are not deleted. Same thing with triggers and may be other objects.

iivs Indeed there was a check if XML contains no objects, they were not touched.
RESOLVED in r47145

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 04 ]

(4) We'll need to implement the same functionality for applications. Additionally, there are two problems with the configuration.import "applications" parameter:

  1. The "createMissing" parameter is not implemented at all. The API always creates applications.
  2. The "updateExisting" must be removed for "applications. There's nothing to update.

iivs RESOLVED in r47154 (checkboxes and v1.8 import), r47157 (trunk import)

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 04 ]

(5) I've exported a host and removed one of the items. When I try to import it with the delete checkbox checked for items I get the following error:

No permissions to referred object or it does not exist! [conf.import.php:182 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->deleteMissingItems() → CItem->delete() → CApiService::exception() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/api/classes/CItem.php:630]

iivs Sames as (1) it was trying to delete webitems. RESOLVED in r47148

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(6) In conf.import.php:

	$cbExist = SPACE;
	$cbMissed = SPACE;
	$cbDeleted = SPACE;

The variables should be set to an empty string, not  .

iivs Seems to work just fine with "null". RESOLVED in r47169

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(7) In conf.import.js.php:

  1. The confirmDeleteMissing() function shouldn't be bound to the form in the php code but in conf.import.js.php instead.
  2. Instead of iterating over jQuery('.deleteMissing') you can use jQuery('.deleteMissing:checked').

iivs RESOLVED in r47168

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(8) In CConfigurationImport::processItems() this check

if (!$this->options['items']['createMissing'] && !$this->options['items']['updateExisting']) {
	return;
}

duplicates the check in getFormattedItems():

if ($this->options['items']['updateExisting']
	|| $this->options['items']['createMissing']
	|| $this->options['items']['deleteMissing']) {

	$this->formattedData['items'] = $this->formatter->getItems();
}

Same problem in other methods.

iivs I took out the option check from each function and put them where the function was called. To keep the integrity, I did it for all getFormated...() functions. RESOLVED in r47180, r47204 (moved option checks for images)

iivs As we last discussed, I removed option checks so that all references are gathered before processing. RESOLVED in r47361

jelisejev It seems that the $formattedData property and all of the CConfigurationImport::getFormatted* methods can now be removed. They don't do anything now.

iivs Discussed this and came to a conclusion that they cannot be removed, since most get* functions from C20ImporteFormatter class don't store information and will re-format objects on each request.

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(9) Regarding the getProcessedHostOrTemplateIds() method: it would be great to move all of the processed host and template storage logic to a separate container object. It doesn't really belong in neither the CConfigurationImport class, not the CImportReferencer class.

iivs RESOLVED in r47204

jelisejev

  1. Now the CImportedObjectContainer class needs a whole bunch of objects just to implement the getHostIds() and getTemplateIds() methods. This can be avoided if you store IDs inside the object container instead of names.
  2. CConfigurationImport shouldn't create the CImportedObjectContainer object itself. It would be great to refactor the CConfigurationImport class so that it would accept the CImportObjectContainer and the CImportReferencer objects as contructor arguments.
  3. In the deleteMissing* methods you use the output of getHostIds() and getTemplateIds() to resolve host IDs. You should use the resolver object for that and these methods should return only IDs.
  4. It's not a good idea to pass the object container to CImporter objects since it's not obvious, that they are adding data to the container. A better solution would be to change their import() methods to return an array of affected IDs and add those IDs to the container in the CConfigurationImport class.

iivs RESOLVED in r47442, r47451 (accidental delete if array_flip())

jelisejev

  1. I've made a few changes in r47473, please review.
  2. Please update the PHP doc of CConfigurationImport::_construct() and CImporter::_construct().

iivs r47473 REVIEWED PHP doc updated and RESOLVED in r47521, r47525

jelisejev Thanks for the update, missed that. CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(10) I've tried to import an XML file that contains a host and a template. The import template checkboxes were checked, hosts - not. I got an undefined index:

Undefined index: Zabbix server [conf.import.php:182 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->deleteMissingItems() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/include/classes/import/CConfigurationImport.php:1340]

iivs Indeed both hosts and templates at the same time resulted in unexpected results, since they usually were not exported in one file. RESOLVED in r47170

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 09 ]

(11) Same XML file as in (9), but now the import template checkboxes are checked. I get a different error:

Undefined index: Template Virt VMware Hypervisor [conf.import.php:182 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->deleteMissingItems() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/include/classes/import/CConfigurationImport.php:1340]

The problem is in the following code in getProcessedHostOrTemplateIds():

$hosts = $this->getFormattedHosts();
if ($hosts) {
	$hosts = zbx_objectValues($hosts, 'host');
}
else {
	$hosts = $this->getFormattedTemplates();
	$hosts = zbx_objectValues($hosts, 'host');
}

It doesn't select templates if hosts are present.

iivs Separated so they both are now selected. RESOLVED in r47170

jelisejev It seems that this commit is incorrect and can be reverted.

iivs Yes, we changed the option checks so all data is gathered including hosts and templates. Changes reverted. RESOLVED in r47527

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 11 ]

(12) In CConfigurationImport::deleteMissingApplications():

foreach ($applications as $application) {
	$applicationId = $this->referencer->resolveApplication($hostId, $application['name']);
	$applicationIdsXML[$applicationId] = $applicationId;
}

Here we need to check whether the application has really been resolved, it may be missing. There are some similar places in the rest of the code.

iivs RESOLVED in r47355

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 11 ]

(13) In CConfigurationImport::deleteMissingTriggers():

if ($triggerId) {
	$triggersXML[$triggerId]['triggerid'] = $triggerId;
	$triggersXML[$triggerId]['hosts'] = $this->referencer->resolveTriggerHostsById($triggerId);
}

You resolve the trigger hosts array but don't use it afterwards.

iivs Probably this was left over due to some previous logic I wrote. Redundant code removed. RESOLVED in r47365

jelisejev

  1. The CImportReferencer::$triggerHosts property needs to be removed as well.
  2. And this line from CXmlImport18:
    $graphsXML[$current_graph['graphid']]['hosts'] = $current_graph['hosts'];
    

iivs RESOLVED in r47429

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 14 ]

(14) The following permission check in CConfigurationImport::deleteMissingGraphs() is not required:

		// check permissions only if there was no update or create. Else permissions are already checked.
		if (!$this->options['graphs']['createMissing'] && !$this->options['graphs']['updateExisting']) {
			$allowedGraphs = API::Graph()->get(array(
				'output' => array('graphid'),
				'graphids' => array_keys($graphsXML),
				'editable' => true,
				'preservekeys' => true
			));
			foreach ($graphsXML as $graph) {
				if (!isset($allowedGraphs[$graph['graphid']])) {
					throw new Exception(_s('No permission for graph "%1$s".', $graph['name']));
				}
			}
		}

iivs Redundant permission check removed. RESOLVED in r47366

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 14 ]

(15) Please implement a CImportReferencer::resolveGraph() method and use it where necessary to keep the code more consistent.

iivs I aslo removed some of the add*Ref() in several places. Either function was not used or was out of place, since we anyway refresh referencer objects.
RESOLVED in r47427

jelisejev

  1. In ConfigurationImport::processDiscoveryRules() and processGraphs() the following loop can be moved inside the main loop:
    foreach ($item['graph_prototypes'] as $graph) {
    	if (isset($graph['graphid'])) {
    		$graphsToUpdate[] = $graph;
    	}
    	else {
    		$graphsToCreate[] = $graph;
    	}
    }
    
  2. Screen importers must also use the import referencer to resolve graphs.
  3. I've removed some unused variables in r47483.

iivs
1. RESOLVED in r47530
2. RESOLVED in r47532
3. REVIEWED. Thanks!

jelisejev I've made a minor improvement in r47580, please review. Otherwise good.

iivs OK, Thanks!
CLOSED

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 14 ]

(16) In CConfigurationImport::deleteMissingApplications() line 1492 you use "nopermissions", while in other similar places you use "editable" instead. You should use "nopermissions" there as well.

iivs RESOLVED in r47367

jelisejev It's better to replace "editable" with "nopermissions" for sake of performance.

iivs RESOLVED in r47447, r47448

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 14 ]

(17) In CTemplateScreenImporter::delete() you use the list of templates returned by the import formatter, when you should use the list of processed templates as in all other places.

CTemplateScreenImporter::import() should also skip hosts that have not been processed.

iivs RESOLVED in r47373

jelisejev I've made a minor correction in r47401, please review.

iivs REVIEWED. Thanks! CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 17 ]

(18) When you delete missing triggers and graphs, you shouldn't delete them one-by-one but in bulk.

iivs RESOLVED in r47451

jelisejev You have a variable named $triggerIdsToDelete which contains triggers and a variable $triggersToDelete that contains IDs. Their names should be switched.

iivs I also renamed deletable graph variables the same way. RESOLVED in r47533

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 17 ]

(19) I suggest to rename the $hostIdsXML variable to something like $processedHostIds, otherwise I'm often confusing them with hosts that are just present in the XML file.

iivs RESOLVED in r47452, r47461 (renamed variables for 1.8)

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 17 ]

(20) I've exported a host from 1.8 and imported it into 2.3. Then I deleted all of the objects from the hosts, opened the import form and tried to import it again with all of the create and delete checkboxes checked. Nothing was imported.

iivs Yes, turns out they were deleted just after import. RESOLVED in r47457, r47458

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 21 ]

(21) CImportReferencer::selectTriggers() and selectGraphs() performs two API requests, when only one is required.

iivs RESOLVED in r47539

jelisejev

  1. Now I can't import a screen that references a discovered graph or trigger.
  2. Please simplify this code in selectTriggers():
    foreach ($dbTriggers as $dbTrigger) {
    	$dbExpr = explode_exp($dbTrigger['expression']);
    	foreach ($this->triggers as $name => $expressions) {
    		if ($name == $dbTrigger['description']) {
    			foreach ($expressions as $expression) {
    				if ($expression == $dbExpr) {
    					$this->triggersRefs[$name][$expression] = $dbTrigger['triggerid'];
    				}
    			}
    		}
    	}
    }
    

iivs RESOLVED in r47605

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 25 ]

(22) We'll need to change the order how objects are imported: first we'll need to delete the missing objects and then import the rest. Otherwise it's possible to import an XML which contains an invalid configuration and it will not trigger any errors.

iivs RESOLVED in r47668

jelisejev CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 31 ]

(23) I've exported a host with some items and applications. Removed all applications from the XML and tried to import it with the remove applications checkbox checked. I received an error:

Item "Available memory" on "Zabbix server": application "Memory" does not exist. [conf.import.php:191 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processItems() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/include/classes/import/CConfigurationImport.php:172]

jelisejev My mistake, this is the intended behavior. CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 31 ]

(24) I've exported a host with an LLD rule with trigger prototypes. Deleted all item prototypes in the XML and imported it back with the delete discovery rule checkbox checked. I received an error:

No permissions to referred object or it does not exist! [conf.import.php:191 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() → CTriggerPrototype->update() → CApiService::exception() in /opt/lampp/htdocs/zabbix/trunk/frontends/php/api/classes/CTriggerPrototype.php:504]

Here's what happens:
1. When I import the XML it calls deleteMissingDiscoveryRules() first and loads the trigger references into the cache.
2. Then it deletes the item prototypes which by cascade deletes the trigger prototypes. But the prototypes remain in the reference cache.
3. When it tries to import the trigger prototypes in the XML, it uses the cached trigger references to decide whether to import or to create the new trigger. Since this triggers is still in the cache, it thinks that it exists in the database and tries to update it. This results in the error.

Before importing trigger prototypes we need to parse the expression and check if all of the items exist. We'll need to do the same for normal triggers. This is for trunk import only.

iivs RESOLVED in r47763

jelisejev I've simplified the code in r47784, please review.

iivs REVIEWED.
Thanks!
CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Jul 31 ]

(25) I've added a check if the items exists before importing a graph or graph prototype in r47703. Please review.

I've also removed some unused vars in r47738.

iivs Thanks! I corrected one error message in r47763 from 'Cannot find item "%1$s" on "%2$s" used in prototype "%3$s".' to 'Cannot find item "%1$s" on "%2$s" used in graph "%3$s".' since the code was graph related, not prototype.

jelisejev Thanks, missed that. CLOSED.

Comment by Pavels Jelisejevs (Inactive) [ 2014 Aug 04 ]

TESTED.

Please review and close (23) before merging.

Comment by Ivo Kurzemnieks [ 2014 Aug 04 ]

Ability to remove resources missing in XML file.

Implemented in pre-2.3.3 trunk r47805

Comment by Ivo Kurzemnieks [ 2014 Aug 04 ]

(26) API changes updated:
https://www.zabbix.com/documentation/2.4/manual/api/reference/configuration/import
https://www.zabbix.com/documentation/2.4/manual/api/changes_2.2_-_2.4

jelisejev I've made some corrections https://www.zabbix.com/documentation/2.4/manual/api/reference/configuration/import?rev=1407160244&do=diff. Otherwise good. CLOSED.

Comment by richlv [ 2014 Aug 22 ]

(27) general documentation
we should explicitly note that host/template macros are removed as well

martins-v Noted in:

RESOLVED.

iivs Look good. If no objections... CLOSED.

Comment by richlv [ 2014 Aug 22 ]

subissues still open : 26 & 27

Generated at Fri Mar 29 09:28:51 EET 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.