diff --git a/CHANGELOG.md b/CHANGELOG.md index 857a77e0..e996ff52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This file is used to list changes made in each version of the aws-parallelcluste - Fix clustermgtd failing to detect compute node bootstrap timeouts, which prevented the cluster from entering protected mode. - Fix an issue where compute nodes are incorrectly replaced when launching a large number of nodes due to eventual consistency. - Fix an issue where starting the compute fleet may not reliably recover the cluster from protected mode. +- Fix an issue where static nodes in a maintenance reservation enter a terminate/relaunch loop when being replaced. 3.15.0 ------ diff --git a/src/slurm_plugin/clustermgtd.py b/src/slurm_plugin/clustermgtd.py index a09924af..d2253452 100644 --- a/src/slurm_plugin/clustermgtd.py +++ b/src/slurm_plugin/clustermgtd.py @@ -741,13 +741,13 @@ def _is_instance_unhealthy(self, instance_status: EC2InstanceHealthState, health return is_instance_status_unhealthy def _update_static_nodes_in_replacement(self, slurm_nodes: List[SlurmNode]): - """Remove from self.static_nodes_in_replacement nodes that are up or that are in maintenance.""" + """Remove from self.static_nodes_in_replacement nodes that are up.""" nodename_to_slurm_nodes_map = {node.name: node for node in slurm_nodes} nodes_still_in_replacement = set() for nodename in self._static_nodes_in_replacement: node = nodename_to_slurm_nodes_map.get(nodename) - # Consider nodename still in replacement if node is not up and not in maintenance - if node and not node.is_up() and not node.is_in_maintenance(): + # Consider nodename still in replacement if node is not up + if node and not node.is_up(): nodes_still_in_replacement.add(nodename) # override self._static_nodes_in_replacement with updated list diff --git a/tests/slurm_plugin/test_clustermgtd.py b/tests/slurm_plugin/test_clustermgtd.py index 3a7b7d2b..6f63dd74 100644 --- a/tests/slurm_plugin/test_clustermgtd.py +++ b/tests/slurm_plugin/test_clustermgtd.py @@ -820,7 +820,7 @@ def test_handle_health_check( StaticNode("queue1-st-c5xlarge-11", "ip", "hostname", "IDLE+CLOUD", "queue1"), StaticNode("queue1-st-c5xlarge-12", "ip", "hostname", "DOWN+CLOUD", "queue1"), ], - {"queue1-st-c5xlarge-2", "queue1-st-c5xlarge-6"}, + {"queue1-st-c5xlarge-2", "queue1-st-c5xlarge-3", "queue1-st-c5xlarge-6"}, ) ], ids=["mixed"], @@ -3980,6 +3980,45 @@ def test_set_ice_compute_resources_to_down( False, True, # disable_capacity_blocks_management ), + # User-created maintenance reservation: a DOWN node in MAINTENANCE+RESERVED that is NOT + # tracked by CapacityBlockManager (not in reserved_nodenames) should still be considered + # unhealthy and enter the replacement flow. + ( + [ + # Node in user-created maintenance reservation - not tracked as CB reserved + StaticNode( + "queue1-st-c5xlarge-1", + "ip-1", + "hostname", + "DOWN+CLOUD+MAINTENANCE+RESERVED", + "queue1", + ), + # Node in CB maintenance reservation - tracked as CB reserved + StaticNode( + "queue1-st-c5xlarge-2", + "ip-2", + "hostname", + "DOWN+CLOUD+MAINTENANCE+RESERVED", + "queue1", + reservation_name="cr-123456", + ), + ], + ["queue1-st-c5xlarge-2"], # reserved_nodenames (only CB node) + [], # expected_unhealthy_dynamic_nodes + [ + StaticNode( + "queue1-st-c5xlarge-1", + "ip-1", + "hostname", + "DOWN+CLOUD+MAINTENANCE+RESERVED", + "queue1", + ), + ], # expected_unhealthy_static_nodes (user maintenance node enters replacement) + [False], # unhealthy_static_node_backing_instance_valid + {}, # expected_ice_compute_resources_and_nodes_map + False, # disable_nodes_on_insufficient_capacity + False, # disable_capacity_blocks_management + ), ], ) @pytest.mark.usefixtures(