Skip to content

Commit

Permalink
[7.7] Lazy test cluster module and plugins (elastic#54852) (elastic#5…
Browse files Browse the repository at this point in the history 
…5087)

This change converts the module and plugin parameters
for testClusters to be lazy. Meaning that the values
are not resolved until they are actually used. This
removes the requirement to use project.afterEvaluate to
be able to resolve the bundle artifact.

Note - this does not completely remove the need for afterEvaluate
since it is still needed for the custom resource extension.
  • Loading branch information
@jakelandis
jakelandis committed Apr 13, 2020
1 parent ee49268 commit 884fa4b
Show file tree
Hide file tree
Showing 18 changed files with 119 additions and 66 deletions.
62 changes: 29 additions & 33 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,37 +61,33 @@ class PluginBuildPlugin implements Plugin<Project> {
PluginPropertiesExtension extension = project.extensions.create(PLUGIN_EXTENSION_NAME, PluginPropertiesExtension, project)
configureDependencies(project)

// this afterEvaluate must happen before the afterEvaluate added by integTest creation,
// so that the file name resolution for installing the plugin will be setup
project.afterEvaluate {
boolean isXPackModule = project.path.startsWith(':x-pack:plugin')
boolean isModule = project.path.startsWith(':modules:') || isXPackModule
PluginPropertiesExtension extension1 = project.getExtensions().getByType(PluginPropertiesExtension.class)
String name = extension1.name
project.archivesBaseName = name
project.description = extension1.description
configurePublishing(project, extension1)
boolean isXPackModule = project.path.startsWith(':x-pack:plugin')
boolean isModule = project.path.startsWith(':modules:') || isXPackModule

project.tasks.integTest.dependsOn(project.tasks.bundlePlugin)
if (isModule) {
project.testClusters.integTest.module(
project.file(project.tasks.bundlePlugin.archiveFile)
)
} else {
project.testClusters.integTest.plugin(
project.file(project.tasks.bundlePlugin.archiveFile)
)
}
createIntegTestTask(project)
createBundleTasks(project, extension)
project.tasks.integTest.dependsOn(project.tasks.bundlePlugin)
if (isModule) {
project.testClusters.integTest.module(project.tasks.bundlePlugin.archiveFile)
} else {
project.testClusters.integTest.plugin(project.tasks.bundlePlugin.archiveFile)
}

project.afterEvaluate {
project.extensions.getByType(PluginPropertiesExtension).extendedPlugins.each { pluginName ->
// Auto add dependent modules to the test cluster
if (project.findProject(":modules:${pluginName}") != null) {
project.integTest.dependsOn(project.project(":modules:${pluginName}").tasks.bundlePlugin)
project.testClusters.integTest.module(
project.file(project.project(":modules:${pluginName}").tasks.bundlePlugin.archiveFile)
project.project(":modules:${pluginName}").tasks.bundlePlugin.archiveFile
)
}
}
PluginPropertiesExtension extension1 = project.getExtensions().getByType(PluginPropertiesExtension.class)
configurePublishing(project, extension1)
String name = extension1.name
project.archivesBaseName = name
project.description = extension1.description

if (extension1.name == null) {
throw new InvalidUserDataException('name is a required setting for esplugin')
Expand All @@ -104,15 +100,15 @@ class PluginBuildPlugin implements Plugin<Project> {
}

Map<String, String> properties = [
'name' : extension1.name,
'description' : extension1.description,
'version' : extension1.version,
'elasticsearchVersion': Version.fromString(VersionProperties.elasticsearch).toString(),
'javaVersion' : project.targetCompatibility as String,
'classname' : extension1.classname,
'extendedPlugins' : extension1.extendedPlugins.join(','),
'hasNativeController' : extension1.hasNativeController,
'requiresKeystore' : extension1.requiresKeystore
'name' : extension1.name,
'description' : extension1.description,
'version' : extension1.version,
'elasticsearchVersion': Version.fromString(VersionProperties.elasticsearch).toString(),
'javaVersion' : project.targetCompatibility as String,
'classname' : extension1.classname,
'extendedPlugins' : extension1.extendedPlugins.join(','),
'hasNativeController' : extension1.hasNativeController,
'requiresKeystore' : extension1.requiresKeystore
]
project.tasks.named('pluginProperties').configure {
expand(properties)
Expand All @@ -122,6 +118,7 @@ class PluginBuildPlugin implements Plugin<Project> {
addNoticeGeneration(project, extension1)
}
}

project.tasks.named('testingConventions').configure {
naming.clear()
naming {
Expand All @@ -135,9 +132,8 @@ class PluginBuildPlugin implements Plugin<Project> {
}
}
}
createIntegTestTask(project)
createBundleTasks(project, extension)
project.configurations.getByName('default').extendsFrom(project.configurations.getByName('runtime'))
project.configurations.getByName('default')
.extendsFrom(project.configurations.getByName('runtimeClasspath'))
// allow running ES with this plugin in the foreground of a build
project.tasks.register('run', RunTask) {
dependsOn(project.tasks.bundlePlugin)
Expand Down
18 changes: 18 additions & 0 deletions buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import org.gradle.api.Named;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
import org.gradle.api.file.RegularFile;
import org.gradle.api.file.RegularFileProperty;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.Nested;

Expand Down Expand Up @@ -144,11 +147,26 @@ public void plugin(File plugin) {
nodes.all(each -> each.plugin(plugin));
}

@Override
public void plugin(Provider<URI> plugin) {
nodes.all(each -> each.plugin(plugin));
}

@Override
public void plugin(RegularFileProperty plugin) {
nodes.all(each -> each.plugin(plugin));
}

@Override
public void module(File module) {
nodes.all(each -> each.module(module));
}

@Override
public void module(Provider<RegularFile> module) {
nodes.all(each -> each.module(module));
}

@Override
public void keystore(String key, String value) {
nodes.all(each -> each.keystore(key, value));
Expand Down
56 changes: 43 additions & 13 deletions buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@
import org.gradle.api.Named;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
import org.gradle.api.file.RegularFile;
import org.gradle.api.file.RegularFileProperty;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.Classpath;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFile;
Expand Down Expand Up @@ -122,8 +126,8 @@ public class ElasticsearchNode implements TestClusterConfiguration {
private final Path workingDir;

private final LinkedHashMap<String, Predicate<TestClusterConfiguration>> waitConditions = new LinkedHashMap<>();
private final List<URI> plugins = new ArrayList<>();
private final List<File> modules = new ArrayList<>();
private final List<Provider<URI>> plugins = new ArrayList<>();
private final List<Provider<File>> modules = new ArrayList<>();
final LazyPropertyMap<String, CharSequence> settings = new LazyPropertyMap<>("Settings", this);
private final LazyPropertyMap<String, CharSequence> keystoreSettings = new LazyPropertyMap<>("Keystore", this);
private final LazyPropertyMap<String, File> keystoreFiles = new LazyPropertyMap<>("Keystore files", this, FileEntry::new);
Expand Down Expand Up @@ -264,7 +268,12 @@ private void setDistributionType(ElasticsearchDistribution distribution, TestDis
}

@Override
public void plugin(URI plugin) {
public void plugin(RegularFileProperty plugin) {
this.plugins.add(plugin.map(p -> p.getAsFile().toURI()));
}

@Override
public void plugin(Provider<URI> plugin) {
requireNonNull(plugin, "Plugin name can't be null");
checkFrozen();
if (plugins.contains(plugin)) {
Expand All @@ -273,14 +282,30 @@ public void plugin(URI plugin) {
this.plugins.add(plugin);
}

@Override
public void plugin(URI plugin) {
Property<URI> uri = project.getObjects().property(URI.class);
uri.set(plugin);
this.plugin(uri);
}

@Override
public void plugin(File plugin) {
plugin(plugin.toURI());
Property<URI> uri = project.getObjects().property(URI.class);
uri.set(plugin.toURI());
this.plugin(uri);
}

@Override
public void module(File module) {
this.modules.add(module);
RegularFileProperty file = project.getObjects().fileProperty();
file.fileValue(module);
this.module(file);
}

@Override
public void module(Provider<RegularFile> module) {
this.modules.add(module.map(RegularFile::getAsFile));
}

@Override
Expand Down Expand Up @@ -426,7 +451,7 @@ public synchronized void start() {

final List<String> pluginsToInstall = new ArrayList<>();
if (plugins.isEmpty() == false) {
pluginsToInstall.addAll(plugins.stream().map(URI::toString).collect(Collectors.toList()));
pluginsToInstall.addAll(plugins.stream().map(Provider::get).map(URI::toString).collect(Collectors.toList()));
}

if (getVersion().before("6.3.0") && testDistribution == TestDistribution.DEFAULT) {
Expand Down Expand Up @@ -577,16 +602,15 @@ private void copyExtraJars() {
private void installModules() {
if (testDistribution == TestDistribution.INTEG_TEST) {
logToProcessStdout("Installing " + modules.size() + "modules");
for (File module : modules) {
for (Provider<File> module : modules) {
Path destination = getDistroDir().resolve("modules")
.resolve(module.getName().replace(".zip", "").replace("-" + getVersion(), "").replace("-SNAPSHOT", ""));

.resolve(module.get().getName().replace(".zip", "").replace("-" + getVersion(), "").replace("-SNAPSHOT", ""));
// only install modules that are not already bundled with the integ-test distribution
if (Files.exists(destination) == false) {
project.copy(spec -> {
if (module.getName().toLowerCase().endsWith(".zip")) {
if (module.get().getName().toLowerCase().endsWith(".zip")) {
spec.from(project.zipTree(module));
} else if (module.isDirectory()) {
} else if (module.get().isDirectory()) {
spec.from(module);
} else {
throw new IllegalArgumentException("Not a valid module " + module + " for " + this);
Expand Down Expand Up @@ -1155,7 +1179,10 @@ private Path getExtractedDistributionDir() {
}

private List<File> getInstalledFileSet(Action<? super PatternFilterable> filter) {
return Stream.concat(plugins.stream().filter(uri -> uri.getScheme().equalsIgnoreCase("file")).map(File::new), modules.stream())
return Stream.concat(
plugins.stream().map(Provider::get).filter(uri -> uri.getScheme().equalsIgnoreCase("file")).map(File::new),
modules.stream().map(p -> p.get())
)
.filter(File::exists)
// TODO: We may be able to simplify this with Gradle 5.6
// https://docs.gradle.org/nightly/release-notes.html#improved-handling-of-zip-archives-on-classpaths
Expand All @@ -1167,7 +1194,10 @@ private List<File> getInstalledFileSet(Action<? super PatternFilterable> filter)

@Input
public Set<URI> getRemotePlugins() {
Set<URI> file = plugins.stream().filter(uri -> uri.getScheme().equalsIgnoreCase("file") == false).collect(Collectors.toSet());
Set<URI> file = plugins.stream()
.map(Provider::get)
.filter(uri -> uri.getScheme().equalsIgnoreCase("file") == false)
.collect(Collectors.toSet());
return file;
}

Expand Down
9 changes: 9 additions & 0 deletions buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@

import org.elasticsearch.gradle.FileSupplier;
import org.elasticsearch.gradle.PropertyNormalization;
import org.gradle.api.file.RegularFile;
import org.gradle.api.file.RegularFileProperty;
import org.gradle.api.logging.Logging;
import org.gradle.api.provider.Provider;
import org.slf4j.Logger;

import java.io.File;
Expand All @@ -45,8 +48,14 @@ public interface TestClusterConfiguration {

void plugin(File plugin);

void plugin(Provider<URI> plugin);

void plugin(RegularFileProperty plugin);

void module(File module);

void module(Provider<RegularFile> module);

void keystore(String key, String value);

void keystore(String key, Supplier<CharSequence> valueSupplier);
Expand Down
2 changes: 1 addition & 1 deletion docs/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ project.rootProject.subprojects.findAll { it.parent.path == ':plugins' }.each {
// FIXME
subproj.afterEvaluate { // need to wait until the project has been configured
testClusters.integTest {
plugin file(subproj.bundlePlugin.archiveFile)
plugin subproj.bundlePlugin.archiveFile
}
tasks.integTest.dependsOn subproj.bundlePlugin
}
Expand Down
2 changes: 1 addition & 1 deletion modules/ingest-common/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ restResources {
testClusters.integTest {
// 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 file(project(':modules:lang-mustache').tasks.bundlePlugin.archiveFile)
module project(':modules:lang-mustache').tasks.bundlePlugin.archiveFile
}
4 changes: 2 additions & 2 deletions modules/lang-painless/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ esplugin {
}

testClusters.integTest {
module file(project(':modules:mapper-extras').tasks.bundlePlugin.archiveFile)
module project(':modules:mapper-extras').tasks.bundlePlugin.archiveFile
systemProperty 'es.scripting.update.ctx_in_params', 'false'
// TODO: remove this once cname is prepended to transport.publish_address by default in 8.0
systemProperty 'es.transport.cname_in_publish_address', 'true'
Expand All @@ -46,7 +46,7 @@ dependencyLicenses {

restResources {
restApi {
includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'search', 'get', 'bulk', 'update',
includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'search', 'get', 'bulk', 'update',
'scripts_painless_execute', 'put_script', 'delete_script'
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/rank-eval/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ esplugin {

restResources {
restApi {
includeCore '_common', 'indices', 'index', 'rank_eval'
includeCore '_common', 'indices', 'index', 'rank_eval'
}
}

testClusters.integTest {
// Modules who's integration is explicitly tested in integration tests
module file(project(':modules:lang-mustache').tasks.bundlePlugin.archiveFile)
module project(':modules:lang-mustache').tasks.bundlePlugin.archiveFile
}
8 changes: 4 additions & 4 deletions modules/reindex/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ esplugin {

testClusters.integTest {
// Modules who's integration is explicitly tested in integration tests
module file(project(':modules:parent-join').tasks.bundlePlugin.archiveFile)
module file(project(':modules:lang-painless').tasks.bundlePlugin.archiveFile)
module project(':modules:parent-join').tasks.bundlePlugin.archiveFile
module project(':modules:lang-painless').tasks.bundlePlugin.archiveFile
// Whitelist reindexing from the local node so we can test reindex-from-remote.
setting 'reindex.remote.whitelist', '127.0.0.1:*'
}
Expand All @@ -61,8 +61,8 @@ dependencies {

restResources {
restApi {
includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'get', 'search', 'mget', 'count',
'update_by_query', 'delete_by_query', 'reindex_rethrottle', 'tasks', 'reindex', 'put_script'
includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'get', 'search', 'mget', 'count',
'update_by_query', 'delete_by_query', 'reindex_rethrottle', 'tasks', 'reindex', 'put_script'
}
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/discovery-ec2/qa/amazon-ec2/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ integTest.enabled = false

testClusters."integTest${action}" {
numberOfNodes = ec2NumberOfNodes
plugin file(project(':plugins:discovery-ec2').bundlePlugin.archiveFile)
plugin project(':plugins:discovery-ec2').bundlePlugin.archiveFile

setting 'discovery.seed_providers', 'ec2'
setting 'network.host', '_ec2_'
Expand Down
2 changes: 1 addition & 1 deletion plugins/discovery-gce/qa/gce/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ integTest {

testClusters.integTest {
numberOfNodes = gceNumberOfNodes
plugin file(project(':plugins:discovery-gce').bundlePlugin.archiveFile)
plugin project(':plugins:discovery-gce').bundlePlugin.archiveFile
// use gce fixture for Auth calls instead of http://metadata.google.internal
environment 'GCE_METADATA_HOST', { "http://${gceFixture.addressAndPort}" }, IGNORE_VALUE
// allows to configure hidden settings (`cloud.gce.host` and `cloud.gce.root_url`)
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-azure/qa/microsoft-azure-storage/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ integTest {
}

testClusters.integTest {
plugin file(project(':plugins:repository-azure').bundlePlugin.archiveFile)
plugin project(':plugins:repository-azure').bundlePlugin.archiveFile
keystore 'azure.client.integration_test.account', azureAccount
if (azureKey != null && azureKey.isEmpty() == false) {
keystore 'azure.client.integration_test.key', azureKey
Expand Down
2 changes: 1 addition & 1 deletion plugins/repository-gcs/qa/google-cloud-storage/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ integTest {
}

testClusters.integTest {
plugin file(project(':plugins:repository-gcs').bundlePlugin.archiveFile)
plugin project(':plugins:repository-gcs').bundlePlugin.archiveFile

keystore 'gcs.client.integration_test.credentials_file', serviceAccountFile, IGNORE_VALUE

Expand Down
Loading

0 comments on commit 884fa4b

Please sign in to comment.