ILM: fix allocate action to allow only total_shards_per_node (#81944)

The allocate action can specify only `number_of_replicas` (without
routing configuration) but failed if it attempted to only specify
`total_shards_per_node`.

This fixes the action to allow specifying only `total_shards_per_node`.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/AllocateAction.java

@@ -88,7 +88,11 @@ public class AllocateAction implements LifecycleAction {
         } else {
             this.require = require;
         }
-        if (this.include.isEmpty() && this.exclude.isEmpty() && this.require.isEmpty() && numberOfReplicas == null) {
+        if (this.include.isEmpty()
+            && this.exclude.isEmpty()
+            && this.require.isEmpty()
+            && numberOfReplicas == null
+            && totalShardsPerNode == null) {
             throw new IllegalArgumentException(
                 "At least one of "
                     + INCLUDE_FIELD.getPreferredName()
@@ -96,8 +100,13 @@ public class AllocateAction implements LifecycleAction {
                     + EXCLUDE_FIELD.getPreferredName()
                     + " or "
                     + REQUIRE_FIELD.getPreferredName()
-                    + "must contain attributes for action "
+                    + " must contain attributes for action "
                     + NAME
+                    + ". Otherwise the "
+                    + NUMBER_OF_REPLICAS_FIELD.getPreferredName()
+                    + " or the "
+                    + TOTAL_SHARDS_PER_NODE_FIELD.getPreferredName()
+                    + " options must be configured."
             );
         }
         if (numberOfReplicas != null && numberOfReplicas < 0) {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/AllocateActionTests.java

@@ -20,7 +20,10 @@ import java.util.List;
 import java.util.Map;
 
 import static org.elasticsearch.cluster.routing.allocation.decider.ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING;
+import static org.elasticsearch.xpack.core.ilm.AllocateAction.NUMBER_OF_REPLICAS_FIELD;
+import static org.elasticsearch.xpack.core.ilm.AllocateAction.TOTAL_SHARDS_PER_NODE_FIELD;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
 
 public class AllocateActionTests extends AbstractActionTestCase<AllocateAction> {
 
@@ -57,7 +60,7 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
             requires = randomBoolean() ? null : Collections.emptyMap();
         }
         Integer numberOfReplicas = randomBoolean() ? null : randomIntBetween(0, 10);
-        Integer totalShardsPerNode = randomBoolean() ? null : randomIntBetween(-1, 300);
+        Integer totalShardsPerNode = randomBoolean() ? null : randomIntBetween(-1, 10);
         return new AllocateAction(numberOfReplicas, totalShardsPerNode, includes, excludes, requires);
     }
 
@@ -73,7 +76,7 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
         Map<String, String> require = instance.getRequire();
         Integer numberOfReplicas = instance.getNumberOfReplicas();
         Integer totalShardsPerNode = instance.getTotalShardsPerNode();
-        switch (randomIntBetween(0, 3)) {
+        switch (randomIntBetween(0, 4)) {
             case 0:
                 include = new HashMap<>(include);
                 include.put(randomAlphaOfLengthBetween(11, 15), randomAlphaOfLengthBetween(1, 20));
@@ -89,6 +92,9 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
             case 3:
                 numberOfReplicas = randomIntBetween(11, 20);
                 break;
+            case 4:
+                totalShardsPerNode = randomIntBetween(11, 20);
+                break;
             default:
                 throw new AssertionError("Illegal randomisation branch");
         }
@@ -110,8 +116,13 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
                 + AllocateAction.EXCLUDE_FIELD.getPreferredName()
                 + " or "
                 + AllocateAction.REQUIRE_FIELD.getPreferredName()
-                + "must contain attributes for action "
-                + AllocateAction.NAME,
+                + " must contain attributes for action "
+                + AllocateAction.NAME
+                + ". Otherwise the "
+                + NUMBER_OF_REPLICAS_FIELD.getPreferredName()
+                + " or the "
+                + TOTAL_SHARDS_PER_NODE_FIELD.getPreferredName()
+                + " options must be configured.",
             exception.getMessage()
         );
     }
@@ -124,7 +135,7 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
             IllegalArgumentException.class,
             () -> new AllocateAction(randomIntBetween(-1000, -1), randomIntBetween(0, 300), include, exclude, require)
         );
-        assertEquals("[" + AllocateAction.NUMBER_OF_REPLICAS_FIELD.getPreferredName() + "] must be >= 0", exception.getMessage());
+        assertEquals("[" + NUMBER_OF_REPLICAS_FIELD.getPreferredName() + "] must be >= 0", exception.getMessage());
     }
 
     public void testInvalidTotalShardsPerNode() {
@@ -135,7 +146,7 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
             IllegalArgumentException.class,
             () -> new AllocateAction(randomIntBetween(0, 300), randomIntBetween(-1000, -2), include, exclude, require)
         );
