Simplify log handling of test clusters (#68333)

buildSrc/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy

@@ -9,10 +9,7 @@
 package org.elasticsearch.gradle
 
 import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest
-import org.gradle.testkit.runner.GradleRunner
 import spock.lang.IgnoreIf
-import spock.lang.Requires
-import spock.util.environment.OperatingSystem
 
 import static org.elasticsearch.gradle.fixtures.DistributionDownloadFixture.withMockedDistributionDownload
 
@@ -59,8 +56,8 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
 
         then:
         result.output.contains("elasticsearch-keystore script executed!")
-        assertEsStdoutContains("myCluster", "Starting Elasticsearch process")
-        assertEsStdoutContains("myCluster", "Stopping node")
+        assertEsLogContains("myCluster", "Starting Elasticsearch process")
+        assertEsLogContains("myCluster", "Stopping node")
         assertNoCustomDistro('myCluster')
     }
 
@@ -84,8 +81,8 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
 
         then:
         result.output.contains("elasticsearch-keystore script executed!")
-        assertEsStdoutContains("myCluster", "Starting Elasticsearch process")
-        assertEsStdoutContains("myCluster", "Stopping node")
+        assertEsLogContains("myCluster", "Starting Elasticsearch process")
+        assertEsLogContains("myCluster", "Stopping node")
         assertNoCustomDistro('myCluster')
     }
 
@@ -111,14 +108,14 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
 
         then:
         result.output.contains("elasticsearch-keystore script executed!")
-        assertEsStdoutContains("myCluster", "Starting Elasticsearch process")
-        assertEsStdoutContains("myCluster", "Stopping node")
+        assertEsLogContains("myCluster", "Starting Elasticsearch process")
+        assertEsLogContains("myCluster", "Stopping node")
         assertCustomDistro('myCluster')
     }
 
-    boolean assertEsStdoutContains(String testCluster, String expectedOutput) {
+    boolean assertEsLogContains(String testCluster, String expectedOutput) {
         assert new File(testProjectDir.root,
-                "build/testclusters/${testCluster}-0/logs/es.stdout.log").text.contains(expectedOutput)
+                "build/testclusters/${testCluster}-0/logs/${testCluster}.log").text.contains(expectedOutput)
         true
     }
 

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

@@ -81,6 +81,7 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
         this.nodes = project.container(ElasticsearchNode.class);
         this.nodes.add(
             new ElasticsearchNode(
+                safeName(clusterName),
                 path,
                 clusterName + "-0",
                 project,
@@ -91,8 +92,6 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
                 workingDirBase
             )
         );
-        // configure the cluster name eagerly so nodes know about it
-        this.nodes.all((node) -> node.defaultConfig.put("cluster.name", safeName(clusterName)));
 
         addWaitForClusterHealth();
     }
