PEREL-5643: support Gi/Mi strings in autotune_limits mem/disk clamping

paasta_tools/config_utils.py

@@ -18,6 +18,20 @@
 
 log = logging.getLogger(__name__)
 
+_RESOURCE_UNIT_TO_MIB = {"Gi": 1024.0, "Mi": 1.0, "Ki": 1.0 / 1024}
+_SUFFIX_RESOURCE_TYPES = {"mem", "disk"}
+
+
+def _normalize_resource_value(value: int | float | str, resource_type: str) -> float:
+    if isinstance(value, (int, float)):
+        return float(value)
+    if isinstance(value, str) and resource_type in _SUFFIX_RESOURCE_TYPES:
+        for suffix, factor in _RESOURCE_UNIT_TO_MIB.items():
+            if value.endswith(suffix):
+                return float(value.removesuffix(suffix)) * factor
+    raise ValueError(f"Cannot parse resource value: {value!r}")
+
+
 # Must have a schema defined
 KNOWN_CONFIG_TYPES = (
     "kubernetes",
@@ -300,14 +314,15 @@ def _clamp_recommendations(
 
             # otherwise, we can do some pretty rote clamping of resource values
             elif unclamped_resource_value is not None:
-                if min_value and unclamped_resource_value < min_value:
+                unclamped_mib = _normalize_resource_value(unclamped_resource_value, limit_type)
+                if min_value and unclamped_mib < _normalize_resource_value(min_value, limit_type):
                     log.debug(
                         f"{limit_type} autotune config under configured limit ({min_value}), using autotune limit lower bound."
                     )
                     clamped_recomendation[limit_type] = min_value
-                if max_value and unclamped_resource_value > max_value:
+                if max_value and unclamped_mib > _normalize_resource_value(max_value, limit_type):
                     log.debug(
-                        f"{limit_type} autotune config over configured limit ({min_value}), using autotune limit upper bound."
+                        f"{limit_type} autotune config over configured limit ({max_value}), using autotune limit upper bound."
                     )
                     clamped_recomendation[limit_type] = max_value
             else:

tests/test_config_utils.py

@@ -5,6 +5,7 @@
 
 import paasta_tools.config_utils as config_utils
 from paasta_tools import yaml_tools as yaml
+from paasta_tools.config_utils import _normalize_resource_value
 from paasta_tools.utils import AUTO_SOACONFIG_SUBDIR
 
 
@@ -383,3 +384,68 @@ def test_auto_config_updater_merge_recommendations_limits(updater):
                 },
             }
         }
+
+
+@pytest.mark.parametrize(
+    "value,resource_type,expected",
+    [
+        (512, "cpu", 512.0),
+        (1024.0, "cpu", 1024.0),
+        ("1Gi", "mem", 1024.0),
+        ("2Gi", "mem", 2048.0),
+        ("512Mi", "mem", 512.0),
+        ("1024Mi", "disk", 1024.0),
+        ("1Ki", "disk", 1.0 / 1024),
+    ],
+)
+def test_normalize_resource_value(value, resource_type, expected):
+    assert _normalize_resource_value(value, resource_type) == pytest.approx(expected)
+
+
+def test_normalize_resource_value_invalid_suffix():
+    with pytest.raises(ValueError):
+        _normalize_resource_value("4GB", "mem")
+
+
+def test_normalize_resource_value_string_cpu_raises():
+    with pytest.raises(ValueError):
+        _normalize_resource_value("1Gi", "cpu")
+
+
+def test_auto_config_updater_merge_recommendations_limits_gi_strings(updater):
+    service = "foo"
+    conf_file = "cassandracluster-norcal-devc"
+    instance = "activity-feed"
+    autotune_data = {instance: {"cpus": 1.5, "mem": "2Gi", "disk": "5Gi"}}
+    user_data = {
+        instance: {
+            "autotune_limits": {
+                "cpus": {"min": 1},
+                "mem": {"min": "4Gi"},
+                "disk": {"max": "10Gi"},
+            }
+        }
+    }
+    recs = {
+        (service, conf_file): {
+            instance: {"mem": "2Gi", "disk": "5Gi", "cpus": 0.5},
+        }
+    }
+
+    with mock.patch.object(
+        updater,
+        "get_existing_configs",
+        autospec=True,
+        side_effect=[autotune_data, user_data, {}],
+    ):
+        result = updater.merge_recommendations(recs)
+
+    assert result == {
+        (service, conf_file): {
+            instance: {
+                "mem": "4Gi",
+                "disk": "5Gi",
+                "cpus": 1,
+            }
+        }
+    }