diff --git a/paasta_tools/config_utils.py b/paasta_tools/config_utils.py index 0ba93b89cf..7c87e1c63e 100644 --- a/paasta_tools/config_utils.py +++ b/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: diff --git a/tests/test_config_utils.py b/tests/test_config_utils.py index 45902fa676..12047112dd 100644 --- a/tests/test_config_utils.py +++ b/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, + } + } + }