@@ -113,6 +112,7 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named {
         for (int i = nodes.size(); i < numberOfNodes; i++) {
             this.nodes.add(
                 new ElasticsearchNode(
+                    safeName(clusterName),
                     path,
                     clusterName + "-" + i,
                     project,

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

@@ -109,6 +109,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private static final String HOSTNAME_OVERRIDE = "LinuxDarwinHostname";
     private static final String COMPUTERNAME_OVERRIDE = "WindowsComputername";
 
+    private final String clusterName;
     private final String path;
     private final String name;
     private final Project project;
@@ -140,8 +141,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private final Path confPathLogs;
     private final Path transportPortFile;
     private final Path httpPortsFile;
-    private final Path esStdoutFile;
-    private final Path esStderrFile;
+    private final Path esLogFile;
     private final Path esStdinFile;
     private final Path tmpDir;
 
@@ -158,6 +158,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private boolean preserveDataDir = false;
 
     ElasticsearchNode(
+        String clusterName,
         String path,
         String name,
         Project project,
@@ -167,6 +168,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         ExecOperations execOperations,
         File workingDirBase
     ) {
+        this.clusterName = clusterName;
         this.path = path;
         this.name = name;
         this.project = project;
@@ -181,11 +183,11 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         confPathLogs = workingDir.resolve("logs");
         transportPortFile = confPathLogs.resolve("transport.ports");
         httpPortsFile = confPathLogs.resolve("http.ports");
-        esStdoutFile = confPathLogs.resolve("es.stdout.log");
-        esStderrFile = confPathLogs.resolve("es.stderr.log");
+        esLogFile = confPathLogs.resolve(clusterName + ".log");
         esStdinFile = workingDir.resolve("es.stdin");
         tmpDir = workingDir.resolve("tmp");
         waitConditions.put("ports files", this::checkPortsFilesExistWithDelay);
+        defaultConfig.put("cluster.name", clusterName);
 
         setTestDistribution(TestDistribution.INTEG_TEST);
         setVersion(VersionProperties.getElasticsearch());
@@ -435,7 +437,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
      * @return stream of log lines
      */
     public Stream<String> logLines() throws IOException {
-        return Files.lines(esStdoutFile, StandardCharsets.UTF_8);
+        return Files.lines(esLogFile, StandardCharsets.UTF_8);
     }
 
     @Override
@@ -551,11 +553,11 @@ public class ElasticsearchNode implements TestClusterConfiguration {
 
     private void logToProcessStdout(String message) {
         try {
-            if (Files.exists(esStdoutFile.getParent()) == false) {
-                Files.createDirectories(esStdoutFile.getParent());
+            if (Files.exists(esLogFile.getParent()) == false) {
+                Files.createDirectories(esLogFile.getParent());
             }
             Files.write(
-                esStdoutFile,
+                esLogFile,
                 ("[" + Instant.now().toString() + "] [BUILD] " + message + "\n").getBytes(StandardCharsets.UTF_8),
                 StandardOpenOption.CREATE,
                 StandardOpenOption.APPEND
@@ -793,9 +795,9 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         environment.clear();
         environment.putAll(getESEnvironment());
 
-        // don't buffer all in memory, make sure we don't block on the default pipes
-        processBuilder.redirectError(ProcessBuilder.Redirect.appendTo(esStderrFile.toFile()));
-        processBuilder.redirectOutput(ProcessBuilder.Redirect.appendTo(esStdoutFile.toFile()));
+        // Just toss the output since we rely on the normal log file written by Elasticsearch
+        processBuilder.redirectOutput(ProcessBuilder.Redirect.DISCARD);
+        processBuilder.redirectError(ProcessBuilder.Redirect.DISCARD);
 
         if (keystorePassword != null && keystorePassword.length() > 0) {
             try {
@@ -892,8 +894,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
-        logFileContents("Standard output of node", esStdoutFile, tailLogs);
-        logFileContents("Standard error of node", esStderrFile, tailLogs);
+        logFileContents("Log output of node", esLogFile, tailLogs);
     }
 
     @Override
@@ -962,7 +963,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
                         String previousMessage = normalizeLogLine(ring.getLast());
                         if (MESSAGES_WE_DONT_CARE_ABOUT.stream().noneMatch(previousMessage::contains)
                             && (previousMessage.contains("ERROR") || previousMessage.contains("WARN"))) {
-                            errorsAndWarnings.put(previousMessage, errorsAndWarnings.getOrDefault(previousMessage, 0) + 1);
+                            errorsAndWarnings.put(ring.getLast(), errorsAndWarnings.getOrDefault(previousMessage, 0) + 1);
                         }
                     } else {
                         // We combine multi line log messages to make sure we never break exceptions apart
@@ -1453,13 +1454,8 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     }
 
     @Internal
-    Path getEsStdoutFile() {
-        return esStdoutFile;
-    }
-
-    @Internal
-    Path getEsStderrFile() {
-        return esStderrFile;
+    Path getEsLogFile() {
+        return esLogFile;
     }
 
     private static class FileEntry implements Named {

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

@@ -144,7 +144,7 @@ public class RunTask extends DefaultTestClustersTask {
         try {
             for (ElasticsearchCluster cluster : getClusters()) {
                 for (ElasticsearchNode node : cluster.getNodes()) {
-                    BufferedReader reader = Files.newBufferedReader(node.getEsStdoutFile());
+                    BufferedReader reader = Files.newBufferedReader(node.getEsLogFile());
                     toRead.add(reader);
                     aliveChecks.add(node::isProcessAlive);
                 }