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.
elastic · Sep 25, 2019 · 1482fc0 · 1482fc0
1 parent 69422b9
commit 1482fc0
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 131 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
@@ -50,6 +50,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)

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy
@@ -26,54 +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 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')
@@ -82,40 +60,11 @@ class RestIntegTestTask extends DefaultTask {
             if (System.getProperty("tests.cluster") != null || System.getProperty("tests.clustername") != null) {
                 throw new IllegalArgumentException("tests.rest.cluster, tests.cluster, and tests.clustername must all 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}")
-                runner.nonInputProperties.systemProperty('tests.clustername', "${-> cluster.getName()}")
-            } 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()}")
-                runner.nonInputProperties.systemProperty('tests.clustername', "${-> nodes[0].clusterName}")
 
-                // 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 || System.getProperty("tests.clustername") == null) {
                 throw new IllegalArgumentException("tests.rest.cluster, tests.cluster, and tests.clustername must all be null or non-null")
@@ -137,13 +86,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)
-            }
         }
     }
 
@@ -152,17 +94,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) {
@@ -189,44 +120,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(

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestTestPlugin.groovy b/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

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy b/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)

diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/TestWithDependenciesPlugin.groovy b/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)

diff --git a/distribution/archives/build.gradle b/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!'
+      }
     }
   }
 

diff --git a/distribution/archives/integ-test-zip/build.gradle b/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--'
   }

diff --git a/...es/integ-test-zip/src/test/java/org/elasticsearch/test/rest/JsonLogsFormatAndParseIT.java b/...es/integ-test-zip/src/test/java/org/elasticsearch/test/rest/JsonLogsFormatAndParseIT.java
@@ -35,7 +35,7 @@
 public class JsonLogsFormatAndParseIT extends JsonLogsIntegTestCase {
     @Override
     protected Matcher<String> nodeNameMatcher() {
-        return is("node-0");
+        return is("integTest-0");
     }
 
     @Override