Complete testclusters backport (#47623)

* Use versions specific distribution folders so we don't need to clean up (#46539)

* Retry deleting distro dir on windows

When retarting the cluster we clean up old distribution files that might
still be in use by the OS.
Windows closes resources of ded processes async, so we do a couple of
retries to get arround it.

Closes #46014

* Avoid having to delete the distro folder.

* Remove the use of ClusterFormationTasks form RestTestTask (#47022)

This PR removes a use-case of the ClusterFormationTasks and converts a
project that flew under the radar so far.
There's probably more clean-up possible here, but for now the goal is
to be able to remove that code after `RunTask` is also updated.

* Migrate some 7.x only projects

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

@@ -53,6 +53,7 @@ class PluginBuildPlugin implements Plugin<Project> {
     @Override
     void apply(Project project) {
         project.pluginManager.apply(BuildPlugin)
+        project.pluginManager.apply(TestClustersPlugin)
 
         PluginPropertiesExtension extension = project.extensions.create(PLUGIN_EXTENSION_NAME, PluginPropertiesExtension, project)
         configureDependencies(project)

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy

@@ -26,55 +26,32 @@ import org.elasticsearch.gradle.tool.Boilerplate
 import org.elasticsearch.gradle.tool.ClasspathUtils
 import org.gradle.api.DefaultTask
 import org.gradle.api.Task
-import org.gradle.api.execution.TaskExecutionAdapter
 import org.gradle.api.file.FileCopyDetails
-import org.gradle.api.logging.Logger
-import org.gradle.api.logging.Logging
 import org.gradle.api.tasks.Copy
 import org.gradle.api.tasks.Input
-import org.gradle.api.tasks.TaskState
-import org.gradle.api.tasks.options.Option
 import org.gradle.api.tasks.testing.Test
 import org.gradle.plugins.ide.idea.IdeaPlugin
-import org.gradle.process.CommandLineArgumentProvider
-
-import java.nio.charset.StandardCharsets
-import java.nio.file.Files
-import java.util.stream.Stream
-
 /**
  * A wrapper task around setting up a cluster and running rest tests.
  */
 class RestIntegTestTask extends DefaultTask {
 
-    private static final Logger LOGGER = Logging.getLogger(RestIntegTestTask)
-
-    protected ClusterConfiguration clusterConfig
-
     protected Test runner
 
-    /** Info about nodes in the integ test cluster. Note this is *not* available until runtime. */
-    List<NodeInfo> nodes
-
     /** Flag indicating whether the rest tests in the rest spec should be run. */
     @Input
     Boolean includePackaged = false
 
     RestIntegTestTask() {
         runner = project.tasks.create("${name}Runner", RestTestRunnerTask.class)
         super.dependsOn(runner)
-        boolean usesTestclusters = project.plugins.hasPlugin(TestClustersPlugin.class)
-        if (usesTestclusters == false) {
-            clusterConfig = project.extensions.create("${name}Cluster", ClusterConfiguration.class, project)
-            runner.outputs.doNotCacheIf("Caching is disabled when using ClusterFormationTasks", { true })
-        } else {
-            project.testClusters {
+
+        project.testClusters {
                 "$name" {
                     javaHome = project.file(project.ext.runtimeJavaHome)
                 }
-            }
-            runner.useCluster project.testClusters."$name"
         }
+        runner.useCluster project.testClusters."$name"
 
         runner.include('**/*IT.class')
         runner.systemProperty('tests.rest.load_packaged', 'false')
@@ -83,38 +60,10 @@ class RestIntegTestTask extends DefaultTask {
             if (System.getProperty("tests.cluster") != null) {
                 throw new IllegalArgumentException("tests.rest.cluster and tests.cluster must both be null or non-null")
             }
-            if (usesTestclusters == true) {
-                ElasticsearchCluster cluster = project.testClusters."${name}"
-                runner.nonInputProperties.systemProperty('tests.rest.cluster', "${-> cluster.allHttpSocketURI.join(",") }")
-                runner.nonInputProperties.systemProperty('tests.cluster', "${-> cluster.transportPortURI }")
-            } else {
-                // we pass all nodes to the rest cluster to allow the clients to round-robin between them
-                // this is more realistic than just talking to a single node
-                runner.nonInputProperties.systemProperty('tests.rest.cluster', "${-> nodes.collect { it.httpUri() }.join(",")}")
-                runner.nonInputProperties.systemProperty('tests.config.dir', "${-> nodes[0].pathConf}")
-                // TODO: our "client" qa tests currently use the rest-test plugin. instead they should have their own plugin
-                // that sets up the test cluster and passes this transport uri instead of http uri. Until then, we pass
-                // both as separate sysprops
-                runner.nonInputProperties.systemProperty('tests.cluster', "${-> nodes[0].transportUri()}")
-
-                // dump errors and warnings from cluster log on failure
-                TaskExecutionAdapter logDumpListener = new TaskExecutionAdapter() {
-                    @Override
-                    void afterExecute(Task task, TaskState state) {
-                        if (task == runner && state.failure != null) {
-                            for (NodeInfo nodeInfo : nodes) {
-                                printLogExcerpt(nodeInfo)
-                            }
-                        }
-                    }
-                }
-                runner.doFirst {
-                    project.gradle.addListener(logDumpListener)
-                }
-                runner.doLast {
-                    project.gradle.removeListener(logDumpListener)
-                }
-            }
+            ElasticsearchCluster cluster = project.testClusters."${name}"
+            runner.nonInputProperties.systemProperty('tests.rest.cluster', "${-> cluster.allHttpSocketURI.join(",")}")
+            runner.nonInputProperties.systemProperty('tests.cluster', "${-> cluster.transportPortURI}")
+            runner.nonInputProperties.systemProperty('tests.clustername', "${-> cluster.getName()}")
         } else {
             if (System.getProperty("tests.cluster") == null) {
                 throw new IllegalArgumentException("tests.rest.cluster and tests.cluster must both be null or non-null")
@@ -135,13 +84,6 @@ class RestIntegTestTask extends DefaultTask {
                 runner.enabled = false
                 return // no need to add cluster formation tasks if the task won't run!
             }
-            if (usesTestclusters == false) {
-                // only create the cluster if needed as otherwise an external cluster to use was specified
-                if (System.getProperty("tests.rest.cluster") == null) {
-                    nodes = ClusterFormationTasks.setup(project, "${name}Cluster", runner, clusterConfig)
-                }
-                super.dependsOn(runner.finalizedBy)
-            }
         }
     }
 
@@ -150,18 +92,6 @@ class RestIntegTestTask extends DefaultTask {
         includePackaged = include
     }
 
-    @Option(
-        option = "debug-jvm",
-        description = "Enable debugging configuration, to allow attaching a debugger to elasticsearch."
-    )
-    public void setDebug(boolean enabled) {
-        clusterConfig.debug = enabled;
-    }
-
-    public List<NodeInfo> getNodes() {
-        return nodes
-    }
-
     @Override
     public Task dependsOn(Object... dependencies) {
         runner.dependsOn(dependencies)
@@ -187,44 +117,6 @@ class RestIntegTestTask extends DefaultTask {
         project.tasks.getByName("${name}Runner").configure(configure)
     }
 
-    /** Print out an excerpt of the log from the given node. */
-    protected static void printLogExcerpt(NodeInfo nodeInfo) {
-        File logFile = new File(nodeInfo.homeDir, "logs/${nodeInfo.clusterName}.log")
-        LOGGER.lifecycle("\nCluster ${nodeInfo.clusterName} - node ${nodeInfo.nodeNum} log excerpt:")
-        LOGGER.lifecycle("(full log at ${logFile})")
-        LOGGER.lifecycle('-----------------------------------------')
-        Stream<String> stream = Files.lines(logFile.toPath(), StandardCharsets.UTF_8)
-        try {
-            boolean inStartup = true
-            boolean inExcerpt = false
-            int linesSkipped = 0
-            for (String line : stream) {
-                if (line.startsWith("[")) {
-                    inExcerpt = false // clear with the next log message
-                }
-                if (line =~ /(\[WARN *\])|(\[ERROR *\])/) {
-                    inExcerpt = true // show warnings and errors
-                }
-                if (inStartup || inExcerpt) {
-                    if (linesSkipped != 0) {
-                        LOGGER.lifecycle("... SKIPPED ${linesSkipped} LINES ...")
-                    }
-                    LOGGER.lifecycle(line)
-                    linesSkipped = 0
-                } else {
-                    ++linesSkipped
-                }
-                if (line =~ /recovered \[\d+\] indices into cluster_state/) {
-                    inStartup = false
-                }
-            }
-        } finally {
-            stream.close()
-        }
-        LOGGER.lifecycle('=========================================')
-
-    }
-
     Copy createCopyRestSpecTask() {
         Boilerplate.maybeCreate(project.configurations, 'restSpec') {
             project.dependencies.add(

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestTestPlugin.groovy

@@ -19,6 +19,7 @@
 package org.elasticsearch.gradle.test
 
 import org.elasticsearch.gradle.BuildPlugin
+import org.elasticsearch.gradle.testclusters.TestClustersPlugin
 import org.gradle.api.InvalidUserDataException
 import org.gradle.api.Plugin
 import org.gradle.api.Project
@@ -43,6 +44,7 @@ public class RestTestPlugin implements Plugin<Project> {
                 + 'elasticsearch.standalone-rest-test')
         }
 
+        project.pluginManager.apply(TestClustersPlugin)
         RestIntegTestTask integTest = project.tasks.create('integTest', RestIntegTestTask.class)
         integTest.description = 'Runs rest tests against an elasticsearch cluster.'
         integTest.group = JavaBasePlugin.VERIFICATION_GROUP

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy

@@ -20,13 +20,12 @@
 
 package org.elasticsearch.gradle.test
 
-
 import groovy.transform.CompileStatic
 import org.elasticsearch.gradle.BuildPlugin
 import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask
-import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.info.GlobalBuildInfoPlugin
 import org.elasticsearch.gradle.precommit.PrecommitTasks
+import org.elasticsearch.gradle.testclusters.TestClustersPlugin
 import org.gradle.api.InvalidUserDataException
 import org.gradle.api.JavaVersion
 import org.gradle.api.Plugin
@@ -42,7 +41,6 @@ import org.gradle.api.tasks.compile.JavaCompile
 import org.gradle.api.tasks.testing.Test
 import org.gradle.plugins.ide.eclipse.model.EclipseModel
 import org.gradle.plugins.ide.idea.model.IdeaModel
-
 /**
  * Configures the build to compile tests against Elasticsearch's test framework
  * and run REST tests. Use BuildPlugin if you want to build main code as well
@@ -60,6 +58,7 @@ class StandaloneRestTestPlugin implements Plugin<Project> {
         }
         project.rootProject.pluginManager.apply(GlobalBuildInfoPlugin)
         project.pluginManager.apply(JavaBasePlugin)
+        project.pluginManager.apply(TestClustersPlugin)
 
         project.getTasks().create("buildResources", ExportElasticsearchBuildResourcesTask)
         BuildPlugin.configureRepositories(project)

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/TestWithDependenciesPlugin.groovy

@@ -56,7 +56,8 @@ class TestWithDependenciesPlugin implements Plugin<Project> {
 
     private static addPluginResources(Project project, Project pluginProject) {
         String outputDir = "${project.buildDir}/generated-resources/${pluginProject.name}"
-        String taskName = ClusterFormationTasks.pluginTaskName("copy", pluginProject.name, "Metadata")
+        String camelName = pluginProject.name.replaceAll(/-(\w)/) { _, c -> c.toUpperCase(Locale.ROOT) }
+        String taskName = "copy" + camelName[0].toUpperCase(Locale.ROOT) + camelName.substring(1) + "Metadata"
         Copy copyPluginMetadata = project.tasks.create(taskName, Copy.class)
         copyPluginMetadata.into(outputDir)
         copyPluginMetadata.from(pluginProject.tasks.pluginProperties)

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java

@@ -295,7 +295,9 @@ private void commonNodeConfig(ElasticsearchNode node, String nodeNames, Elastics
                 .filter(name -> name.startsWith("discovery.zen."))
                 .collect(Collectors.toList())
                 .forEach(node.defaultConfig::remove);
-            if (nodeNames != null) {
+            if (nodeNames != null &&
+                node.settings.getOrDefault("discovery.type", "anything").equals("single-node") == false
+            ) {
                 node.defaultConfig.put("cluster.initial_master_nodes", "[" + nodeNames + "]");
             }
             node.defaultConfig.put("discovery.seed_providers", "file");

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

@@ -140,7 +140,6 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private final Path esStdoutFile;
     private final Path esStderrFile;
     private final Path tmpDir;
-    private final Path distroDir;
 
     private int currentDistro = 0;
     private TestDistribution testDistribution;
@@ -156,7 +155,6 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         this.project = project;
         this.reaper = reaper;
         workingDir = workingDirBase.toPath().resolve(safeName(name)).toAbsolutePath();
-        distroDir = workingDir.resolve("distro");
         confPathRepo = workingDir.resolve("repo");
         configFile = workingDir.resolve("config/elasticsearch.yml");
         confPathData = workingDir.resolve("data");
@@ -183,6 +181,10 @@ public Version getVersion() {
         return distributions.get(currentDistro).getVersion();
     }
 
+    public Path getDistroDir() {
+        return workingDir.resolve("distro").resolve(getVersion() + "-" + testDistribution);
+    }
+
     @Override
     public void setVersion(String version) {
         requireNonNull(version, "null version passed when configuring test cluster `" + this + "`");
@@ -530,7 +532,7 @@ private void installModules() {
         if (testDistribution == TestDistribution.INTEG_TEST) {
             logToProcessStdout("Installing " + modules.size() + "modules");
             for (File module : modules) {
-                Path destination = distroDir.resolve("modules").resolve(module.getName().replace(".zip", "")
+                Path destination = getDistroDir().resolve("modules").resolve(module.getName().replace(".zip", "")
                     .replace("-" + getVersion(), "")
                     .replace("-SNAPSHOT", ""));
 
@@ -590,16 +592,16 @@ public void user(Map<String, String> userSpec) {
 
     private void runElaticsearchBinScriptWithInput(String input, String tool, String... args) {
         if (
-            Files.exists(distroDir.resolve("bin").resolve(tool)) == false &&
-                Files.exists(distroDir.resolve("bin").resolve(tool + ".bat")) == false
+            Files.exists(getDistroDir().resolve("bin").resolve(tool)) == false &&
+                Files.exists(getDistroDir().resolve("bin").resolve(tool + ".bat")) == false
         ) {
             throw new TestClustersException("Can't run bin script: `" + tool + "` does not exist. " +
                 "Is this the distribution you expect it to be ?");
         }
         try (InputStream byteArrayInputStream = new ByteArrayInputStream(input.getBytes(StandardCharsets.UTF_8))) {
             LoggedExec.exec(project, spec -> {
                 spec.setEnvironment(getESEnvironment());
-                spec.workingDir(distroDir);
+                spec.workingDir(getDistroDir());
                 spec.executable(
                     OS.conditionalString()
                         .onUnix(() -> "./bin/" + tool)
@@ -684,8 +686,8 @@ private void startElasticsearchProcess() {
         final ProcessBuilder processBuilder = new ProcessBuilder();
 
         List<String> command = OS.<List<String>>conditional()
-            .onUnix(() -> Arrays.asList(distroDir.getFileName().resolve("./bin/elasticsearch").toString()))
-            .onWindows(() -> Arrays.asList("cmd", "/c", distroDir.getFileName().resolve("bin\\elasticsearch.bat").toString()))
+            .onUnix(() -> Arrays.asList(workingDir.relativize(getDistroDir()).resolve("./bin/elasticsearch").toString()))
+            .onWindows(() -> Arrays.asList("cmd", "/c", workingDir.relativize(getDistroDir()).resolve("bin\\elasticsearch.bat").toString()))
             .supply();
         processBuilder.command(command);
         processBuilder.directory(workingDir.toFile());
@@ -915,7 +917,9 @@ private void waitForProcessToExit(ProcessHandle processHandle) {
     }
 
     private void createWorkingDir(Path distroExtractDir) throws IOException {
-        syncWithLinks(distroExtractDir, distroDir);
+        if (Files.exists(getDistroDir()) == false) {
+            syncWithLinks(distroExtractDir, getDistroDir());
+        }
         // Start configuration from scratch in case of a restart
         project.delete(configFile.getParent());
         Files.createDirectories(configFile.getParent());
@@ -934,10 +938,7 @@ private void createWorkingDir(Path distroExtractDir) throws IOException {
      * @param destinationRoot destination to link to
      */
     private void syncWithLinks(Path sourceRoot, Path destinationRoot) {
-        if (Files.exists(destinationRoot)) {
-            project.delete(destinationRoot);
-        }
-
+        assert Files.exists(destinationRoot) == false;
         try (Stream<Path> stream = Files.walk(sourceRoot)) {
             stream.forEach(source -> {
                 Path relativeDestination = sourceRoot.relativize(source);
@@ -1040,7 +1041,7 @@ private void createConfiguration() {
             );
 
             final List<Path> configFiles;
-            try (Stream<Path> stream = Files.list(distroDir.resolve("config"))) {
+            try (Stream<Path> stream = Files.list(getDistroDir().resolve("config"))) {
                 configFiles = stream.collect(Collectors.toList());
             }
             logToProcessStdout("Copying additional config files from distro " + configFiles);

distribution/archives/build.gradle

@@ -310,17 +310,13 @@ configure(subprojects.findAll { it.name == 'integ-test-zip' }) {
   group = "org.elasticsearch.distribution.integ-test-zip"
 
   integTest {
-    includePackaged = true
-  }
-
-  integTestCluster {
     dependsOn assemble
-    distribution = project.name
-  }
-  integTestRunner {
-    if (Os.isFamily(Os.FAMILY_WINDOWS) && System.getProperty('tests.timeoutSuite') == null) {
-      // override the suite timeout to 30 mins for windows, because it has the most inefficient filesystem known to man
-      systemProperty 'tests.timeoutSuite', '1800000!'
+    includePackaged = true
+    runner {
+      if (Os.isFamily(Os.FAMILY_WINDOWS) && System.getProperty('tests.timeoutSuite') == null) {
+        // override the suite timeout to 30 mins for windows, because it has the most inefficient filesystem known to man
+        systemProperty 'tests.timeoutSuite', '1800000!'
+      }
     }
   }
 

distribution/archives/integ-test-zip/build.gradle

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-integTestRunner {
+integTest.runner {
   /*
    * There are two unique things going on here:
    * 1. These tests can be run against an external cluster with
@@ -27,7 +27,7 @@ integTestRunner {
    */
   if (System.getProperty("tests.rest.cluster") == null) {
     nonInputProperties.systemProperty 'tests.logfile',
-      "${ -> integTest.nodes[0].homeDir}/logs/${ -> integTest.nodes[0].clusterName }_server.json"
+      "${ -> testClusters.integTest.singleNode().getServerLog()}"
   } else {
     systemProperty 'tests.logfile', '--external--'
   }