From 5bf270bb93a2a08f28d32d3b8e55bf396fe661a5 Mon Sep 17 00:00:00 2001 From: weizijun Date: Mon, 11 Oct 2021 19:10:40 +0800 Subject: [PATCH] improve --- .../elasticsearch/ingest/IngestService.java | 35 +++++++----- .../ingest/IngestServiceTests.java | 55 ++++++++++++++++++- 2 files changed, 74 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 81267aefa7487..6ec268a485ae2 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -67,6 +67,7 @@ import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.IntConsumer; +import java.util.stream.Collectors; /** * Holder class for several ingest related services. @@ -295,30 +296,36 @@ static ClusterState innerDelete(DeletePipelineRequest request, ClusterState curr } static void validateNotInUse(String pipeline, ImmutableOpenMap indices) { + List defaultPipelineIndices = new ArrayList<>(); + List finalPipelineIndices = new ArrayList<>(); for (ObjectCursor cursor : indices.values()) { IndexMetadata indexMetadata = cursor.value; String defaultPipeline = IndexSettings.DEFAULT_PIPELINE.get(indexMetadata.getSettings()); String finalPipeline = IndexSettings.FINAL_PIPELINE.get(indexMetadata.getSettings()); if (pipeline.equals(defaultPipeline)) { - throw new IllegalArgumentException( - "unable to remove pipeline [" - + pipeline - + "], as it is in use by index [" - + indexMetadata.getIndex().getName() - + "] default pipeline settings" - ); + defaultPipelineIndices.add(indexMetadata.getIndex().getName()); } if (pipeline.equals(finalPipeline)) { - throw new IllegalArgumentException( - "unable to remove pipeline [" - + pipeline - + "], as it is in use by index [" - + indexMetadata.getIndex().getName() - + "] final pipeline settings" - ); + finalPipelineIndices.add(indexMetadata.getIndex().getName()); } } + + if (defaultPipelineIndices.size() > 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(",")) + + "]" + ); + } } /** diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index a20fdc1a3e4a7..965105d5654ab 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -35,6 +35,7 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; @@ -78,6 +79,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.instanceOf; @@ -283,6 +285,55 @@ public void testValidateNoIngestInfo() throws Exception { ingestService.validatePipeline(Collections.singletonMap(discoveryNode, ingestInfo), putRequest); } + 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); + 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++; + } + + builder.settings(settingsBuilder); + IndexMetadata indexMetadata = builder.settings(settingsBuilder).numberOfShards(1).numberOfReplicas(1).build(); + indices.put(indexMetadata.getIndex().getName(), 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 (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)); + } + } + public void testGetProcessorsInPipeline() throws Exception { IngestService ingestService = createWithProcessors(); String id = "_id"; @@ -579,7 +630,7 @@ public void testDeleteWithIndexUsePipeline() { IllegalArgumentException.class, () -> IngestService.innerDelete(deleteRequest, finalClusterState) ); - assertTrue(e.getMessage().contains("default pipeline settings")); + assertThat(e.getMessage(), containsString("default pipeline for 1 indices including [pipeline-index]")); } { @@ -596,7 +647,7 @@ public void testDeleteWithIndexUsePipeline() { IllegalArgumentException.class, () -> IngestService.innerDelete(deleteRequest, finalClusterState) ); - assertTrue(e.getMessage().contains("final pipeline settings")); + assertThat(e.getMessage(), containsString("final pipeline for 1 indices including [pipeline-index]")); } // Delete pipeline: