Reland: mounts: make params a dict

Change-Id: I31b6b8d655119d53e978c7a98fc27d2139d29fb0

cmk/base/legacy_checks/mounts.py

@@ -9,30 +9,50 @@
 # to
 # knvmsapprd:/transreorg/sap/trans /transreorg/sap/trans\040(deleted) nfs4 rw,relatime,vers=4.0,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,clientaddr=172.24.98.63,local_lock=none,addr=172.24.98.57 0 0
 
-#
-# (mo): This plugin is a good example for plugins that would massively benefit from a
-#       section definition (parse_function).
-#       However, I need an exapmle for a plugin without one, so for now I keep it this way.
-#
-
+from collections.abc import Iterable, Mapping, Sequence
+from dataclasses import dataclass
+from typing import Any
 
 from cmk.base.check_api import LegacyCheckDefinition
 from cmk.base.config import check_info
+from cmk.base.plugins.agent_based.agent_based_api.v1.type_defs import StringTable
 
 
-def discovery_mounts(info):
-    devices = []
-    for dev, mountpoint, fstype, options, _dump, _fsck in info:
-        if fstype == "tmpfs":
-            continue
+@dataclass(frozen=True, kw_only=True)
+class Mount:
+    mountpoint: str
+    options: Sequence[str]  # this could be improved...
+    fs_type: str
+    is_stale: bool
 
-        if mountpoint in ["/etc/resolv.conf", "/etc/hostname", "/etc/hosts"]:
+
+def parse_mounts(string_table: StringTable) -> Mapping[str, Mount]:
+    devices = set()
+    section = {}
+    for dev, mountpoint, fs_type, options, _dump, _fsck in string_table:
+        if dev in devices:
             continue
 
-        if dev not in devices:
-            devices.append(dev)
-            opts = sorted(options.split(","))
-            yield mountpoint.replace("\\040(deleted)", ""), opts
+        devices.add(dev)
+
+        mountname = mountpoint.replace("\\040(deleted)", "")
+        section[mountname] = Mount(
+            mountpoint=mountname,
+            options=sorted(options.split(",")),
+            fs_type=fs_type,
+            is_stale=mountpoint.endswith("\\040(deleted)"),
+        )
+
+    return section
+
+
+def discovery_mounts(section: Mapping[str, Mount]) -> Iterable[tuple[str, Mapping]]:
+    yield from (
+        (m.mountpoint, {"expected_mount_options": m.options})
+        for m in section.values()
+        if m.fs_type != "tmpfs"
+        and m.mountpoint not in ["/etc/resolv.conf", "/etc/hostname", "/etc/hosts"]
+    )
 
 
 def _should_ignore_option(option):
@@ -42,45 +62,45 @@ def _should_ignore_option(option):
     return False
 
 
-def check_mounts(item, targetopts, info):
-    # Ignore options that are allowed to change
-    for _dev, mountpoint, _fstype, options, _dump, _fsck in info:
-        if item == mountpoint.replace("\\040(deleted)", ""):
-            if mountpoint.endswith("\\040(deleted)"):
-                yield 1, "Mount point detected as stale"
-                return
+def check_mounts(
+    item: str, params: Mapping[str, Any], section: Mapping[str, Mount]
+) -> Iterable[tuple[int, str]]:
+    if (mount := section.get(item)) is None:
+        return
+
+    if mount.is_stale:
+        yield 1, "Mount point detected as stale"
+        return
 
-            opts = options.split(",")
-            # Now compute the exact difference.
+    targetopts = params["expected_mount_options"]
 
-            exceeding = [
-                opt for opt in opts if opt not in targetopts and not _should_ignore_option(opt)
-            ]
+    # Now compute the exact difference.
+    exceeding = [
+        opt for opt in mount.options if opt not in targetopts and not _should_ignore_option(opt)
+    ]
 
