[checkmk-commits] 7217 FIX HW/SW-Inventory: Execution order of plugins

Module: check_mk Branch: master Commit: 6ada74d0c575aacfb8e56975959dbf519891b45f URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6ada74d0c575aa… Author: Moritz Kiemer &lt;mo(a)mathias-kettner.de&gt; Date: Fri Mar 15 14:35:12 2019 +0100 7217 FIX HW/SW-Inventory: Execution order of plugins The order in which the HW/SW inventory plugins are executed is now deterministic. Previously dependencies between inventory plugins could lead to missing data if the plugins happend to be executed in the wrong order. Plugins are now executed in alphabetical order by default. You can force a plugin to be executed after another plugin by setting the <tt>depends_on</tt> key in the inventory info variable <tt>inv_info</tt>. For instance writing <tt>inv_info['plugin_a'] = { ... 'depends_on' = ['plugin_b'], }</tt> will enforce "plugin_b" to be executed before "plugin_a". Change-Id: I388bf0c0fe334ad87ca38905f66bf2d5141be98f --- .werks/7217 | 21 ++++++++++++ cmk_base/inventory.py | 5 +-- cmk_base/inventory_plugins.py | 46 +++++++++++++++++++++++++ inventory/cisco_vlans | 1 + tests/unit/cmk_base/test_inventory_plugins.py | 48 +++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/.werks/7217 b/.werks/7217 new file mode 100644 index 0000000..732fe11 --- /dev/null +++ b/.werks/7217 @@ -0,0 +1,21 @@ +Title: HW/SW-Inventory: Execution order of plugins +Level: 1 +Component: checks +Compatible: compat +Edition: cre +Version: 1.6.0i1 +Date: 1552656409 +Class: fix + +The order in which the HW/SW inventory plugins are executed is now deterministic. +Previously dependencies between inventory plugins could lead to missing data if +the plugins happend to be executed in the wrong order. + +Plugins are now executed in alphabetical order by default. You can force a plugin +to be executed after another plugin by setting the <tt>depends_on</tt> key in +the inventory info variable <tt>inv_info</tt>. For instance writing +<tt>inv_info['plugin_a'] = { + ... + 'depends_on' = ['plugin_b'], +}</tt> +will enforce "plugin_b" to be executed before "plugin_a". diff --git a/cmk_base/inventory.py b/cmk_base/inventory.py index cb6794f..f1f54fb 100644 --- a/cmk_base/inventory.py +++ b/cmk_base/inventory.py @@ -237,12 +237,9 @@ def _do_inv_for_realhost(sources, multi_host_sections, hostname, ipaddress, inve console.step("Executing inventory plugins") import cmk_base.inventory_plugins as inventory_plugins console.verbose("Plugins:") - for section_name, plugin in inventory_plugins.inv_info.items(): + for section_name, plugin in inventory_plugins.sorted_inventory_plugins(): section_content = multi_host_sections.get_section_content( hostname, ipaddress, section_name, for_discovery=False) - if section_content is None: # No data for this check type - continue - # TODO: Don't we need to take config.check_info[check_plugin_name]["handle_empty_info"]: # like it is done in checking.execute_check()? Standardize this! if not section_content: # section not present (None or []) diff --git a/cmk_base/inventory_plugins.py b/cmk_base/inventory_plugins.py index 076682b..366143a 100644 --- a/cmk_base/inventory_plugins.py +++ b/cmk_base/inventory_plugins.py @@ -29,6 +29,7 @@ from typing import Any, Dict # pylint: disable=unused-import import cmk.utils.paths import cmk.utils.debug +from cmk.utils.exceptions import MKGeneralException import cmk_base.config as config import cmk_base.console as console @@ -127,3 +128,48 @@ def is_snmp_plugin(plugin_type): section_name = cmk_base.check_utils.section_name_of(plugin_type) return "snmp_info" in inv_info.get(section_name, {}) \ or cmk_base.check_utils.is_snmp_check(plugin_type) + + +def sorted_inventory_plugins(): + + # First resolve *all* dependencies. This ensures that there + # are no cyclic dependencies, and that the 'depends on' + # relation is transitive. + resolved_dependencies = {} + + def resolve_plugin_dependencies(plugin_name, known_dependencies=None): + '''recursively aggregate all plugin dependencies''' + if known_dependencies is None: + known_dependencies = set() + if plugin_name in resolved_dependencies: + known_dependencies.update(resolved_dependencies[plugin_name]) + return known_dependencies + + try: + direct_dependencies = set(inv_info[plugin_name].get('depends_on', [])) + except KeyError: + raise MKGeneralException("unknown plugin dependency: %r" % plugin_name) + + new_dependencies = direct_dependencies - known_dependencies + known_dependencies.update(new_dependencies) + for dependency in new_dependencies: + known_dependencies = resolve_plugin_dependencies(dependency, known_dependencies) + return known_dependencies + + for plugin_name in inv_info: + resolved_dependencies[plugin_name] = resolve_plugin_dependencies(plugin_name) + if plugin_name in resolved_dependencies[plugin_name]: + raise MKGeneralException("cyclic plugin dependencies for %r" % plugin_name) + + # The plugins are now a partially ordered set with respect to + # the 'depends on' relation. That means we can iteratively + # yield the minimal elements + remaining_plugins = set(inv_info.keys()) + yielded_plugins = set() + while remaining_plugins: + for plugin_name in sorted(remaining_plugins): + dependencies = resolved_dependencies[plugin_name] + if dependencies <= yielded_plugins: + yield plugin_name, inv_info[plugin_name] + yielded_plugins.add(plugin_name) + remaining_plugins.remove(plugin_name) diff --git a/inventory/cisco_vlans b/inventory/cisco_vlans index 795417b..7ec6b6d 100644 --- a/inventory/cisco_vlans +++ b/inventory/cisco_vlans @@ -112,6 +112,7 @@ def inv_cisco_vlans(info, params): inv_info['inv_cisco_vlans'] = { + 'depends_on': ['inv_if'], "inv_function": inv_cisco_vlans, 'snmp_info': ( ".1.3.6.1.4.1.9.9.68.1.2.2.1", diff --git a/tests/unit/cmk_base/test_inventory_plugins.py b/tests/unit/cmk_base/test_inventory_plugins.py new file mode 100644 index 0000000..35499cf --- /dev/null +++ b/tests/unit/cmk_base/test_inventory_plugins.py @@ -0,0 +1,48 @@ +import pytest +import cmk_base.inventory_plugins +from cmk.utils.exceptions import MKGeneralException + + +(a)pytest.mark.parametrize(&quot;inv_info,expected_order&quot;rder", [ + ({ + 'a_plugin': {}, + 'b_plugin': {} + }, ['a_plugin', 'b_plugin']), + ({ + 'a_plugin': { + 'depends_on': ['b_plugin'] + }, + 'b_plugin': {} + }, ['b_plugin', 'a_plugin']), +]) +def test_iteritems_sorted_by_dependency(monkeypatch, inv_info, expected_order): + monkeypatch.setattr(cmk_base.inventory_plugins, "inv_info", inv_info) + order = [item[0] for item in cmk_base.inventory_plugins.sorted_inventory_plugins()] + assert order == expected_order + + +def test_iteritems_sorted_by_dependency_cyclic(monkeypatch): + monkeypatch.setattr( + cmk_base.inventory_plugins, "inv_info", { + "a_plugin": { + 'depends_on': ["b_plugin"] + }, + "b_plugin": { + 'depends_on': ["c_plugin"] + }, + "c_plugin": { + 'depends_on': ["a_plugin"] + }, + }) + with pytest.raises(MKGeneralException, match="cyclic plugin dependencies for '._plugin'"): + list(cmk_base.inventory_plugins.sorted_inventory_plugins()) + + +def test_iteritems_sorted_by_dependency_unknown(monkeypatch): + monkeypatch.setattr(cmk_base.inventory_plugins, "inv_info", { + "a_plugin": { + 'depends_on': ["whoopdeedoo"] + }, + }) + with pytest.raises(MKGeneralException, match="unknown plugin dependency: 'whoopdeedoo'"): + list(cmk_base.inventory_plugins.sorted_inventory_plugins())