New custom property support#2972
Open
raviks789 wants to merge 93 commits into
Open
Conversation
raviks789
force-pushed
the
dictionary-support
branch
9 times, most recently
from
dd916d4 to
f19501d
Compare
raviks789
force-pushed
the
dictionary-support
branch
2 times, most recently
from
2ddc75b to
3583514
Compare
raviks789
force-pushed
the
dictionary-support
branch
10 times, most recently
from
e818e3e to
3260c35
Compare
raviks789
force-pushed
the
dictionary-support
branch
8 times, most recently
from
01bbc41 to
e1fac6d
Compare
…es in apply-for-rule
…eption TypeError extends Error, not Exception, so a TypeError thrown inside the wrapped callable previously skipped the rollback entirely, leaving the transaction open. This helper is shared by ImportExport, BranchMerger, and DbObjectStore in addition to DeleteCustomVariableForm.
…-level fixed array Deleting an item from a fixed-array that is itself the root custom variable indexed into the stored value using the parent's key_name, but the stored value for a root-level fixed array is the bare list itself, not a dictionary keyed by that name. This threw when the update ran.
…lexicographically key_name is a varchar column, so ordering the reindex query by it sorted '10' right after '1' and before '2' once a fixed array had more than ten items, scrambling the remaining items' order after a deletion.
…through onSuccess() deleted datalist links, object vars, and the property row across several separate statements with no transaction, so a failure partway through (e.g. a malformed stored value) left the database in a half-deleted state. Wraps the whole thing in Db::runFailSafeTransaction().
… nested fields removeObjectCustomVars() and removeFromOverrideServiceVars() both assumed the root property was at most one hop above the deleted field's parent. Dictionaries can nest arbitrarily deep, so anything nested three or more levels down was resolved against the wrong root, causing the update to silently touch nothing (or the wrong path). resolveRootProperty() now walks parent_uuid all the way up instead of guessing the depth.
… deleted field removeObjectCustomVars() looped over a dynamic dictionary's entries by value and mutated the loop copy, so the removal was only written back to the stored value when an entry became fully empty -- any entry that survived with other fields intact kept the deleted field. Extracts the correct by-reference version (already used by removeFromOverrideServiceVars) into a shared removeDictionaryItemFromEveryEntry() used by both.
… of delete+reinsert updateFixedArrayItems() deleted every sibling item and reinserted them with new key_names, which needlessly cascaded away (and had to manually restore) each item's director_property_datalist link, and would silently drop any other table's reference to an item's uuid, such as an icinga_host_property "required" binding on an individual item. Deletes only the removed item and updates key_name in place on survivors.
… mode The skip for datafields with an unsupported value type only ran when verbose output was enabled, since the continue was nested inside the isVerbose check along with the message. Without --verbose the loop fell through and tried to insert the unsupported value_type into the director_property table, which failed because the column is an enum.
migrateDatafields used a bare beginTransaction/commit around its insert loop instead of the fail safe transaction helper, so if any insert in the loop threw partway through, the transaction was left open instead of being rolled back. Wrapping the loop body in a closure and running it through Db::runFailSafeTransaction lets it roll back cleanly on any error, while dry runs still skip the transaction entirely since they never write.
…lly written migratedDataFields got the same entry added twice, once unconditionally right before the dry run check and once again inside the write branch. Because of the unconditional write, a dry run with --delete --verbose would print datafields as having been migrated and deleted even though nothing was ever touched. Dropping the duplicate outside the write branch means the list only ever reflects what was actually inserted.
upgrade_186.sql already adds a unique index on director_datalist.uuid. But this is not added in the mysql.sql. Hence, there is a need to check if the unique index was already created for the uuid column in director_datalist before creating it.
This index is about making the property-uuid-first lookups not scan the whole table.
…remove duplicated code
… during migration
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Icinga 2 supports dictionaries as native variable type, but Director had not completely supported the dictionary type custom variables. Operators using service apply-for rules to generate services from dictionary type host custom variables were forced to define those variables outside Director entirely, losing managed configuration. This PR closes both gaps together: Director now supports a dictionary type custom variable and service apply-for rules can iterate over them directly.
Changes
--dry-runand--deleteoptions. The export command includes custom variable definitions.