[Validators] Add validator ExtraChefAttributesValidator to validate format of ExtraChefAttributes and notify the user about downsides of in_place_update_on_fleet_enabled=False

CHANGELOG.md

@@ -4,6 +4,9 @@ CHANGELOG
 3.15.0
 ------
 
+**CHANGES**
+- Add validator that warns against the downsides of disabling in-place updates on compute and login nodes through DevSettings.
+
 **BUG FIXES**
 - Reduce EFA installation time for Ubuntu by ~20 minutes by only holding kernel packages for the installed kernel.
 

cli/src/pcluster/config/common.py

@@ -21,6 +21,7 @@
 from typing import List, Set
 
 from pcluster.validators.common import AsyncValidator, FailureLevel, ValidationResult, Validator, ValidatorContext
+from pcluster.validators.dev_settings_validators import ExtraChefAttributesValidator
 from pcluster.validators.iam_validators import AdditionalIamPolicyValidator
 from pcluster.validators.networking_validators import LambdaFunctionsVpcConfigValidator
 from pcluster.validators.s3_validators import UrlValidator
@@ -333,6 +334,8 @@ def __init__(self, chef_cookbook: str = None, extra_chef_attributes: str = None)
     def _register_validators(self, context: ValidatorContext = None):
         if self.chef_cookbook is not None:
             self._register_validator(UrlValidator, url=self.chef_cookbook)
+        if self.extra_chef_attributes:
+            self._register_validator(ExtraChefAttributesValidator, extra_chef_attributes=self.extra_chef_attributes)
 
 
 class LambdaFunctionsVpcConfig(Resource):

cli/src/pcluster/validators/dev_settings_validators.py

@@ -0,0 +1,62 @@
+# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance
+# with the License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+import json
+
+from pcluster.validators.common import FailureLevel, Validator
+from pcluster.validators.utils import dig, is_boolean_string, str_to_bool
+
+EXTRA_CHEF_ATTRIBUTES_PATH = "DevSettings/Cookbook/ExtraChefAttributes"
+ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED = "in_place_update_on_fleet_enabled"
+
+
+class ExtraChefAttributesValidator(Validator):
+    """Validate DevSettings/Cookbook/ExtraChefAttributes."""
+
+    def _validate(self, extra_chef_attributes: str = None):
+        """Validate extra Chef attributes.
+
+        Args:
+            extra_chef_attributes: JSON string containing Chef attributes.
+                                 Schema validation ensures this is valid JSON.
+        """
+        if not extra_chef_attributes:
+            return
+        else:
+            self._validate_in_place_update_on_fleet_enabled(json.loads(extra_chef_attributes))
+
+    def _validate_in_place_update_on_fleet_enabled(self, extra_chef_attributes: dict = None):
+        """Validate attribute cluster.in_place_update_on_fleet_enabled.
+
+        It returns an error if the attribute is set to a non-boolean value.
+        It returns a warning if the in-place update is disabled.
+
+        Args:
+            extra_chef_attributes: Dictionary of Chef attributes to validate.
+        """
+        in_place_update_on_fleet_enabled = dig(extra_chef_attributes, "cluster", ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED)
+
+        if in_place_update_on_fleet_enabled is None:
+            return
+
+        if not is_boolean_string(str(in_place_update_on_fleet_enabled)):
+            self._add_failure(
+                f"Invalid value in {EXTRA_CHEF_ATTRIBUTES_PATH}: "
+                f"attribute '{ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED}' must be a boolean value.",
+                FailureLevel.ERROR,
+            )
+            return
+
+        if str_to_bool(str(in_place_update_on_fleet_enabled)) is False:
+            self._add_failure(
+                "When in-place updates are disabled, cluster updates are applied "
+                "by replacing compute and login nodes according to the selected QueueUpdateStrategy.",
+                FailureLevel.WARNING,
+            )

cli/src/pcluster/validators/utils.py

@@ -12,7 +12,54 @@
 # This module contains all the classes representing the Resources objects.
 # These objects are obtained from the configuration file through a conversion based on the Schema classes.
 #
