From 322a5c307bb28b0009e8197289abf0ad0619ffb7 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Mon, 18 Oct 2021 08:59:35 -0500 Subject: [PATCH] [7.x] Tweak error message when pipeline cannot be deleted (#79357) --- .../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..fde635ca5a993 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().sorted().limit(3).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().sorted().limit(3).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..8786e36c852fa 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(defaultIndicesCount > 0 ? 0 : 1, 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().sorted().limit(3).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().sorted().limit(3).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: