From 23db141218d8a3cd0ff506f2a05c60db484034aa Mon Sep 17 00:00:00 2001 From: Samuel Walladge Date: Thu, 14 Sep 2023 11:45:48 +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 | 41 +++++++++++++++++++++++++++++------ unit_tests/test_ceph_hooks.py | 21 ++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index 474389a2..3dff4f06 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -15,6 +15,7 @@ # limitations under the License. import base64 +from enum import Enum import glob import json import netifaces @@ -25,6 +26,7 @@ import socket import subprocess import sys import traceback +from typing import NamedTuple sys.path.append('lib') import charms_ceph.utils as ceph @@ -386,7 +388,21 @@ def warn_if_memory_outside_bounds(value): "This is not recommended.", level=WARNING) -def get_osd_memory_target(): +class OSDMemoryTargetType(Enum): + EMPTY = 1 + GB = 2 + PERCENT = 3 + INVALID = 4 + + +class OSDMemoryTarget(NamedTuple): + # the value in bytes, as a string + value: str + # the original parsed type from the tune-osd-memory-target config value + parsed_type: OSDMemoryTargetType + + +def get_osd_memory_target() -> OSDMemoryTarget: """ Processes the config value of tune-osd-memory-target. @@ -398,13 +414,16 @@ def get_osd_memory_target(): tune_osd_memory_target = config('tune-osd-memory-target') if not tune_osd_memory_target: - return "" + return OSDMemoryTarget(value="", parsed_type=OSDMemoryTargetType.EMPTY) match = re.match(r"(\d+)GB$", tune_osd_memory_target) if match: osd_memory_target = int(match.group(1)) * 1024 * 1024 * 1024 warn_if_memory_outside_bounds(osd_memory_target) - return str(osd_memory_target) + return OSDMemoryTarget( + value=str(osd_memory_target), + parsed_type=OSDMemoryTargetType.GB + ) match = re.match(r"(\d+)%$", tune_osd_memory_target) if match: @@ -412,11 +431,14 @@ def get_osd_memory_target(): num_osds = len(kv().get("osd-devices", [])) osd_memory_target = int(get_total_ram() * percentage / num_osds) warn_if_memory_outside_bounds(osd_memory_target) - return str(osd_memory_target) + return OSDMemoryTarget( + value=str(osd_memory_target), + parsed_type=OSDMemoryTargetType.PERCENT + ) log("tune-osd-memory-target value invalid," " leaving the OSD memory target unchanged", level=ERROR) - return "" + return OSDMemoryTarget(value="", parsed_type=OSDMemoryTargetType.INVALID) def get_ceph_context(upgrading=False): @@ -539,7 +561,7 @@ def config_changed(): relation_id=r_id, relation_settings={ 'osd-host': socket.gethostname(), - 'osd-memory-target': get_osd_memory_target(), + 'osd-memory-target': get_osd_memory_target().value, } ) @@ -633,7 +655,7 @@ def prepare_disks_and_activate(): hookenv.config('source') or 'distro' ), 'osd-host': socket.gethostname(), - 'osd-memory-target': get_osd_memory_target(), + 'osd-memory-target': get_osd_memory_target().value, } ) @@ -869,6 +891,11 @@ VERSION_PACKAGE = 'ceph-common' def assess_status(): """Assess status of current unit""" # check to see if the unit is paused. + + if get_osd_memory_target().parsed_type == OSDMemoryTargetType.INVALID: + status_set('blocked', 'tune-osd-memory-target config value is invalid') + return + application_version_set(get_upstream_version(VERSION_PACKAGE)) if is_unit_upgrading_set(): status_set("blocked", diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 2bfa2d3c..b83720d0 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -755,7 +755,8 @@ class CephHooksTestCase(unittest.TestCase): mock_config.side_effect = config_func target = ceph_hooks.get_osd_memory_target() - self.assertEqual(target, str(5 * 1024 * 1024 * 1024)) # 5GB + self.assertEqual(target.value, str(5 * 1024 * 1024 * 1024)) # 5GB + self.assertEqual(target.parsed_type, ceph_hooks.OSDMemoryTargetType.GB) @patch.object(ceph_hooks, "config") @patch.object(ceph_hooks, "get_total_ram") @@ -776,7 +777,11 @@ class CephHooksTestCase(unittest.TestCase): target = ceph_hooks.get_osd_memory_target() # should be 50% of 16GB / 2 osd devices = 4GB - self.assertEqual(target, str(4 * 1024 * 1024 * 1024)) # 4GB + self.assertEqual(target.value, str(4 * 1024 * 1024 * 1024)) # 4GB + self.assertEqual( + target.parsed_type, + ceph_hooks.OSDMemoryTargetType.PERCENT + ) @patch.object(ceph_hooks, "config") @patch.object(ceph_hooks, "get_total_ram") @@ -792,7 +797,11 @@ class CephHooksTestCase(unittest.TestCase): mock_config.side_effect = lambda _: None target = ceph_hooks.get_osd_memory_target() - self.assertEqual(target, "") + self.assertEqual(target.value, "") + self.assertEqual( + target.parsed_type, + ceph_hooks.OSDMemoryTargetType.EMPTY + ) @patch.object(ceph_hooks, "config") @patch.object(ceph_hooks, "get_total_ram") @@ -812,7 +821,11 @@ class CephHooksTestCase(unittest.TestCase): mock_config.side_effect = config_func target = ceph_hooks.get_osd_memory_target() - self.assertEqual(target, "") + self.assertEqual(target.value, "") + self.assertEqual( + target.parsed_type, + ceph_hooks.OSDMemoryTargetType.INVALID + ) mock_log.assert_called_with( "tune-osd-memory-target value invalid," " leaving the OSD memory target unchanged",