From cf87e9ea7b8c375edd9e4e0f8b3a87b24abb53df Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 13 Oct 2021 06:58:40 -0500 Subject: [PATCH 1/3] Tweak error message when pipeline cannot be deleted (#79016) --- .../elasticsearch/ingest/IngestService.java | 38 ++++++--- .../ingest/IngestServiceTests.java | 84 +++++++++++-------- 2 files changed, 77 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 542ecf88234b5..827237ec51c02 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -12,6 +12,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.logging.log4j.util.Strings; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ResourceNotFoundException; @@ -62,6 +63,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -314,18 +316,30 @@ static void validateNotInUse(String pipeline, ImmutableOpenMap 0 || finalPipelineIndices.size() > 0) { - throw new IllegalArgumentException( - "pipeline [" - + pipeline - + "] cannot be deleted because it is the default pipeline for " - + defaultPipelineIndices.size() - + " indices including [" - + defaultPipelineIndices.stream().limit(3).collect(Collectors.joining(",")) - + "] and the final pipeline for " - + finalPipelineIndices.size() - + " indices including [" - + finalPipelineIndices.stream().limit(3).collect(Collectors.joining(",")) - + "]" + throw new IllegalArgumentException(String.format( + Locale.ROOT, + "pipeline [%s] cannot be deleted because it is %s%s%s", + pipeline, + defaultPipelineIndices.size() > 0 + ? String.format( + Locale.ROOT, + "the default pipeline for %s index(es) including [%s]", + defaultPipelineIndices.size(), + defaultPipelineIndices.stream().limit(3).sorted().collect(Collectors.joining(",")) + ) + : Strings.EMPTY, + defaultPipelineIndices.size() > 0 && finalPipelineIndices.size() > 0 + ? " and " + : Strings.EMPTY, + finalPipelineIndices.size() > 0 + ? String.format( + Locale.ROOT, + "the final pipeline for %s index(es) including [%s]", + finalPipelineIndices.size(), + finalPipelineIndices.stream().limit(3).sorted().collect(Collectors.joining(",")) + ) + : Strings.EMPTY + ) ); } } diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index 1b6e4899315e2..4de08c65f27d3 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -67,11 +67,13 @@ import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.concurrent.CountDownLatch; @@ -320,49 +322,65 @@ public void testValidateNoIngestInfo() throws Exception { public void testValidateNotInUse() { String pipeline = "pipeline"; ImmutableOpenMap.Builder indices = ImmutableOpenMap.builder(); - int defaultPipelineCount = 0; - int finalPipelineCount = 0; - int indicesCount = randomIntBetween(5, 10); - for (int i = 0; i < indicesCount; i++) { - IndexMetadata.Builder builder = IndexMetadata.builder("index" + i).numberOfShards(1).numberOfReplicas(1); + int defaultIndicesCount = randomIntBetween(0, 4); + List defaultIndices = new ArrayList<>(); + for (int i = 0; i < defaultIndicesCount; i++) { + String indexName = "index" + i; + defaultIndices.add(indexName); + IndexMetadata.Builder builder = IndexMetadata.builder(indexName); Settings.Builder settingsBuilder = settings(Version.CURRENT); - if (randomBoolean()) { - settingsBuilder.put(IndexSettings.DEFAULT_PIPELINE.getKey(), pipeline); - defaultPipelineCount++; - } - - if (randomBoolean()) { - settingsBuilder.put(IndexSettings.FINAL_PIPELINE.getKey(), pipeline); - finalPipelineCount++; - } + settingsBuilder.put(IndexSettings.DEFAULT_PIPELINE.getKey(), pipeline); + builder.settings(settingsBuilder); + IndexMetadata indexMetadata = builder.settings(settingsBuilder).numberOfShards(1).numberOfReplicas(1).build(); + indices.put(indexName, indexMetadata); + } + int finalIndicesCount = randomIntBetween(0, 4); + List finalIndices = new ArrayList<>(); + for (int i = defaultIndicesCount; i < (finalIndicesCount + defaultIndicesCount); i++) { + String indexName = "index" + i; + finalIndices.add(indexName); + IndexMetadata.Builder builder = IndexMetadata.builder(indexName); + Settings.Builder settingsBuilder = settings(Version.CURRENT); + settingsBuilder.put(IndexSettings.FINAL_PIPELINE.getKey(), pipeline); builder.settings(settingsBuilder); IndexMetadata indexMetadata = builder.settings(settingsBuilder).numberOfShards(1).numberOfReplicas(1).build(); - indices.put(indexMetadata.getIndex().getName(), indexMetadata); + indices.put(indexName, indexMetadata); } IllegalArgumentException e = expectThrows( IllegalArgumentException.class, () -> IngestService.validateNotInUse(pipeline, indices.build()) ); - assertThat(e.getMessage(), containsString("default pipeline for " + defaultPipelineCount + " indices including")); - assertThat(e.getMessage(), containsString("final pipeline for " + finalPipelineCount + " indices including")); - if (defaultPipelineCount >= 3) { - // assert index limit - String content = "default pipeline for " + defaultPipelineCount + " indices including ["; - int start = e.getMessage().indexOf(content) + content.length(); - int end = e.getMessage().indexOf("] and the final pipeline"); - // indices content length, eg: index0,index1,index2 - assertEquals(end - start, (6 + 1 + 6 + 1 + 6)); + + if (defaultIndices.size() > 0) { + assertThat( + e.getMessage(), + containsString(String.format( + Locale.ROOT, + "default pipeline for %s index(es) including [%s]", + defaultIndices.size(), + defaultIndices.stream().limit(3).sorted().collect(Collectors.joining(",")) + ) + ) + ); } - if (finalPipelineCount >= 3) { - // assert index limit - String content = "final pipeline for " + finalPipelineCount + " indices including ["; - int start = e.getMessage().indexOf(content) + content.length(); - int end = e.getMessage().lastIndexOf("]"); - // indices content length, eg: index0,index1,index2 - assertEquals(end - start, (6 + 1 + 6 + 1 + 6)); + if (defaultIndices.size() > 0 && finalIndices.size() > 0) { + assertThat(e.getMessage(), containsString(" and ")); + } + + if (finalIndices.size() > 0) { + assertThat( + e.getMessage(), + containsString(String.format( + Locale.ROOT, + "final pipeline for %s index(es) including [%s]", + finalIndices.size(), + finalIndices.stream().limit(3).sorted().collect(Collectors.joining(",")) + ) + ) + ); } } @@ -662,7 +680,7 @@ public void testDeleteWithIndexUsePipeline() { IllegalArgumentException.class, () -> IngestService.innerDelete(deleteRequest, finalClusterState) ); - assertThat(e.getMessage(), containsString("default pipeline for 1 indices including [pipeline-index]")); + assertThat(e.getMessage(), containsString("default pipeline for 1 index(es) including [pipeline-index]")); } { @@ -679,7 +697,7 @@ public void testDeleteWithIndexUsePipeline() { IllegalArgumentException.class, () -> IngestService.innerDelete(deleteRequest, finalClusterState) ); - assertThat(e.getMessage(), containsString("final pipeline for 1 indices including [pipeline-index]")); + assertThat(e.getMessage(), containsString("final pipeline for 1 index(es) including [pipeline-index]")); } // Delete pipeline: From 18fb7878c5ed386d486ddc4feff2c2f82c653957 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Wed, 13 Oct 2021 10:42:21 -0500 Subject: [PATCH 2/3] Fix failing IngestServiceTests testValidateNotInUse (#79059) --- .../src/main/java/org/elasticsearch/ingest/IngestService.java | 4 ++-- .../java/org/elasticsearch/ingest/IngestServiceTests.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 827237ec51c02..fde635ca5a993 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -325,7 +325,7 @@ static void validateNotInUse(String pipeline, ImmutableOpenMap 0 && finalPipelineIndices.size() > 0 @@ -336,7 +336,7 @@ static void validateNotInUse(String pipeline, ImmutableOpenMap Date: Thu, 14 Oct 2021 08:14:02 -0500 Subject: [PATCH 3/3] Fix failure in IngestServiceTests > testValidateNotInUse (#79145) --- .../test/java/org/elasticsearch/ingest/IngestServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index 5e380d6129474..8786e36c852fa 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -335,7 +335,7 @@ public void testValidateNotInUse() { indices.put(indexName, indexMetadata); } - int finalIndicesCount = randomIntBetween(0, 4); + int finalIndicesCount = randomIntBetween(defaultIndicesCount > 0 ? 0 : 1, 4); List finalIndices = new ArrayList<>(); for (int i = defaultIndicesCount; i < (finalIndicesCount + defaultIndicesCount); i++) { String indexName = "index" + i;