-        assertEquals("[" + AllocateAction.TOTAL_SHARDS_PER_NODE_FIELD.getPreferredName() + "] must be >= -1", exception.getMessage());
+        assertEquals("[" + TOTAL_SHARDS_PER_NODE_FIELD.getPreferredName() + "] must be >= -1", exception.getMessage());
     }
 
     public static Map<String, String> randomAllocationRoutingMap(int minEntries, int maxEntries) {
@@ -206,6 +217,10 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
         steps = action.toSteps(null, phase, nextStepKey);
         firstStep = (UpdateSettingsStep) steps.get(0);
         assertEquals(null, firstStep.getSettings().get(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()));
+
+        // allow an allocate action that only specifies total shards per node (don't expect any exceptions in this case)
+        action = new AllocateAction(null, 5, null, null, null);
+        assertThat(action.getTotalShardsPerNode(), is(5));
     }
 
 }

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingService.java

@@ -383,8 +383,8 @@ public final class MetadataMigrateToDataTiersRoutingService {
                 Map<String, LifecycleAction> actionMap = new HashMap<>(phase.getActions());
                 // this phase contains an allocate action that defines a require rule for the attribute name so we'll remove all the
                 // rules to allow for the migrate action to be injected
-                if (allocateAction.getNumberOfReplicas() != null) {
-                    // keep the number of replicas configuration
+                if (allocateAction.getNumberOfReplicas() != null || allocateAction.getTotalShardsPerNode() != null) {
+                    // keep the number of replicas configuration and/or the total shards per node configuration
                     AllocateAction updatedAllocateAction = new AllocateAction(
                         allocateAction.getNumberOfReplicas(),
                         allocateAction.getTotalShardsPerNode(),

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/cluster/metadata/MetadataMigrateToDataTiersRoutingServiceTests.java

@@ -267,6 +267,65 @@ public class MetadataMigrateToDataTiersRoutingServiceTests extends ESTestCase {
             assertThat(allocateDef.get("require"), is(Map.of()));
         }
 
+        {
+            // index is in the cold phase and the migrated allocate action is not removed due to allocate specifying
+            // total_shards_per_node
+            LifecyclePolicyMetadata policyMetadataWithTotalShardsPerNode = getWarmColdPolicyMeta(
+                warmSetPriority,
+                shrinkAction,
+                warmAllocateAction,
+                new AllocateAction(null, 1, null, null, Map.of("data", "cold"))
+            );
+
+            LifecycleExecutionState preMigrationExecutionState = LifecycleExecutionState.builder()
+                .setPhase("cold")
+                .setAction("allocate")
+                .setStep("allocate")
+                .setPhaseDefinition(getColdPhaseDefinitionWithTotalShardsPerNode())
+                .build();
+
+            IndexMetadata.Builder indexMetadata = IndexMetadata.builder(indexName)
+                .settings(getBaseIndexSettings())
+                .putCustom(ILM_CUSTOM_METADATA_KEY, preMigrationExecutionState.asMap());
+
+            ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
+                .metadata(
+                    Metadata.builder()
+                        .putCustom(
+                            IndexLifecycleMetadata.TYPE,
+                            new IndexLifecycleMetadata(
+                                Collections.singletonMap(
+                                    policyMetadataWithTotalShardsPerNode.getName(),
+                                    policyMetadataWithTotalShardsPerNode
+                                ),
+                                OperationMode.STOPPED
+                            )
+                        )
+                        .put(indexMetadata)
+                        .build()
+                )
+                .build();
+
+            Metadata.Builder newMetadata = Metadata.builder(state.metadata());
+            List<String> migratedPolicies = migrateIlmPolicies(newMetadata, state, "data", REGISTRY, client, null);
+
+            assertThat(migratedPolicies.get(0), is(lifecycleName));
+            ClusterState newState = ClusterState.builder(state).metadata(newMetadata).build();
+            LifecycleExecutionState newLifecycleState = LifecycleExecutionState.fromIndexMetadata(newState.metadata().index(indexName));
+
+            Map<String, Object> migratedPhaseDefAsMap = getPhaseDefinitionAsMap(newLifecycleState);
+
+            // expecting the phase definition to be refreshed with the migrated phase representation
+            // ie. allocate action does not contain any allocation rules
+            Map<String, Object> actions = (Map<String, Object>) migratedPhaseDefAsMap.get("actions");
+            assertThat(actions.size(), is(1));
+            Map<String, Object> allocateDef = (Map<String, Object>) actions.get(AllocateAction.NAME);
+            assertThat(allocateDef, notNullValue());
+            assertThat(allocateDef.get("include"), is(Map.of()));
+            assertThat(allocateDef.get("exclude"), is(Map.of()));
+            assertThat(allocateDef.get("require"), is(Map.of()));
+        }
+
         {
             // index is in the warm phase executing the allocate action, the migrated allocate action is removed
             LifecycleExecutionState preMigrationExecutionState = LifecycleExecutionState.builder()
@@ -945,6 +1004,26 @@ public class MetadataMigrateToDataTiersRoutingServiceTests extends ESTestCase {
             }""".formatted(lifecycleName);
     }
 
+    private String getColdPhaseDefinitionWithTotalShardsPerNode() {
+        return """
+            {
+              "policy": "%s",
+              "phase_definition": {
+                "min_age": "0m",
+                "actions": {
+                  "allocate": {
+                    "total_shards_per_node": "1",
+                    "require": {
+                      "data": "cold"
+                    }
+                  }
+                }
+              },
+              "version": 1,
+              "modified_date_in_millis": 1578521007076
+            }""".formatted(lifecycleName);
+    }
+
     private String getColdPhaseDefinition() {
         return """
             {