Merge ~nikolay.vinogradov/charm-nrpe:lp2008926 into charm-nrpe:master

[Nikolay Vinogradov]

Exclude mounted Kubernetes PVs from the check_space check.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:2175153e817b93291f37def2b0e8b37db5352c4f
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/8/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/264/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/92/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/8//rebuild

review: Approve (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:f964148020004937d32464f81efc108112ba9280
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/9/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/269/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/97/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/9//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:b2d8609693a9862f81d1b8df7eca1923e1a73c9c
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/10/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/270/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/98/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/10//rebuild

review: Approve (continuous-integration)

[Andrea Ieri (aieri)]

get_partitions_to_check() is already producing the list of mountpoints that need to be checked, filtering out the ones that need to be skipped. Wouldn't it be better to handle skipping pod mountpoints there?

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:214848164e5ea87c29d64684b45f06442601e96a
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/11/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/272/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/100/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/11//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:15469dbcde04506c4dbf8fa49c8956d4acb1a654
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/12/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/273/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/101/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/12//rebuild

review: Approve (continuous-integration)

[Andrea Ieri (aieri)]

in-line comment

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:ee5cdbc8d7550d0ddcec0cdc4ce9f550f274620b
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/13/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/275/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/103/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/13//rebuild

review: Approve (continuous-integration)

[Andrea Ieri (aieri)]

Thanks, this looks good, we're only missing test cases. I see we have a TestDiskSpaceCheck class where we should plug a test or two. Apologies for not mentioning this in my previous review...

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:fa9402ff622c20a8783c05d64bb0900036dc9895
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/14/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/404/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/156/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/14//rebuild

review: Approve (continuous-integration)

[Andrea Ieri (aieri)]

lgtm, although I've submitted MP#440218 to slightly improve the test

review: Approve

[Erhan Sunar (esunar)]

review: Approve

[Sudeep Bhandari (sudeephb)]

LGTM

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 2a396fcb7f8c3f1e9775b18a30c37c654f2b21fe

diff --git a/hooks/nrpe_helpers.py b/hooks/nrpe_helpers.py
index 3aa9238..b485822 100644
--- a/hooks/nrpe_helpers.py
+++ b/hooks/nrpe_helpers.py
@@ -818,6 +818,11 @@ def is_valid_partition(device):
     return True
 
 
+def is_kubernetes_pv(mountpoint):
+    """Check if a mountpoint is Kubernetes persistent volume."""
+    return mountpoint.startswith("/var/lib/kubelet/pods")
+
+
 def get_partitions_to_check():
     """Get a list of partitions to be checked by check_disk."""
     lsblk_cmd = "lsblk -J".split()
@@ -828,18 +833,18 @@ def get_partitions_to_check():
     for dev in block_devices:
         if not is_valid_partition(dev):
             continue
-        if dev.get("mountpoint", ""):
-            partitions_to_check.add(dev.get("mountpoint"))
+        mnt = dev.get("mountpoint", "")
+        if mnt and not is_kubernetes_pv(mnt):
+            partitions_to_check.add(mnt)
         for child in dev.get("children", []):
             if not is_valid_partition(child):
                 continue
-            if child.get("mountpoint", ""):
-                partitions_to_check.add(child.get("mountpoint"))
             # Jammy returns a list of "mountpoints" instead of a single value
             # in the key "mountpoint"
-            for mountpoint in child.get("mountpoints", []):
-                if mountpoint:
-                    partitions_to_check.add(mountpoint)
+            mountpoints = child.get("mountpoints", []) or [child.get("mountpoint")]
+            for mnt in filter(bool, mountpoints):
+                if not is_kubernetes_pv(mnt):
+                    partitions_to_check.add(mnt)
 
     skipped_partitions = {"[SWAP]", "/boot/efi"}
 
diff --git a/tests/unit/test_nrpe_helpers.py b/tests/unit/test_nrpe_helpers.py
index d9ea79a..9f48f17 100644
--- a/tests/unit/test_nrpe_helpers.py
+++ b/tests/unit/test_nrpe_helpers.py
@@ -279,7 +279,16 @@ class TestDiskSpaceCheck(unittest.TestCase):
                    ]
                 }
              ]
-          }
+            },
+            {
+             "name":"vdb",
+             "maj:min":"252:16",
+             "rm":false,
+             "size":"1G",
+             "ro":false,
+             "type":"disk",
+             "mountpoint": "/var/lib/kubelet/pods/../k..s.io~csi/pvc../mount"
+            }
         ]
     }"""
 
@@ -292,6 +301,9 @@ class TestDiskSpaceCheck(unittest.TestCase):
         self.assertEqual("/" in result, True)
         self.assertEqual("/srv/instances" in result, True)
         self.assertEqual("/srv/jammy" in result, True)
+        self.assertEqual(
+            "/var/lib/kubelet/pods/../k..s.io~csi/pvc../mount" in result, False
+        )
 
 
 def load_default_config():