+from typing import Any, Union
+
+BOOLEAN_VALUES = ["true", "false"]
 
 
 def get_bucket_name_from_s3_url(import_path):
     return import_path.split("/")[2]
+
+
+def str_to_bool(string: str = None) -> bool:
+    """Convert string to boolean value.
+
+    Args:
+        string: String to convert. Defaults to None.
+
+    Returns:
+        True if string is "true" (case-insensitive), False otherwise.
+    """
+    return str(string).lower() == "true"
+
+
+def is_boolean_string(value: str) -> bool:
+    """Check if value is a valid boolean string.
+
+    Args:
+        value: String value to check.
+
+    Returns:
+        True if value is "true" or "false" (case-insensitive), False otherwise.
+    """
+    return str(value).lower() in BOOLEAN_VALUES
+
+
+def dig(dictionary: dict, *keys: str) -> Union[dict, None, Any]:
+    """Navigate nested dictionary using key path.
+
+    Args:
+        dictionary: Dictionary to navigate.
+        *keys: Sequence of keys to traverse.
+
+    Returns:
+        Value at the specified key path, or None if path doesn't exist.
+    """
+    if dictionary is None:
+        return None
+    value = dictionary
+    for key in keys:
+        if value is None or not isinstance(value, dict):
+            return None
+        value = value.get(key)
+    return value

cli/tests/pcluster/validators/test_all_validators.py

@@ -19,6 +19,7 @@
 from pcluster.validators import (
     cluster_validators,
     database_validators,
+    dev_settings_validators,
     ebs_validators,
     ec2_validators,
     feature_validators,
@@ -67,6 +68,7 @@ async def _validate_async(*args, **kwargs):
     modules = [
         cluster_validators,
         database_validators,
+        dev_settings_validators,
         ebs_validators,
         ec2_validators,
         fsx_validators,
@@ -282,6 +284,10 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker)
     compute_resource_tags_validator = mocker.patch(
         tags_validators + ".ComputeResourceTagsValidator._validate", return_value=[]
     )
+    dev_settings_validators = validators_path + ".dev_settings_validators"
+    extra_chef_attributes_validator = mocker.patch(
+        dev_settings_validators + ".ExtraChefAttributesValidator._validate", return_value=[]
+    )
     mocker.patch(
         "pcluster.config.cluster_config.HeadNode.architecture", new_callable=PropertyMock(return_value="x86_64")
     )
@@ -451,6 +457,8 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker)
         any_order=True,
     )
 
+    extra_chef_attributes_validator.assert_has_calls([call(extra_chef_attributes='{"attr1": {"attr2": "value"}}')])
+
     # No assertion on the argument for minor validators
     name_validator.assert_called()
     feature_region_validator.assert_called()

cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml

@@ -101,3 +101,7 @@ Tags:
     Value: String
   - Key: cluster_tag2
     Value: String
+DevSettings:
+  Cookbook:
+    ExtraChefAttributes: |
+      {"attr1": {"attr2": "value"}}

cli/tests/pcluster/validators/test_dev_settings_validators.py

