[8.13] [ILM] Delete step deletes data stream with only one index (#105772)

docs/changelog/105772.yaml

@@ -0,0 +1,5 @@
+pr: 105772
+summary: "[ILM] Delete step deletes data stream with only one index"
+area: ILM+SLM
+type: bug
+issues: []

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

@@ -41,7 +41,10 @@ public void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState cu
 
         if (dataStream != null) {
             assert dataStream.getWriteIndex() != null : dataStream.getName() + " has no write index";
-            if (dataStream.getIndices().size() == 1 && dataStream.getIndices().get(0).equals(indexMetadata.getIndex())) {
+
+            // using index name equality across this if/else branch as the UUID of the index might change via restoring a data stream
+            // with one index from snapshot
+            if (dataStream.getIndices().size() == 1 && dataStream.getWriteIndex().getName().equals(indexName)) {
                 // This is the last index in the data stream, the entire stream
                 // needs to be deleted, because we can't have an empty data stream
                 DeleteDataStreamAction.Request deleteReq = new DeleteDataStreamAction.Request(new String[] { dataStream.getName() });
@@ -62,7 +65,8 @@ public void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState cu
                     policyName
                 );
                 logger.debug(errorMessage);
-                throw new IllegalStateException(errorMessage);
+                listener.onFailure(new IllegalStateException(errorMessage));
+                return;
             }
         }
 

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

@@ -21,7 +21,10 @@
 
 import java.util.List;
 
+import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
 import static org.hamcrest.Matchers.is;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
 
 public class DeleteStepTests extends AbstractStepTestCase<DeleteStep> {
 
@@ -76,7 +79,7 @@ public void testDeleted() throws Exception {
             assertEquals(indexMetadata.getIndex().getName(), request.indices()[0]);
             listener.onResponse(null);
             return null;
-        }).when(indicesClient).delete(Mockito.any(), Mockito.any());
+        }).when(indicesClient).delete(any(), any());
 
         DeleteStep step = createRandomInstance();
         ClusterState clusterState = ClusterState.builder(emptyClusterState())
@@ -86,7 +89,7 @@ public void testDeleted() throws Exception {
 
         Mockito.verify(client, Mockito.only()).admin();
         Mockito.verify(adminClient, Mockito.only()).indices();
-        Mockito.verify(indicesClient, Mockito.only()).delete(Mockito.any(), Mockito.any());
+        Mockito.verify(indicesClient, Mockito.only()).delete(any(), any());
     }
 
     public void testExceptionThrown() {
@@ -102,7 +105,7 @@ public void testExceptionThrown() {
             assertEquals(indexMetadata.getIndex().getName(), request.indices()[0]);
             listener.onFailure(exception);
             return null;
-        }).when(indicesClient).delete(Mockito.any(), Mockito.any());
+        }).when(indicesClient).delete(any(), any());
 
         DeleteStep step = createRandomInstance();
         ClusterState clusterState = ClusterState.builder(emptyClusterState())
@@ -117,7 +120,13 @@ public void testExceptionThrown() {
         );
     }
 
