[DevSettings] Add support for selective ExtraChefAttributes updates.

Accept updates for the attribute `cluster/slurm/reconfigure_timeout`.

Author: gmarciani

cli/src/pcluster/config/update_policy.py

@@ -20,6 +20,7 @@
     SLURM,
     STORAGE_TYPES_SUPPORTING_LIVE_UPDATES,
 )
+from pcluster.utils import get_dictionary_diff, parse_json_string
 
 
 class UpdatePolicy:
@@ -510,6 +511,40 @@ def get_pool_name_from_change_paths(change):
     return ""
 
 
+# ExtraChefAttributes update policy helpers
+# Define which paths within ExtraChefAttributes JSON are updatable (dot-notation)
+# IMPORTANT: We assume this set to contain the path to leaf fields.
+EXTRA_CHEF_ATTRIBUTES_UPDATABLE_PATHS: set[str] = {
+    "cluster.slurm.reconfigure_timeout",
+}
+
+
+def _get_non_updatable_changes(old_value: str, new_value: str) -> list[str]:
+    """
+    Parse and compare two ExtraChefAttributes JSON strings, returning paths that are not updatable.
+    """
+    old_attrs = parse_json_string(old_value, raise_on_error=False, default={}) if old_value else {}
+    new_attrs = parse_json_string(new_value, raise_on_error=False, default={}) if new_value else {}
+
+    return [p for p in get_dictionary_diff(old_attrs, new_attrs) if p not in EXTRA_CHEF_ATTRIBUTES_UPDATABLE_PATHS]
+
+
+def condition_checker_extra_chef_attributes(change, patch) -> bool:
+    """
+    Check if ExtraChefAttributes changes are allowed.
+
+    Only changes to paths defined in EXTRA_CHEF_ATTRIBUTES_UPDATABLE_PATHS are allowed.
+    """
+    return len(_get_non_updatable_changes(change.old_value, change.new_value)) == 0
+
+
+def fail_reason_extra_chef_attributes(change, patch):
+    """Generate fail reason for ExtraChefAttributes update."""
+    non_updatable_changes = _get_non_updatable_changes(change.old_value, change.new_value)
+    paths_str = ", ".join(sorted(non_updatable_changes))
+    return f"The following ExtraChefAttributes fields cannot be updated: {paths_str}"
+
+
 # Common fail_reason messages
 UpdatePolicy.FAIL_REASONS = {
     "ebs_volume_resize": "Updating the file system after a resize operation requires commands specific to your "
@@ -526,6 +561,7 @@ def get_pool_name_from_change_paths(change):
     "compute_or_login_nodes_running": lambda change, patch: (
         "The update is not supported when compute or login nodes are running"
     ),
+    "extra_chef_attributes_update": fail_reason_extra_chef_attributes,
 }
 
 # Common action_needed messages
@@ -548,6 +584,9 @@ def get_pool_name_from_change_paths(change):
         "Stop the login nodes by setting Count parameter to 0 "
         "and update the cluster with the pcluster update-cluster command"
     ),
+    "extra_chef_attributes_update": lambda change, patch: (
+        "Revert the non-updatable ExtraChefAttributes fields to their original values."
+    ),
 }
 
 # Base policies
@@ -567,6 +606,15 @@ def get_pool_name_from_change_paths(change):
     name="SUPPORTED", level=0, fail_reason="-", condition_checker=(lambda change, patch: True)
 )
 
+# Update policy for ExtraChefAttributes - allows updates to specific fields only
+UpdatePolicy.EXTRA_CHEF_ATTRIBUTES = UpdatePolicy(
+    name="EXTRA_CHEF_ATTRIBUTES",
+    level=5,
+    fail_reason=UpdatePolicy.FAIL_REASONS["extra_chef_attributes_update"],
+    action_needed=UpdatePolicy.ACTIONS_NEEDED["extra_chef_attributes_update"],
+    condition_checker=condition_checker_extra_chef_attributes,
+)
+
 # Checks resize of max_vcpus in Batch Compute Environment
 UpdatePolicy.AWSBATCH_CE_MAX_RESIZE = UpdatePolicy(
     name="AWSBATCH_CE_MAX_RESIZE",

cli/src/pcluster/schemas/common_schema.py

@@ -216,13 +216,7 @@ class CookbookSchema(BaseSchema):
             )
         }
     )
