Check ILM status before reporting node migration STALLED (#98367)

This commit adjusts the behavior of the node shutdown status
calculation to avoid reporting a `STALLED` status when ILM is
in the process of doing something that might result in the
migration being temporarily stalled (at time of writing, that
means shrinking an index).

This check is only made if ILM is not stopped.

docs/changelog/98367.yaml

@@ -0,0 +1,6 @@
+pr: 98367
+summary: Check ILM status before reporting node migration STALLED
+area: Infra/Node Lifecycle
+type: enhancement
+issues:
+ - 89486

x-pack/plugin/shutdown/src/main/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusAction.java

@@ -17,6 +17,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.cluster.metadata.LifecycleExecutionState;
 import org.elasticsearch.cluster.metadata.NodesShutdownMetadata;
 import org.elasticsearch.cluster.metadata.ShutdownPersistentTasksStatus;
 import org.elasticsearch.cluster.metadata.ShutdownPluginsStatus;
@@ -38,6 +39,9 @@ import org.elasticsearch.snapshots.SnapshotsInfoService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
+import org.elasticsearch.xpack.core.ilm.ErrorStep;
+import org.elasticsearch.xpack.core.ilm.OperationMode;
+import org.elasticsearch.xpack.core.ilm.ShrinkAction;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -50,6 +54,7 @@ import java.util.stream.Collectors;
 
 import static org.elasticsearch.cluster.metadata.ShutdownShardMigrationStatus.NODE_ALLOCATION_DECISION_KEY;
 import static org.elasticsearch.core.Strings.format;
+import static org.elasticsearch.xpack.core.ilm.LifecycleOperationMetadata.currentILMMode;
 
 public class TransportGetShutdownStatusAction extends TransportMasterNodeAction<
     GetShutdownStatusAction.Request,
@@ -278,6 +283,8 @@ public class TransportGetShutdownStatusAction extends TransportMasterNodeAction<
                 }
                 return hasShardCopyOnOtherNode == false;
             })
