Skip to content

[7.x] Do not create unused testCluster #78312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions BUILDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ The major difference between these two syntaxes is, that the configuration block

By actually doing less in the gradle configuration time as only creating tasks that are requested as part of the build and by only running the configurations for those requested tasks, using the task avoidance api contributes a major part in keeping our build fast.

#### Registering test clusters

When using the elasticsearch test cluster plugin we want to use (similar to the task avoidance API) a Gradle API to create domain objects lazy or only if required by the build.
Therefore we register test cluster by using the following syntax:

def someClusterProvider = testClusters.register('someCluster') { ... }

This registers a potential testCluster named `somecluster` and provides a provider instance, but doesn't create it yet nor configures it. This makes the gradle configuration phase more efficient by
doing less.

To wire this registered cluster into a `TestClusterAware` task (e.g. `RestIntegTest`) you can resolve the actual cluster from the provider instance:

tasks.register('someClusterTest', RestIntegTestTask) {
useCluster someClusterProvider
nonInputProperties.systemProperty 'tests.leader_host', "${-> someClusterProvider.get().getAllHttpSocketURI().get(0)}"
}

#### Adding additional integration tests

Additional integration tests for a certain elasticsearch modules that are specific to certain cluster configuration can be declared in a separate so called `qa` subproject of your module.
Expand Down
12 changes: 5 additions & 7 deletions build-tools-internal/src/main/groovy/elasticsearch.run.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@ import org.elasticsearch.gradle.testclusters.RunTask
// precompiled script plugins (see https://github.com/gradle/gradle/issues/17004)
// apply plugin: 'elasticsearch.internal-testclusters'

