[checkmk-commits] Fixed broken checks when check includes depend on other includes

Module: check_mk Branch: master Commit: 7810a3743458e17f55649014e2ed372a9c527fe0 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=7810a3743458e1… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Wed Jul 5 11:57:48 2017 +0200 Fixed broken checks when check includes depend on other includes Change-Id: I8acbc4919d29fe9b8db50d4059e8b0f72a7f661b --- cmk_base/checks.py | 78 +++++++++++++++++++++---------------------- cmk_base/inventory_plugins.py | 53 +++++++++++++++++------------ 2 files changed, 70 insertions(+), 61 deletions(-) diff --git a/cmk_base/checks.py b/cmk_base/checks.py index 7ab93a3..53a72fb 100644 --- a/cmk_base/checks.py +++ b/cmk_base/checks.py @@ -44,8 +44,8 @@ import cmk_base.check_api as check_api # like for example check_exists(...) _check_contexts = {} # The checks are loaded into this dictionary. Each check - # becomes a separat sub-dictionary, named by the check name -_include_contexts = {} # These are the contexts of the check include files + # has a separate sub-dictionary, named by the check name. + # It is populated with the includes and the check itself. # The following data structures will be filled by the checks check_info = {} # all known checks @@ -94,11 +94,7 @@ def get_plugin_paths(*dirs): filelist = [] for dir in dirs: filelist += _plugin_pathnames_in_directory(dir) - - # read include files always first, but still in the sorted - # order with local ones last (possibly overriding variables) - return [ f for f in filelist if f.endswith(".include") ] + \ - [ f for f in filelist if not f.endswith(".include") ] + return filelist # Now read in all checks. Note: this is done *before* reading the @@ -115,7 +111,6 @@ def load_checks(filelist): continue # ignore editor backup / temp files file_name = os.path.basename(f) - is_include = file_name.endswith(".include") if file_name in loaded_files: continue # skip already loaded files (e.g. from local) @@ -125,6 +120,8 @@ def load_checks(filelist): known_vars = check_context.keys() known_checks = check_info.keys() + load_check_includes(f, check_context) + execfile(f, check_context) loaded_files.add(file_name) @@ -136,12 +133,10 @@ def load_checks(filelist): continue new_checks = set(check_info.keys()).difference(known_checks) - if is_include: - _include_contexts[file_name] = check_context - else: - # Now store the check context for all checks found in this file - for check_name in new_checks: - _check_contexts[check_name] = check_context + + # Now store the check context for all checks found in this file + for check_name in new_checks: + _check_contexts[check_name] = check_context # Collect all variables that the check file did introduce compared to the # default check context @@ -171,17 +166,7 @@ def load_checks(filelist): # Keep track of which variable needs to be set to which context context_ident_list = _check_variables.setdefault(varname, []) - if is_include: - context_ident_list.append(file_name) - else: - context_ident_list += new_checks - - # Hack to make dependencies between multiple includes work. In case we - # need more here we need to find another solution. - # TODO(lm): This needs to be cleaned up. Try to move the includes to - # python modules that are separated from each other and can refer to - # each other. - _include_contexts["if64.include"].update(_include_contexts["if.include"]) + context_ident_list += new_checks config.add_check_variables(check_variable_defaults) @@ -214,19 +199,35 @@ def _new_check_context(check_file_path): for k, v in check_api._get_check_context(): context[k] = v - # Load the definitions of the required include files for this check - # Working with imports when specifying the includes would be much cleaner, - # sure. But we need to deal with the current check API. - if check_file_path and not check_file_path.endswith(".include"): - for include_file_name in includes_of_plugin(check_file_path): - try: - context.update(_include_contexts[include_file_name]) - except KeyError, e: - raise MKGeneralException("The include file %s does not exist" % include_file_name) - return context +# Load the definitions of the required include files for this check +# Working with imports when specifying the includes would be much cleaner, +# sure. But we need to deal with the current check API. +def load_check_includes(check_file_path, check_context): + for include_file_name in includes_of_plugin(check_file_path): + include_file_path = check_include_file_path(include_file_name) + try: + execfile(include_file_path, check_context) + except Exception, e: + console.error("Error in check include file %s: %s\n", include_file_path, e) + if cmk.debug.enabled(): + raise + else: + continue + + +def check_include_file_path(include_file_name): + include_file_path = os.path.join(cmk.paths.checks_dir, include_file_name) + + local_path = os.path.join(cmk.paths.local_checks_dir, include_file_name) + if os.path.exists(local_path): + include_file_path = local_path + + return include_file_path + + # Parse the check file without executing the code to find the check include # files the check uses. The following statements are extracted: # check_info[...] = { "includes": [...] } @@ -276,7 +277,7 @@ def _plugin_pathnames_in_directory(path): return sorted([ path + "/" + f for f in os.listdir(path) - if not f.startswith(".") + if not f.startswith(".") and not f.endswith(".include") ]) else: return [] @@ -301,10 +302,7 @@ def get_include_context(include_file_name): def set_check_variable(varname, value): globals()[varname] = value for context_ident in _check_variables[varname]: - if context_ident.endswith(".include"): - _include_contexts[context_ident][varname] = value - else: - _check_contexts[context_ident][varname] = value + _check_contexts[context_ident][varname] = value # FIXME: Clear / unset all legacy variables to prevent confusions in other code trying to diff --git a/cmk_base/inventory_plugins.py b/cmk_base/inventory_plugins.py index 43b396a..221ec20 100644 --- a/cmk_base/inventory_plugins.py +++ b/cmk_base/inventory_plugins.py @@ -57,13 +57,15 @@ def load(): # pylint: disable=function-redefined continue # ignore editor backup / temp files file_name = os.path.basename(f) - is_include = file_name.endswith(".include") if file_name in loaded_files: continue # skip already loaded files (e.g. from local) try: plugin_context = _new_inv_context(f) known_plugins = inv_info.keys() + + load_plugin_includes(f, check_context) + execfile(f, plugin_context) loaded_files.add(file_name) except Exception, e: @@ -73,12 +75,9 @@ def load(): # pylint: disable=function-redefined else: continue - if is_include: - _include_contexts[file_name] = plugin_context - else: - # Now store the check context for all plugins found in this file - for check_name in set(inv_info.keys()).difference(known_plugins): - _plugin_contexts[check_name] = plugin_context + # Now store the check context for all plugins found in this file + for check_name in set(inv_info.keys()).difference(known_plugins): + _plugin_contexts[check_name] = plugin_context def _new_inv_context(plugin_file_path): @@ -95,23 +94,35 @@ def _new_inv_context(plugin_file_path): for k, v in check_api._get_check_context() + _get_inventory_context(): context[k] = v - # Load the definitions of the required include files for this check - # Working with imports when specifying the includes would be much cleaner, - # sure. But we need to deal with the current check API. - if plugin_file_path and not plugin_file_path.endswith(".include"): - for include_file_name in checks.includes_of_plugin(plugin_file_path): - try: - context.update(_include_contexts[include_file_name]) - except KeyError, e: - # inventory plugins may also use check includes. Try to find one. - try: - context.update(checks.get_include_context(include_file_name)) - except KeyError, e: - raise MKGeneralException("The include file %s does not exist" % include_file_name) - return context +# Load the definitions of the required include files for this check +# Working with imports when specifying the includes would be much cleaner, +# sure. But we need to deal with the current check API. +def load_plugin_includes(check_file_path, plugin_context): + for include_file_name in checks.includes_of_plugin(check_file_path): + include_file_path = os.path.join(cmk.paths.inventory_dir, include_file_name) + + local_path = os.path.join(cmk.paths.local_inventory_dir, include_file_name) + if os.path.exists(local_path): + include_file_path = local_path + + # inventory plugins may also use check includes. Try to find one. + if not os.path.exists(include_file_path): + include_file_path = check_include_file_path(include_file_name) + + try: + execfile(include_file_path, check_context) + except Exception, e: + console.error("Error in include file %s: %s\n", include_file_path, e) + if cmk.debug.enabled(): + raise + else: + continue + + + def is_snmp_plugin(plugin_type): info_type = plugin_type.split(".")[0] return "snmp_info" in inv_info.get(info_type, {})