bi: re-organize output

Use the check API as it is intended to
avoid accessing the `state_marker`.

Note that the `notice` fields will only be
shown in the summary, if the state is not OK.

Change-Id: Ic006c9505dfdad1cf2ee8c46df775b822e04c648

Author: mo-ki

cmk/plugins/collection/agent_based/bi_aggregation.py

@@ -7,8 +7,6 @@
 from dataclasses import dataclass, field
 from typing import Any, TypedDict
 
-from cmk.checkengine.checkresults import state_markers  # pylint: disable=cmk-module-layer-violation
-
 from cmk.agent_based.v2 import (
     AgentSection,
     CheckPlugin,
@@ -50,7 +48,9 @@ class Aggregation:
 
 @dataclass
 class AggregationError:
-    output: str
+    state: State
+    notice: str
+    details: str
     affects_state: bool
 
 
@@ -90,22 +90,35 @@ def get_aggregation_errors(
 
     if aggr.error_state is not None:
         yield AggregationError(
-            output=f"{state_markers[aggr.error_state]} {aggr.error_output}",
+            state=State(aggr.error_state),
+            notice=aggr.error_output or "",
+            details=aggr.error_output or "",
             affects_state=affects_state,
         )
 
     if aggr.custom_output is not None:
-        yield AggregationError(output=aggr.custom_output, affects_state=affects_state)
+        yield AggregationError(
+            state=State.OK,
+            notice=aggr.custom_output,
+            details=aggr.custom_output,
+            affects_state=affects_state,
+        )
 
     for child in aggr.children:
         if errors := list(get_aggregation_errors(child, affects_state)):
             yield AggregationError(
-                output=f"+-- {errors[0].output}", affects_state=errors[0].affects_state
+                state=errors[0].state,
+                notice=errors[0].notice,
+                details=f"+-- {errors[0].details}",
+                affects_state=errors[0].affects_state,
             )
 
             for error in errors[1:]:
                 yield AggregationError(
-                    output=f"| {error.output}", affects_state=error.affects_state
+                    state=error.state,
+                    notice=error.notice,
+                    details=f"| {error.details}",
+                    affects_state=error.affects_state,
                 )
 
 
@@ -136,23 +149,22 @@ def check_bi_aggregation(item: str, section: Section) -> CheckResult:
         summary="Acknowledged: %s" % ("yes" if bi_data["acknowledged"] else "no"),
     )
 
-    if bi_data["infos"]:
-        aggregations = get_aggregations(bi_data["infos"])
-        errors = list(get_aggregation_errors(aggregations, bool(overall_state)))
-
-        errors_affecting_state = [error.output for error in errors if error.affects_state]
-        other_errors = [error.output for error in errors if not error.affects_state]
+    if not bi_data["infos"]:
+        return
 
-        infos = []
-        if errors_affecting_state:
-            infos.extend(["", "Aggregation problems affecting the state:"])
-            infos.extend(errors_affecting_state)
+    aggregations = get_aggregations(bi_data["infos"])
+    errors = list(get_aggregation_errors(aggregations, bool(overall_state)))
 
-        if other_errors:
-            infos.extend(["", "Aggregation problems not affecting the state:"])
-            infos.extend(other_errors)
+    if errors_affecting_state := [e for e in errors if e.affects_state]:
+        yield Result(state=State.OK, notice="Aggregation problems affecting the state:")
+        yield from (
+            Result(state=e.state, notice=e.notice, details=e.details)
+            for e in errors_affecting_state
+        )
 
-        yield Result(state=State.OK, notice="\n".join(infos))
+    if other_errors := [e for e in errors if not e.affects_state]:
+        yield Result(state=State.OK, notice="Aggregation problems not affecting the state:")
+        yield from (Result(state=e.state, notice=e.notice, details=e.details) for e in other_errors)
 
 
 check_plugin_bi_aggregation = CheckPlugin(

tests/unit/cmk/plugins/collection/agent_based/test_bi_aggregation.py

@@ -76,21 +76,33 @@
 
 
 def test_check_bi_aggregation() -> None:
-    expected_notice = (
-        "\n"
-        "Aggregation problems affecting the state:\n"
-        "(!) Host test\n+-- (!) General State\n"
-        "| +-- (!) Check_MK\n"
-        "| | +-- (!) Check_MK Discovery, Services unmonitored: 1 (cpu_loads: 1)(!), Host labels: all up to date\n"
-        "\n"
-        "Aggregation problems not affecting the state:\n"
-        "+-- +-- (!) Aggr Host test, Aggregation state: Warning(!), In downtime: no, Acknowledged: no\n"
-        "| +-- (!!) OMD test_gestern Notification Spooler, Version: 2.3.0-2024.02.01, Status last updated 42 days 1 hour ago, spooler seems crashed or busy(!!)\n"
-        "| +-- (!!) OMD stable Notification Spooler, Version: 2.2.0-2024.03.13, Status last updated 22 hours 17 minutes ago, spooler seems crashed or busy(!!)"
-    )
     assert list(check_bi_aggregation("Host test", TEST_INFO)) == [
         Result(state=State.WARN, summary="Aggregation state: Warning"),
         Result(state=State.OK, summary="In downtime: no"),
         Result(state=State.OK, summary="Acknowledged: no"),
-        Result(state=State.OK, notice=expected_notice),
+        Result(state=State.OK, notice="Aggregation problems affecting the state:"),
+        Result(state=State.WARN, notice="Host test"),
+        Result(state=State.WARN, notice="General State", details="+-- General State"),
+        Result(state=State.WARN, notice="Check_MK", details="| +-- Check_MK"),
+        Result(
+            state=State.WARN,
+            notice="Check_MK Discovery, Services unmonitored: 1 (cpu_loads: 1)(!), Host labels: all up to date",
+            details="| | +-- Check_MK Discovery, Services unmonitored: 1 (cpu_loads: 1)(!), Host labels: all up to date",
+        ),
+        Result(state=State.OK, notice="Aggregation problems not affecting the state:"),
+        Result(
+            state=State.WARN,
+            notice="Aggr Host test, Aggregation state: Warning(!), In downtime: no, Acknowledged: no",
+            details="+-- +-- Aggr Host test, Aggregation state: Warning(!), In downtime: no, Acknowledged: no",
+        ),
+        Result(
+            state=State.CRIT,
+            notice="OMD test_gestern Notification Spooler, Version: 2.3.0-2024.02.01, Status last updated 42 days 1 hour ago, spooler seems crashed or busy(!!)",
+            details="| +-- OMD test_gestern Notification Spooler, Version: 2.3.0-2024.02.01, Status last updated 42 days 1 hour ago, spooler seems crashed or busy(!!)",
+        ),
+        Result(
+            state=State.CRIT,
+            notice="OMD stable Notification Spooler, Version: 2.2.0-2024.03.13, Status last updated 22 hours 17 minutes ago, spooler seems crashed or busy(!!)",
+            details="| +-- OMD stable Notification Spooler, Version: 2.2.0-2024.03.13, Status last updated 22 hours 17 minutes ago, spooler seems crashed or busy(!!)",
+        ),
     ]