-    extra_chef_attributes = fields.Str(
-        metadata={
-            "update_policy": UpdatePolicy(
-                UpdatePolicy.UNSUPPORTED, fail_reason=UpdatePolicy.FAIL_REASONS["cookbook_update"]
-            )
-        }
-    )
+    extra_chef_attributes = fields.Str(metadata={"update_policy": UpdatePolicy.EXTRA_CHEF_ATTRIBUTES})
 
     @post_load()
     def make_resource(self, data, **kwargs):
@@ -275,7 +269,7 @@ def make_resource(self, data, **kwargs):
 class BaseDevSettingsSchema(BaseSchema):
     """Represent the common schema of Dev Setting for ImageBuilder and Cluster."""
 
-    cookbook = fields.Nested(CookbookSchema, metadata={"update_policy": UpdatePolicy.UNSUPPORTED})
+    cookbook = fields.Nested(CookbookSchema, metadata={"update_policy": UpdatePolicy.IGNORED})
     node_package = fields.Str(metadata={"update_policy": UpdatePolicy.UNSUPPORTED})
     aws_batch_cli_package = fields.Str(metadata={"update_policy": UpdatePolicy.UNSUPPORTED})
 

cli/src/pcluster/utils.py

@@ -614,3 +614,77 @@ def get_needed_ultraserver_capacity_block_statuses(statuses, capacity_reservatio
         ):
             needed_capacity_block_statuses.append(status)
     return needed_capacity_block_statuses
+
+
+def parse_json_string(value: str, raise_on_error: bool = False, default: any = None) -> any:
+    """
+    Parse a JSON string into a Python object.
+
+    :param value: JSON string to parse
+    :param raise_on_error: If True, raises exception on parse failure; if False, returns default
+    :param default: Value to return on parse failure when raise_on_error is False
+    :return: Parsed JSON object or default value on failure
+    """
+    try:
+        return json.loads(value)
+    except (json.JSONDecodeError, TypeError) as e:
+        LOGGER.error("Failed to parse JSON string: %s. Error: %s", value, e)
+        if raise_on_error:
+            raise e
+        return default
+
+
+def get_dictionary_diff(d1: dict, d2: dict, path: str = "") -> set[str]:
+    """
+    Recursively find all leaf paths where two dictionaries differ.
+
+    Returns a set of dot-notation paths (e.g., "cluster.settings.enabled") where
+    the dictionaries have different values, including added or removed keys.
+    Always returns leaf paths, never intermediate dict paths.
+
+    :param d1: First dictionary to compare (if None, treated as empty dict)
+    :param d2: Second dictionary to compare (if None, treated as empty dict)
+    :param path: Current path prefix (used for recursion)
+    :return: Set of dot-notation paths where dictionaries differ (never None)
+    """
+    d1 = d1 or {}
+    d2 = d2 or {}
+
+    diffs: set[str] = set()
+    keys1, keys2 = set(d1.keys()), set(d2.keys())
+
+    def collect_leaves(d: dict, prefix: str) -> set[str]:
+        """Collect all leaf paths from a dict."""
+        leaves = set()
+        for k, v in d.items():
+            key_path = f"{prefix}.{k}" if prefix else str(k)
+            if isinstance(v, dict) and v:
+                leaves.update(collect_leaves(v, key_path))
+            else:
+                leaves.add(key_path)
+        return leaves if leaves else {prefix} if prefix else set()
+
+    # Added or removed keys - collect their leaf paths
+    for k in keys1 - keys2:
+        key_path = f"{path}.{k}" if path else str(k)
+        if isinstance(d1[k], dict) and d1[k]:
+            diffs.update(collect_leaves(d1[k], key_path))
+        else:
+            diffs.add(key_path)
+
+    for k in keys2 - keys1:
+        key_path = f"{path}.{k}" if path else str(k)
+        if isinstance(d2[k], dict) and d2[k]:
+            diffs.update(collect_leaves(d2[k], key_path))
+        else:
+            diffs.add(key_path)
+
+    # Common keys - recurse or compare
+    for k in keys1 & keys2:
+        key_path = f"{path}.{k}" if path else str(k)
+        if isinstance(d1[k], dict) and isinstance(d2[k], dict):
+            diffs.update(get_dictionary_diff(d1[k], d2[k], key_path))
+        elif d1[k] != d2[k]:
+            diffs.add(key_path)
+
+    return diffs

