From 462c3d6adf14db81fd3342ada49a216ec586e075 Mon Sep 17 00:00:00 2001 From: Esduard <42816427+Esduard@users.noreply.github.com> Date: Tue, 13 Jul 2021 17:45:45 -0300 Subject: [PATCH] Update ILM message for out of order phases error (#75099) --- .../core/ilm/TimeseriesLifecycleType.java | 34 ++++++++++++-- .../ilm/TimeseriesLifecycleTypeTests.java | 47 ++++++++++++++++--- x-pack/plugin/ilm/qa/rest/build.gradle | 7 +++ .../rest-api-spec/test/ilm/10_basic.yml | 2 +- 4 files changed, 78 insertions(+), 12 deletions(-) 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 index 90d89195604aa..97c03d52bfad3 100644 --- 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 @@ -8,8 +8,8 @@ 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.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 static String validateMonotonicallyIncreasingPhaseTimings(Collection 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 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); } } } 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 index a82938541dda7..ad5ab8d75da86 100644 --- 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 @@ -754,8 +754,9 @@ public void testValidatingIncreasingAges() { 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 void testValidatingIncreasingAges() { 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 void testValidatingIncreasingAges() { 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.")); } } diff --git a/x-pack/plugin/ilm/qa/rest/build.gradle b/x-pack/plugin/ilm/qa/rest/build.gradle index 7c459e4808ed2..adbf72c977b71 100644 --- a/x-pack/plugin/ilm/qa/rest/build.gradle +++ b/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(',') +} diff --git a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml b/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml index 2383b858f2c0c..89f51590861a6 100644 --- a/x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/10_basic.yml +++ b/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: |