Refactor ILMs `UpdateSettingsStep` to be more generic (#133329)

By making the step more generic, we can reuse it in other actions (such
as the upcoming improvements to the force-merge action).

The change is relatively straightforward as the name of the step is
already generic enough.

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

@@ -12,10 +12,12 @@ import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.ClusterStateObserver;
 import org.elasticsearch.cluster.ProjectState;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.LifecycleExecutionState;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.TimeValue;
 
-import java.util.Objects;
+import java.util.function.BiFunction;
+import java.util.function.Function;
 
 /**
  * Updates the settings for an index.
@@ -23,11 +25,32 @@ import java.util.Objects;
 public class UpdateSettingsStep extends AsyncActionStep {
     public static final String NAME = "update-settings";
 
-    private final Settings settings;
+    private static final BiFunction<String, LifecycleExecutionState, String> DEFAULT_TARGET_INDEX_NAME_SUPPLIER = (index, state) -> index;
 
+    private final BiFunction<String, LifecycleExecutionState, String> targetIndexNameSupplier;
+    private final Function<IndexMetadata, Settings> settingsSupplier;
+
+    /**
+     * Use this constructor when you want to update the index that ILM runs on with <i>constant</i> settings.
+     */
     public UpdateSettingsStep(StepKey key, StepKey nextStepKey, Client client, Settings settings) {
+        this(key, nextStepKey, client, DEFAULT_TARGET_INDEX_NAME_SUPPLIER, indexMetadata -> settings);
+    }
+
+    /**
+     * Use this constructor when you want to update an index other than the one ILM runs on, and/or when you have non-constant settings
+     * (i.e., settings that depend on the index metadata).
+     */
+    public UpdateSettingsStep(
+        StepKey key,
+        StepKey nextStepKey,
+        Client client,
+        BiFunction<String, LifecycleExecutionState, String> targetIndexNameSupplier,
+        Function<IndexMetadata, Settings> settingsSupplier
+    ) {
         super(key, nextStepKey, client);
-        this.settings = settings;
+        this.targetIndexNameSupplier = targetIndexNameSupplier;
+        this.settingsSupplier = settingsSupplier;
     }
 
     @Override
@@ -42,32 +65,16 @@ public class UpdateSettingsStep extends AsyncActionStep {
         ClusterStateObserver observer,
         ActionListener<Void> listener
     ) {
-        UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexMetadata.getIndex().getName()).masterNodeTimeout(
-            TimeValue.MAX_VALUE
-        ).settings(settings);
+        String indexName = targetIndexNameSupplier.apply(indexMetadata.getIndex().getName(), indexMetadata.getLifecycleExecutionState());
+        Settings settings = settingsSupplier.apply(indexMetadata);
+        UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexName).masterNodeTimeout(TimeValue.MAX_VALUE)
+            .settings(settings);
         getClient(currentState.projectId()).admin()
             .indices()
             .updateSettings(updateSettingsRequest, listener.delegateFailureAndWrap((l, r) -> l.onResponse(null)));
     }
 
-    public Settings getSettings() {
-        return settings;
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(super.hashCode(), settings);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (obj == null) {
-            return false;
-        }
-        if (getClass() != obj.getClass()) {
-            return false;
-        }
-        UpdateSettingsStep other = (UpdateSettingsStep) obj;
-        return super.equals(obj) && Objects.equals(settings, other.settings);
+    public Function<IndexMetadata, Settings> getSettingsSupplier() {
+        return settingsSupplier;
     }
 }

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

@@ -186,7 +186,7 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
             expectedSettings.put(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), action.getTotalShardsPerNode());
         }
 
-        assertThat(firstStep.getSettings(), equalTo(expectedSettings.build()));
+        assertThat(firstStep.getSettingsSupplier().apply(null), equalTo(expectedSettings.build()));
         AllocationRoutedStep secondStep = (AllocationRoutedStep) steps.get(1);
         assertEquals(expectedSecondStepKey, secondStep.getKey());
         assertEquals(nextStepKey, secondStep.getNextStepKey());
@@ -204,13 +204,15 @@ public class AllocateActionTests extends AbstractActionTestCase<AllocateAction>
         );
         List<Step> steps = action.toSteps(null, phase, nextStepKey);
         UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
-        assertEquals(totalShardsPerNode, firstStep.getSettings().getAsInt(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), null));
+        Settings actualSettings = firstStep.getSettingsSupplier().apply(null);
+        assertEquals(totalShardsPerNode, actualSettings.getAsInt(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), null));
 
         totalShardsPerNode = null;
         action = new AllocateAction(numberOfReplicas, totalShardsPerNode, null, null, null);
         steps = action.toSteps(null, phase, nextStepKey);
         firstStep = (UpdateSettingsStep) steps.get(0);
-        assertEquals(null, firstStep.getSettings().get(INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()));
+        actualSettings = firstStep.getSettingsSupplier().apply(null);
+        assertNull(actualSettings.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);

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

