[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: |
|
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 ? |
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. |
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:
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:
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. 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:
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:
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:
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
iivs RESOLVED in r47442, r47451 (accidental delete if array_flip())
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:
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:
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
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.
iivs jelisejev I've made a minor improvement in r47580, please review. Otherwise good. iivs OK, Thanks! |
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
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:
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:
Here's what happens: 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. |
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: 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 martins-v Noted in:
RESOLVED. iivs Look good. If no objections... CLOSED. |
Comment by richlv [ 2014 Aug 22 ] |
subissues still open : |