From 1e9c165bf59a4ab4cc5f579d4603c8d5e4667883 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 14 Sep 2023 11:21:24 -0700 Subject: [PATCH 1/6] Add utility for loading SPI extensions (#99557) A recurring pattern in extension classes is to include a static load method. Extensions are mostly meant as a way for serverless to extend functionality, and no other extensions are acceptable, so there is either one or zero extensions found. This commit adds a utility class for loading SPI extensions, which expects the singleton pattern. BuildExtension and VersionExtension uses are changed to using the new helper class. --- .../main/java/org/elasticsearch/Build.java | 10 +- .../org/elasticsearch/TransportVersion.java | 4 +- .../org/elasticsearch/index/IndexVersion.java | 4 +- .../internal/BuildExtension.java | 16 --- .../internal/VersionExtension.java | 16 --- .../plugins/ExtensionLoader.java | 49 ++++++++ .../plugins/ExtensionLoaderTests.java | 107 ++++++++++++++++++ .../test/PrivilegedOperations.java | 7 ++ 8 files changed, 173 insertions(+), 40 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/plugins/ExtensionLoader.java create mode 100644 server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java diff --git a/server/src/main/java/org/elasticsearch/Build.java b/server/src/main/java/org/elasticsearch/Build.java index acb61e5fb46b6..c34a9737bf799 100644 --- a/server/src/main/java/org/elasticsearch/Build.java +++ b/server/src/main/java/org/elasticsearch/Build.java @@ -14,10 +14,12 @@ import org.elasticsearch.core.Booleans; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.internal.BuildExtension; +import org.elasticsearch.plugins.ExtensionLoader; import java.io.IOException; import java.net.URL; import java.security.CodeSource; +import java.util.ServiceLoader; import java.util.jar.JarInputStream; import java.util.jar.Manifest; @@ -41,12 +43,8 @@ private static class CurrentHolder { // finds the pluggable current build, or uses the local build as a fallback private static Build findCurrent() { - var buildExtension = BuildExtension.load(); - if (buildExtension == null) { - return findLocalBuild(); - } - var build = buildExtension.getCurrentBuild(); - return build; + var buildExtension = ExtensionLoader.loadSingleton(ServiceLoader.load(BuildExtension.class), () -> Build::findLocalBuild); + return buildExtension.getCurrentBuild(); } } diff --git a/server/src/main/java/org/elasticsearch/TransportVersion.java b/server/src/main/java/org/elasticsearch/TransportVersion.java index efb3cca9a7c59..2aae6befb673a 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersion.java +++ b/server/src/main/java/org/elasticsearch/TransportVersion.java @@ -12,8 +12,10 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.internal.VersionExtension; +import org.elasticsearch.plugins.ExtensionLoader; import java.io.IOException; +import java.util.ServiceLoader; /** * Represents the version of the wire protocol used to communicate between a pair of ES nodes. @@ -109,7 +111,7 @@ private static class CurrentHolder { // finds the pluggable current version, or uses the given fallback private static TransportVersion findCurrent() { - var versionExtension = VersionExtension.load(); + var versionExtension = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class), () -> null); if (versionExtension == null) { return TransportVersions.LATEST_DEFINED; } diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersion.java b/server/src/main/java/org/elasticsearch/index/IndexVersion.java index 34f415e46462a..ef238d1fe87f0 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexVersion.java +++ b/server/src/main/java/org/elasticsearch/index/IndexVersion.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Assertions; import org.elasticsearch.internal.VersionExtension; +import org.elasticsearch.plugins.ExtensionLoader; import org.elasticsearch.xcontent.ToXContentFragment; import org.elasticsearch.xcontent.XContentBuilder; @@ -25,6 +26,7 @@ import java.util.HashMap; import java.util.Map; import java.util.NavigableMap; +import java.util.ServiceLoader; import java.util.Set; import java.util.TreeMap; @@ -154,7 +156,7 @@ private static class CurrentHolder { // finds the pluggable current version, or uses the given fallback private static IndexVersion findCurrent(IndexVersion fallback) { - var versionExtension = VersionExtension.load(); + var versionExtension = ExtensionLoader.loadSingleton(ServiceLoader.load(VersionExtension.class), () -> null); if (versionExtension == null) { return fallback; } diff --git a/server/src/main/java/org/elasticsearch/internal/BuildExtension.java b/server/src/main/java/org/elasticsearch/internal/BuildExtension.java index 8826045b93e38..0d1a9880f8d32 100644 --- a/server/src/main/java/org/elasticsearch/internal/BuildExtension.java +++ b/server/src/main/java/org/elasticsearch/internal/BuildExtension.java @@ -10,8 +10,6 @@ import org.elasticsearch.Build; -import java.util.ServiceLoader; - /** * Allows plugging in current build info. */ @@ -21,18 +19,4 @@ public interface BuildExtension { * Returns the {@link Build} that represents the running Elasticsearch code. */ Build getCurrentBuild(); - - /** - * Loads a single BuildExtension, or returns {@code null} if none are found. - */ - static BuildExtension load() { - var loader = ServiceLoader.load(BuildExtension.class); - var extensions = loader.stream().toList(); - if (extensions.size() > 1) { - throw new IllegalStateException("More than one build extension found"); - } else if (extensions.size() == 0) { - return null; - } - return extensions.get(0).get(); - } } diff --git a/server/src/main/java/org/elasticsearch/internal/VersionExtension.java b/server/src/main/java/org/elasticsearch/internal/VersionExtension.java index 8afef81455a95..83974b3b65158 100644 --- a/server/src/main/java/org/elasticsearch/internal/VersionExtension.java +++ b/server/src/main/java/org/elasticsearch/internal/VersionExtension.java @@ -11,8 +11,6 @@ import org.elasticsearch.TransportVersion; import org.elasticsearch.index.IndexVersion; -import java.util.ServiceLoader; - /** * Allows plugging in current version elements. */ @@ -30,18 +28,4 @@ public interface VersionExtension { * This must be at least equal to the latest version found in {@link IndexVersion} V_* constants. */ IndexVersion getCurrentIndexVersion(); - - /** - * Loads a single VersionExtension, or returns {@code null} if none are found. - */ - static VersionExtension load() { - var loader = ServiceLoader.load(VersionExtension.class); - var extensions = loader.stream().toList(); - if (extensions.size() > 1) { - throw new IllegalStateException("More than one version extension found"); - } else if (extensions.size() == 0) { - return null; - } - return extensions.get(0).get(); - } } diff --git a/server/src/main/java/org/elasticsearch/plugins/ExtensionLoader.java b/server/src/main/java/org/elasticsearch/plugins/ExtensionLoader.java new file mode 100644 index 0000000000000..7dfb64c989ea2 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/plugins/ExtensionLoader.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins; + +import java.util.Locale; +import java.util.ServiceLoader; +import java.util.function.Supplier; + +/** + * A utility for loading SPI extensions. + */ +public class ExtensionLoader { + + /** + * Loads a single SPI extension. + * + * There should be no more than one extension found. If no service providers + * are found, the supplied fallback is used. + * + * Note: A ServiceLoader is needed rather than the service class because ServiceLoaders + * must be loaded by a module with the {@code uses} declaration. Since this + * utility class is in server, it will not have uses (or even know about) all the + * service classes it may load. Thus, the caller must load the ServiceLoader. + * + * @param loader a service loader instance to find the singleton extension in + * @param fallback a supplier for an instance if no extensions are found + * @return an instance of the extension + * @param the SPI extension type + */ + public static T loadSingleton(ServiceLoader loader, Supplier fallback) { + var extensions = loader.stream().toList(); + if (extensions.size() > 1) { + // It would be really nice to give the actual extension class here directly, but that would require passing it + // in effectively twice in the call site, once to ServiceLoader, and then to this method directly as well. + // It's annoying that ServiceLoader hangs onto the service class, but does not expose it. It does at least + // print the service class from its toString, which is better tha nothing + throw new IllegalStateException(String.format(Locale.ROOT, "More than one extension found for %s", loader)); + } else if (extensions.isEmpty()) { + return fallback.get(); + } + return extensions.get(0).get(); + } +} diff --git a/server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java b/server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java new file mode 100644 index 0000000000000..35f65ebedf9b9 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/plugins/ExtensionLoaderTests.java @@ -0,0 +1,107 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.plugins; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.PrivilegedOperations.ClosableURLClassLoader; +import org.elasticsearch.test.compiler.InMemoryJavaCompiler; +import org.elasticsearch.test.jar.JarUtils; + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Locale; +import java.util.Map; +import java.util.ServiceLoader; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; + +public class ExtensionLoaderTests extends ESTestCase { + public interface TestService { + int getValue(); + } + + private ClosableURLClassLoader buildProviderJar(Map sources) throws Exception { + var classToBytes = InMemoryJavaCompiler.compile(sources); + + Map jarEntries = new HashMap<>(); + for (var entry : sources.entrySet()) { + var classname = entry.getKey(); + var filename = classname.replace(".", "/") + ".class"; + jarEntries.put(filename, classToBytes.get(classname)); + } + String serviceFile = String.join("\n", sources.keySet()); + jarEntries.put( + "META-INF/services/org.elasticsearch.plugins.ExtensionLoaderTests$TestService", + serviceFile.getBytes(StandardCharsets.UTF_8) + ); + + Path topLevelDir = createTempDir(getTestName()); + Path jar = topLevelDir.resolve("provider.jar"); + JarUtils.createJarWithEntries(jar, jarEntries); + URL[] urls = new URL[] { jar.toUri().toURL() }; + + return new ClosableURLClassLoader(URLClassLoader.newInstance(urls, this.getClass().getClassLoader())); + } + + private String defineProvider(String name, int value) { + return String.format(Locale.ROOT, """ + package p; + import org.elasticsearch.plugins.ExtensionLoaderTests.TestService; + public class %s implements TestService { + @Override + public int getValue() { + return %d; + } + } + """, name, value); + } + + public void testNoProviderNullFallback() { + TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class), () -> null); + assertThat(service, nullValue()); + } + + public void testNoProvider() { + TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class), () -> () -> 2); + assertThat(service, not(nullValue())); + assertThat(service.getValue(), equalTo(2)); + } + + public void testOneProvider() throws Exception { + Map sources = Map.of("p.FooService", defineProvider("FooService", 1)); + try (var loader = buildProviderJar(sources)) { + TestService service = ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()), () -> null); + assertThat(service, not(nullValue())); + assertThat(service.getValue(), equalTo(1)); + } + } + + public void testManyProviders() throws Exception { + Map sources = Map.of( + "p.FooService", + defineProvider("FooService", 1), + "p.BarService", + defineProvider("BarService", 2) + ); + try (var loader = buildProviderJar(sources)) { + var e = expectThrows( + IllegalStateException.class, + () -> ExtensionLoader.loadSingleton(ServiceLoader.load(TestService.class, loader.classloader()), () -> null) + ); + assertThat(e.getMessage(), containsString("More than one extension found")); + assertThat(e.getMessage(), containsString("TestService")); + } + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/PrivilegedOperations.java b/test/framework/src/main/java/org/elasticsearch/test/PrivilegedOperations.java index 3c0a84b15a55e..ae819a57173cf 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/PrivilegedOperations.java +++ b/test/framework/src/main/java/org/elasticsearch/test/PrivilegedOperations.java @@ -53,6 +53,13 @@ public static void closeURLClassLoader(URLClassLoader loader) throws IOException } } + public record ClosableURLClassLoader(URLClassLoader classloader) implements AutoCloseable { + @Override + public void close() throws Exception { + closeURLClassLoader(classloader); + } + } + public static Boolean compilationTaskCall(JavaCompiler.CompilationTask compilationTask) { return AccessController.doPrivileged( (PrivilegedAction) () -> compilationTask.call(), From 9a8f13fd3be12b8e05591b9e8fac65a980a1f4eb Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 14 Sep 2023 15:29:31 -0400 Subject: [PATCH 2/6] [buildkite] Add more memory to platform-support job agents (#99594) --- .buildkite/pipelines/periodic-platform-support.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.buildkite/pipelines/periodic-platform-support.yml b/.buildkite/pipelines/periodic-platform-support.yml index 520089286ec36..8522ead742768 100644 --- a/.buildkite/pipelines/periodic-platform-support.yml +++ b/.buildkite/pipelines/periodic-platform-support.yml @@ -27,7 +27,7 @@ steps: provider: gcp image: family/elasticsearch-{{matrix.image}} diskSizeGb: 350 - machineType: custom-32-98304 + machineType: n1-standard-32 env: {} - group: platform-support-windows steps: @@ -50,7 +50,7 @@ steps: agents: provider: gcp image: family/elasticsearch-{{matrix.image}} - machineType: custom-32-98304 + machineType: n1-standard-32 diskType: pd-ssd diskSizeGb: 350 env: From a42145957fecc45a15297d296ed6298723f54038 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 14 Sep 2023 16:33:46 -0400 Subject: [PATCH 3/6] [buildkite] Disable all jenkins PR jobs when buildkite-opt-in label is present (#99597) --- ...elastic+elasticsearch+pull-request+build-benchmark-part1.yml | 2 ++ ...elastic+elasticsearch+pull-request+build-benchmark-part2.yml | 2 ++ ...elastic+elasticsearch+pull-request+bwc-snapshots-windows.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+cloud-deploy.yml | 2 ++ .ci/jobs.t/elastic+elasticsearch+pull-request+docs-check.yml | 1 + .../elastic+elasticsearch+pull-request+eql-correctness.yml | 1 + .../elastic+elasticsearch+pull-request+example-plugins.yml | 2 ++ .ci/jobs.t/elastic+elasticsearch+pull-request+full-bwc.yml | 1 + ...c+elasticsearch+pull-request+packaging-tests-unix-sample.yml | 1 + .../elastic+elasticsearch+pull-request+packaging-tests-unix.yml | 1 + ...elasticsearch+pull-request+packaging-tests-windows-nojdk.yml | 1 + ...search+pull-request+packaging-tests-windows-sample-nojdk.yml | 1 + ...lasticsearch+pull-request+packaging-tests-windows-sample.yml | 1 + ...astic+elasticsearch+pull-request+packaging-tests-windows.yml | 1 + ...astic+elasticsearch+pull-request+packaging-upgrade-tests.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+part-1-fips.yml | 1 + .../elastic+elasticsearch+pull-request+part-1-windows.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+part-2-fips.yml | 1 + .../elastic+elasticsearch+pull-request+part-2-windows.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+part-3-fips.yml | 1 + .../elastic+elasticsearch+pull-request+part-3-windows.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+part-3.yml | 1 + .ci/jobs.t/elastic+elasticsearch+pull-request+precommit.yml | 2 ++ .ci/jobs.t/elastic+elasticsearch+pull-request+release-tests.yml | 2 ++ .../elastic+elasticsearch+pull-request+rest-compatibility.yml | 1 + .ci/templates.t/pull-request-gradle-unix.yml | 1 + 27 files changed, 33 insertions(+) diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part1.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part1.yml index 064ac3022141f..a3f1345a07f13 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part1.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part1.yml @@ -34,6 +34,8 @@ - ^x-pack/docs/.* white-list-labels: - 'build-benchmark' + black-list-labels: + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part2.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part2.yml index bf72ffa54cdae..f1b11ab1ec75a 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part2.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+build-benchmark-part2.yml @@ -34,6 +34,8 @@ - ^x-pack/docs/.* white-list-labels: - 'build-benchmark' + black-list-labels: + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots-windows.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots-windows.yml index 629700daa70e2..c0ed9bf998159 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots-windows.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots-windows.yml @@ -29,6 +29,7 @@ - 'test-windows' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: slave diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots.yml index 86da8986f27e5..676f5f6f629b7 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+bwc-snapshots.yml @@ -26,6 +26,7 @@ black-list-labels: - '>test-mute' - 'test-full-bwc' + - 'buildkite-opt-in' axes: - axis: type: slave diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+cloud-deploy.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+cloud-deploy.yml index eedbf7bba5789..24548954d8a10 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+cloud-deploy.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+cloud-deploy.yml @@ -26,6 +26,8 @@ - ^x-pack/docs/.* white-list-labels: - 'cloud-deploy' + black-list-labels: + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+docs-check.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+docs-check.yml index 77c499f455b22..c766b4379a1f6 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+docs-check.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+docs-check.yml @@ -23,6 +23,7 @@ - ^x-pack/docs/.* black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+eql-correctness.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+eql-correctness.yml index fcdbf2ea87084..0b9eea62ad9bf 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+eql-correctness.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+eql-correctness.yml @@ -25,6 +25,7 @@ - ^x-pack/docs/.* black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+example-plugins.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+example-plugins.yml index f79c98c00101f..320a9c6176d5f 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+example-plugins.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+example-plugins.yml @@ -23,6 +23,8 @@ - build-tools/.* - build-tools-internal/.* - plugins/examples/.* + black-list-labels: + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+full-bwc.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+full-bwc.yml index 5276d39f956d3..2a7920e4bae89 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+full-bwc.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+full-bwc.yml @@ -27,6 +27,7 @@ - 'test-full-bwc' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: slave diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix-sample.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix-sample.yml index c283da8e32479..04e48036a8e9e 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix-sample.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix-sample.yml @@ -27,6 +27,7 @@ black-list-labels: - '>test-mute' - ':Delivery/Packaging' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix.yml index 95a4c4273ebb7..a7413699ff6c3 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-unix.yml @@ -28,6 +28,7 @@ - ':Delivery/Packaging' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-nojdk.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-nojdk.yml index ecd4a1a084755..ea4097b1a0b93 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-nojdk.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-nojdk.yml @@ -32,6 +32,7 @@ - ':Delivery/Packaging' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample-nojdk.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample-nojdk.yml index 091dcf9eb77a0..ec644445ef8de 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample-nojdk.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample-nojdk.yml @@ -31,6 +31,7 @@ black-list-labels: - '>test-mute' - ':Delivery/Packaging' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample.yml index a438335529a3c..242e137cb1d83 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows-sample.yml @@ -30,6 +30,7 @@ black-list-labels: - '>test-mute' - ':Delivery/Packaging' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows.yml index 1dc0127284c47..a2ffc7b4050ec 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-tests-windows.yml @@ -32,6 +32,7 @@ - ':Delivery/Packaging' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-upgrade-tests.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-upgrade-tests.yml index 01770b7d94c6e..2b73d0144cab7 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-upgrade-tests.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+packaging-upgrade-tests.yml @@ -29,6 +29,7 @@ - ':Delivery/Packaging' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' axes: - axis: type: label-expression diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-fips.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-fips.yml index 793ade87a1fd9..a661230d3b93b 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-fips.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-fips.yml @@ -27,6 +27,7 @@ - 'Team:Security' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: # Use FIPS-specific Java versions diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-windows.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-windows.yml index 9a55d8dc6eeac..d7afdd0ac3277 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-windows.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-1-windows.yml @@ -28,6 +28,7 @@ - 'test-windows' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-fips.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-fips.yml index 0795172b916e2..913820709dabc 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-fips.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-fips.yml @@ -27,6 +27,7 @@ - 'Team:Security' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: # Use FIPS-specific Java versions diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-windows.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-windows.yml index de09d5044d466..ae590872be16e 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-windows.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-2-windows.yml @@ -28,6 +28,7 @@ - 'test-windows' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-fips.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-fips.yml index 3383e81ae61ed..6bf6544d40310 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-fips.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-fips.yml @@ -28,6 +28,7 @@ - 'Team:Security' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: # Use FIPS-specific Java versions diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-windows.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-windows.yml index 65ed5c58335a9..58bad17954b24 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-windows.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3-windows.yml @@ -29,6 +29,7 @@ - 'test-windows' black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3.yml index 325b9ecb68fd9..0158b909903b4 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+part-3.yml @@ -23,6 +23,7 @@ - ^x-pack/docs/.* black-list-labels: - '>test-mute' + - 'buildkite-opt-in' black-list-target-branches: - 6.8 - 7.17 diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+precommit.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+precommit.yml index 60878f1519555..1267b6a21778e 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+precommit.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+precommit.yml @@ -20,6 +20,8 @@ cancel-builds-on-update: true white-list-labels: - '>test-mute' + black-list-labels: + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+release-tests.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+release-tests.yml index 98ada6e2080e4..1ab6bd1ce0e5d 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+release-tests.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+release-tests.yml @@ -25,6 +25,8 @@ - ^x-pack/docs/.* white-list-labels: - 'test-release' + black-list-labels: + - 'buildkite-opt-in' black-list-target-branches: - 7.15 - 6.8 diff --git a/.ci/jobs.t/elastic+elasticsearch+pull-request+rest-compatibility.yml b/.ci/jobs.t/elastic+elasticsearch+pull-request+rest-compatibility.yml index 417ce525880d6..216f8ceae2078 100644 --- a/.ci/jobs.t/elastic+elasticsearch+pull-request+rest-compatibility.yml +++ b/.ci/jobs.t/elastic+elasticsearch+pull-request+rest-compatibility.yml @@ -28,6 +28,7 @@ - ^x-pack/docs/.* black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' diff --git a/.ci/templates.t/pull-request-gradle-unix.yml b/.ci/templates.t/pull-request-gradle-unix.yml index b4b5c48739097..c09e64c56f32d 100644 --- a/.ci/templates.t/pull-request-gradle-unix.yml +++ b/.ci/templates.t/pull-request-gradle-unix.yml @@ -23,6 +23,7 @@ - ^x-pack/docs/.* black-list-labels: - '>test-mute' + - 'buildkite-opt-in' builders: - inject: properties-file: '.ci/java-versions.properties' From b1e69ff21bc7f92dd6f14175ac05dd6cb6a67b18 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 14 Sep 2023 14:13:00 -0700 Subject: [PATCH 4/6] AwaitFix EsqlActionTaskIT Relates #99589 --- .../org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java index 92b81057ec698..ff6a7b15c3462 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.action; +import org.apache.lucene.tests.util.LuceneTestCase; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionFuture; import org.elasticsearch.action.admin.cluster.node.tasks.cancel.CancelTasksAction; @@ -65,6 +66,7 @@ value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.tasks.TaskCancellationService:TRACE", reason = "These tests are failing frequently; we need logs before muting them" ) +@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/99589") public class EsqlActionTaskIT extends AbstractEsqlIntegTestCase { private static int PAGE_SIZE; private static int NUM_DOCS; From bba26ae388a706c0de0458f43ae6cea9fbef6540 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 14 Sep 2023 14:32:28 -0700 Subject: [PATCH 5/6] Add rest filtering to RestExtension (#99559) This commit modifies the recently added RestExtension so that it can also filter rest handlers. The construction of the extension must change slightly so that the extension may bind to elements from a plugin (eg ThreadPool to get ThreadContext) which it would not have in a pure static environment of Java's SPI. --- .../elasticsearch/action/ActionModule.java | 24 ++++--------- .../java/org/elasticsearch/node/Node.java | 4 ++- .../elasticsearch/plugins/PluginsService.java | 22 ++++++++++++ .../plugins/internal/RestExtension.java | 36 +++++++++++++------ .../action/ActionModuleTests.java | 16 ++++++--- .../AbstractHttpServerTransportTests.java | 4 ++- .../xpack/security/SecurityTests.java | 4 ++- 7 files changed, 73 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 5683ace63ba3b..31355aac21d67 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -510,6 +510,7 @@ public class ActionModule extends AbstractModule { private final ThreadPool threadPool; private final ReservedClusterStateService reservedClusterStateService; private final boolean serverlessEnabled; + private final RestExtension restExtension; public ActionModule( Settings settings, @@ -525,7 +526,8 @@ public ActionModule( SystemIndices systemIndices, Tracer tracer, ClusterService clusterService, - List> reservedStateHandlers + List> reservedStateHandlers, + RestExtension restExtension ) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; @@ -572,6 +574,7 @@ public ActionModule( restController = new RestController(restInterceptor, nodeClient, circuitBreakerService, usageService, tracer); } reservedClusterStateService = new ReservedClusterStateService(clusterService, reservedStateHandlers); + this.restExtension = restExtension; } private static T getRestServerComponent( @@ -851,15 +854,10 @@ private static ActionFilters setupActionFilters(List actionPlugins public void initRestHandlers(Supplier nodesInCluster) { List catActions = new ArrayList<>(); - var restExtension = RestExtension.load(() -> new RestExtension() { - @Override - public Predicate getCatActionsFilter() { - return action -> true; - } - }); Predicate catActionsFilter = restExtension.getCatActionsFilter(); + Predicate restFilter = restExtension.getActionsFilter(); Consumer registerHandler = handler -> { - if (shouldKeepRestHandler(handler)) { + if (restFilter.test(handler)) { if (handler instanceof AbstractCatAction catAction && catActionsFilter.test(catAction)) { catActions.add(catAction); } @@ -1066,16 +1064,6 @@ public Predicate getCatActionsFilter() { registerHandler.accept(new RestDeleteSynonymRuleAction()); } - /** - * This method is used to determine whether a RestHandler ought to be kept in memory or not. Returns true if serverless mode is - * disabled, or if there is any ServlerlessScope annotation on the RestHandler. - * @param handler - * @return - */ - private boolean shouldKeepRestHandler(final RestHandler handler) { - return serverlessEnabled == false || handler.getServerlessScope() != null; - } - @Override protected void configure() { bind(ActionFilters.class).toInstance(actionFilters); diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 111dc5ec72165..1ae3aaa9e09db 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -183,6 +183,7 @@ import org.elasticsearch.plugins.internal.DocumentParsingObserver; import org.elasticsearch.plugins.internal.DocumentParsingObserverPlugin; import org.elasticsearch.plugins.internal.ReloadAwarePlugin; +import org.elasticsearch.plugins.internal.RestExtension; import org.elasticsearch.plugins.internal.SettingsExtension; import org.elasticsearch.readiness.ReadinessService; import org.elasticsearch.repositories.RepositoriesModule; @@ -814,7 +815,8 @@ protected Node( systemIndices, tracer, clusterService, - reservedStateHandlers + reservedStateHandlers, + pluginsService.loadSingletonServiceProvider(RestExtension.class, RestExtension::allowAll) ); modules.add(actionModule); diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index be8f9b303f4eb..96f3eedde165c 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -59,6 +59,7 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -345,6 +346,27 @@ public List loadServiceProviders(Class service) { return Collections.unmodifiableList(result); } + /** + * Loads a single SPI extension. + * + * There should be no more than one extension found. If no service providers + * are found, the supplied fallback is used. + * + * @param service the SPI class that should be loaded + * @param fallback a supplier for an instance if no providers are found + * @return an instance of the service + * @param the SPI service type + */ + public T loadSingletonServiceProvider(Class service, Supplier fallback) { + var services = loadServiceProviders(service); + if (services.size() > 1) { + throw new IllegalStateException(String.format(Locale.ROOT, "More than one extension found for %s", service.getSimpleName())); + } else if (services.size() == 0) { + return fallback.get(); + } + return services.get(0); + } + private static void loadExtensionsForPlugin(ExtensiblePlugin extensiblePlugin, List extendingPlugins) { ExtensiblePlugin.ExtensionLoader extensionLoader = new ExtensiblePlugin.ExtensionLoader() { @Override diff --git a/server/src/main/java/org/elasticsearch/plugins/internal/RestExtension.java b/server/src/main/java/org/elasticsearch/plugins/internal/RestExtension.java index da5de4f784a22..4864e6bf31222 100644 --- a/server/src/main/java/org/elasticsearch/plugins/internal/RestExtension.java +++ b/server/src/main/java/org/elasticsearch/plugins/internal/RestExtension.java @@ -8,11 +8,10 @@ package org.elasticsearch.plugins.internal; +import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.action.cat.AbstractCatAction; -import java.util.ServiceLoader; import java.util.function.Predicate; -import java.util.function.Supplier; public interface RestExtension { /** @@ -23,14 +22,29 @@ public interface RestExtension { */ Predicate getCatActionsFilter(); - static RestExtension load(Supplier fallback) { - var loader = ServiceLoader.load(RestExtension.class); - var extensions = loader.stream().toList(); - if (extensions.size() > 1) { - throw new IllegalStateException("More than one rest extension found"); - } else if (extensions.size() == 0) { - return fallback.get(); - } - return extensions.get(0).get(); + /** + * Returns a filter that determines which rest actions are exposed. + * + * The filter should return {@code false} if an action should be included, + * or {@code false} if the paths + * @return + */ + Predicate getActionsFilter(); + + /** + * Returns a rest extension which allows all rest endpoints through. + */ + static RestExtension allowAll() { + return new RestExtension() { + @Override + public Predicate getCatActionsFilter() { + return action -> true; + } + + @Override + public Predicate getActionsFilter() { + return handler -> true; + } + }; } } diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index 6975cc22adaa9..1279ea810f0a6 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -27,6 +27,7 @@ import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; import org.elasticsearch.plugins.interceptor.RestServerActionPlugin; +import org.elasticsearch.plugins.internal.RestExtension; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; @@ -123,7 +124,8 @@ public void testSetupRestHandlerContainsKnownBuiltin() { null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -182,7 +184,8 @@ public String getName() { null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/_nodes] for method: GET")); @@ -234,7 +237,8 @@ public List getRestHandlers( null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -281,7 +285,8 @@ public void test3rdPartyHandlerIsNotInstalled() { null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ) ); assertThat( @@ -319,7 +324,8 @@ public void test3rdPartyRestControllerIsNotInstalled() { null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ) ); assertThat( diff --git a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java index e32fa310ec5c8..b8d102db7e8ae 100644 --- a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java +++ b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.core.TimeValue; import org.elasticsearch.indices.TestIndexNameExpressionResolver; import org.elasticsearch.plugins.ActionPlugin; +import org.elasticsearch.plugins.internal.RestExtension; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestControllerTests; import org.elasticsearch.rest.RestHeaderDefinition; @@ -1152,7 +1153,8 @@ public Collection getRestHeaders() { null, null, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 72f3c2be7087f..fa7ff7b12994c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -48,6 +48,7 @@ import org.elasticsearch.plugins.ExtensiblePlugin; import org.elasticsearch.plugins.MapperPlugin; import org.elasticsearch.plugins.internal.DocumentParsingObserver; +import org.elasticsearch.plugins.internal.RestExtension; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; @@ -775,7 +776,8 @@ public void testSecurityRestHandlerInterceptorCanBeInstalled() throws IllegalAcc null, Tracer.NOOP, mock(ClusterService.class), - List.of() + List.of(), + RestExtension.allowAll() ); actionModule.initRestHandlers(null); From 460c2ee0f6e6f915f3266994206c894ee05d73d9 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 14 Sep 2023 14:58:33 -0700 Subject: [PATCH 6/6] Avoid using query pragmas in ESQL tests (#99600) This PR avoids enabling QueryPragmas in ESQL tests since it's only available in the snapshot build. --- .../xpack/esql/action/AbstractEsqlIntegTestCase.java | 6 ++++-- .../elasticsearch/xpack/esql/action/EsqlActionIT.java | 3 +-- .../xpack/esql/action/EsqlActionTaskIT.java | 1 + .../elasticsearch/xpack/esql/action/ManyShardsIT.java | 10 +++++----- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java index 9598d8d4498cb..2c904a39129db 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/AbstractEsqlIntegTestCase.java @@ -85,8 +85,7 @@ protected EsqlQueryResponse run(EsqlQueryRequest request) { protected static QueryPragmas randomPragmas() { Settings.Builder settings = Settings.builder(); - // pragmas are only enabled on snapshot builds - if (Build.current().isSnapshot()) { + if (canUseQueryPragmas()) { if (randomBoolean()) { settings.put("task_concurrency", randomLongBetween(1, 10)); } @@ -118,4 +117,7 @@ protected static QueryPragmas randomPragmas() { return new QueryPragmas(settings.build()); } + protected static boolean canUseQueryPragmas() { + return Build.current().isSnapshot(); + } } diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java index c567edda97018..fdc83161dcd06 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java @@ -445,8 +445,7 @@ public void testFromEvalStats() { assertEquals(0.034d, (double) getValuesList(results).get(0).get(0), 0.001d); } - public void testFromStatsEvalWithPragma() { - assumeTrue("pragmas only enabled on snapshot builds", Build.current().isSnapshot()); + public void testFromStatsThenEval() { EsqlQueryResponse results = run("from test | stats avg_count = avg(count) | eval x = avg_count + 7"); logger.info(results); Assert.assertEquals(1, getValuesList(results).size()); diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java index ff6a7b15c3462..16f704aa8f7c3 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionTaskIT.java @@ -82,6 +82,7 @@ protected Collection> nodePlugins() { @Before public void setupIndex() throws IOException { + assumeTrue("requires query pragmas", canUseQueryPragmas()); PAGE_SIZE = between(10, 100); NUM_DOCS = between(4 * PAGE_SIZE, 5 * PAGE_SIZE); READ_DESCRIPTION = """ diff --git a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java index a95f601d88ca0..c1476c8c52de5 100644 --- a/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java +++ b/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/ManyShardsIT.java @@ -59,11 +59,11 @@ public void testConcurrentQueries() throws Exception { } catch (InterruptedException e) { throw new AssertionError(e); } - var pragmas = Settings.builder() - .put(randomPragmas().getSettings()) - .put("exchange_concurrent_clients", between(1, 2)) - .build(); - run("from test-* | stats count(user) by tags", new QueryPragmas(pragmas)); + final var pragmas = Settings.builder(); + if (canUseQueryPragmas()) { + pragmas.put(randomPragmas().getSettings()).put("exchange_concurrent_clients", between(1, 2)); + } + run("from test-* | stats count(user) by tags", new QueryPragmas(pragmas.build())); }); } for (Thread thread : threads) {