Update ILM message for out of order phases error (#75099)

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

@@ -8,8 +8,8 @@ package org.elasticsearch.xpack.core.ilm;
 
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamOutput;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.util.set.Sets;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.rollup.RollupV2;
 
 import java.io.IOException;
@@ -19,6 +19,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -399,10 +400,33 @@ public class TimeseriesLifecycleType implements LifecycleType {
                     .collect(Collectors.toSet());
                 if (phasesWithBadAges.size() > 0) {
                     phasesWithBadAges.forEach(p -> invalidPhases.add(p.getName()));
-                    errors.add("phases [" + phasesWithBadAges.stream().map(Phase::getName).collect(Collectors.joining(",")) +
-                        "] configure a [min_age] value less than the [min_age] of [" + phase.getMinimumAge() +
-                        "] for the [" + phaseName + "] phase, configuration: " +
-                        phasesWithBadAges.stream().collect(Collectors.toMap(Phase::getName, Phase::getMinimumAge)));
+
+                    //build an error message string
+                    Iterator<Phase> it = phasesWithBadAges.iterator();
+                    Phase badPhase = it.next();
+
+                    String error = "Your policy is configured to run the "
+                        + badPhase.getName() + " phase (min_age: " + badPhase.getMinimumAge() + ")";
+
+                    if (phasesWithBadAges.size() > 1) {
+                        while (it.hasNext()) {
+                            badPhase = it.next();
+                            error = error + ", the " + badPhase.getName() + " phase (min_age: "
+                                + badPhase.getMinimumAge()+ ")";
+                        }
+                            //if multiple phases are cited
+                            //replace last occurrence of "," with " and"
+                            StringBuilder builder = new StringBuilder();
+                            int last_comma_index = error.lastIndexOf(",");
+                            builder.append(error, 0, last_comma_index);
+                            builder.append(" and");
+                            builder.append(error.substring(last_comma_index + 1));
+                            error = builder.toString();
+                    }
+                    error = error + " before the " + phaseName + " phase (min_age: " + phase.getMinimumAge() +
+                        "). You should change the phase timing so that the phases will execute in the order of hot, warm, then cold.";
+
+                    errors.add(error);
                 }
             }
         }

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

@@ -754,8 +754,9 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
                 validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));
 
             assertThat(err,
-                containsString("phases [cold] configure a [min_age] value less than the" +
-                    " [min_age] of [1d] for the [hot] phase, configuration: {cold=12h}"));
+                containsString("Your policy is configured to run the cold phase "+
+                    "(min_age: 12h) before the hot phase (min_age: 1d). You should change "+
+                    "the phase timing so that the phases will execute in the order of hot, warm, then cold."));
         }
 
         {
@@ -769,8 +770,9 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
                 validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));
 
             assertThat(err,
-                containsString("phases [frozen,delete] configure a [min_age] value less " +
-                    "than the [min_age] of [3d] for the [warm] phase, configuration: {frozen=1d, delete=2d}"));
+                containsString("Your policy is configured to run the frozen phase "+
+                    "(min_age: 1d) and the delete phase (min_age: 2d) before the warm phase (min_age: 3d)."+
+                    " You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
         }
 
         {
@@ -784,8 +786,41 @@ public class TimeseriesLifecycleTypeTests extends ESTestCase {
                 validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));
 
             assertThat(err,
-                containsString("phases [frozen,delete] configure a [min_age] value less than " +
-                    "the [min_age] of [3d] for the [warm] phase, configuration: {frozen=2d, delete=1d}"));
+                containsString("Your policy is configured to run the frozen phase "+
+                    "(min_age: 2d) and the delete phase (min_age: 1d) before the warm phase (min_age: 3d)."+
+                    " You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
+        }
+
+        {
+            Phase hotPhase = new Phase(HOT_PHASE, TimeValue.timeValueDays(3), Collections.emptyMap());
+            Phase warmPhase = new Phase(WARM_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
+            Phase coldPhase = new Phase(COLD_PHASE, null, Collections.emptyMap());
+            Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
+            Phase deletePhase = new Phase(DELETE_PHASE, TimeValue.timeValueDays(1), Collections.emptyMap());
+
+            String err =
+                validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));
+
+            assertThat(err,
+                containsString("Your policy is configured to run the frozen phase "+
+                    "(min_age: 2d), the delete phase (min_age: 1d) and the warm phase (min_age: 2d) before the hot phase (min_age: 3d)."+
+                    " You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
+        }
+
+        {
+            Phase hotPhase = new Phase(HOT_PHASE, TimeValue.timeValueDays(3), Collections.emptyMap());
+            Phase warmPhase = new Phase(WARM_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
+            Phase coldPhase = new Phase(COLD_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
+            Phase frozenPhase = new Phase(FROZEN_PHASE, TimeValue.timeValueDays(2), Collections.emptyMap());
+            Phase deletePhase = new Phase(DELETE_PHASE, TimeValue.timeValueDays(1), Collections.emptyMap());
+
+            String err =
+                validateMonotonicallyIncreasingPhaseTimings(Arrays.asList(hotPhase, warmPhase, coldPhase, frozenPhase, deletePhase));
+
+            assertThat(err,
+                containsString("Your policy is configured to run the cold phase (min_age: 2d), the frozen phase "+
+                    "(min_age: 2d), the delete phase (min_age: 1d) and the warm phase (min_age: 2d) before the hot phase (min_age: 3d)."+
+                    " You should change the phase timing so that the phases will execute in the order of hot, warm, then cold."));
         }
     }
 

x-pack/plugin/ilm/qa/rest/build.gradle

@@ -26,3 +26,10 @@ testClusters.all {
   setting 'xpack.license.self_generated.type', 'trial'
   user clusterCredentials
 }
+
+tasks.named("yamlRestCompatTest").configure {
+  systemProperty 'tests.rest.blacklist', [
+    //TODO: remove once #75099 is back ported to 7.x
+    'ilm/10_basic/Test increasing phase timings validated'
+  ].join(',')
+}

x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml

@@ -281,7 +281,7 @@ setup:
 "Test increasing phase timings validated":
 
   - do:
-      catch: /phases \[delete\] configure a \[min_age\] value less than the \[min_age\] of \[10s\] for the \[warm\] phase/
+      catch: /Your policy is configured to run the delete phase \(min_age\:\ 5s\) before the warm phase \(min_age\:\ 10s\). You should change the phase timing so that the phases will execute in the order of hot, warm, then cold./
       ilm.put_lifecycle:
         policy: "bad_policy"
         body: |