all changes

Signed-off-by: Hanwen <[email protected]>

Author: hanwen-cluster

cli/src/pcluster/config/config_patch.py

@@ -137,15 +137,26 @@ def _compare_section(self, base_section: dict, target_section: dict, section_sch
                             )
             else:
                 # Simple param
-                target_value = target_section.get(data_key, None) if target_section else None
-                base_value = base_section.get(data_key, None) if base_section else None
+                target_value, data_key = self._get_value_from_section(data_key, target_section)
+                base_value, data_key = self._get_value_from_section(data_key, base_section)
 
                 if target_value != base_value:
                     # Add param change information
                     self.changes.append(
                         Change(param_path, data_key, base_value, target_value, change_update_policy, is_list=False)
                     )
 
+    def _get_value_from_section(self, data_key, target_section):
+        target_value = None
+        if target_section:
+            target_value = target_section.get(data_key, None)
+            if data_key == "Script" and target_value is None:
+                # This handles special case when multiple installation scripts are provided.
+                # The schema of script sequence is customized. The following code correctly detect changes.
+                target_value = target_section.get("Sequence", None)
+                data_key = "Sequence"
+        return target_value, data_key
+
     def _compare_nested_section(self, param_path, data_key, base_value, target_value, field_obj):
         # Compare nested sections and params
         nested_path = copy.deepcopy(param_path)

cli/tests/pcluster/config/test_config_patch.py

@@ -439,6 +439,35 @@ def _test_no_updatable_custom_actions(base_conf, target_conf):
     )
 
 
+def _test_no_updatable_custom_actions_sequence(base_conf, target_conf):
+    base_conf["HeadNode"].update(
+        {
+            "CustomActions": {
+                "OnNodeConfigured": {"Sequence": [{"Script": "test-to-edit.sh", "Args": ["1", "2"]}]},
+            }
+        }
+    )
+    target_conf["HeadNode"].update(
+        {"CustomActions": {"OnNodeConfigured": {"Sequence": [{"Script": "test-to-edit.sh", "Args": ["2"]}]}}}
+    )
+
+    _check_patch(
+        base_conf,
+        target_conf,
+        [
+            Change(
+                ["HeadNode", "CustomActions", "OnNodeConfigured"],
+                "Sequence",
+                [{"Script": "test-to-edit.sh", "Args": ["1", "2"]}],
+                [{"Script": "test-to-edit.sh", "Args": ["2"]}],
+                UpdatePolicy.UNSUPPORTED,
+                is_list=False,
+            ),
+        ],
+        UpdatePolicy.UNSUPPORTED,
+    )
+
+
 def _test_updatable_custom_actions_attributes(base_conf, target_conf):
     base_conf["HeadNode"].update(
         {"CustomActions": {"OnNodeUpdated": {"Script": "test-to-edit.sh", "Args": ["1", "2"]}}}
@@ -491,6 +520,71 @@ def _test_updatable_custom_actions(base_conf, target_conf):
     )
 
 
+def _test_updatable_custom_actions_sequence_add(base_conf, target_conf):
+    target_conf["HeadNode"].update(
+        {"CustomActions": {"OnNodeUpdated": {"Sequence": [{"Script": "test-to-remove.sh"}]}}}
+    )
+
+    _check_patch(
+        base_conf,
+        target_conf,
+        [
+            Change(
+                ["HeadNode", "CustomActions"],
+                "OnNodeUpdated",
+                "-",
+                {"Sequence": [{"Script": "test-to-remove.sh"}]},
+                UpdatePolicy.SUPPORTED,
+                is_list=False,
+            ),
+        ],
+        UpdatePolicy.SUPPORTED,
+    )
+
+
+def _test_updatable_custom_actions_sequence_change(base_conf, target_conf):
+    base_conf["HeadNode"].update({"CustomActions": {"OnNodeUpdated": {"Sequence": [{"Script": "test-to-remove.sh"}]}}})
+    target_conf["HeadNode"].update(
+        {"CustomActions": {"OnNodeUpdated": {"Sequence": [{"Script": "another-script.sh"}]}}}
+    )
+
+    _check_patch(
+        base_conf,
+        target_conf,
+        [
+            Change(
+                ["HeadNode", "CustomActions", "OnNodeUpdated"],
+                "Sequence",
+                [{"Script": "test-to-remove.sh"}],
+                [{"Script": "another-script.sh"}],
+                UpdatePolicy.SUPPORTED,
+                is_list=False,
+            ),
+        ],
+        UpdatePolicy.SUPPORTED,
+    )
+
+
+def _test_updatable_custom_actions_sequence_remove(base_conf, target_conf):
+    base_conf["HeadNode"].update({"CustomActions": {"OnNodeUpdated": {"Sequence": [{"Script": "test-to-remove.sh"}]}}})
+
+    _check_patch(
+        base_conf,
+        target_conf,
+        [
+            Change(
+                ["HeadNode", "CustomActions"],
+                "OnNodeUpdated",
+                {"Sequence": [{"Script": "test-to-remove.sh"}]},
+                "-",
+                UpdatePolicy.SUPPORTED,
+                is_list=False,
+            ),
+        ],
+        UpdatePolicy.SUPPORTED,
+    )
+
+
 def _test_less_target_sections(base_conf, target_conf):
     # Remove an ebs section in the target conf
     assert_that(_get_storage_by_name(target_conf, "ebs1")).is_not_none()
@@ -905,8 +999,12 @@ def _test_iam(base_conf, target_conf):
         _test_compute_resources,
         _test_queues,
         _test_no_updatable_custom_actions,
+        _test_no_updatable_custom_actions_sequence,
         _test_updatable_custom_actions,
         _test_updatable_custom_actions_attributes,
+        _test_updatable_custom_actions_sequence_add,
+        _test_updatable_custom_actions_sequence_change,
+        _test_updatable_custom_actions_sequence_remove,
         _test_storage,
         _test_iam,
     ],