From b392a4fd13889d420fecffe54c82861db434f682 Mon Sep 17 00:00:00 2001
From: Samuel Walladge <samuel.walladge@canonical.com>
Date: Fri, 29 Sep 2023 13:30:32 +0930
Subject: [PATCH] Warn in status if tune-osd-memory-target invalid

This is useful, because if an invalid value is set,
the value is ignored and not overridden,
and an error logged.
So now we warn about this in the status to be more obvious to the user.

Change-Id: Idc4a7706f30cbcea8aee83a1406fa84139fe510d
---
 hooks/ceph_hooks.py           | 16 ++++++++++++++++
 unit_tests/test_ceph_hooks.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py
index 474389a2..f7c4bef1 100755
--- a/hooks/ceph_hooks.py
+++ b/hooks/ceph_hooks.py
@@ -386,6 +386,17 @@ def warn_if_memory_outside_bounds(value):
             "This is not recommended.", level=WARNING)
 
 
+def is_tune_osd_memory_target_valid() -> bool:
+    """
+    Check if the tune-osd-memory-target value is valid
+
+    :returns: True if valid, else False
+    :rtype: bool
+    """
+    value = config('tune-osd-memory-target')
+    return not value or bool(re.match(r"\d+(?:GB|%)$", value))
+
+
 def get_osd_memory_target():
     """
     Processes the config value of tune-osd-memory-target.
@@ -868,6 +879,11 @@ VERSION_PACKAGE = 'ceph-common'
 
 def assess_status():
     """Assess status of current unit"""
+
+    if not is_tune_osd_memory_target_valid():
+        status_set('blocked', 'tune-osd-memory-target config value is invalid')
+        return
+
     # check to see if the unit is paused.
     application_version_set(get_upstream_version(VERSION_PACKAGE))
     if is_unit_upgrading_set():
diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py
index 2bfa2d3c..a127c601 100644
--- a/unit_tests/test_ceph_hooks.py
+++ b/unit_tests/test_ceph_hooks.py
@@ -737,6 +737,37 @@ class CephHooksTestCase(unittest.TestCase):
             level=ceph_hooks.WARNING
         )
 
+    @patch.object(ceph_hooks, "config")
+    def test_is_tune_osd_memory_target_valid(self, mock_config):
+        def tune(value):
+            return lambda k: (
+                value if k == "tune-osd-memory-target" else KeyError
+            )
+
+        # value, is_valid
+        scenarios = [
+            ("", True),
+            ("5GB", True),
+            ("020GB", True),
+            ("34GB", True),
+            ("5%", True),
+            ("05%", True),
+            ("50%", True),
+            ("test", False),
+            ("   ", False),
+            ("5", False),
+            ("GB", False),
+            ("%", False),
+            ("test5GB", False),
+            ("50%%", False),
+        ]
+        for value, expected_valid in scenarios:
+            mock_config.side_effect = tune(value)
+            self.assertEqual(
+                ceph_hooks.is_tune_osd_memory_target_valid(),
+                expected_valid
+            )
+
     @patch.object(ceph_hooks, "config")
     @patch.object(ceph_hooks, "get_total_ram")
     @patch.object(ceph_hooks, "kv")