Refactor CleanupShrinkIndexStep to CleanupGeneratedIndexStep

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CleanupShrinkIndexStep.java → x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CleanupGeneratedIndexStep.java

@@ -19,15 +19,28 @@
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.IndexNotFoundException;
 
+import java.util.function.BiFunction;
+
 /**
- * Deletes the index identified by the shrink index name stored in the lifecycle state of the managed index (if any was generated)
+ * Deletes the index identified by the index name supplier.
  */
-public class CleanupShrinkIndexStep extends AsyncRetryDuringSnapshotActionStep {
-    public static final String NAME = "cleanup-shrink-index";
-    private static final Logger logger = LogManager.getLogger(CleanupShrinkIndexStep.class);
+public class CleanupGeneratedIndexStep extends AsyncRetryDuringSnapshotActionStep {
+
+    public static final String OLD_NAME = "cleanup-shrink-index";
+    public static final String NAME = "cleanup-prefixed-index";
+
+    private static final Logger logger = LogManager.getLogger(CleanupGeneratedIndexStep.class);
 
-    public CleanupShrinkIndexStep(StepKey key, StepKey nextStepKey, Client client) {
+    private final BiFunction<String, LifecycleExecutionState, String> targetIndexNameSupplier;
+
+    public CleanupGeneratedIndexStep(
+        StepKey key,
+        StepKey nextStepKey,
+        Client client,
+        BiFunction<String, LifecycleExecutionState, String> targetIndexNameSupplier
+    ) {
         super(key, nextStepKey, client);
+        this.targetIndexNameSupplier = targetIndexNameSupplier;
     }
 
     @Override
@@ -37,40 +50,41 @@ public boolean isRetryable() {
 
     @Override
     void performDuringNoSnapshot(IndexMetadata indexMetadata, ProjectMetadata currentProject, ActionListener<Void> listener) {
-        final String shrunkenIndexSource = IndexMetadata.INDEX_RESIZE_SOURCE_NAME.get(indexMetadata.getSettings());
-        if (Strings.isNullOrEmpty(shrunkenIndexSource) == false) {
-            // the current managed index is a shrunk index
-            if (currentProject.index(shrunkenIndexSource) == null) {
-                // if the source index does not exist, we'll skip deleting the
-                // (managed) shrunk index as that will cause data loss
+        // If the index was generated by a resize operation, and the source index does not exist anymore, we skip the deletion to avoid
+        // data loss.
+        final String generatedIndexSource = IndexMetadata.INDEX_RESIZE_SOURCE_NAME.get(indexMetadata.getSettings());
+        if (Strings.isNullOrEmpty(generatedIndexSource) == false) {
+            if (currentProject.index(generatedIndexSource) == null) {
                 String policyName = indexMetadata.getLifecyclePolicyName();
                 logger.warn(
                     "managed index [{}] as part of policy [{}] is a shrunk index and the source index [{}] does not exist "
                         + "anymore. will skip the [{}] step",
                     indexMetadata.getIndex().getName(),
                     policyName,
-                    shrunkenIndexSource,
+                    generatedIndexSource,
                     NAME
                 );
                 listener.onResponse(null);
                 return;
             }
         }
 
-        LifecycleExecutionState lifecycleState = indexMetadata.getLifecycleExecutionState();
-        final String shrinkIndexName = lifecycleState.shrinkIndexName();
-        // if the shrink index was not generated there is nothing to delete so we move on
-        if (Strings.hasText(shrinkIndexName) == false) {
+        final String targetIndexName = targetIndexNameSupplier.apply(
+            indexMetadata.getIndex().getName(),
+            indexMetadata.getLifecycleExecutionState()
+        );
+        // If no index name was generated, there is nothing for us to delete, so we can move on
+        if (Strings.hasText(targetIndexName) == false) {
             listener.onResponse(null);
             return;
         }
         getClient(currentProject.id()).admin()
             .indices()
-            .delete(new DeleteIndexRequest(shrinkIndexName).masterNodeTimeout(TimeValue.MAX_VALUE), new ActionListener<>() {
+            .delete(new DeleteIndexRequest(targetIndexName).masterNodeTimeout(TimeValue.MAX_VALUE), new ActionListener<>() {
                 @Override
                 public void onResponse(AcknowledgedResponse acknowledgedResponse) {
                     // even if not all nodes acked the delete request yet we can consider this operation as successful as
-                    // we'll generate a new index name and attempt to shrink into the newly generated name
+                    // we'll generate a new index name and attempt to create a new index with the newly generated name
                     listener.onResponse(null);
                 }
 
@@ -86,4 +100,7 @@ public void onFailure(Exception e) {
             });
     }
 
+    public BiFunction<String, LifecycleExecutionState, String> getTargetIndexNameSupplier() {
+        return targetIndexNameSupplier;
+    }
 }

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

@@ -168,7 +168,8 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
         StepKey waitTimeSeriesEndTimePassesKey = new StepKey(phase, NAME, WaitUntilTimeSeriesEndTimePassesStep.NAME);
         StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyAction.NAME);
         StepKey checkTargetShardsCountKey = new StepKey(phase, NAME, CheckTargetShardsCountStep.NAME);
-        StepKey cleanupShrinkIndexKey = new StepKey(phase, NAME, CleanupShrinkIndexStep.NAME);
+        StepKey oldCleanupShrinkIndexKey = new StepKey(phase, NAME, CleanupGeneratedIndexStep.OLD_NAME);
+        StepKey cleanupShrinkIndexKey = new StepKey(phase, NAME, CleanupGeneratedIndexStep.NAME);
         StepKey generateShrinkIndexNameKey = new StepKey(phase, NAME, GenerateUniqueIndexNameStep.NAME);
         StepKey setSingleNodeKey = new StepKey(phase, NAME, SetSingleNodeAllocateStep.NAME);
         StepKey allocationRoutedKey = new StepKey(phase, NAME, CheckShrinkReadyStep.NAME);
@@ -240,13 +241,16 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
             cleanupShrinkIndexKey,
             numberOfShards
         );
+        // The cleanup step was renamed, so we need to forward indices in the old step to the new one, i.e. during an upgrade
+        NoopStep oldCleanupShrinkIndexStep = new NoopStep(oldCleanupShrinkIndexKey, cleanupShrinkIndexKey);
         // We generate a unique shrink index name but we also retry if the allocation of the shrunk index is not possible, so we want to
         // delete the "previously generated" shrink index (this is a no-op if it's the first run of the action and we haven't generated a
         // shrink index name)
-        CleanupShrinkIndexStep cleanupShrinkIndexStep = new CleanupShrinkIndexStep(
+        CleanupGeneratedIndexStep cleanupShrinkIndexStep = new CleanupGeneratedIndexStep(
             cleanupShrinkIndexKey,
             generateShrinkIndexNameKey,
-            client
+            client,
+            ShrinkIndexNameSupplier::getShrinkIndexName
         );
         // generate a unique shrink index name and store it in the ILM execution state
         GenerateUniqueIndexNameStep generateUniqueIndexNameStep = new GenerateUniqueIndexNameStep(
@@ -313,6 +317,7 @@ public List<Step> toSteps(Client client, String phase, Step.StepKey nextStepKey)
             waitUntilTimeSeriesEndTimeStep,
             readOnlyStep,
             checkTargetShardsCountStep,
+            oldCleanupShrinkIndexStep,
             cleanupShrinkIndexStep,
             generateUniqueIndexNameStep,
             setSingleNodeStep,

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CleanupShrinkIndexStepTests.java → x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CleanupGeneratedIndexStepTests.java

@@ -28,30 +28,35 @@
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.is;
 
-public class CleanupShrinkIndexStepTests extends AbstractStepTestCase<CleanupShrinkIndexStep> {
+public class CleanupGeneratedIndexStepTests extends AbstractStepTestCase<CleanupGeneratedIndexStep> {
 
     @Override
-    public CleanupShrinkIndexStep createRandomInstance() {
+    public CleanupGeneratedIndexStep createRandomInstance() {
         StepKey stepKey = randomStepKey();
         StepKey nextStepKey = randomStepKey();
-        return new CleanupShrinkIndexStep(stepKey, nextStepKey, client);
+        return new CleanupGeneratedIndexStep(stepKey, nextStepKey, client, (index, state) -> randomAlphaOfLength(5) + index);
     }
 
     @Override
-    protected CleanupShrinkIndexStep copyInstance(CleanupShrinkIndexStep instance) {
-        return new CleanupShrinkIndexStep(instance.getKey(), instance.getNextStepKey(), instance.getClientWithoutProject());
+    protected CleanupGeneratedIndexStep copyInstance(CleanupGeneratedIndexStep instance) {
+        return new CleanupGeneratedIndexStep(
+            instance.getKey(),
+            instance.getNextStepKey(),
+            instance.getClientWithoutProject(),
+            instance.getTargetIndexNameSupplier()
+        );
     }
 
     @Override
-    public CleanupShrinkIndexStep mutateInstance(CleanupShrinkIndexStep instance) {
+    public CleanupGeneratedIndexStep mutateInstance(CleanupGeneratedIndexStep instance) {
         StepKey key = instance.getKey();
         StepKey nextKey = instance.getNextStepKey();
         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));
             default -> throw new AssertionError("Illegal randomisation branch");
         }
-        return new CleanupShrinkIndexStep(key, nextKey, instance.getClientWithoutProject());
+        return new CleanupGeneratedIndexStep(key, nextKey, instance.getClientWithoutProject(), instance.getTargetIndexNameSupplier());
     }
 
     public void testPerformActionDoesntFailIfShrinkingIndexNameIsMissing() {
@@ -67,7 +72,7 @@ public void testPerformActionDoesntFailIfShrinkingIndexNameIsMissing() {
 
         ProjectState state = projectStateFromProject(ProjectMetadata.builder(randomProjectIdOrDefault()).put(indexMetadata, true));
 
-        CleanupShrinkIndexStep cleanupShrinkIndexStep = createRandomInstance();
+        CleanupGeneratedIndexStep cleanupShrinkIndexStep = createRandomInstance();
         cleanupShrinkIndexStep.performAction(indexMetadata, state, null, new ActionListener<>() {
             @Override
             public void onResponse(Void unused) {}
@@ -100,7 +105,12 @@ public void testPerformAction() {
 
         try (var threadPool = createThreadPool()) {
             final var client = getDeleteIndexRequestAssertingClient(threadPool, shrinkIndexName);
-            CleanupShrinkIndexStep step = new CleanupShrinkIndexStep(randomStepKey(), randomStepKey(), client);
+            CleanupGeneratedIndexStep step = new CleanupGeneratedIndexStep(
+                randomStepKey(),
+                randomStepKey(),
+                client,
+                ShrinkIndexNameSupplier::getShrinkIndexName
+            );
             step.performAction(indexMetadata, state, null, ActionListener.noop());
         }
     }
@@ -125,7 +135,7 @@ public void testDeleteSkippedIfManagedIndexIsShrunkAndSourceDoesntExist() {
 
         try (var threadPool = createThreadPool()) {
             final var client = getFailingIfCalledClient(threadPool);
-            CleanupShrinkIndexStep step = new CleanupShrinkIndexStep(randomStepKey(), randomStepKey(), client);
+            CleanupGeneratedIndexStep step = new CleanupGeneratedIndexStep(randomStepKey(), randomStepKey(), client, (index, s) -> index);
             step.performAction(shrunkIndexMetadata, state, null, ActionListener.noop());
         }
     }