Do not daemonize when testing error output in packaging tests (#56971)

The packaging tests start elasticsearch in various ways. All of these
currently expect it is started asynchronously, yet some tests expect it
will fail to start and want to check the error output. This commit adds
a daemonize flag to the utility methods to start elasticsearch for such
cases, so that when the start method returns, all the error output
should already be available since the process will have exited.

relates #51716

qa/os/src/test/java/org/elasticsearch/packaging/test/KeystoreManagementTests.java

@@ -158,7 +158,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         assertPasswordProtectedKeystore();
 
-        awaitElasticsearchStartup(startElasticsearchStandardInputPassword(password));
+        awaitElasticsearchStartup(startElasticsearchStandardInputPassword(password, true));
         ServerUtils.runElasticsearchTests();
         stopElasticsearch();
     }
@@ -169,7 +169,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         assertPasswordProtectedKeystore();
 
-        Shell.Result result = startElasticsearchStandardInputPassword("wrong");
+        Shell.Result result = startElasticsearchStandardInputPassword("wrong", false);
         assertElasticsearchFailure(result, Arrays.asList(ERROR_INCORRECT_PASSWORD, ERROR_CORRUPTED_KEYSTORE), null);
     }
 
@@ -187,7 +187,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         assertPasswordProtectedKeystore();
 
-        awaitElasticsearchStartup(startElasticsearchTtyPassword(password));
+        awaitElasticsearchStartup(startElasticsearchTtyPassword(password, true));
         ServerUtils.runElasticsearchTests();
         stopElasticsearch();
     }
@@ -200,7 +200,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
 
         assertPasswordProtectedKeystore();
 
-        Shell.Result result = startElasticsearchTtyPassword("wrong");
+        Shell.Result result = startElasticsearchTtyPassword("wrong", false);
         // error will be on stdout for "expect"
         assertThat(result.stdout, anyOf(containsString(ERROR_INCORRECT_PASSWORD), containsString(ERROR_CORRUPTED_KEYSTORE)));
     }