testClusters {
runTask {
testDistribution = System.getProperty('run.distribution', 'default')
if (System.getProperty('run.distribution', 'default') == 'default') {
String licenseType = System.getProperty("run.license_type", "basic")
testClusters.register("runTask") {
testDistribution = providers.systemProperty('run.distribution').orElse('default').forUseAtConfigurationTime().get()
if (providers.systemProperty('run.distribution').orElse('default').forUseAtConfigurationTime().get() == 'default') {
String licenseType = providers.systemProperty("run.license_type").orElse("basic").forUseAtConfigurationTime().get()
if (licenseType == 'trial') {
setting 'xpack.ml.enabled', 'true'
setting 'xpack.graph.enabled', 'true'
Expand All @@ -30,11 +29,10 @@ testClusters {
keystore 'bootstrap.password', 'password'
user username: 'elastic-admin', password: 'elastic-password', role: 'superuser'
}
}
}

tasks.register("run", RunTask) {
useCluster testClusters.runTask;
useCluster testClusters.named("runTask")
description = 'Runs elasticsearch in the foreground'
group = 'Verification'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void apply(final Project project) {
// Auto add dependent modules to the test cluster
if (project1.findProject(":modules:" + pluginName) != null) {
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = testClusters(project, "testClusters");
testClusters.all(elasticsearchCluster -> elasticsearchCluster.module(":modules:" + pluginName));
testClusters.configureEach(elasticsearchCluster -> elasticsearchCluster.module(":modules:" + pluginName));
}
});
final PluginPropertiesExtension extension1 = project1.getExtensions().getByType(PluginPropertiesExtension.class);
Expand Down Expand Up @@ -124,18 +124,18 @@ public void apply(final Project project) {
project.getConfigurations().getByName("default").extendsFrom(project.getConfigurations().getByName("runtimeClasspath"));

// allow running ES with this plugin in the foreground of a build
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = testClusters(project, TestClustersPlugin.EXTENSION_NAME);
final ElasticsearchCluster runCluster = testClusters.create("runTask", cluster -> {
var testClusters = testClusters(project, TestClustersPlugin.EXTENSION_NAME);
var runCluster = testClusters.register("runtTask", c -> {
if (GradleUtils.isModuleProject(project.getPath())) {
cluster.module(bundleTask.flatMap((Transformer<Provider<RegularFile>, Zip>) zip -> zip.getArchiveFile()));
c.module(bundleTask.flatMap((Transformer<Provider<RegularFile>, Zip>) zip -> zip.getArchiveFile()));
} else {
cluster.plugin(bundleTask.flatMap((Transformer<Provider<RegularFile>, Zip>) zip -> zip.getArchiveFile()));
c.plugin(bundleTask.flatMap((Transformer<Provider<RegularFile>, Zip>) zip -> zip.getArchiveFile()));
}
});

project.getTasks().register("run", RunTask.class, runTask -> {
runTask.useCluster(runCluster);
runTask.dependsOn(project.getTasks().named(BUNDLE_PLUGIN_TASK_NAME));
project.getTasks().register("run", RunTask.class, r -> {
r.useCluster(runCluster.get());
r.dependsOn(project.getTasks().named(BUNDLE_PLUGIN_TASK_NAME));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,16 @@ public void apply(Project project) {
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
.getExtensions()
.getByName(TestClustersPlugin.EXTENSION_NAME);
var cluster = testClusters.maybeCreate(JAVA_REST_TEST);
var clusterProvider = testClusters.register(JAVA_REST_TEST);

// Register test task
TaskProvider<StandaloneRestIntegTestTask> javaRestTestTask = project.getTasks()
.register(JAVA_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
task.useCluster(cluster);
task.useCluster(clusterProvider);
task.setTestClassesDirs(testSourceSet.getOutput().getClassesDirs());
task.setClasspath(testSourceSet.getRuntimeClasspath());

var cluster = clusterProvider.get();
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
nonInputProperties.systemProperty("tests.rest.cluster", () -> String.join(",", cluster.getAllHttpSocketURI()));
nonInputProperties.systemProperty("tests.cluster", () -> String.join(",", cluster.getAllTransportPortURI()));
Expand All @@ -67,7 +68,7 @@ public void apply(Project project) {
// Register plugin bundle with test cluster
project.getPlugins().withType(PluginBuildPlugin.class, p -> {
TaskProvider<Zip> bundle = project.getTasks().withType(Zip.class).named(BUNDLE_PLUGIN_TASK_NAME);
cluster.plugin(bundle.flatMap(Zip::getArchiveFile));
clusterProvider.configure( c-> c.plugin(bundle.flatMap(Zip::getArchiveFile)));
javaRestTestTask.configure(t -> t.dependsOn(bundle));
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
package org.elasticsearch.gradle.test;

import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Input;
import org.gradle.process.CommandLineArgumentProvider;

Expand All @@ -18,6 +19,10 @@
public class SystemPropertyCommandLineArgumentProvider implements CommandLineArgumentProvider {
private final Map<String, Object> systemProperties = new LinkedHashMap<>();

public void systemProperty(String key, Provider<Object> value) {
systemProperties.put(key, (Supplier<String>) () -> String.valueOf(value.get()));
}

public void systemProperty(String key, Supplier<String> value) {
systemProperties.put(key, value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.gradle.transform.UnzipTransform;
import org.gradle.api.Action;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.NamedDomainObjectProvider;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.Task;
Expand Down Expand Up @@ -81,11 +82,11 @@ public void apply(Project project) {
testSourceSet.getOutput().dir(copyRestTestSpecs.map(Task::getOutputs));
Configuration yamlRestTestImplementation = configurations.getByName(testSourceSet.getImplementationConfigurationName());
setupDefaultDependencies(project.getDependencies(), restTestSpecs, yamlRestTestImplementation);
var cluster = testClusters.maybeCreate(YAML_REST_TEST);
var cluster = testClusters.register(YAML_REST_TEST);
TaskProvider<StandaloneRestIntegTestTask> yamlRestTestTask = setupTestTask(project, testSourceSet, cluster);
project.getPlugins().withType(PluginBuildPlugin.class, p -> {
TaskProvider<Zip> bundle = project.getTasks().withType(Zip.class).named(BUNDLE_PLUGIN_TASK_NAME);
cluster.plugin(bundle.flatMap(Zip::getArchiveFile));
cluster.configure(c -> c.plugin(bundle.flatMap(Zip::getArchiveFile)));
yamlRestTestTask.configure(t -> t.dependsOn(bundle));
});

Expand All @@ -109,15 +110,16 @@ private static void setupDefaultDependencies(
}

private TaskProvider<StandaloneRestIntegTestTask> setupTestTask(
Project project,
SourceSet testSourceSet,
ElasticsearchCluster cluster
Project project,
SourceSet testSourceSet,
NamedDomainObjectProvider<ElasticsearchCluster> clusterProvider
) {
return project.getTasks().register(YAML_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
task.useCluster(cluster);
task.useCluster(clusterProvider.get());
task.setTestClassesDirs(testSourceSet.getOutput().getClassesDirs());
task.setClasspath(testSourceSet.getRuntimeClasspath());

var cluster = clusterProvider.get();
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
nonInputProperties.systemProperty("tests.rest.cluster", () -> String.join(",", cluster.getAllHttpSocketURI()));
nonInputProperties.systemProperty("tests.cluster", () -> String.join(",", cluster.getAllTransportPortURI()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.gradle.Jdk;
import org.gradle.api.Task;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Nested;

import java.util.Collection;
Expand All @@ -32,5 +33,9 @@ default void useCluster(ElasticsearchCluster cluster) {
getClusters().add(cluster);
}

default void useCluster(Provider<ElasticsearchCluster> cluster) {
useCluster(cluster.get());
}

default void beforeStart() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ private NamedDomainObjectContainer<ElasticsearchCluster> createTestClustersConta
);
});
project.getExtensions().add(EXTENSION_NAME, container);
container.all(cluster -> cluster.systemProperty("ingest.geoip.downloader.enabled.default", "false"));
container.configureEach(cluster -> cluster.systemProperty("ingest.geoip.downloader.enabled.default", "false"));
return container;
}

Expand Down
2 changes: 1 addition & 1 deletion client/rest-high-level/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ tasks.named("check").configure {
dependsOn(asyncIntegTest)
}

testClusters.all {
testClusters.configureEach {
testDistribution = 'DEFAULT'
systemProperty 'es.scripting.update.ctx_in_params', 'false'
setting 'reindex.remote.whitelist', '[ "[::1]:*", "127.0.0.1:*" ]'
Expand Down
3 changes: 1 addition & 2 deletions distribution/archives/integ-test-zip/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ tasks.named("integTest").configure {
* when running against an external cluster.
*/
if (project.providers.systemProperty("tests.rest.cluster").forUseAtConfigurationTime().isPresent()) {
nonInputProperties.systemProperty 'tests.logfile',
"${-> testClusters.integTest.singleNode().getServerLog()}"
nonInputProperties.systemProperty 'tests.logfile', testClusters.named('integTest').map(c -> c.singleNode().serverLog)
} else {
systemProperty 'tests.logfile', '--external--'
}
Expand Down
2 changes: 1 addition & 1 deletion modules/ingest-common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ restResources {
}
}

testClusters.all {
testClusters.configureEach {
// Needed in order to test ingest pipeline templating:
// (this is because the integTest node is not using default distribution, but only the minimal number of required modules)
module ':modules:lang-mustache'
Expand Down
4 changes: 2 additions & 2 deletions modules/ingest-geoip/qa/file-based-update/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-test'

testClusters.all {
testClusters.configureEach {
testDistribution = 'DEFAULT'
setting 'resource.reload.interval.high', '100ms'
setting 'xpack.security.enabled', 'true'
Expand All @@ -18,7 +18,7 @@ testClusters.all {

tasks.named("integTest").configure {
systemProperty 'tests.security.manager', 'false' // Allows the test the add databases to config directory.
nonInputProperties.systemProperty 'tests.config.dir', "${-> testClusters.integTest.singleNode().getConfigDir()}"
nonInputProperties.systemProperty 'tests.config.dir', testClusters.named("integTest").map(c -> c.singleNode().getConfigDir())
}

tasks.named("forbiddenPatterns").configure {
Expand Down
2 changes: 1 addition & 1 deletion modules/ingest-user-agent/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ restResources {
}
}

testClusters.all {
testClusters.configureEach {
extraConfigFile 'ingest-user-agent/test-regexes.yml', file('src/test/test-regexes.yml')
}
2 changes: 1 addition & 1 deletion modules/kibana/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ dependencies {
api project(path: ':modules:reindex')
}

testClusters.all {
testClusters.configureEach {
module ':modules:reindex'
}

24 changes: 10 additions & 14 deletions modules/lang-painless/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ esplugin {
classname 'org.elasticsearch.painless.PainlessPlugin'
}

testClusters.all {
testClusters.configureEach {
module ':modules:mapper-extras'
systemProperty 'es.scripting.update.ctx_in_params', 'false'
// TODO: remove this once cname is prepended to transport.publish_address by default in 8.0
Expand Down Expand Up @@ -108,42 +108,38 @@ dependencies {
}
}

testClusters {
generateContextCluster {
testDistribution = 'DEFAULT'
}
def generateContextCluster = testClusters.register("generateContextCluster") {
testDistribution = 'DEFAULT'
}

tasks.register("generateContextDoc", DefaultTestClustersTask) {
dependsOn sourceSets.doc.runtimeClasspath
useCluster testClusters.generateContextCluster
useCluster generateContextCluster
doFirst {
project.javaexec {
mainClass = 'org.elasticsearch.painless.ContextDocGenerator'
classpath = sourceSets.doc.runtimeClasspath
systemProperty "cluster.uri", "${-> testClusters.generateContextCluster.singleNode().getAllHttpSocketURI().get(0)}"
systemProperty "cluster.uri", "${-> generateContextCluster.get().singleNode().getAllHttpSocketURI().get(0)}"
}.assertNormalExitValue()
}
}
/**********************************************
* Context JSON Generation *
**********************************************/
testClusters {
generateContextApiSpecCluster {
def generateContextApiSpecCluster = testClusters.register("generateContextApiSpecCluster") {
testDistribution = 'DEFAULT'
}
}

tasks.register("generateContextApiSpec", DefaultTestClustersTask) {
dependsOn sourceSets.doc.runtimeClasspath
useCluster testClusters.generateContextApiSpecCluster
useCluster generateContextApiSpecCluster
doFirst {
project.javaexec {
mainClass = 'org.elasticsearch.painless.ContextApiSpecGenerator'
classpath = sourceSets.doc.runtimeClasspath
systemProperty "cluster.uri", "${-> testClusters.generateContextApiSpecCluster.singleNode().getAllHttpSocketURI().get(0)}"
systemProperty "jdksrc", System.getProperty("jdksrc")
systemProperty "packageSources", System.getProperty("packageSources")
systemProperty "cluster.uri", "${-> generateContextApiSpecCluster.get().singleNode().getAllHttpSocketURI().get(0)}"
systemProperty "jdksrc", providers.systemProperty("jdksrc").getOrNull()
systemProperty "packageSources", providers.systemProperty("packageSources").forUseAtConfigurationTime().getOrNull()
}.assertNormalExitValue()
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/rank-eval/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ restResources {
}
}

testClusters.all {
testClusters.configureEach {
// Modules who's integration is explicitly tested in integration tests
module ':modules:lang-mustache'
}
4 changes: 2 additions & 2 deletions modules/reindex/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ esplugin {
hasClientJar = true
}

testClusters.all {
testClusters.configureEach {
// Modules who's integration is explicitly tested in integration tests
module ':modules:parent-join'
module ':modules:lang-painless'
Expand Down Expand Up @@ -151,7 +151,7 @@ if (Os.isFamily(Os.FAMILY_WINDOWS)) {
/* Use a closure on the string to delay evaluation until right before we
* run the integration tests so that we can be sure that the file is
* ready. */
nonInputProperties.systemProperty "es${version}.port", "${-> fixture.get().addressAndPort}"
nonInputProperties.systemProperty "es${version}.port", fixture.map(f->f.addressAndPort)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/repository-url/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def fixtureAddress = { fixtureName ->

File repositoryDir = fixture.fsRepositoryDir as File

testClusters.all {
testClusters.configureEach {
// repositoryDir is used by a FS repository to create snapshots
setting 'path.repo', "${repositoryDir.absolutePath}", PropertyNormalization.IGNORE_VALUE
// repositoryDir is used by two URL repositories to restore snapshots
Expand Down
2 changes: 1 addition & 1 deletion plugins/examples/custom-settings/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ esplugin {
noticeFile rootProject.file('NOTICE.txt')
}

testClusters.all {
testClusters.configureEach {
// Adds a setting in the Elasticsearch keystore before running the integration tests
keystore 'custom.secured', 'password'
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/examples/custom-suggester/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ esplugin {
noticeFile rootProject.file('NOTICE.txt')
}

testClusters.all {
testClusters.configureEach {
numberOfNodes = 2
}
2 changes: 1 addition & 1 deletion plugins/examples/painless-whitelist/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies {
compileOnly "org.elasticsearch.plugin:elasticsearch-scripting-painless-spi:${elasticsearchVersion}"
}

testClusters.all {
testClusters.configureEach {
testDistribution = 'DEFAULT'
setting 'xpack.security.enabled', 'false'
}
2 changes: 1 addition & 1 deletion qa/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import org.elasticsearch.gradle.testclusters.TestClustersPlugin

subprojects { Project subproj ->
plugins.withType(TestClustersPlugin).whenPluginAdded {
testClusters.all {
testClusters.configureEach {
testDistribution = 'DEFAULT'
setting "xpack.security.enabled", "false"
}
Expand Down
Loading