Handle duplicate errors (only report the first one)

dakrone · dakrone · commit 77dea577b02c · 2021-03-08T11:03:27.000-07:00
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java
@@ -370,6 +370,7 @@ static void validateAllSearchableSnapshotActionsUseSameRepository(Collection<Pha
      */
     public static String validateMonotonicallyIncreasingPhaseTimings(Collection<Phase> phases) {
         List<String> errors = new ArrayList<>();
+        Set<String> invalidPhases = new HashSet<>();
 
         // Loop through all phases in order, for each phase with a min_age
         // configured, look at all the future phases to see if their ages are
@@ -385,6 +386,12 @@ public static String validateMonotonicallyIncreasingPhaseTimings(Collection<Phas
 
             if (maybePhase.isPresent()) {
                 Phase phase = maybePhase.get();
+                // We only consider a phase bad once, otherwise we can duplicate
+                // errors, so we keep track of the invalid phases we've seen and
+                // ignore them if they come around again.
+                if (invalidPhases.contains(phase.getName())) {
+                    continue;
+                }
                 TimeValue phaseMinAge = phase.getMinimumAge();
                 Set<String> followingPhases = new HashSet<>(ORDERED_VALID_PHASES.subList(i + 1, ORDERED_VALID_PHASES.size()));
                 Set<Phase> phasesWithBadAges = phases.stream()
@@ -393,6 +400,7 @@ public static String validateMonotonicallyIncreasingPhaseTimings(Collection<Phas
                     .filter(p -> p.getMinimumAge().compareTo(phaseMinAge) < 0)
                     .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: " +
diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java
@@ -771,6 +771,21 @@ public void testValidatingIncreasingAges() {
                 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}"));
         }
+
+        {
+            Phase hotPhase = new Phase(HOT_PHASE, TimeValue.timeValueDays(1), Collections.emptyMap());
+            Phase warmPhase = new Phase(WARM_PHASE, TimeValue.timeValueDays(3), 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("phases [frozen,delete] configure a [min_age] value less than " +
+                    "the [min_age] of [3d] for the [warm] phase, configuration: {frozen=2d, delete=1d}"));
+        }
     }
 
     private void assertNextActionName(String phaseName, String currentAction, String expectedNextAction, String... availableActionNames) {
