Revert "17042 FIX Systemd Service Summary: Make default state of fail…

…ed units CRIT" This reverts commit a42941e. Change-Id: I389d89f4cb0bfb3214cddcc7899969ab6c212c6c
Checkmk · Jan 29, 2025 · ac3ebe4 · ac3ebe4
1 parent 69d7eaa
commit ac3ebe4
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 140 deletions.
diff --git a/.werks/17042.md b/.werks/17042.md
diff --git a/cmk/base/plugins/agent_based/systemd_units.py b/cmk/base/plugins/agent_based/systemd_units.py
@@ -606,14 +606,11 @@ def check_systemd_units_summary(
     yield Result(state=State.OK, summary=f"Total: {len(units):d}")
     services_organised = _services_split(units, blacklist)
     yield Result(state=State.OK, summary=f"Disabled: {len(services_organised['disabled']):d}")
-
+    # some of the failed ones might be ignored, so this is OK:
     yield Result(
-        state=State(params["states"].get("failed", params["states_default"]))
-        if (number_of_failed_units := sum(s.active_status == "failed" for s in units))
-        else State.OK,
-        summary=f"Failed: {number_of_failed_units:d}",
+        state=State.OK,
+        summary=f"Failed: {sum(s.active_status == 'failed' for s in units):d}",
     )
-
     included_template = "{count:d} {unit_type} {status} ({service_text})"
     yield from _check_non_ok_services(
         services_organised["included"], params, included_template, unit_type

diff --git a/cmk/gui/plugins/wato/check_parameters/systemd_services_summary.py b/cmk/gui/plugins/wato/check_parameters/systemd_services_summary.py
@@ -3,128 +3,109 @@
 # This file is part of Checkmk (https://checkmk.com). It is subject to the terms and
 # conditions defined in the file COPYING, which is part of this source code package.
 
-from typing import Any
-
 from cmk.gui.i18n import _
 from cmk.gui.plugins.wato.utils import (
     CheckParameterRulespecWithoutItem,
     rulespec_registry,
     RulespecGroupCheckParametersApplications,
 )
 from cmk.gui.plugins.wato.utils.simple_levels import SimpleLevels
-from cmk.gui.valuespec import Age, Dictionary, ListOf, Migrate, MonitoringState, RegExp
-
-REQUIRED_STATE_KEYS_AND_STATES = {"active": 0, "inactive": 0, "failed": 2}
-
+from cmk.gui.valuespec import Age, Dictionary, ListOf, MonitoringState, RegExp
 
-def _migrate(params: dict[str, Any]) -> dict[str, Any]:
-    if "states" not in params:
-        return params
 
-    for key, value in REQUIRED_STATE_KEYS_AND_STATES.items():
-        if key not in params["states"]:
-            params["states"][key] = value
-    return params
-
-
-def _parameter_valuespec_systemd_units_summary() -> Migrate:
-    return Migrate(
-        valuespec=Dictionary(
-            elements=[
-                (
-                    "states",
-                    Dictionary(
-                        title=_("Map systemd states to monitoring states"),
-                        elements=[
-                            (
-                                "active",
-                                MonitoringState(
-                                    title=_("Monitoring state if unit is active"),
-                                    default_value=0,
-                                ),
+def _parameter_valuespec_systemd_units_summary() -> Dictionary:
+    return Dictionary(
+        elements=[
+            (
+                "states",
+                Dictionary(
+                    title=_("Map systemd states to monitoring states"),
+                    elements=[
+                        (
+                            "active",
+                            MonitoringState(
+                                title=_("Monitoring state if unit is active"),
+                                default_value=0,
                             ),
-                            (
-                                "inactive",
-                                MonitoringState(
-                                    title=_("Monitoring state if unit is inactive"),
-                                    default_value=0,
-                                ),
+                        ),
+                        (
+                            "inactive",
+                            MonitoringState(
+                                title=_("Monitoring state if unit is inactive"),
+                                default_value=0,
                             ),
-                            (
-                                "failed",
-                                MonitoringState(
-                                    title=_("Monitoring state if unit is failed"),
-                                    default_value=2,
-                                ),
+                        ),
+                        (
+                            "failed",
+                            MonitoringState(
+                                title=_("Monitoring state if unit is failed"),
+                                default_value=2,
                             ),
-                        ],
-                        required_keys=list(REQUIRED_STATE_KEYS_AND_STATES.keys()),
-                        default_keys=list(REQUIRED_STATE_KEYS_AND_STATES.keys()),
-                    ),
+                        ),
+                    ],
                 ),
-                (
-                    "states_default",
-                    MonitoringState(
-                        title=_("Monitoring state for any other unit state"),
-                        default_value=2,
-                    ),
+            ),
+            (
+                "states_default",
+                MonitoringState(
+                    title=_("Monitoring state for any other unit state"),
+                    default_value=2,
                 ),
-                (
-                    "ignored",
-                    ListOf(
-                        valuespec=RegExp(
-                            title=_("Pattern (Regex)"),
-                            size=40,
-                            mode=RegExp.infix,
-                        ),
-                        title=_("Exclude units matching provided regex patterns"),
-                        help=_(
-                            "You can optionally define one or multiple regular expressions."
-                            " Matching units are excluded."
-                            " This allows to ignore services which are known to fail beforehand."
-                        ),
-                        add_label=_("Add pattern"),
+            ),
+            (
+                "ignored",
+                ListOf(
+                    valuespec=RegExp(
+                        title=_("Pattern (Regex)"),
+                        size=40,
+                        mode=RegExp.infix,
                     ),
+                    title=_("Exclude units matching provided regex patterns"),
+                    help=_(
+                        "You can optionally define one or multiple regular expressions."
+                        " Matching units are excluded."
+                        " This allows to ignore services which are known to fail beforehand."
+                    ),
+                    add_label=_("Add pattern"),
                 ),
-                (
-                    "activating_levels",
-                    SimpleLevels(
-                        Age,
-                        title=_("Tolerance period for 'activating' state"),
-                        help=_(
-                            "Choose time levels for which a unit is allowed to be in an 'activating' state"
-                        ),
-                        default_levels=(30, 60),
+            ),
+            (
+                "activating_levels",
+                SimpleLevels(
+                    Age,
+                    title=_("Tolerance period for 'activating' state"),
+                    help=_(
+                        "Choose time levels for which a unit is allowed to be in an 'activating' state"
                     ),
+                    default_levels=(30, 60),
                 ),
-                (
-                    "deactivating_levels",
-                    SimpleLevels(
-                        Age,
-                        title=_("Tolerance period for 'deactivating' state"),
-                        help=_(
-                            "Choose time levels (in seconds) for which a unti is allowed to be in an 'deactivating' state"
-                        ),
-                        default_value=(30, 60),
+            ),
+            (
+                "deactivating_levels",
+                SimpleLevels(
+                    Age,
+                    title=_("Tolerance period for 'deactivating' state"),
+                    help=_(
+                        "Choose time levels (in seconds) for which a unti is allowed to be in an 'deactivating' state"
                     ),
+                    default_value=(30, 60),
                 ),
-                (
-                    "reloading_levels",
-                    SimpleLevels(
-                        Age,
-                        title=_("Tolerance period for 'reloading' state"),
-                        help=_(
-                            "Choose time levels (in seconds) for which a unit is allowed to be in a 'reloading' state"
-                        ),
-                        default_value=(30, 60),
+            ),
+            (
+                "reloading_levels",
+                SimpleLevels(
+                    Age,
+                    title=_("Tolerance period for 'reloading' state"),
+                    help=_(
+                        "Choose time levels (in seconds) for which a unit is allowed to be in a 'reloading' state"
                     ),
+                    default_value=(30, 60),
                 ),
-            ],
-            help=_(
-                "This ruleset only applies to the summary Systemd service and not the individual one."
             ),
+        ],
+        help=_(
+            "This ruleset only applies to the summary Systemd service and not the individual one."
         ),
-        migrate=_migrate,
     )
 
 

diff --git a/tests/unit/cmk/base/plugins/agent_based/test_systemd_units.py b/tests/unit/cmk/base/plugins/agent_based/test_systemd_units.py
@@ -1026,30 +1026,11 @@ def test_check_systemd_units_sockets(
             [
                 Result(state=State.OK, summary="Total: 5"),
                 Result(state=State.OK, summary="Disabled: 0"),
-                Result(state=State.CRIT, summary="Failed: 2"),
+                Result(state=State.OK, summary="Failed: 2"),
                 Result(state=State.CRIT, summary="2 services failed (bar, foo)"),
             ],
             id="'Normal' test case",
         ),
-        pytest.param(
-            {
-                "else": 2,
-                "states": {"active": 0, "failed": 1, "inactive": 0},
-                "states_default": 2,
-                "ignored": [],
-                "activating_levels": (30, 60),
-                "deactivating_levels": (30, 60),
-                "reloading_levels": (30, 60),
-            },
-            SECTION,
-            [
-                Result(state=State.OK, summary="Total: 5"),
-                Result(state=State.OK, summary="Disabled: 0"),
-                Result(state=State.WARN, summary="Failed: 2"),
-                Result(state=State.WARN, summary="2 services failed (bar, foo)"),
-            ],
-            id="Custom state for failed",
-        ),
         pytest.param(
             {
                 "ignored": ["virtual"],