cli/src/pcluster/validators/dev_settings_validators.py

@@ -15,6 +15,8 @@
 
 EXTRA_CHEF_ATTRIBUTES_PATH = "DevSettings/Cookbook/ExtraChefAttributes"
 ATTR_IN_PLACE_UPDATE_ON_FLEET_ENABLED = "in_place_update_on_fleet_enabled"
+ATTR_RECONFIGURE_TIMEOUT = "cluster.slurm.reconfigure_timeout"
+MIN_SLURM_RECONFIGURE_TIMEOUT = 300
 
 
 class ExtraChefAttributesValidator(Validator):
@@ -29,8 +31,10 @@ def _validate(self, extra_chef_attributes: str = None):
         """
         if not extra_chef_attributes:
             return
-        else:
-            self._validate_in_place_update_on_fleet_enabled(json.loads(extra_chef_attributes))
+
+        attrs = json.loads(extra_chef_attributes)
+        self._validate_in_place_update_on_fleet_enabled(attrs)
+        self._validate_slurm_reconfigure_timeout(attrs)
 
     def _validate_in_place_update_on_fleet_enabled(self, extra_chef_attributes: dict = None):
         """Validate attribute cluster.in_place_update_on_fleet_enabled.
@@ -60,3 +64,33 @@ def _validate_in_place_update_on_fleet_enabled(self, extra_chef_attributes: dict
                 "by replacing compute and login nodes according to the selected QueueUpdateStrategy.",
                 FailureLevel.WARNING,
             )
+
+    def _validate_slurm_reconfigure_timeout(self, extra_chef_attributes: dict = None):
+        """Validate attribute cluster.slurm.reconfigure-timeout.
+
+        Must be an integer greater than 300.
+
+        Args:
+            extra_chef_attributes: Dictionary of Chef attributes to validate.
+        """
+        reconfigure_timeout = dig(extra_chef_attributes, *ATTR_RECONFIGURE_TIMEOUT.split("."))
+
+        if reconfigure_timeout is None:
+            return
+
+        # Reject booleans explicitly (bool is subclass of int in Python)
+        if isinstance(reconfigure_timeout, bool) or not isinstance(reconfigure_timeout, int):
+            self._add_failure(
+                f"Invalid value in {EXTRA_CHEF_ATTRIBUTES_PATH}: "
+                f"attribute '{ATTR_RECONFIGURE_TIMEOUT}' must be an integer.",
+                FailureLevel.ERROR,
+            )
+            return
+
+        if reconfigure_timeout <= MIN_SLURM_RECONFIGURE_TIMEOUT:
+            self._add_failure(
+                f"Invalid value in {EXTRA_CHEF_ATTRIBUTES_PATH}: "
+                f"attribute '{ATTR_RECONFIGURE_TIMEOUT}' "
+                f"must be greater than {MIN_SLURM_RECONFIGURE_TIMEOUT}.",
+                FailureLevel.ERROR,
+            )