[7.7] Lazy test cluster module and plugins (#54852) (#55087) (#55118)

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.
elastic · Apr 13, 2020 · 6b2c398 · 6b2c398
1 parent ee49268
commit 6b2c398
Show file tree

Hide file tree

Showing 18 changed files with 119 additions and 66 deletions.
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy
@@ -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')
@@ -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)
@@ -122,6 +118,7 @@ class PluginBuildPlugin implements Plugin<Project> {
                 addNoticeGeneration(project, extension1)
             }
         }
+
         project.tasks.named('testingConventions').configure {
             naming.clear()
             naming {
@@ -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)

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java
@@ -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;
 
@@ -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));

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java
@@ -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;
@@ -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);
@@ -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)) {
@@ -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
@@ -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) {
@@ -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);
@@ -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
@@ -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;
     }
 

diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java
@@ -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;
@@ -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);

diff --git a/docs/build.gradle b/docs/build.gradle
@@ -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
   }

diff --git a/modules/ingest-common/build.gradle b/modules/ingest-common/build.gradle
@@ -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
 }
diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle
@@ -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'
@@ -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'
   }
 }

diff --git a/modules/rank-eval/build.gradle b/modules/rank-eval/build.gradle
@@ -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
 }
diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle
@@ -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:*'
 }
@@ -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'
   }
 }
 

diff --git a/plugins/discovery-ec2/qa/amazon-ec2/build.gradle b/plugins/discovery-ec2/qa/amazon-ec2/build.gradle
@@ -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_'

diff --git a/plugins/discovery-gce/qa/gce/build.gradle b/plugins/discovery-gce/qa/gce/build.gradle
@@ -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`)

diff --git a/plugins/repository-azure/qa/microsoft-azure-storage/build.gradle b/plugins/repository-azure/qa/microsoft-azure-storage/build.gradle
@@ -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

diff --git a/plugins/repository-gcs/qa/google-cloud-storage/build.gradle b/plugins/repository-gcs/qa/google-cloud-storage/build.gradle
@@ -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