Skip to content

Do not create unused testCluster #77581

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
Show all changes
30 commits
Select commit Hold shift + click to select a range
e85c8f2
Do not create unused testCluster
breskeby Sep 10, 2021
ff5ea3a
Do not fail on run task (debug)
breskeby Sep 10, 2021
44b6f78
Create more test cluster lazy
breskeby Sep 10, 2021
f1a12d9
Make more test cluster lazy
breskeby Sep 11, 2021
45ced90
Avoid creating unused testcluster
breskeby Sep 13, 2021
11d05e3
Fix PluginBuildPlugin
breskeby Sep 13, 2021
c53c242
Fix disabling geo db download
breskeby Sep 13, 2021
6d70cab
Fix cluster setup in repository-multi-version
breskeby Sep 13, 2021
6b9298d
Polishing
breskeby Sep 13, 2021
4dcfde8
Fix issue with irretic groovy ogic
breskeby Sep 13, 2021
2ee6420
Fix bwc tests
breskeby Sep 13, 2021
32d6688
Fix more bwcTests
breskeby Sep 13, 2021
1a803ff
Fix more bwc tests
breskeby Sep 13, 2021
b31c6d7
Fix more bwc tests
breskeby Sep 13, 2021
b701692
Fix more bwc tests
breskeby Sep 13, 2021
65aa706
Fix typo
breskeby Sep 13, 2021
fd9139a
Minor polishing
breskeby Sep 13, 2021
3695edd
Fix rolling upgrade tests
breskeby Sep 16, 2021
d1974a5
Fix cluster config in sql qa mixedcluster project
breskeby Sep 20, 2021
54aa231
Fix more bwc tests
breskeby Sep 20, 2021
cf33648
Clean up before review
breskeby Sep 21, 2021
e05e5c2
Document test cluster usage
breskeby Sep 21, 2021
23f1fab
Api polising after Review
breskeby Sep 22, 2021
f7dab15
Allow gradle provider as value for nonSystemProperties
breskeby Sep 22, 2021
9d76d4b
Some simplification on test configuration
breskeby Sep 22, 2021
5bd6919
Fix typo in rest test config
breskeby Sep 22, 2021
92a11c5
Fix more typos
breskeby Sep 22, 2021
7a00e4c
Fix another typo
breskeby Sep 22, 2021
69e3d1a
Fix more typos
breskeby Sep 22, 2021
1fd16dd
Merge branch 'master' into create-less-unused-cluster
breskeby Sep 23, 2021
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
6 changes: 2 additions & 4 deletions build-tools-internal/src/main/groovy/elasticsearch.run.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import org.elasticsearch.gradle.testclusters.RunTask
// precompiled script plugins (see https://github.com/gradle/gradle/issues/17004)
// apply plugin: 'elasticsearch.internal-testclusters'

testClusters {
runTask {
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()
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 @@ -77,7 +77,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 var extension1 = project1.getExtensions().getByType(PluginPropertiesExtension.class);
Expand Down Expand Up @@ -120,17 +120,17 @@ public void apply(final Project project) {

// allow running ES with this plugin in the foreground of a build
var testClusters = testClusters(project, TestClustersPlugin.EXTENSION_NAME);
final var runCluster = testClusters.create("runTask", cluster -> {
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.gradle.api.Action;
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 @@ -31,5 +32,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 @@ -121,7 +121,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 @@ -93,7 +93,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 @@ -29,7 +29,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 @@ -19,7 +19,7 @@ restResources {
}
}

testClusters.all {
testClusters.configureEach {
extraConfigFile 'ingest-user-agent/test-regexes.yml', file('src/test/test-regexes.yml')
}

Expand Down
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'
}

22 changes: 9 additions & 13 deletions modules/lang-painless/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,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 @@ -109,41 +109,37 @@ 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", providers.systemProperty("jdksrc").forUseAtConfigurationTime().getOrNull()
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 {
classname 'org.elasticsearch.reindex.ReindexPlugin'
}

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 @@ -54,7 +54,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'
}
}
Expand Down
Loading