Merge ~xavpaice/charm-nrpe:bug/1824882 into ~nrpe-charmers/charm-nrpe:master

[Xav Paice]

[Wouter van Bommel (woutervb)]

Please see the inline comment(s)

review: Needs Fixing

[Xav Paice (xavpaice)]

updated; added python3 to the package list (although it's there on the current Trusty cloud image), added some validation on the inputs, and fixed up the syntax errors (forgot to push that change).

[Wouter van Bommel (woutervb)]

lgtm

review: Approve

[Alvaro Uria (aluria)]

I've suggested a few changes for readability, completeness (verify all read-only fs that are not excluded instead of returning the first one found), and a doubt about the input validation (match substrings could induce to errors, should we look for "start of string"?).

review: Needs Fixing

[Xav Paice (xavpaice)]

Many thanks for the detailed review! Have updated the commit with responses to the feedback.

[Jeremy Lounder (jldev)]

review: Approve

diff --git a/config.yaml b/config.yaml
index 44a6680..2c9eefd 100644
--- a/config.yaml
+++ b/config.yaml
@@ -179,3 +179,12 @@ options:
         dmesg history length to check for xfs errors, in minutes
         .
         Defaults to disabled, set the time to enable.
+  ro_filesystem_excludes:
+    default: "/snap/,/sys/fs/cgroup"
+    type: string
+    description: |
+      Comma separated list of mount points to exclude from checks for readonly filesystem.
+      Can be a substring rather than the entire mount point, e.g. /sys will match all filesystems
+      beginning with the string /sys.
+      The check is disabled on all LXD units, and also for non-container units if this parameter is
+      set to ''.
diff --git a/files/plugins/check_ro_filesystem.py b/files/plugins/check_ro_filesystem.py
 new file mode 100755
index 0000000..c19ae31
--- /dev/null
+++ b/files/plugins/check_ro_filesystem.py
@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+# -*- coding: us-ascii -*-
+
+# Copyright (C) 2020 Canonical
+# All rights reserved
+#
+
+import argparse
+
+from nagios_plugin3 import (
+    CriticalError,
+    UnknownError,
+    try_check,
+)
+
+EXCLUDE = {"/snap/", "/sys/fs/cgroup"}
+
+
+def check_ro_filesystem(excludes=""):
+    """Loops /proc/mounts looking for readonly mounts.
+
+    :param excludes: list of mount points to exclude from checks
+    """
+    # read /proc/mounts, add each line to a list
+    try:
+        with open("/proc/mounts") as fd:
+            mounts = [mount.strip() for mount in fd.readlines()]
+    except Exception as e:
+        raise UnknownError("UNKNOWN: unable to read mounts with {}".format(e))
+
+    exclude_mounts = EXCLUDE
+    ro_filesystems = []
+    # if excludes != "" and excludes is not None:
+    if excludes:
+        try:
+            exclude_mounts = EXCLUDE.union(set(excludes.split(",")))
+        except Exception as e:
+            msg = "UNKNOWN: unable to read list of mounts to exclude {}".format(e)
+            raise UnknownError(msg)
+    for mount in mounts:
+        # for each line in the list, split by space to a new list
+        split_mount = mount.split()
+        # if mount[1] matches EXCLUDE_FS then next, else check it's not readonly
+        if not any(split_mount[1].startswith(exclusion.strip()) for exclusion in exclude_mounts):
+            mount_options = split_mount[3].split(",")
+            if "ro" in mount_options:
+                ro_filesystems.append(split_mount[1])
+    if len(ro_filesystems) > 0:
+        msg = "CRITICAL: filesystem(s) {} readonly".format(','.join(ro_filesystems))
+        raise CriticalError(msg)
+
+    print("OK: no readonly filesystems found")
+
+
+def parse_args():
+    parser = argparse.ArgumentParser(description="Check for readonly filesystems")
+    parser.add_argument(
+        "--exclude",
+        "-e",
+        type=str,
+        help="""Comma separated list of mount points to exclude from checks for readonly filesystem.
+                Can be just a substring of the whole mount point.""",
+        default='',
+    )
+    args = parser.parse_args()
+    return args
+
+
+def main():
+    args = parse_args()
+    try_check(check_ro_filesystem, args.exclude)
+
+
+if __name__ == "__main__":
+    main()
diff --git a/hooks/nrpe_helpers.py b/hooks/nrpe_helpers.py
index 63c7ba4..fac74ed 100644
--- a/hooks/nrpe_helpers.py
+++ b/hooks/nrpe_helpers.py
@@ -437,6 +437,23 @@ class SubordinateCheckDefinitions(dict):
                 'cmd_params': '-w 60 -c 80',  # Specify params here to enable the check, not required otherwise.
             }
             checks.append(arp_check)
+            ro_filesystem_excludes = hookenv.config('ro_filesystem_excludes')
+            if ro_filesystem_excludes == '':
+                # specify cmd_params = '' to disable/remove the check from nrpe
+                check_ro_filesystem = {
+                    'description': 'Readonly filesystems',
+                    'cmd_name': 'check_ro_filesystem',
+                    'cmd_exec': os.path.join(local_plugin_dir, 'check_ro_filesystem.py'),
+                    'cmd_params': '',
+                }
+            else:
+                check_ro_filesystem = {
+                    'description': 'Readonly filesystems',
+                    'cmd_name': 'check_ro_filesystem',
+                    'cmd_exec': os.path.join(local_plugin_dir, 'check_ro_filesystem.py'),
+                    'cmd_params': '-e {}'.format(hookenv.config('ro_filesystem_excludes')),
+                }
+            checks.append(check_ro_filesystem)
 
         if hookenv.config('lacp_bonds').strip():
             for bond_iface in hookenv.config('lacp_bonds').strip().split():
diff --git a/hooks/nrpe_utils.py b/hooks/nrpe_utils.py
index 6e2e9ab..7d294d7 100644
--- a/hooks/nrpe_utils.py
+++ b/hooks/nrpe_utils.py
@@ -31,7 +31,8 @@ def determine_packages():
     pkgs = [
         'nagios-nrpe-server',
         'nagios-plugins-basic',
-        'nagios-plugins-standard'
+        'nagios-plugins-standard',
+        'python3',
     ]
     if hookenv.config('export_nagios_definitions'):
         pkgs.append('rsync')