@@ -265,7 +265,7 @@ public class KeystoreManagementTests extends PackagingTestCase {
             Files.write(esKeystorePassphraseFile, List.of("wrongpassword"));
 
             Packages.JournaldWrapper journaldWrapper = new Packages.JournaldWrapper(sh);
-            Shell.Result result = runElasticsearchStartCommand();
+            Shell.Result result = runElasticsearchStartCommand(false);
             assertElasticsearchFailure(result, Arrays.asList(ERROR_INCORRECT_PASSWORD, ERROR_CORRUPTED_KEYSTORE), journaldWrapper);
         } finally {
             sh.run("sudo systemctl unset-environment ES_KEYSTORE_PASSPHRASE_FILE");

qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java

@@ -357,7 +357,7 @@ public class PackageTests extends PackagingTestCase {
 
             // Make sure we don't pick up the journal entries for previous ES instances.
             Packages.JournaldWrapper journald = new Packages.JournaldWrapper(sh);
-            runElasticsearchStartCommand();
+            runElasticsearchStartCommand(true);
             final Result logs = journald.getLogs();
 
             assertThat(logs.stdout, containsString("Failed to load settings from [elasticsearch.yml]"));

qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -155,18 +155,24 @@ public abstract class PackagingTestCase extends Assert {
     public void teardown() throws Exception {
         // move log file so we can avoid false positives when grepping for
         // messages in logs during test
-        if (installation != null && Files.exists(installation.logs)) {
-            Path logFile = installation.logs.resolve("elasticsearch.log");
-            String prefix = this.getClass().getSimpleName() + "." + testNameRule.getMethodName();
-            if (Files.exists(logFile)) {
-                Path newFile = installation.logs.resolve(prefix + ".elasticsearch.log");
-                FileUtils.mv(logFile, newFile);
+        if (installation != null) {
+            if (Files.exists(installation.logs)) {
+                Path logFile = installation.logs.resolve("elasticsearch.log");
+                String prefix = this.getClass().getSimpleName() + "." + testNameRule.getMethodName();
+                if (Files.exists(logFile)) {
+                    Path newFile = installation.logs.resolve(prefix + ".elasticsearch.log");
+                    FileUtils.mv(logFile, newFile);
+                }
+                for (Path rotatedLogFile : FileUtils.lsGlob(installation.logs, "elasticsearch*.tar.gz")) {
+                    Path newRotatedLogFile = installation.logs.resolve(prefix + "." + rotatedLogFile.getFileName());
+                    FileUtils.mv(rotatedLogFile, newRotatedLogFile);
+                }
             }
-            for (Path rotatedLogFile : FileUtils.lsGlob(installation.logs, "elasticsearch*.tar.gz")) {
-                Path newRotatedLogFile = installation.logs.resolve(prefix + "." + rotatedLogFile.getFileName());
-                FileUtils.mv(rotatedLogFile, newRotatedLogFile);
+            if (Files.exists(Archives.getPowershellErrorPath(installation))) {
+                FileUtils.rmWithRetries(Archives.getPowershellErrorPath(installation));
             }
         }
+
     }
 
     /** The {@link Distribution} that should be tested in this case */
@@ -200,7 +206,7 @@ public abstract class PackagingTestCase extends Assert {
      */
     protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Exception {
         try {
-            awaitElasticsearchStartup(runElasticsearchStartCommand());
+            awaitElasticsearchStartup(runElasticsearchStartCommand(true));
         } catch (Exception e) {
             if (Files.exists(installation.home.resolve("elasticsearch.pid"))) {
                 String pid = FileUtils.slurp(installation.home.resolve("elasticsearch.pid")).trim();
@@ -235,11 +241,11 @@ public abstract class PackagingTestCase extends Assert {
      * @return Shell results of the startup command.
      * @throws Exception when command fails immediately.
      */
-    public Shell.Result runElasticsearchStartCommand() throws Exception {
+    public Shell.Result runElasticsearchStartCommand(boolean daemonize) throws Exception {
         switch (distribution.packaging) {
             case TAR:
             case ZIP:
-                return Archives.runElasticsearchStartCommand(installation, sh, "");
+                return Archives.runElasticsearchStartCommand(installation, sh, null, daemonize);
             case DEB:
             case RPM:
                 return Packages.runElasticsearchStartCommand(sh);
@@ -290,21 +296,21 @@ public abstract class PackagingTestCase extends Assert {
 
     /**
      * Start Elasticsearch and wait until it's up and running. If you just want to run
-     * the start command, use {@link #runElasticsearchStartCommand()}.
+     * the start command, use {@link #runElasticsearchStartCommand(boolean)}.
      * @throws Exception if Elasticsearch can't start
      */
     public void startElasticsearch() throws Exception {
-        awaitElasticsearchStartup(runElasticsearchStartCommand());
+        awaitElasticsearchStartup(runElasticsearchStartCommand(true));
     }
 
-    public Shell.Result startElasticsearchStandardInputPassword(String password) {
+    public Shell.Result startElasticsearchStandardInputPassword(String password, boolean daemonize) {
         assertTrue("Only archives support passwords on standard input", distribution().isArchive());
-        return Archives.runElasticsearchStartCommand(installation, sh, password);
+        return Archives.runElasticsearchStartCommand(installation, sh, password, daemonize);
     }
 
-    public Shell.Result startElasticsearchTtyPassword(String password) throws Exception {
+    public Shell.Result startElasticsearchTtyPassword(String password, boolean daemonize) throws Exception {
         assertTrue("Only archives support passwords on TTY", distribution().isArchive());
-        return Archives.startElasticsearchWithTty(installation, sh, password);
+        return Archives.startElasticsearchWithTty(installation, sh, password, daemonize);
     }
 
     public void assertElasticsearchFailure(Shell.Result result, String expectedMessage, Packages.JournaldWrapper journaldWrapper) {
@@ -330,7 +336,7 @@ public abstract class PackagingTestCase extends Assert {
             Shell.Result error = journaldWrapper.getLogs();
             assertThat(error.stdout, anyOf(stringMatchers));
 
-        } else if (Platforms.WINDOWS) {
+        } else if (Platforms.WINDOWS && Files.exists(Archives.getPowershellErrorPath(installation))) {
 
             // In Windows, we have written our stdout and stderr to files in order to run
             // in the background

qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java

@@ -25,6 +25,7 @@ import org.apache.logging.log4j.Logger;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
 import java.util.stream.Stream;
@@ -231,18 +232,26 @@ public class Archives {
     }
 
     public static Shell.Result startElasticsearch(Installation installation, Shell sh) {
-        return runElasticsearchStartCommand(installation, sh, "");
+        return runElasticsearchStartCommand(installation, sh, null, true);
     }
 
-    public static Shell.Result startElasticsearchWithTty(Installation installation, Shell sh, String keystorePassword) throws Exception {
+    public static Shell.Result startElasticsearchWithTty(Installation installation, Shell sh, String keystorePassword, boolean daemonize)
+        throws Exception {
         final Path pidFile = installation.home.resolve("elasticsearch.pid");
         final Installation.Executables bin = installation.executables();
 
         // requires the "expect" utility to be installed
+        List<String> command = new ArrayList<>();
+        command.add("sudo -E -u %s %s -p %s");
+        if (daemonize) {
+            command.add("-d");
+        }
         String script = String.format(
             Locale.ROOT,
             "expect -c \"$(cat<<EXPECT\n"
-                + "spawn -ignore HUP sudo -E -u %s %s -d -p %s \n"
+                + "spawn -ignore HUP "
+                + String.join(" ", command)
+                + "\n"
                 + "expect \"Elasticsearch keystore password:\"\n"
                 + "send \"%s\\r\"\n"
                 + "expect eof\n"
@@ -258,7 +267,12 @@ public class Archives {
         return sh.runIgnoreExitCode(script);
     }
 
-    public static Shell.Result runElasticsearchStartCommand(Installation installation, Shell sh, String keystorePassword) {
+    public static Shell.Result runElasticsearchStartCommand(
+        Installation installation,
+        Shell sh,
+        String keystorePassword,
+        boolean daemonize
+    ) {
         final Path pidFile = installation.home.resolve("elasticsearch.pid");
 
         assertThat(pidFile, fileDoesNotExist());
@@ -275,71 +289,93 @@ public class Archives {
 
             // We need to give Elasticsearch enough time to print failures to stderr before exiting
             sh.getEnv().put("ES_STARTUP_SLEEP_TIME", ES_STARTUP_SLEEP_TIME_SECONDS);
-            return sh.runIgnoreExitCode(
-                "sudo -E -u " + ARCHIVE_OWNER + " " + bin.elasticsearch + " -d -p " + pidFile + " <<<'" + keystorePassword + "'"
-            );
+
+            List<String> command = new ArrayList<>();
+            command.add("sudo -E -u ");
+            command.add(ARCHIVE_OWNER);
+            command.add(bin.elasticsearch.toString());
+            if (daemonize) {
+                command.add("-d");
+            }
+            command.add("-p");
+            command.add(pidFile.toString());
+            if (keystorePassword != null) {
+                command.add("<<<'" + keystorePassword + "'");
+            }
+            return sh.runIgnoreExitCode(String.join(" ", command));
         }
-        final Path stdout = getPowershellOutputPath(installation);
-        final Path stderr = getPowershellErrorPath(installation);
-
-        String powerShellProcessUserSetup;
-        if (System.getenv("username").equals("vagrant")) {
-            // the tests will run as Administrator in vagrant.
-            // we don't want to run the server as Administrator, so we provide the current user's
-            // username and password to the process which has the effect of starting it not as Administrator.
-            powerShellProcessUserSetup = "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; "
-                + "$processInfo.Username = 'vagrant'; "
-                + "$processInfo.Password = $password; ";
+
+        if (daemonize) {
+            final Path stdout = getPowershellOutputPath(installation);
+            final Path stderr = getPowershellErrorPath(installation);
+
+            String powerShellProcessUserSetup;
+            if (System.getenv("username").equals("vagrant")) {
+                // the tests will run as Administrator in vagrant.
+                // we don't want to run the server as Administrator, so we provide the current user's
+                // username and password to the process which has the effect of starting it not as Administrator.
+                powerShellProcessUserSetup = "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; "
+                    + "$processInfo.Username = 'vagrant'; "
+                    + "$processInfo.Password = $password; ";
+            } else {
+                powerShellProcessUserSetup = "";
+            }
+            // this starts the server in the background. the -d flag is unsupported on windows
+            return sh.run(
+                "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; "
+                    + "$processInfo.FileName = '"
+                    + bin.elasticsearch
+                    + "'; "
+                    + "$processInfo.Arguments = '-p "
+                    + installation.home.resolve("elasticsearch.pid")
+                    + "'; "
+                    + powerShellProcessUserSetup
+                    + "$processInfo.RedirectStandardOutput = $true; "
+                    + "$processInfo.RedirectStandardError = $true; "
+                    + "$processInfo.RedirectStandardInput = $true; "
+                    + sh.env.entrySet()
+                        .stream()
+                        .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
+                        .collect(joining())
+                    + "$processInfo.UseShellExecute = $false; "
+                    + "$process = New-Object System.Diagnostics.Process; "
+                    + "$process.StartInfo = $processInfo; "
+                    +
+
+                    // set up some asynchronous output handlers
+                    "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
+                    + stdout
+                    + "' }; "
+                    + "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
+                    + stderr
+                    + "' }; "
+                    + "$stdOutEvent = Register-ObjectEvent -InputObject $process "
+                    + "-Action $outScript -EventName 'OutputDataReceived'; "
+                    + "$stdErrEvent = Register-ObjectEvent -InputObject $process "
+                    + "-Action $errScript -EventName 'ErrorDataReceived'; "
+                    +
+
+                    "$process.Start() | Out-Null; "
+                    + "$process.BeginOutputReadLine(); "
+                    + "$process.BeginErrorReadLine(); "
+                    + "$process.StandardInput.WriteLine('"
+                    + keystorePassword
+                    + "'); "
+                    + "Wait-Process -Timeout "
+                    + ES_STARTUP_SLEEP_TIME_SECONDS
+                    + " -Id $process.Id; "
+                    + "$process.Id;"
+            );
         } else {
-            powerShellProcessUserSetup = "";
+            List<String> command = new ArrayList<>();
+            if (keystorePassword != null) {
+                command.add("echo '" + keystorePassword + "' |");
+            }
+            command.add(bin.elasticsearch.toString());
+            command.add("-p");
+            command.add(installation.home.resolve("elasticsearch.pid").toString());
+            return sh.runIgnoreExitCode(String.join(" ", command));
         }
-
-        // this starts the server in the background. the -d flag is unsupported on windows
-        return sh.run(
-            "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; "
-                + "$processInfo.FileName = '"
-                + bin.elasticsearch
-                + "'; "
-                + "$processInfo.Arguments = '-p "
-                + installation.home.resolve("elasticsearch.pid")
-                + "'; "
-                + powerShellProcessUserSetup
-                + "$processInfo.RedirectStandardOutput = $true; "
-                + "$processInfo.RedirectStandardError = $true; "
-                + "$processInfo.RedirectStandardInput = $true; "
-                + sh.env.entrySet()
-                    .stream()
-                    .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
-                    .collect(joining())
-                + "$processInfo.UseShellExecute = $false; "
-                + "$process = New-Object System.Diagnostics.Process; "
-                + "$process.StartInfo = $processInfo; "
-                +
-
-                // set up some asynchronous output handlers
-                "$outScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
-                + stdout
-                + "' }; "
-                + "$errScript = { $EventArgs.Data | Out-File -Encoding UTF8 -Append '"
-                + stderr
-                + "' }; "
-                + "$stdOutEvent = Register-ObjectEvent -InputObject $process "
-                + "-Action $outScript -EventName 'OutputDataReceived'; "
-                + "$stdErrEvent = Register-ObjectEvent -InputObject $process "
-                + "-Action $errScript -EventName 'ErrorDataReceived'; "
-                +
-
-                "$process.Start() | Out-Null; "
-                + "$process.BeginOutputReadLine(); "
-                + "$process.BeginErrorReadLine(); "
-                + "$process.StandardInput.WriteLine('"
-                + keystorePassword
-                + "'); "
-                + "Wait-Process -Timeout "
-                + ES_STARTUP_SLEEP_TIME_SECONDS
-                + " -Id $process.Id; "
-                + "$process.Id;"
-        );
     }
 
     public static void assertElasticsearchStarted(Installation installation) throws Exception {