-    public void testPerformActionThrowsExceptionIfIndexIsTheDataStreamWriteIndex() {
+    public void testPerformActionCallsFailureListenerIfIndexIsTheDataStreamWriteIndex() {
+        doThrow(
+            new IllegalStateException(
+                "the client must not be called in this test as we should fail in the step validation phase before we call the delete API"
+            )
+        ).when(indicesClient).delete(any(DeleteIndexRequest.class), anyActionListener());
+
         String policyName = "test-ilm-policy";
         String dataStreamName = randomAlphaOfLength(10);
 
@@ -149,31 +158,27 @@ public void testPerformActionThrowsExceptionIfIndexIsTheDataStreamWriteIndex() {
             .metadata(Metadata.builder().put(index1, false).put(sourceIndexMetadata, false).put(dataStream).build())
             .build();
 
-        IllegalStateException illegalStateException = expectThrows(
-            IllegalStateException.class,
-            () -> createRandomInstance().performDuringNoSnapshot(sourceIndexMetadata, clusterState, new ActionListener<>() {
-                @Override
-                public void onResponse(Void complete) {
-                    fail("unexpected listener callback");
-                }
-
-                @Override
-                public void onFailure(Exception e) {
-                    fail("unexpected listener callback");
-                }
-            })
-        );
-        assertThat(
-            illegalStateException.getMessage(),
-            is(
-                "index ["
-                    + sourceIndexMetadata.getIndex().getName()
-                    + "] is the write index for data stream ["
-                    + dataStreamName
-                    + "]. stopping execution of lifecycle [test-ilm-policy] as a data stream's write index cannot be deleted. "
-                    + "manually rolling over the index will resume the execution of the policy as the index will not be the "
-                    + "data stream's write index anymore"
-            )
-        );
+        createRandomInstance().performDuringNoSnapshot(sourceIndexMetadata, clusterState, new ActionListener<>() {
+            @Override
+            public void onResponse(Void complete) {
+                fail("unexpected listener callback");
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                assertThat(
+                    e.getMessage(),
+                    is(
+                        "index ["
+                            + sourceIndexMetadata.getIndex().getName()
+                            + "] is the write index for data stream ["
+                            + dataStreamName
+                            + "]. stopping execution of lifecycle [test-ilm-policy] as a data stream's write index cannot be deleted. "
+                            + "manually rolling over the index will resume the execution of the policy as the index will not be the "
+                            + "data stream's write index anymore"
+                    )
+                );
+            }
+        });
     }
 }

x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/TimeSeriesDataStreamsIT.java

@@ -11,12 +11,14 @@
 import org.elasticsearch.client.Response;
 import org.elasticsearch.cluster.metadata.DataStream;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.Template;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.index.engine.EngineConfig;
 import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.xcontent.XContentType;
 import org.elasticsearch.xpack.core.ilm.CheckNotDataStreamWriteIndexStep;
 import org.elasticsearch.xpack.core.ilm.DeleteAction;
+import org.elasticsearch.xpack.core.ilm.DeleteStep;
 import org.elasticsearch.xpack.core.ilm.ForceMergeAction;
 import org.elasticsearch.xpack.core.ilm.FreezeAction;
 import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
@@ -37,6 +39,7 @@
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createSnapshotRepo;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.explainIndex;
+import static org.elasticsearch.xpack.TimeSeriesRestDriver.getBackingIndices;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.getOnlyIndexSettings;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.getStepKeyForIndex;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.getTemplate;
@@ -45,6 +48,7 @@
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.waitAndGetShrinkIndexName;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
 
 public class TimeSeriesDataStreamsIT extends ESRestTestCase {
@@ -303,4 +307,46 @@ public void testDeleteOnlyIndexInDataStreamDeletesDataStream() throws Exception
         });
     }
 
+    @SuppressWarnings("unchecked")
+    public void testDataStreamWithMultipleIndicesAndWriteIndexInDeletePhase() throws Exception {
+        createComposableTemplate(client(), template, dataStream + "*", new Template(null, null, null, null));
+        indexDocument(client(), dataStream, true);
+
+        createNewSingletonPolicy(client(), policyName, "delete", DeleteAction.NO_SNAPSHOT_DELETE);
+        // let's update the index template so the new write index (after rollover) is managed by an ILM policy that sents it to the
+        // delete step - note that we'll have here a data stream with generation 000001 not managed and the write index 000002 in the
+        // delete phase (the write index in this case, being not the only backing index must NOT be deleted).
+        createComposableTemplate(client(), template, dataStream + "*", getTemplate(policyName));
+
+        client().performRequest(new Request("POST", dataStream + "/_rollover"));
+        indexDocument(client(), dataStream, true);
+
+        String secondGenerationIndex = getBackingIndices(client(), dataStream).get(1);
+        assertBusy(() -> {
+            Request explainRequest = new Request("GET", "/_data_stream/" + dataStream);
+            Response response = client().performRequest(explainRequest);
+            Map<String, Object> responseMap;
+            try (InputStream is = response.getEntity().getContent()) {
+                responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true);
+            }
+
+            List<Object> dataStreams = (List<Object>) responseMap.get("data_streams");
+            assertThat(dataStreams.size(), is(1));
+            Map<String, Object> dataStream = (Map<String, Object>) dataStreams.get(0);
+
+            List<Object> indices = (List<Object>) dataStream.get("indices");
+            // no index should be deleted
+            assertThat(indices.size(), is(2));
+
+            Map<String, Object> explainIndex = explainIndex(client(), secondGenerationIndex);
+            assertThat(explainIndex.get("failed_step"), is(DeleteStep.NAME));
+            assertThat((Integer) explainIndex.get("failed_step_retry_count"), is(greaterThan(1)));
+        });
+
+        // rolling the data stream again would see 000002 not be the write index anymore and should be deleted automatically
+        client().performRequest(new Request("POST", dataStream + "/_rollover"));
+
+        assertBusy(() -> assertThat(indexExists(secondGenerationIndex), is(false)));
+    }
+
 }