Add default rollover conditions to ILM explain API response (#104721)

The ILM `_explain` API displays the user-configured rollover action with the user-defined rollover conditions. However, the implicit conditions were not visible anywhere (except in the documentation and some debug-level logging).
elastic · Jan 26, 2024 · 0381954 · 0381954
1 parent a107107
commit 0381954
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 29 deletions.
diff --git a/docs/changelog/104721.yaml b/docs/changelog/104721.yaml
@@ -0,0 +1,6 @@
+pr: 104721
+summary: Add default rollover conditions to ILM explain API response
+area: ILM+SLM
+type: enhancement
+issues:
+ - 103395
diff --git a/docs/reference/ilm/apis/explain.asciidoc b/docs/reference/ilm/apis/explain.asciidoc
@@ -180,13 +180,15 @@ phase completes.
           "min_age": "0ms",
           "actions": {
             "rollover": {
-              "max_age": "30s"
+              "max_age": "30s",
+              "max_primary_shard_docs": 200000000, <2>
+              "min_docs": 1
             }
           }
         },
-        "version": 3, <2>
-        "modified_date": "2018-10-15T13:21:41.576Z", <3>
-        "modified_date_in_millis": 1539609701576 <4>
+        "version": 3, <3>
+        "modified_date": "2018-10-15T13:21:41.576Z", <4>
+        "modified_date_in_millis": 1539609701576 <5>
       }
     }
   }
@@ -196,9 +198,10 @@ phase completes.
 
 <1> The JSON phase definition loaded from the specified policy when the index
 entered this phase
-<2> The version of the policy that was loaded
-<3> The date the loaded policy was last modified
-<4> The epoch time when the loaded policy was last modified
+<2> The rollover action includes the default `max_primary_shard_docs` and `min_docs` conditions. See <<ilm-rollover-options,ILM Rollover Options>> for more information.
+<3> The version of the policy that was loaded
+<4> The date the loaded policy was last modified
+<5> The epoch time when the loaded policy was last modified
 
 If {ilm-init} is waiting for a step to complete, the response includes status
 information for the step that's being performed on the index.

diff --git a/.../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForRolloverReadyStep.java b/.../plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/WaitForRolloverReadyStep.java
@@ -10,8 +10,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
-import org.elasticsearch.action.admin.indices.rollover.Condition;
-import org.elasticsearch.action.admin.indices.rollover.MaxPrimaryShardDocsCondition;
 import org.elasticsearch.action.admin.indices.rollover.RolloverConditions;
 import org.elasticsearch.action.admin.indices.rollover.RolloverRequest;
 import org.elasticsearch.client.internal.Client;
@@ -25,9 +23,7 @@
 import org.elasticsearch.index.Index;
 import org.elasticsearch.xpack.core.ilm.step.info.EmptyInfo;
 
-import java.util.HashMap;
 import java.util.Locale;
