From 8e1ba278c61d4af7de1078f913aa86525c413857 Mon Sep 17 00:00:00 2001 From: Michael Froh Date: Wed, 5 Apr 2023 17:15:38 +0000 Subject: [PATCH] More feature flag fixes for search pipeline testing - Don't use system properties for SearchPipelineServiceTests. - Enable feature flag for multinode smoke tests. Signed-off-by: Michael Froh --- qa/smoke-test-multinode/build.gradle | 1 + .../pipeline/SearchPipelineService.java | 16 +++++++++++++--- .../pipeline/SearchPipelineServiceTests.java | 19 ++++--------------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/qa/smoke-test-multinode/build.gradle b/qa/smoke-test-multinode/build.gradle index 25261f5e3ff7d..8f13e4bf8736e 100644 --- a/qa/smoke-test-multinode/build.gradle +++ b/qa/smoke-test-multinode/build.gradle @@ -43,6 +43,7 @@ File repo = file("$buildDir/testclusters/repo") testClusters.integTest { numberOfNodes = 2 setting 'path.repo', repo.absolutePath + setting 'opensearch.experimental.feature.search_pipeline.enabled', 'true' } integTest { diff --git a/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java b/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java index 5f30f7d9a0127..95ae2b9c81caf 100644 --- a/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java +++ b/server/src/main/java/org/opensearch/search/pipeline/SearchPipelineService.java @@ -79,6 +79,8 @@ public class SearchPipelineService implements ClusterStateApplier, ReportingServ private final NamedWriteableRegistry namedWriteableRegistry; private volatile ClusterState state; + private boolean forceEnabled = false; + public SearchPipelineService( ClusterService clusterService, ThreadPool threadPool, @@ -212,7 +214,7 @@ public void putPipeline( PutSearchPipelineRequest request, ActionListener listener ) throws Exception { - if (FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE) == false) { + if (isFeatureEnabled() == false) { throw new IllegalArgumentException("Experimental search pipeline feature is not enabled"); } @@ -330,7 +332,7 @@ static ClusterState innerDelete(DeleteSearchPipelineRequest request, ClusterStat public SearchRequest transformRequest(SearchRequest originalRequest) { String pipelineId = originalRequest.pipeline(); - if (pipelineId != null && FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE)) { + if (pipelineId != null && isFeatureEnabled()) { PipelineHolder pipeline = pipelines.get(pipelineId); if (pipeline == null) { throw new IllegalArgumentException("Pipeline " + pipelineId + " is not defined"); @@ -353,7 +355,7 @@ public SearchRequest transformRequest(SearchRequest originalRequest) { public SearchResponse transformResponse(SearchRequest request, SearchResponse searchResponse) { String pipelineId = request.pipeline(); - if (pipelineId != null && FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE)) { + if (pipelineId != null && isFeatureEnabled()) { PipelineHolder pipeline = pipelines.get(pipelineId); if (pipeline == null) { throw new IllegalArgumentException("Pipeline " + pipelineId + " is not defined"); @@ -426,4 +428,12 @@ static class PipelineHolder { this.pipeline = Objects.requireNonNull(pipeline); } } + + private boolean isFeatureEnabled() { + return forceEnabled || FeatureFlags.isEnabled(FeatureFlags.SEARCH_PIPELINE); + } + + void setForceEnabled(boolean forceEnabled) { + this.forceEnabled = forceEnabled; + } } diff --git a/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java b/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java index 1664bbc7dd159..c0748edfb3efb 100644 --- a/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java +++ b/server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java @@ -11,9 +11,7 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.lucene.search.TotalHits; -import org.junit.AfterClass; import org.junit.Before; -import org.junit.BeforeClass; import org.opensearch.OpenSearchParseException; import org.opensearch.ResourceNotFoundException; import org.opensearch.Version; @@ -29,11 +27,9 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.SuppressForbidden; import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.io.stream.NamedWriteableRegistry; import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.FeatureFlags; import org.opensearch.common.util.concurrent.OpenSearchExecutors; import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.query.TermQueryBuilder; @@ -58,17 +54,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -@SuppressForbidden(reason = "feature flag overrides") public class SearchPipelineServiceTests extends OpenSearchTestCase { - @BeforeClass - public static void enableFeature() { - System.setProperty(FeatureFlags.SEARCH_PIPELINE, "true"); - } - - @AfterClass - public static void disableFeature() { - System.setProperty(FeatureFlags.SEARCH_PIPELINE, "false"); - } private static final SearchPipelinePlugin DUMMY_PLUGIN = new SearchPipelinePlugin() { @Override @@ -137,6 +123,7 @@ public void testExecuteSearchPipelineDoesNotExist() { List.of(DUMMY_PLUGIN), client ); + searchPipelineService.setForceEnabled(true); final SearchRequest searchRequest = new SearchRequest("_index").pipeline("bar"); IllegalArgumentException e = expectThrows( IllegalArgumentException.class, @@ -232,7 +219,7 @@ private SearchPipelineService createWithProcessors(Map getProcessors(Processor.Parameters paramet }), client ); + searchPipelineService.setForceEnabled(true); + return searchPipelineService; } public void testUpdatePipelines() {