+            // If ILM is shrinking the index this shard is part of, it'll look like it's unmovable, but we can just wait for ILM to finish
+            .filter(pair -> isIlmRestrictingShardMovement(currentState, pair.v1()) == false)
             .peek(pair -> {
                 logger.debug(
                     "node [{}] shutdown of type [{}] stalled: found shard [{}][{}] from index [{}] with negative decision: [{}]",
@@ -321,6 +328,29 @@ public class TransportGetShutdownStatusAction extends TransportMasterNodeAction<
         }
     }
 
+    private static boolean isIlmRestrictingShardMovement(ClusterState currentState, ShardRouting pair) {
+        if (OperationMode.STOPPED.equals(currentILMMode(currentState)) == false) {
+            LifecycleExecutionState ilmState = currentState.metadata().index(pair.index()).getLifecycleExecutionState();
+            // Specifically, if 1) ILM is running, 2) ILM is currently shrinking the index this shard is part of, and 3) it hasn't
+            // errored out, we can disregard this shard under the assumption that ILM will get it movable eventually
+            boolean ilmWillMoveShardEventually = ilmState != null
+                && ShrinkAction.NAME.equals(ilmState.action())
+                && ErrorStep.NAME.equals(ilmState.step()) == false;
+            if (ilmWillMoveShardEventually) {
+                logger.debug(
+                    format(
+                        "shard [%s] [%s] of index [%s] cannot move, but ILM is shrinking that index so assuming it will move",
+                        pair.shardId().getId(),
+                        pair.primary() ? "primary" : "replica",
+                        pair.index().getName()
+                    )
+                );
+            }
+            return ilmWillMoveShardEventually;
+        }
+        return false;
+    }
+
     private static boolean hasShardCopyOnAnotherNode(ClusterState clusterState, ShardRouting shardRouting, Set<String> shuttingDownNodes) {
         return clusterState.routingTable()
             .allShards(shardRouting.index().getName())

x-pack/plugin/shutdown/src/test/java/org/elasticsearch/xpack/shutdown/TransportGetShutdownStatusActionTests.java

@@ -13,6 +13,7 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.EmptyClusterInfoService;
 import org.elasticsearch.cluster.TestShardRoutingRoleStrategies;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.LifecycleExecutionState;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.NodesShutdownMetadata;
 import org.elasticsearch.cluster.metadata.ShutdownShardMigrationStatus;
@@ -44,9 +45,16 @@ import org.elasticsearch.snapshots.SnapshotShardSizeInfo;
 import org.elasticsearch.snapshots.SnapshotsInfoService;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.gateway.TestGatewayAllocator;
+import org.elasticsearch.xpack.core.ilm.ErrorStep;
+import org.elasticsearch.xpack.core.ilm.LifecycleOperationMetadata;
+import org.elasticsearch.xpack.core.ilm.OperationMode;
+import org.elasticsearch.xpack.core.ilm.ShrinkAction;
+import org.elasticsearch.xpack.core.ilm.ShrinkStep;
+import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType;
 import org.hamcrest.Matcher;
 import org.junit.Before;
 
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -54,6 +62,8 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Function;
 
 import static java.util.stream.Collectors.toMap;
+import static org.elasticsearch.cluster.metadata.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY;
+import static org.elasticsearch.xpack.core.ilm.LifecycleOperationMetadata.currentSLMMode;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -551,6 +561,107 @@ public class TransportGetShutdownStatusActionTests extends ESTestCase {
         assertShardMigration(status, SingleNodeShutdownMetadata.Status.NOT_STARTED, 0, is("node is not currently part of the cluster"));
     }
 
+    public void testIlmShrinkingIndexAvoidsStall() {
+        LifecycleExecutionState executionState = LifecycleExecutionState.builder()
+            .setAction(ShrinkAction.NAME)
+            .setStep(ShrinkStep.NAME)
+            .setPhase(randomFrom("hot", "warm"))
+            .build();
+        checkStalledShardWithIlmState(executionState, OperationMode.RUNNING, SingleNodeShutdownMetadata.Status.IN_PROGRESS);
+    }
+
+    public void testIlmShrinkingWithIlmStoppingIndexAvoidsStall() {
+        LifecycleExecutionState executionState = LifecycleExecutionState.builder()
+            .setAction(ShrinkAction.NAME)
+            .setStep(ShrinkStep.NAME)
+            .build();
+        checkStalledShardWithIlmState(executionState, OperationMode.STOPPING, SingleNodeShutdownMetadata.Status.IN_PROGRESS);
+    }
+
+    public void testIlmShrinkingButIlmStoppedDoesNotAvoidStall() {
+        LifecycleExecutionState executionState = LifecycleExecutionState.builder()
+            .setAction(ShrinkAction.NAME)
+            .setStep(ShrinkStep.NAME)
+            .build();
+        checkStalledShardWithIlmState(executionState, OperationMode.STOPPED, SingleNodeShutdownMetadata.Status.STALLED);
+    }
+
+    public void testIlmShrinkingButErroredDoesNotAvoidStall() {
+        LifecycleExecutionState executionState = LifecycleExecutionState.builder()
+            .setAction(ShrinkAction.NAME)
+            .setStep(ErrorStep.NAME)
+            .build();
+        checkStalledShardWithIlmState(
+            executionState,
+            randomFrom(OperationMode.STOPPING, OperationMode.RUNNING),
+            SingleNodeShutdownMetadata.Status.STALLED
+        );
+    }
+
+    public void testIlmInAnyOtherActionDoesNotAvoidStall() {
+        Set<String> allOtherIlmActions = new HashSet<>();
+        allOtherIlmActions.addAll(TimeseriesLifecycleType.ORDERED_VALID_HOT_ACTIONS);
+        allOtherIlmActions.addAll(TimeseriesLifecycleType.ORDERED_VALID_WARM_ACTIONS);
+        allOtherIlmActions.addAll(TimeseriesLifecycleType.ORDERED_VALID_COLD_ACTIONS);
+        allOtherIlmActions.addAll(TimeseriesLifecycleType.ORDERED_VALID_FROZEN_ACTIONS);
+        allOtherIlmActions.addAll(TimeseriesLifecycleType.ORDERED_VALID_DELETE_ACTIONS);
+        allOtherIlmActions.remove(ShrinkAction.NAME);
+        for (String action : allOtherIlmActions) {
+            LifecycleExecutionState executionState = LifecycleExecutionState.builder().setAction(action).build();
+            checkStalledShardWithIlmState(
+                executionState,
+                randomFrom(OperationMode.STOPPING, OperationMode.RUNNING),
+                SingleNodeShutdownMetadata.Status.STALLED
+            );
+        }
+    }
+
+    private void checkStalledShardWithIlmState(
+        LifecycleExecutionState executionState,
+        OperationMode operationMode,
+        SingleNodeShutdownMetadata.Status expectedStatus
+    ) {
+        Index index = new Index(randomAlphaOfLength(5), randomAlphaOfLengthBetween(1, 20));
+        IndexMetadata imd = IndexMetadata.builder(generateIndexMetadata(index, 3, 0))
+            .putCustom(ILM_CUSTOM_METADATA_KEY, executionState.asMap())
+            .build();
+        IndexRoutingTable indexRoutingTable = IndexRoutingTable.builder(index)
+            .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), LIVE_NODE_ID, true, ShardRoutingState.STARTED))
+            .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), LIVE_NODE_ID, true, ShardRoutingState.STARTED))
+            .addShard(TestShardRouting.newShardRouting(new ShardId(index, 2), SHUTTING_DOWN_NODE_ID, true, ShardRoutingState.STARTED))
+            .build();
+
+        // Force a decision of NO for all moves and new allocations, simulating a decider that's stuck
+        canAllocate.set((r, n, a) -> Decision.NO);
+        // And the remain decider simulates NodeShutdownAllocationDecider
+        canRemain.set((r, n, a) -> n.nodeId().equals(SHUTTING_DOWN_NODE_ID) ? Decision.NO : Decision.YES);
+
+        RoutingTable.Builder routingTable = RoutingTable.builder();
+        routingTable.add(indexRoutingTable);
+        ClusterState state = createTestClusterState(routingTable.build(), List.of(imd), SingleNodeShutdownMetadata.Type.REMOVE);
+        state = setIlmOperationMode(state, operationMode);
+
+        ShutdownShardMigrationStatus status = TransportGetShutdownStatusAction.shardMigrationStatus(
+            state,
+            SHUTTING_DOWN_NODE_ID,
+            SingleNodeShutdownMetadata.Type.REMOVE,
+            true,
+            clusterInfoService,
+            snapshotsInfoService,
+            allocationService,
+            allocationDeciders
+        );
+
+        assertShardMigration(
+            status,
+            expectedStatus,
+            1,
+            expectedStatus == SingleNodeShutdownMetadata.Status.STALLED
+                ? allOf(containsString(index.getName()), containsString("[2] [primary]"))
+                : nullValue()
+        );
+    }
+
     private IndexMetadata generateIndexMetadata(Index index, int numberOfShards, int numberOfReplicas) {
         return IndexMetadata.builder(index.getName())
             .settings(
@@ -638,6 +749,15 @@ public class TransportGetShutdownStatusActionTests extends ESTestCase {
             .build();
     }
 
+    private ClusterState setIlmOperationMode(ClusterState state, OperationMode operationMode) {
+        return ClusterState.builder(state)
+            .metadata(
+                Metadata.builder(state.metadata())
+                    .putCustom(LifecycleOperationMetadata.TYPE, new LifecycleOperationMetadata(operationMode, currentSLMMode(state)))
+            )
+            .build();
+    }
+
     private UnassignedInfo makeUnassignedInfo(String nodeId) {
         return new UnassignedInfo(
             UnassignedInfo.Reason.ALLOCATION_FAILED,