-            missing = [
-                opt for opt in targetopts if opt not in opts and not _should_ignore_option(opt)
-            ]
+    missing = [
+        opt for opt in targetopts if opt not in mount.options and not _should_ignore_option(opt)
+    ]
 
-            if not missing and not exceeding:
-                yield 0, "Mount options exactly as expected"
-                return
+    if not missing and not exceeding:
+        yield 0, "Mount options exactly as expected"
+        return
 
-            infos = []
-            if missing:
-                infos.append("Missing: %s" % ",".join(missing))
-            if exceeding:
-                infos.append("Exceeding: %s" % ",".join(exceeding))
-            infotext = ", ".join(infos)
+    if missing:
+        yield 1, "Missing: %s" % ",".join(missing)
+    if exceeding:
+        yield 1, "Exceeding: %s" % ",".join(exceeding)
 
-            yield 1, infotext
-            if "ro" in exceeding:
-                yield 2, "Filesystem has switched to read-only and is probably corrupted"
-            return
+    if "ro" in exceeding:
+        yield 2, "Filesystem has switched to read-only and is probably corrupted"
 
 
 check_info["mounts"] = LegacyCheckDefinition(
     service_name="Mount options of %s",
-    discovery_function=discovery_mounts,
+    parse_function=parse_mounts,
+    discovery_function=discovery_mounts,  # type: ignore[typeddict-item]  # fix coming up
     check_function=check_mounts,
     check_ruleset_name="fs_mount_options",
+    check_default_parameters={"expected_mount_options": []},  # will be overwritten by dicsovery
 )

cmk/gui/plugins/wato/check_parameters/fs_mount_options.py

@@ -9,19 +9,30 @@
     rulespec_registry,
     RulespecGroupCheckParametersStorage,
 )
-from cmk.gui.valuespec import ListOfStrings, TextInput
+from cmk.gui.valuespec import Dictionary, ListOfStrings, Migrate, TextInput
 
 
-def _parameter_valuespec_fs_mount_options():
-    return ListOfStrings(
-        title=_("Expected mount options"),
-        help=_(
-            "Specify all expected mount options here. If the list of "
-            "actually found options differs from this list, the check will go "
-            "warning or critical. Just the option <tt>commit</tt> is being "
-            "ignored since it is modified by the power saving algorithms."
+def _parameter_valuespec_fs_mount_options() -> Migrate:
+    return Migrate(
+        valuespec=Dictionary(
+            elements=[
+                (
+                    "expected_mount_options",
+                    ListOfStrings(
+                        title=_("Expected mount options"),
+                        help=_(
+                            "Specify all expected mount options here. If the list of "
+                            "actually found options differs from this list, the check will go "
+                            "warning or critical. Just the option <tt>commit</tt> is being "
+                            "ignored since it is modified by the power saving algorithms."
+                        ),
+                        valuespec=TextInput(),
+                    ),
+                ),
+            ],
+            optional_keys=[],
         ),
-        valuespec=TextInput(),
+        migrate=lambda p: p if isinstance(p, dict) else {"expected_mount_options": p},
     )
 
 

tests/unit/cmk/base/plugins/agent_based/test_section_properties.py

@@ -281,7 +281,6 @@ def test_section_parse_function_does_something(fix_register: FixRegister) -> Non
         "mongodb_flushing",
         "mongodb_instance",
         "mongodb_locks",
-        "mounts",
         "mq_queues",
         "msexch_replhealth",
         "msoffice_serviceplans",

tests/unit/cmk/gui/plugins/wato/check_parameters/test_plugin_vs_wato.py

@@ -607,11 +607,6 @@ class ErrorReporter:
             "mongodb_collections",
             RuleGroup.CheckgroupParameters("mongodb_collections"),
         ),
-        (
-            "check",
-            "mounts",
-            RuleGroup.CheckgroupParameters("fs_mount_options"),
-        ),
         ("check", "mq_queues", RuleGroup.CheckgroupParameters("mq_queues")),
         (
             "check",