@@ -0,0 +1,69 @@
+# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance
+# with the License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+import pytest
+
+from pcluster.validators.common import FailureLevel
+from pcluster.validators.dev_settings_validators import ExtraChefAttributesValidator
+from tests.pcluster.validators.utils import assert_failure_level, assert_failure_messages
+
+
+@pytest.mark.parametrize(
+    "extra_chef_attributes, expected_message, expected_failure_level",
+    [
+        pytest.param(None, None, None, id="No extra chef attributes"),
+        pytest.param('{"other_attribute": "value"}', None, None, id="in_place_update_on_fleet_enabled not set"),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": "true"}}',
+            None,
+            None,
+            id="in_place_update_true_string 'true' passes",
+        ),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": true}}',
+            None,
+            None,
+            id="in_place_update_true_string true passes",
+        ),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": "false"}}',
+            "When in-place updates are disabled, cluster updates are applied "
+            "by replacing compute and login nodes according to the selected QueueUpdateStrategy.",
+            FailureLevel.WARNING,
+            id="in_place_update_true_string 'false' throws a warning",
+        ),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": false}}',
+            "When in-place updates are disabled, cluster updates are applied "
+            "by replacing compute and login nodes according to the selected QueueUpdateStrategy.",
+            FailureLevel.WARNING,
+            id="in_place_update_true_string false throws a warning",
+        ),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": "invalid"}}',
+            "Invalid value in DevSettings/Cookbook/ExtraChefAttributes: "
+            "attribute 'in_place_update_on_fleet_enabled' must be a boolean value.",
+            FailureLevel.ERROR,
+            id="in_place_update_true_string invalid (string) throws an error",
+        ),
+        pytest.param(
+            '{"cluster": {"in_place_update_on_fleet_enabled": 123}}',
+            "Invalid value in DevSettings/Cookbook/ExtraChefAttributes: "
+            "attribute 'in_place_update_on_fleet_enabled' must be a boolean value.",
+            FailureLevel.ERROR,
+            id="n_place_update_true_string invalid (number) throws an error",
+        ),
+    ],
+)
+def test_extra_chef_attributes_validator(extra_chef_attributes, expected_message, expected_failure_level):
+    actual_failures = ExtraChefAttributesValidator().execute(extra_chef_attributes=extra_chef_attributes)
+    assert_failure_messages(actual_failures, expected_message)
+    if expected_failure_level:
+        assert_failure_level(actual_failures, expected_failure_level)

cli/tests/pcluster/validators/test_utils.py

@@ -0,0 +1,86 @@
+# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance
+# with the License. A copy of the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
+# OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and
+# limitations under the License.
+import pytest
+
+from pcluster.validators.utils import dig, is_boolean_string, str_to_bool
+
+
+@pytest.mark.parametrize(
+    "string_value, expected_result",
+    [
+        # Valid cases
+        ("true", True),
+        ("True", True),
+        ("TRUE", True),
+        ("false", False),
+        ("False", False),
+        ("FALSE", False),
+        # Invalid cases
+        ("yes", False),
+        ("no", False),
+        ("1", False),
+        ("0", False),
+        ("", False),
+        (None, False),
+    ],
+)
+def test_str_to_bool(string_value, expected_result):
+    assert str_to_bool(string_value) == expected_result
+
+
+@pytest.mark.parametrize(
+    "value, expected_result",
+    [
+        # Valid cases
+        ("true", True),
+        ("True", True),
+        ("TRUE", True),
+        ("false", True),
+        ("False", True),
+        ("FALSE", True),
+        (True, True),
+        (False, True),
+        # Invalid cases
+        (None, False),
+        ("", False),
+        ("yes", False),
+        ("no", False),
+        ("1", False),
+        ("0", False),
+        (1, False),
+        (0, False),
+    ],
+)
+def test_is_boolean_string(value, expected_result):
+    assert is_boolean_string(value) == expected_result
+
+
+@pytest.mark.parametrize(
+    "dictionary, keys, expected_result",
+    [
+        # Cases where value is found
+        ({"a": {"b": {"c": "value"}}}, ("a", "b", "c"), "value"),
+        ({"a": {"b": "value"}}, ("a", "b"), "value"),
+        ({"a": "value"}, ("a",), "value"),
+        ({"a": {"b": {"c": "value"}}}, ("a", "b"), {"c": "value"}),
+        # Cases where value is not found
+        ({"a": {"b": {"c": "value"}}}, ("a", "nonexistent"), None),
+        ({"a": {"b": {"c": "value"}}}, ("nonexistent",), None),
+        ({"a": {"b": {"c": "value"}}}, ("a", "b", "c", "d"), None),
+        ({}, ("a",), None),
+        (None, ("a",), None),
+        ({"a": None}, ("a", "b"), None),
+        ({"a": "not_dict"}, ("a", "b"), None),
+        ({"a": {"b": None}}, ("a", "b", "c"), None),
+    ],
+)
+def test_dig(dictionary, keys, expected_result):
+    assert dig(dictionary, *keys) == expected_result