-import java.util.Map;
 import java.util.Objects;
 
 /**
@@ -233,20 +229,28 @@ public void evaluateCondition(Metadata metadata, Index index, Listener listener,
     RolloverRequest createRolloverRequest(String rolloverTarget, TimeValue masterTimeout, boolean rolloverOnlyIfHasDocuments) {
         RolloverRequest rolloverRequest = new RolloverRequest(rolloverTarget, null).masterNodeTimeout(masterTimeout);
         rolloverRequest.dryRun(true);
+        rolloverRequest.setConditions(applyDefaultConditions(conditions, rolloverOnlyIfHasDocuments));
+        return rolloverRequest;
+    }
+
+    /**
+     * Apply default conditions to the set of user-defined conditions.
+     *
+     * @param conditions the existing conditions
+     * @param rolloverOnlyIfHasDocuments whether to inject a min_docs 1 condition if there is not already a min_docs
+     *                                   (or min_primary_shard_docs) condition
+     * @return the rollover conditions with the default conditions applied.
+     */
+    public static RolloverConditions applyDefaultConditions(RolloverConditions conditions, boolean rolloverOnlyIfHasDocuments) {
+        var builder = RolloverConditions.newBuilder(conditions);
         if (rolloverOnlyIfHasDocuments && (conditions.getMinDocs() == null && conditions.getMinPrimaryShardDocs() == null)) {
-            rolloverRequest.setConditions(RolloverConditions.newBuilder(conditions).addMinIndexDocsCondition(1L).build());
-        } else {
-            rolloverRequest.setConditions(conditions);
+            builder.addMinIndexDocsCondition(1L);
         }
-        long currentMaxPrimaryShardDocs = rolloverRequest.getConditions().getMaxPrimaryShardDocs() != null
-            ? rolloverRequest.getConditions().getMaxPrimaryShardDocs()
+        long currentMaxPrimaryShardDocs = conditions.getMaxPrimaryShardDocs() != null
+            ? conditions.getMaxPrimaryShardDocs()
             : Long.MAX_VALUE;
-        if (currentMaxPrimaryShardDocs > MAX_PRIMARY_SHARD_DOCS) {
-            Map<String, Condition<?>> conditions = new HashMap<>(rolloverRequest.getConditions().getConditions());
-            conditions.put(MaxPrimaryShardDocsCondition.NAME, new MaxPrimaryShardDocsCondition(MAX_PRIMARY_SHARD_DOCS));
-            rolloverRequest.setConditions(new RolloverConditions(conditions));
-        }
-        return rolloverRequest;
+        builder.addMaxPrimaryShardDocsCondition(Math.min(currentMaxPrimaryShardDocs, MAX_PRIMARY_SHARD_DOCS));
+        return builder.build();
     }
 
     public RolloverConditions getConditions() {

diff --git a/...ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java b/...ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/TimeSeriesRestDriver.java
@@ -12,6 +12,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.client.Request;
+import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.RestClient;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -94,9 +95,16 @@ public static Map<String, Object> explainIndex(RestClient client, String indexNa
 
     public static Map<String, Map<String, Object>> explain(RestClient client, String indexPattern, boolean onlyErrors, boolean onlyManaged)
         throws IOException {
+        RequestOptions consumeWarningsOptions = RequestOptions.DEFAULT.toBuilder()
+            .setWarningsHandler(warnings -> warnings.isEmpty() == false && List.of("""
+                [indices.lifecycle.rollover.only_if_has_documents] setting was deprecated in Elasticsearch \
+                and will be removed in a future release.""").equals(warnings) == false)
+            .build();
+
         Request explainRequest = new Request("GET", indexPattern + "/_ilm/explain");
         explainRequest.addParameter("only_errors", Boolean.toString(onlyErrors));
         explainRequest.addParameter("only_managed", Boolean.toString(onlyManaged));
+        explainRequest.setOptions(consumeWarningsOptions);
         Response response = client.performRequest(explainRequest);
         Map<String, Object> responseMap;
         try (InputStream is = response.getEntity().getContent()) {

diff --git a/...ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportExplainLifecycleAction.java b/...ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportExplainLifecycleAction.java
@@ -33,7 +33,9 @@
 import org.elasticsearch.xpack.core.ilm.ExplainLifecycleRequest;
 import org.elasticsearch.xpack.core.ilm.ExplainLifecycleResponse;
 import org.elasticsearch.xpack.core.ilm.IndexLifecycleExplainResponse;
+import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.PhaseExecutionInfo;
+import org.elasticsearch.xpack.core.ilm.RolloverAction;
 import org.elasticsearch.xpack.core.ilm.action.ExplainLifecycleAction;
 import org.elasticsearch.xpack.ilm.IndexLifecycleService;
 
@@ -42,6 +44,7 @@
 import java.util.TreeMap;
 
 import static org.elasticsearch.index.IndexSettings.LIFECYCLE_ORIGINATION_DATE;
+import static org.elasticsearch.xpack.core.ilm.WaitForRolloverReadyStep.applyDefaultConditions;
 
 public class TransportExplainLifecycleAction extends TransportClusterInfoAction<ExplainLifecycleRequest, ExplainLifecycleResponse> {
 
@@ -80,6 +83,9 @@ protected void doMasterOperation(
         ClusterState state,
         ActionListener<ExplainLifecycleResponse> listener
     ) {
+        boolean rolloverOnlyIfHasDocuments = LifecycleSettings.LIFECYCLE_ROLLOVER_ONLY_IF_HAS_DOCUMENTS_SETTING.get(
+            state.metadata().settings()
+        );
         Map<String, IndexLifecycleExplainResponse> indexResponses = new TreeMap<>();
         for (String index : concreteIndices) {
             final IndexLifecycleExplainResponse indexResponse;
@@ -90,7 +96,8 @@ protected void doMasterOperation(
                     request.onlyErrors(),
                     request.onlyManaged(),
                     indexLifecycleService,
-                    xContentRegistry
+                    xContentRegistry,
+                    rolloverOnlyIfHasDocuments
                 );
             } catch (IOException e) {
                 listener.onFailure(new ElasticsearchParseException("failed to parse phase definition for index [" + index + "]", e));
@@ -111,7 +118,8 @@ static IndexLifecycleExplainResponse getIndexLifecycleExplainResponse(
         boolean onlyErrors,
         boolean onlyManaged,
         IndexLifecycleService indexLifecycleService,
-        NamedXContentRegistry xContentRegistry
+        NamedXContentRegistry xContentRegistry,
+        boolean rolloverOnlyIfHasDocuments
     ) throws IOException {
         IndexMetadata indexMetadata = metadata.index(indexName);
         Settings idxSettings = indexMetadata.getSettings();
@@ -136,6 +144,16 @@ static IndexLifecycleExplainResponse getIndexLifecycleExplainResponse(
                 )
             ) {
                 phaseExecutionInfo = PhaseExecutionInfo.parse(parser, currentPhase);
+
+                // Try to add default rollover conditions to the response.
+                var phase = phaseExecutionInfo.getPhase();
+                if (phase != null) {
+                    var rolloverAction = (RolloverAction) phase.getActions().get(RolloverAction.NAME);
+                    if (rolloverAction != null) {
+                        var conditions = applyDefaultConditions(rolloverAction.getConditions(), rolloverOnlyIfHasDocuments);
+                        phase.getActions().put(RolloverAction.NAME, new RolloverAction(conditions));
+                    }
+                }
             }
         }
 

diff --git a/...rc/test/java/org/elasticsearch/xpack/ilm/action/TransportExplainLifecycleActionTests.java b/...rc/test/java/org/elasticsearch/xpack/ilm/action/TransportExplainLifecycleActionTests.java
@@ -85,7 +85,8 @@ public void testGetIndexLifecycleExplainResponse() throws IOException {
                 true,
                 true,
                 indexLifecycleService,
-                REGISTRY
+                REGISTRY,
+                randomBoolean()
             );
             assertThat(onlyErrorsResponse, notNullValue());
             assertThat(onlyErrorsResponse.getIndex(), is(indexInErrorStep));
@@ -118,7 +119,8 @@ public void testGetIndexLifecycleExplainResponse() throws IOException {
                 true,
                 true,
                 indexLifecycleService,
-                REGISTRY
+                REGISTRY,
+                randomBoolean()
             );
             assertThat(onlyErrorsResponse, nullValue());
 
@@ -128,7 +130,8 @@ public void testGetIndexLifecycleExplainResponse() throws IOException {
                 false,
                 true,
                 indexLifecycleService,
-                REGISTRY
+                REGISTRY,
+                randomBoolean()
             );
             assertThat(allManagedResponse, notNullValue());
             assertThat(allManagedResponse.getIndex(), is(indexInCheckRolloverStep));
@@ -154,7 +157,8 @@ public void testGetIndexLifecycleExplainResponse() throws IOException {
                 true,
                 true,
                 indexLifecycleService,
-                REGISTRY
+                REGISTRY,
+                randomBoolean()
             );
             assertThat(onlyErrorsResponse, notNullValue());
             assertThat(onlyErrorsResponse.getPolicyName(), is("random-policy"));
@@ -179,9 +183,43 @@ public void testGetIndexLifecycleExplainResponse() throws IOException {
                 false,
                 true,
                 indexLifecycleService,
-                REGISTRY
+                REGISTRY,
+                randomBoolean()
             );
             assertThat(onlyManaged, nullValue());
         }
+
+        {
+            // validate addition of default condition with `rolloverOnlyIfHasDocuments` true
+            IndexLifecycleService indexLifecycleService = mock(IndexLifecycleService.class);
+            when(indexLifecycleService.policyExists("my-policy")).thenReturn(true);
+
+            LifecycleExecutionState.Builder checkRolloverReadyStepState = LifecycleExecutionState.builder()
+                .setPhase("hot")
+                .setAction("rollover")
+                .setStep(WaitForRolloverReadyStep.NAME)
+                .setPhaseDefinition(PHASE_DEFINITION);
+            String indexInCheckRolloverStep = "index_in_check_rollover";
+            IndexMetadata indexMetadata = IndexMetadata.builder(indexInCheckRolloverStep)
+                .settings(settings(IndexVersion.current()).put(LifecycleSettings.LIFECYCLE_NAME, "my-policy"))
+                .numberOfShards(randomIntBetween(1, 5))
+                .numberOfReplicas(randomIntBetween(0, 5))
+                .putCustom(ILM_CUSTOM_METADATA_KEY, checkRolloverReadyStepState.build().asMap())
+                .build();
+            Metadata metadata = Metadata.builder().put(indexMetadata, true).build();
+
+            IndexLifecycleExplainResponse response = getIndexLifecycleExplainResponse(
+                indexInCheckRolloverStep,
+                metadata,
+                false,
+                true,
+                indexLifecycleService,
+                REGISTRY,
+                true
+            );
+            var rolloverAction = ((RolloverAction) response.getPhaseExecutionInfo().getPhase().getActions().get(RolloverAction.NAME));
+            assertThat(rolloverAction, notNullValue());
+            assertThat(rolloverAction.getConditions().getMinDocs(), is(1L));
+        }
     }
 }