[checkmk-commits] Fixed handling of edge case on service discovery page with snmp check interval configured

Module: check_mk Branch: master Commit: 2a6bf53693c5304fc4d4e701959f6f7b1faac886 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=2a6bf53693c530… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Thu Nov 16 12:22:57 2017 +0100 Fixed handling of edge case on service discovery page with snmp check interval configured Change-Id: I1c1133ebd225b212c1e1e8891c1753b22758ba0d --- cmk_base/checking.py | 3 ++- cmk_base/data_sources/abstract.py | 18 +++++++++++------- cmk_base/data_sources/piggyback.py | 2 +- cmk_base/discovery.py | 10 ++++++++++ tests/cmk_base/test_data_sources.py | 10 +++++----- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/cmk_base/checking.py b/cmk_base/checking.py index 046a447..eb55c6b 100644 --- a/cmk_base/checking.py +++ b/cmk_base/checking.py @@ -305,6 +305,7 @@ def execute_check(all_host_infos, hostname, ipaddress, check_name, item, params, x = e.exc_info() raise x[0], x[1], x[2] # re-raise the original exception to not destory the trace + # TODO: Move this to a helper function if info is None: # No data for this check type return False @@ -314,7 +315,7 @@ def execute_check(all_host_infos, hostname, ipaddress, check_name, item, params, # In case of SNMP checks but missing agent response, skip this check. # Special checks which still need to be called even with empty data # may declare this. - if info == [] and checks.is_snmp_check(check_name) \ + if not info and checks.is_snmp_check(check_name) \ and not checks.check_info[check_name]["handle_empty_info"]: return False diff --git a/cmk_base/data_sources/abstract.py b/cmk_base/data_sources/abstract.py index d005145..d17c938 100644 --- a/cmk_base/data_sources/abstract.py +++ b/cmk_base/data_sources/abstract.py @@ -78,14 +78,14 @@ class DataSource(object): try: cpu_tracking.push_phase(self._cpu_tracking_id()) - raw_data = self._get_raw_data(hostname, ipaddress) + raw_data, is_cached_data = self._get_raw_data(hostname, ipaddress) if get_raw_data: return raw_data host_info = self._convert_to_infos(raw_data, hostname) assert isinstance(host_info, HostInfo) - if host_info.persisted_info: + if host_info.persisted_info and not is_cached_data: self._store_persisted_info(hostname, host_info.persisted_info) # Add information from previous persisted infos @@ -115,7 +115,7 @@ class DataSource(object): raw_data = self._read_cache_file(hostname) if raw_data: self._logger.verbose("[%s] Use cached data" % self.id()) - return raw_data + return raw_data, True elif raw_data is None and config.simulation_mode: raise MKAgentError("Got no data (Simulation mode enabled and no cachefile present)") @@ -125,7 +125,7 @@ class DataSource(object): self._write_cache_file(hostname, raw_data) - return raw_data + return raw_data, False def run_raw(self, hostname, ipaddress): @@ -164,7 +164,7 @@ class DataSource(object): may_use_outdated = config.simulation_mode or self._use_outdated_cache_file if not may_use_outdated and cmk_base.utils.cachefile_age(cachefile) > self._max_cachefile_age: self._logger.debug("[%s] Not using cache (Too old. Age is %d sec, allowed is %s sec)" % - (cmk_base.utils.cachefile_age(cachefile), self._max_cachefile_age)) + (self.id(), cmk_base.utils.cachefile_age(cachefile), self._max_cachefile_age)) return # TODO: Use some generic store file read function to generalize error handling, @@ -338,7 +338,12 @@ class DataSource(object): persisted_info = store.load_data_from_file(file_path, {}) persisted_info = self._filter_outdated_persisted_info(persisted_info, hostname) - self._logger.debug("[%s] Loaded persisted sections: %s" % (self.id(), ", ".join(persisted_info.keys()))) + + if not persisted_info: + self._logger.debug("[%s] No persisted sections loaded" % (self.id())) + else: + self._logger.debug("[%s] Loaded persisted sections: %s" % (self.id(), ", ".join(persisted_info.keys()))) + return persisted_info @@ -367,7 +372,6 @@ class DataSource(object): pass elif modified: - print "XXXXXX" self._store_persisted_info(hostname, persisted_info) return persisted_info diff --git a/cmk_base/data_sources/piggyback.py b/cmk_base/data_sources/piggyback.py index c5e51e9..d6e96d4 100644 --- a/cmk_base/data_sources/piggyback.py +++ b/cmk_base/data_sources/piggyback.py @@ -53,4 +53,4 @@ class PiggyBackDataSource(CheckMKAgentDataSource): Special for piggyback: No caching of raw data """ self._logger.verbose("[%s] Execute data source" % self.id()) - return self._execute(hostname, ipaddress) + return self._execute(hostname, ipaddress), False diff --git a/cmk_base/discovery.py b/cmk_base/discovery.py index 160f083..dadbc8e 100644 --- a/cmk_base/discovery.py +++ b/cmk_base/discovery.py @@ -1106,6 +1106,16 @@ def get_check_preview(hostname, use_caches, do_snmp_scan, on_error): exitcode = 3 output = "Error: %s" % e + # TODO: Move this to a helper function + if info is None: # No data for this check type + exitcode = 3 + output = "Received no data" + + if not info and checks.is_snmp_check(check_type) \ + and not checks.check_info[check_type]["handle_empty_info"]: + exitcode = 0 + output = "Received no data" + item_state.set_item_state_prefix(check_type, item) if exitcode == None: diff --git a/tests/cmk_base/test_data_sources.py b/tests/cmk_base/test_data_sources.py index acbee69..d1fbe9e 100644 --- a/tests/cmk_base/test_data_sources.py +++ b/tests/cmk_base/test_data_sources.py @@ -189,7 +189,7 @@ def test_mode_inventory_caching(test_cfg, hosts, cache, force, monkeypatch): assert _counter_run == num_runs finally: # TODO: Can't the mode clean this up on it's own? - cmk_base.data_sources.restore_original_agent_caching_usage() + cmk_base.data_sources.abstract.DataSource.set_use_outdated_cache_file(False) def test_mode_inventory_as_check(test_cfg, monkeypatch, mock): @@ -229,7 +229,7 @@ def test_mode_check_discovery_cached(test_cfg, monkeypatch, mock): assert _counter_run == 2 finally: # TODO: Can't the mode clean this up on it's own? - cmk_base.data_sources.restore_original_agent_caching_usage() + cmk_base.data_sources.abstract.DataSource.set_use_outdated_cache_file(False) def test_mode_discover_all_hosts(test_cfg, monkeypatch, mock): @@ -253,7 +253,7 @@ def test_mode_discover_explicit_hosts_cache(test_cfg, monkeypatch): assert _counter_run == 2 finally: # TODO: Can't the mode clean this up on it's own? - cmk_base.data_sources.restore_original_agent_caching_usage() + cmk_base.data_sources.abstract.DataSource.set_use_outdated_cache_file(False) def test_mode_discover_explicit_hosts_no_cache(test_cfg, monkeypatch): @@ -277,7 +277,7 @@ def test_mode_check_explicit_host_cache(test_cfg, monkeypatch): assert _counter_run == 2 finally: # TODO: Can't the mode clean this up on it's own? - cmk_base.data_sources.restore_original_agent_caching_usage() + cmk_base.data_sources.abstract.DataSource.set_use_outdated_cache_file(False) def test_mode_check_explicit_host_no_cache(test_cfg, monkeypatch): @@ -301,7 +301,7 @@ def test_mode_dump_agent_explicit_host_cache(test_cfg, monkeypatch): assert _counter_run == 2 finally: # TODO: Can't the mode clean this up on it's own? - cmk_base.data_sources.restore_original_agent_caching_usage() + cmk_base.data_sources.abstract.DataSource.set_use_outdated_cache_file(False) def test_mode_dump_agent_explicit_host_no_cache(test_cfg, monkeypatch):