@@ -140,7 +140,7 @@ public class ForceMergeActionTests extends AbstractActionTestCase<ForceMergeActi
 
         UpdateSettingsStep updateCodecStep = (UpdateSettingsStep) steps.get(5);
         assertThat(
-            updateCodecStep.getSettings().get(EngineConfig.INDEX_CODEC_SETTING.getKey()),
+            updateCodecStep.getSettingsSupplier().apply(null).get(EngineConfig.INDEX_CODEC_SETTING.getKey()),
             equalTo(CodecService.BEST_COMPRESSION_CODEC)
         );
     }

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

@@ -92,17 +92,20 @@ public class MigrateActionTests extends AbstractActionTestCase<MigrateAction> {
         {
             List<Step> steps = MigrateAction.ENABLED.toSteps(null, HOT_PHASE, nextStepKey);
             UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
-            assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_HOT));
+            assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)), is(DATA_HOT));
         }
         {
             List<Step> steps = MigrateAction.ENABLED.toSteps(null, WARM_PHASE, nextStepKey);
             UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
-            assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_WARM + "," + DATA_HOT));
+            assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)), is(DATA_WARM + "," + DATA_HOT));
         }
         {
             List<Step> steps = MigrateAction.ENABLED.toSteps(null, COLD_PHASE, nextStepKey);
             UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1);
-            assertThat(DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettings()), is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT));
+            assertThat(
+                DataTier.TIER_PREFERENCE_SETTING.get(firstStep.getSettingsSupplier().apply(null)),
+                is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)
+            );
         }
     }
 

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

@@ -69,8 +69,8 @@ public class SetPriorityActionTests extends AbstractActionTestCase<SetPriorityAc
         UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
         assertThat(firstStep.getKey(), equalTo(expectedFirstStepKey));
         assertThat(firstStep.getNextStepKey(), equalTo(nextStepKey));
-        assertThat(firstStep.getSettings().size(), equalTo(1));
-        assertEquals(priority, (long) IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettings()));
+        assertThat(firstStep.getSettingsSupplier().apply(null).size(), equalTo(1));
+        assertEquals(priority, (long) IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettingsSupplier().apply(null)));
     }
 
     public void testNullPriorityStep() {
@@ -88,10 +88,10 @@ public class SetPriorityActionTests extends AbstractActionTestCase<SetPriorityAc
         UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0);
         assertThat(firstStep.getKey(), equalTo(expectedFirstStepKey));
         assertThat(firstStep.getNextStepKey(), equalTo(nextStepKey));
-        assertThat(firstStep.getSettings().size(), equalTo(1));
+        assertThat(firstStep.getSettingsSupplier().apply(null).size(), equalTo(1));
         assertThat(
-            IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettings()),
-            equalTo(IndexMetadata.INDEX_PRIORITY_SETTING.getDefault(firstStep.getSettings()))
+            IndexMetadata.INDEX_PRIORITY_SETTING.get(firstStep.getSettingsSupplier().apply(null)),
+            equalTo(IndexMetadata.INDEX_PRIORITY_SETTING.getDefault(firstStep.getSettingsSupplier().apply(null)))
         );
     }
 

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

@@ -32,16 +32,14 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
     public UpdateSettingsStep mutateInstance(UpdateSettingsStep instance) {
         StepKey key = instance.getKey();
         StepKey nextKey = instance.getNextStepKey();
-        Settings settings = instance.getSettings();
 
-        switch (between(0, 2)) {
+        switch (between(0, 1)) {
             case 0 -> key = new StepKey(key.phase(), key.action(), key.name() + randomAlphaOfLength(5));
             case 1 -> nextKey = new StepKey(nextKey.phase(), nextKey.action(), nextKey.name() + randomAlphaOfLength(5));
-            case 2 -> settings = Settings.builder().put(settings).put(randomAlphaOfLength(10), randomInt()).build();
             default -> throw new AssertionError("Illegal randomisation branch");
         }
 
-        return new UpdateSettingsStep(key, nextKey, client, settings);
+        return new UpdateSettingsStep(key, nextKey, client, instance.getSettingsSupplier().apply(null));
     }
 
     @Override
@@ -50,7 +48,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
             instance.getKey(),
             instance.getNextStepKey(),
             instance.getClientWithoutProject(),
-            instance.getSettings()
+            instance.getSettingsSupplier().apply(null)
         );
     }
 
@@ -71,7 +69,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
             UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
             @SuppressWarnings("unchecked")
             ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
-            assertThat(request.settings(), equalTo(step.getSettings()));
+            assertThat(request.settings(), equalTo(step.getSettingsSupplier().apply(indexMetadata)));
             assertThat(request.indices(), equalTo(new String[] { indexMetadata.getIndex().getName() }));
             listener.onResponse(AcknowledgedResponse.TRUE);
             return null;
@@ -96,7 +94,7 @@ public class UpdateSettingsStepTests extends AbstractStepTestCase<UpdateSettings
             UpdateSettingsRequest request = (UpdateSettingsRequest) invocation.getArguments()[0];
             @SuppressWarnings("unchecked")
             ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
-            assertThat(request.settings(), equalTo(step.getSettings()));
+            assertThat(request.settings(), equalTo(step.getSettingsSupplier().apply(indexMetadata)));
             assertThat(request.indices(), equalTo(new String[] { indexMetadata.getIndex().getName() }));
             listener.onFailure(exception);
             return null;