From 9f3b3df3752838d294431132349b1a75542f41c5 Mon Sep 17 00:00:00 2001 From: Moritz Kiemer Date: Mon, 16 Sep 2024 12:59:16 +0200 Subject: [PATCH] SSC backend: remove 'get_active_service_description' This function originated from a time when it was cheaper to only compute the description, but this is not the case anymore. These two functions only differ in the check that the description is not empty and the deduplication. In both callsites we actually want that, though (or it makes no difference). Change-Id: Ib7baf5db49d2320258d6a7711aa13c14aea9652e --- cmk/base/automations/check_mk.py | 50 ++++---- cmk/base/server_side_calls/_active_checks.py | 25 ---- .../server_side_calls/test_active_checks.py | 108 ------------------ 3 files changed, 23 insertions(+), 160 deletions(-) diff --git a/cmk/base/automations/check_mk.py b/cmk/base/automations/check_mk.py index dc21619ea68..ea977360609 100644 --- a/cmk/base/automations/check_mk.py +++ b/cmk/base/automations/check_mk.py @@ -543,7 +543,7 @@ def _active_check_preview_rows( host_name: HostName, ip_address_of: config.IPLookup, ) -> Sequence[CheckPreviewEntry]: - active_checks_ = config_cache.active_checks(host_name) + active_check_rules = config_cache.active_checks(host_name) host_attrs = config_cache.get_host_attributes(host_name, ip_address_of) ignored_services = config.IgnoredServices(config_cache, host_name) ruleset_matcher = config_cache.ruleset_matcher @@ -586,30 +586,26 @@ def make_final_service_name(sn: ServiceName) -> ServiceName: password_store_file, ) - return list( - { - active_service.description: CheckPreviewEntry( - check_source=make_check_source(active_service.description), - check_plugin_name=active_service.plugin_name, - ruleset_name=None, - discovery_ruleset_name=None, - item=active_service.description, - old_discovered_parameters={}, - new_discovered_parameters={}, - effective_parameters={}, - description=active_service.description, - state=None, - output=make_output(active_service.description), - metrics=[], - old_labels={}, - new_labels={}, - found_on_nodes=[host_name], - ) - for active_service in active_check_config.get_active_service_descriptions( - active_checks_ - ) - }.values() - ) + return [ + CheckPreviewEntry( + check_source=make_check_source(active_service.description), + check_plugin_name=active_service.plugin_name, + ruleset_name=None, + discovery_ruleset_name=None, + item=active_service.description, + old_discovered_parameters={}, + new_discovered_parameters={}, + effective_parameters=active_service.configuration, + description=active_service.description, + state=None, + output=make_output(active_service.description), + metrics=[], + old_labels={}, + new_labels={}, + found_on_nodes=[host_name], + ) + for active_service in active_check_config.get_active_service_data(active_check_rules) + ] def _execute_discovery( @@ -1541,12 +1537,12 @@ def _get_service_info( ) active_checks = config_cache.active_checks(host_name) - for active_service in active_check_config.get_active_service_descriptions(active_checks): + for active_service in active_check_config.get_active_service_data(active_checks): if active_service.description == servicedesc: return { "origin": "active", "checktype": active_service.plugin_name, - "parameters": active_service.params, + "parameters": active_service.configuration, } return {} # not found diff --git a/cmk/base/server_side_calls/_active_checks.py b/cmk/base/server_side_calls/_active_checks.py index 89b6ba3ec8f..0879fd8a92f 100644 --- a/cmk/base/server_side_calls/_active_checks.py +++ b/cmk/base/server_side_calls/_active_checks.py @@ -33,13 +33,6 @@ class ActiveServiceData: command: tuple[str, ...] -@dataclass(frozen=True) -class ActiveServiceDescription: - plugin_name: str - description: ServiceName - params: Mapping[str, object] | object - - class ActiveCheck: def __init__( self, @@ -144,24 +137,6 @@ def _sanitize_service_descriptions( yield service - def get_active_service_descriptions( - self, active_checks_rules: Iterable[SSCRules] - ) -> Iterator[ActiveServiceDescription]: - for plugin_name, plugin_params in active_checks_rules: - try: - for raw_service in self._iterate_services(plugin_name, plugin_params): - yield ActiveServiceDescription( - plugin_name=raw_service.plugin_name, - description=raw_service.description, - params=raw_service.configuration, - ) - except Exception as e: - if cmk.ccc.debug.enabled(): - raise - config_warnings.warn( - f"Config creation for active check {plugin_name} failed on {self.host_name}: {e}" - ) - def _autodetect_plugin(command: str, module_name: str | None) -> str: libexec_paths = (family_libexec_dir(module_name),) if module_name else () diff --git a/tests/unit/cmk/base/server_side_calls/test_active_checks.py b/tests/unit/cmk/base/server_side_calls/test_active_checks.py index 83b29bd0b7b..714c1f152b7 100644 --- a/tests/unit/cmk/base/server_side_calls/test_active_checks.py +++ b/tests/unit/cmk/base/server_side_calls/test_active_checks.py @@ -14,7 +14,6 @@ from cmk.utils.hostaddress import HostName from cmk.base.server_side_calls import _active_checks, ActiveCheck, ActiveServiceData -from cmk.base.server_side_calls._active_checks import ActiveServiceDescription from cmk.discover_plugins import PluginLocation from cmk.server_side_calls.v1 import ( @@ -631,110 +630,3 @@ def test_get_active_service_data_warnings( captured = capsys.readouterr() assert captured.out == expected_warning - - -@pytest.mark.parametrize( - "active_check_rules, active_check_plugins, hostname, host_attrs, expected_result", - [ - pytest.param( - [ - ( - "my_active_check", - [ - { - "description": "My active check", - "password": ( - "cmk_postprocessed", - "explicit_password", - (":uuid:1234", "myp4ssw0rd"), - ), - } - ], - ), - ], - { - PluginLocation( - # this is not what we'd expect here, but we need a module that we know to be importable. - f"{__name__}", - "active_check_my_active_check", - ): ActiveCheckConfig( - name="my_active_check", - parameter_parser=lambda p: p, - commands_function=lambda p, *_: ( - [ - ActiveCheckCommand( - service_description="My service", - command_arguments=["--password", p["password"]], - ), - ] - ), - ) - }, - HostName("myhost"), - { - "alias": "my_host_alias", - "_ADDRESS_4": "127.0.0.1", - "address": "127.0.0.1", - "_ADDRESS_FAMILY": "4", - "_ADDRESSES_4": "127.0.0.1", - "_ADDRESSES_6": "", - "display_name": "my_host", - }, - [ - ActiveServiceDescription( - plugin_name="my_active_check", - description="My service", - params={ - "description": "My active check", - "password": ( - "cmk_postprocessed", - "explicit_password", - (":uuid:1234", "myp4ssw0rd"), - ), - }, - ), - ], - id="one_service", - ), - pytest.param( - [ - ("my_active_check", [{"description": "My active check", "param1": "param1"}]), - ], - {}, - HostName("myhost"), - { - "alias": "my_host_alias", - "_ADDRESS_4": "127.0.0.1", - "address": "127.0.0.1", - "_ADDRESS_FAMILY": "4", - "_ADDRESSES_4": "127.0.0.1", - "_ADDRESSES_6": "", - "display_name": "my_host", - }, - [], - id="unimplemented_plugin", - ), - ], -) -def test_get_active_service_descriptions( - monkeypatch: pytest.MonkeyPatch, - active_check_rules: Sequence[tuple[str, Sequence[Mapping[str, object]]]], - active_check_plugins: Mapping[PluginLocation, ActiveCheckConfig], - hostname: HostName, - host_attrs: Mapping[str, str], - expected_result: Sequence[ActiveServiceDescription], -) -> None: - monkeypatch.setitem(password_store.hack.HACK_CHECKS, "my_active_check", True) - active_check_config = ActiveCheck( - active_check_plugins, - hostname, - HOST_CONFIG, - host_attrs, - http_proxies={}, - service_name_finalizer=lambda x: x, - stored_passwords={}, - password_store_file=Path("/pw/store"), - ) - - descriptions = list(active_check_config.get_active_service_descriptions(active_check_rules)) - assert descriptions == expected_result