From d16f2b996d50df3bd4100c7fb0f63d5c39e5bc1a Mon Sep 17 00:00:00 2001 From: hanwenli Date: Fri, 23 Oct 2026 11:07:54 -0700 Subject: [PATCH] Fix dynamic nodes stuck in COMPLETING after their instance is terminated and there is a new instance reusing the same IP Source: github.com/aws/aws-parallelcluster/issues/7671 ### Summary A dynamic node that still has a job can get stuck as `IDLE+CLOUD+COMPLETING+POWER_DOWN` after clustermgtd terminates its EC2 instance. The job never finishes completing, the node is never powered down, and clustermgtd takes no further action. ### Root cause 1. clustermgtd finds the node unhealthy, terminates its instance and runs `scontrol update state=power_down_force`, which also resets NodeAddr to the node name. 2. `POWER_DOWN_FORCE` kills the job but leaves the node's base state alone. Slurm still treats the node as reachable and waits for `EPILOG_COMPLETE`. `SuspendProgram` doesn't run while the node is COMPLETING. 3. Normally slurmd stops answering, and after `SlurmdTimeout` Slurm marks the node DOWN and recovers it. But if EC2 gives the old private IP to another compute node, that node's slurmd answers in its place. So the node keeps looking alive, and the epilog never completes. 4. clustermgtd never notices. It decides whether a node should have an instance from NodeAddr, and NodeAddr has already been reset. Slurm still holds the old InstanceId, because it only clears InstanceId at POWERED_DOWN. Change Add SlurmNode.is_instance_expected() and use it in is_backing_instance_valid(). A node is now expected to have an instance if NodeAddr is set, or if InstanceId is set and the node isn't powering down or powered down. Slurm keeps InstanceId until POWERED_DOWN, so a stuck node is now reported as missing its instance. clustermgtd then treats it as unhealthy and sends another power_down_force. For a job that is already completing, that clears the node's epilog wait (kill_running_job_by_node_ptr), so the jobs are requeued and the node powers down. Scope - Detection logic only; the scontrol commands clustermgtd sends are unchanged. - We considered setting nodes `DOWN` before power_down_force, which releases jobs immediately. It's deferred because it changes behavior for every power-down caller. --- CHANGELOG.md | 7 ++ src/slurm_plugin/slurm_resources.py | 16 +++- .../slurm_resources/test_slurm_resources.py | 96 +++++++++++++++++++ 3 files changed, 117 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 042d6ec7..c013b333 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ aws-parallelcluster-node CHANGELOG This file is used to list changes made in each version of the aws-parallelcluster-node package. +3.17.0 +------ + +**BUG FIXES** +- Fix dynamic nodes getting stuck in `COMPLETING` state indefinitely, holding their jobs, when the backing instance + of a node with jobs is terminated and its private IP is reused by another compute node. + 3.16.2 ------ diff --git a/src/slurm_plugin/slurm_resources.py b/src/slurm_plugin/slurm_resources.py index 77d9b472..e4455c5f 100644 --- a/src/slurm_plugin/slurm_resources.py +++ b/src/slurm_plugin/slurm_resources.py @@ -452,19 +452,31 @@ def is_powering_down_with_nodeaddr(self): # for example because of a short SuspendTimeout return self.is_nodeaddr_set() and (self.is_power() or self.is_powering_down()) + def is_instance_expected(self): + """ + Check if a slurm node is expected to be backed by an EC2 instance. + + A node is expected to have an instance if its nodeaddr is set, or if its InstanceId is set and the node is + not powering down or powered down. The InstanceId still identifies the node's instance after the nodeaddr + has been reset to the node name, until Slurm clears it when the node reaches POWERED_DOWN. + """ + return self.is_nodeaddr_set() or ( + bool(self.instance_id) and not (self.is_powering_down() or self.is_powered_down()) + ) + def is_backing_instance_valid( self, ec2_instance_missing_max_count, nodes_without_backing_instance_count_map: dict[str, MissingInstance], log_warn_if_unhealthy=True, ): - """Check if a slurm node's addr is set, it points to a valid instance in EC2.""" + """Check if a slurm node expected to be backed by an instance points to a valid instance in EC2.""" # Perform this logic only once and return the result thereafter if self.ec2_backing_instance_valid is not None: return self.ec2_backing_instance_valid # Set ec2_backing_instance_valid to True since it will be the result most often self.ec2_backing_instance_valid = True - if self.is_nodeaddr_set() and not self.instance: + if self.is_instance_expected() and not self.instance: if log_warn_if_unhealthy: logger.warning( "Node state check: no corresponding instance in EC2 for node %s, node state: %s", diff --git a/tests/slurm_plugin/slurm_resources/test_slurm_resources.py b/tests/slurm_plugin/slurm_resources/test_slurm_resources.py index 73454fd1..232c840f 100644 --- a/tests/slurm_plugin/slurm_resources/test_slurm_resources.py +++ b/tests/slurm_plugin/slurm_resources/test_slurm_resources.py @@ -1252,6 +1252,37 @@ def test_slurm_node_is_powering_down_with_nodeaddr(node, expected_result): {"queue1-st-c5xlarge-1": MissingInstance("queue1-st-c5xlarge-1", "ip-1", 1)}, True, ), + ( + # Node of GitHub issue #7671: nodeaddr reset to the node name, InstanceId of a terminated instance + DynamicNode( + "node-dy-c5xlarge-1", + "node-dy-c5xlarge-1", + "hostname", + "IDLE+CLOUD+COMPLETING+POWER_DOWN", + "node", + instance_id="i-1", + ), + None, + 0, + {}, + {}, + False, + ), + ( + DynamicNode( + "node-dy-c5xlarge-1", + "node-dy-c5xlarge-1", + "hostname", + "IDLE+CLOUD+POWERING_DOWN", + "node", + instance_id="i-1", + ), + None, + 0, + {}, + {}, + True, + ), ], ids=[ "static_no_backing_zero_max_count", @@ -1262,6 +1293,8 @@ def test_slurm_node_is_powering_down_with_nodeaddr(node, expected_result): "static_no_backing_with_count_exceeded", "static_backed_with_count_exceeded", "static_no_backing_count_not_exceeded_with_wrong_ip", + "dynamic_nodeaddr_reset_with_instance_id_no_backing", + "dynamic_powering_down_with_instance_id_no_backing", ], ) def test_slurm_node_is_backing_instance_valid(node, instance, max_count, count_map, final_map, expected_result): @@ -1278,6 +1311,69 @@ def test_slurm_node_is_backing_instance_valid(node, instance, max_count, count_m assert_that(count_map[node.name].ip).is_equal_to(node.nodeaddr) +@pytest.mark.parametrize( + "node, expected_result", + [ + (DynamicNode("queue1-dy-c5xlarge-1", "ip-1", "hostname", "MIXED+CLOUD", "queue1"), True), + ( + DynamicNode( + "queue1-dy-c5xlarge-1", + "queue1-dy-c5xlarge-1", + "hostname", + "IDLE+CLOUD+COMPLETING+POWER_DOWN", + "queue1", + instance_id="i-1", + ), + True, + ), + ( + DynamicNode( + "queue1-dy-c5xlarge-1", + "queue1-dy-c5xlarge-1", + "hostname", + "IDLE+CLOUD+POWERING_DOWN", + "queue1", + instance_id="i-1", + ), + False, + ), + ( + DynamicNode( + "queue1-dy-c5xlarge-1", + "queue1-dy-c5xlarge-1", + "hostname", + "IDLE+CLOUD+POWERED_DOWN", + "queue1", + instance_id="i-1", + ), + False, + ), + ( + DynamicNode( + "queue1-dy-c5xlarge-1", "queue1-dy-c5xlarge-1", "hostname", "IDLE+CLOUD+POWERED_DOWN", "queue1" + ), + False, + ), + ( + DynamicNode( + "queue1-dy-c5xlarge-1", "queue1-dy-c5xlarge-1", "hostname", "MIXED+CLOUD+POWERING_UP", "queue1" + ), + False, + ), + ], + ids=[ + "nodeaddr_set", + "nodeaddr_reset_with_instance_id", + "powering_down_with_instance_id", + "powered_down_with_instance_id", + "powered_down", + "powering_up_before_resume", + ], +) +def test_slurm_node_is_instance_expected(node, expected_result): + assert_that(node.is_instance_expected()).is_equal_to(expected_result) + + @pytest.mark.parametrize( "node, expected_result", [