From 50fdbf0bd9d8edb8468cf20ec4d808cf061a88ed Mon Sep 17 00:00:00 2001
From: Henning Andersen <33268011+henningandersen@users.noreply.github.com>
Date: Tue, 10 Mar 2020 08:02:02 +0100
Subject: [PATCH] ILM Freeze step retry when not acknowledged (#53287)

A freeze operation can partially fail in multiple places, including the
close verification step. This left the index in an unfrozen but
partially closed state. Now throw an exception to retry the freeze step
instead.
---
 .../xpack/core/ilm/FreezeStep.java            |  8 +++-
 .../xpack/core/ilm/FreezeStepTests.java       | 37 ++++++++++++++++++-
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java
index 32f4fc94ce97e..837bfd8703c06 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/FreezeStep.java
@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.core.ilm;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterState;
@@ -26,6 +27,11 @@ public FreezeStep(StepKey key, StepKey nextStepKey, Client client) {
     public void performDuringNoSnapshot(IndexMetaData indexMetaData, ClusterState currentState, Listener listener) {
         getClient().admin().indices().execute(FreezeIndexAction.INSTANCE,
             new FreezeRequest(indexMetaData.getIndex().getName()),
-            ActionListener.wrap(response -> listener.onResponse(true), listener::onFailure));
+            ActionListener.wrap(response -> {
+                if (response.isAcknowledged() == false) {
+                    throw new ElasticsearchException("freeze index request failed to be acknowledged");
+                }
+                listener.onResponse(true);
+            }, listener::onFailure));
     }
 }
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java
index fcd22d369533a..38d266f5b23e5 100644
--- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java
+++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/FreezeStepTests.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.client.IndicesAdminClient;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.protocol.xpack.frozen.FreezeRequest;
+import org.elasticsearch.protocol.xpack.frozen.FreezeResponse;
 import org.elasticsearch.xpack.core.frozen.action.FreezeIndexAction;
 import org.elasticsearch.xpack.core.ilm.Step.StepKey;
 import org.junit.Before;
@@ -86,7 +87,7 @@ public void testFreeze() {
             assertNotNull(request);
             assertEquals(1, request.indices().length);
             assertEquals(indexMetaData.getIndex().getName(), request.indices()[0]);
-            listener.onResponse(null);
+            listener.onResponse(new FreezeResponse(true, true));
             return null;
         }).when(indicesClient).execute(Mockito.any(), Mockito.any(), Mockito.any());
 
@@ -151,4 +152,38 @@ public void onFailure(Exception e) {
 
         assertThat(exceptionThrown.get(), equalTo(true));
     }
+
+    public void testNotAcknowledged() {
+        IndexMetaData indexMetaData = IndexMetaData.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT))
+            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
+
+        AdminClient adminClient = Mockito.mock(AdminClient.class);
+        IndicesAdminClient indicesClient = Mockito.mock(IndicesAdminClient.class);
+
+        Mockito.when(client.admin()).thenReturn(adminClient);
+        Mockito.when(adminClient.indices()).thenReturn(indicesClient);
+        Mockito.doAnswer(invocation -> {
+            @SuppressWarnings("unchecked")
+            ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[2];
+            listener.onResponse(new FreezeResponse(false, false));
+            return null;
+        }).when(indicesClient).execute(Mockito.any(), Mockito.any(), Mockito.any());
+
+        SetOnce<Boolean> exceptionThrown = new SetOnce<>();
+        FreezeStep step = createRandomInstance();
+        step.performAction(indexMetaData, null, null, new AsyncActionStep.Listener() {
+            @Override
+            public void onResponse(boolean complete) {
+                throw new AssertionError("Unexpected method call");
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                assertEquals("freeze index request failed to be acknowledged", e.getMessage());
+                exceptionThrown.set(true);
+            }
+        });
+
+        assertThat(exceptionThrown.get(), equalTo(true));
+    }
 }