Browse Source

ServerProcess refactoring (separate options construction from ServerProcess start) (#102973)

* Refactoring: move command line and environment build to a separate class
* Refactoring: adding a builder
* Moving tmp dir setup and JVM option parsing outside of builder
Lorenzo Dematté 1 year ago
parent
commit
0e5f485058

+ 13 - 10
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java

@@ -10,6 +10,7 @@ package org.elasticsearch.server.cli;
 
 import org.elasticsearch.bootstrap.ServerArgs;
 import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.ProcessInfo;
 import org.elasticsearch.cli.UserException;
 
 import java.io.BufferedReader;
@@ -39,7 +40,7 @@ import java.util.stream.StreamSupport;
 /**
  * Parses JVM options from a file and prints a single line with all JVM options to standard output.
  */
-final class JvmOptionsParser {
+public final class JvmOptionsParser {
 
     static class JvmOptionsFileParserException extends Exception {
 
@@ -59,7 +60,6 @@ final class JvmOptionsParser {
             this.jvmOptionsFile = jvmOptionsFile;
             this.invalidLines = invalidLines;
         }
-
     }
 
     /**
@@ -70,25 +70,27 @@ final class JvmOptionsParser {
      * variable.
      *
      * @param args            the start-up arguments
-     * @param configDir       the ES config dir
+     * @param processInfo     information about the CLI process.
      * @param tmpDir          the directory that should be passed to {@code -Djava.io.tmpdir}
-     * @param envOptions      the options passed through the ES_JAVA_OPTS env var
      * @return the list of options to put on the Java command line
      * @throws InterruptedException if the java subprocess is interrupted
      * @throws IOException          if there is a problem reading any of the files
      * @throws UserException        if there is a problem parsing the `jvm.options` file or `jvm.options.d` files
      */
-    static List<String> determineJvmOptions(ServerArgs args, Path configDir, Path tmpDir, String envOptions) throws InterruptedException,
+    public static List<String> determineJvmOptions(ServerArgs args, ProcessInfo processInfo, Path tmpDir) throws InterruptedException,
         IOException, UserException {
-
         final JvmOptionsParser parser = new JvmOptionsParser();
 
         final Map<String, String> substitutions = new HashMap<>();
         substitutions.put("ES_TMPDIR", tmpDir.toString());
-        substitutions.put("ES_PATH_CONF", configDir.toString());
+        substitutions.put("ES_PATH_CONF", args.configDir().toString());
+
+        final String envOptions = processInfo.envVars().get("ES_JAVA_OPTS");
 
         try {
-            return parser.jvmOptions(args, configDir, tmpDir, envOptions, substitutions);
+            return Collections.unmodifiableList(
+                parser.jvmOptions(args, args.configDir(), tmpDir, envOptions, substitutions, processInfo.sysprops())
+            );
         } catch (final JvmOptionsFileParserException e) {
             final String errorMessage = String.format(
                 Locale.ROOT,
@@ -122,7 +124,8 @@ final class JvmOptionsParser {
         final Path config,
         Path tmpDir,
         final String esJavaOpts,
-        final Map<String, String> substitutions
+        final Map<String, String> substitutions,
+        final Map<String, String> cliSysprops
     ) throws InterruptedException, IOException, JvmOptionsFileParserException, UserException {
 
         final List<String> jvmOptions = readJvmOptionsFiles(config);
@@ -137,7 +140,7 @@ final class JvmOptionsParser {
         );
         substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(config, substitutedJvmOptions));
         final List<String> ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions);
-        final List<String> systemJvmOptions = SystemJvmOptions.systemJvmOptions(args.nodeSettings());
+        final List<String> systemJvmOptions = SystemJvmOptions.systemJvmOptions(args.nodeSettings(), cliSysprops);
 
         final List<String> apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), args.logsDir(), tmpDir);
 

+ 9 - 2
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java

@@ -243,8 +243,15 @@ class ServerCli extends EnvironmentAwareCommand {
     }
 
     // protected to allow tests to override
-    protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws UserException {
-        return ServerProcess.start(terminal, processInfo, args);
+    protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws Exception {
+        var tempDir = ServerProcessUtils.setupTempDir(processInfo);
+        var jvmOptions = JvmOptionsParser.determineJvmOptions(args, processInfo, tempDir);
+        var serverProcessBuilder = new ServerProcessBuilder().withTerminal(terminal)
+            .withProcessInfo(processInfo)
+            .withServerArgs(args)
+            .withTempDir(tempDir)
+            .withJvmOptions(jvmOptions);
+        return serverProcessBuilder.start();
     }
 
     // protected to allow tests to override

+ 1 - 169
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java

@@ -9,34 +9,17 @@
 package org.elasticsearch.server.cli;
 
 import org.elasticsearch.bootstrap.BootstrapInfo;
-import org.elasticsearch.bootstrap.ServerArgs;
-import org.elasticsearch.cli.ExitCodes;
-import org.elasticsearch.cli.ProcessInfo;
-import org.elasticsearch.cli.Terminal;
-import org.elasticsearch.cli.UserException;
-import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
 import org.elasticsearch.core.IOUtils;
-import org.elasticsearch.core.PathUtils;
-import org.elasticsearch.core.SuppressForbidden;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.io.UncheckedIOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.nio.file.attribute.FileAttribute;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
 
 import static org.elasticsearch.server.cli.ProcessUtil.nonInterruptible;
 
 /**
  * A helper to control a {@link Process} running the main Elasticsearch server.
  *
- * <p> The process can be started by calling {@link #start(Terminal, ProcessInfo, ServerArgs)}.
+ * <p> The process can be started by calling {@link ServerProcessBuilder#start()}.
  * The process is controlled by internally sending arguments and control signals on stdin,
  * and receiving control signals on stderr. The start method does not return until the
  * server is ready to process requests and has exited the bootstrap thread.
@@ -64,68 +47,6 @@ public class ServerProcess {
         this.errorPump = errorPump;
     }
 
-    // this allows mocking the process building by tests
-    interface OptionsBuilder {
-        List<String> getJvmOptions(ServerArgs args, Path configDir, Path tmpDir, String envOptions) throws InterruptedException,
-            IOException, UserException;
-    }
-
-    // this allows mocking the process building by tests
-    interface ProcessStarter {
-        Process start(ProcessBuilder pb) throws IOException;
-    }
-
-    /**
-     * Start a server in a new process.
-     *
-     * @param terminal        A terminal to connect the standard inputs and outputs to for the new process.
-     * @param processInfo     Info about the current process, for passing through to the subprocess.
-     * @param args            Arguments to the server process.
-     * @return A running server process that is ready for requests
-     * @throws UserException If the process failed during bootstrap
-     */
-    public static ServerProcess start(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws UserException {
-        return start(terminal, processInfo, args, JvmOptionsParser::determineJvmOptions, ProcessBuilder::start);
-    }
-
-    // package private so tests can mock options building and process starting
-    static ServerProcess start(
-        Terminal terminal,
-        ProcessInfo processInfo,
-        ServerArgs args,
-        OptionsBuilder optionsBuilder,
-        ProcessStarter processStarter
-    ) throws UserException {
-        Process jvmProcess = null;
-        ErrorPumpThread errorPump;
-
-        boolean success = false;
-        try {
-            jvmProcess = createProcess(args, processInfo, args.configDir(), optionsBuilder, processStarter);
-            errorPump = new ErrorPumpThread(terminal.getErrorWriter(), jvmProcess.getErrorStream());
-            errorPump.start();
-            sendArgs(args, jvmProcess.getOutputStream());
-
-            String errorMsg = errorPump.waitUntilReady();
-            if (errorMsg != null) {
-                // something bad happened, wait for the process to exit then rethrow
-                int exitCode = jvmProcess.waitFor();
-                throw new UserException(exitCode, errorMsg);
-            }
-            success = true;
-        } catch (InterruptedException e) {
-            throw new RuntimeException(e);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        } finally {
-            if (success == false && jvmProcess != null && jvmProcess.isAlive()) {
-                jvmProcess.destroyForcibly();
-            }
-        }
-
-        return new ServerProcess(jvmProcess, errorPump);
-    }
-
     /**
      * Return the process id of the server.
      */
@@ -169,19 +90,6 @@ public class ServerProcess {
         waitFor(); // ignore exit code, we are already shutting down
     }
 
-    private static void sendArgs(ServerArgs args, OutputStream processStdin) {
-        // DO NOT close the underlying process stdin, since we need to be able to write to it to signal exit
-        var out = new OutputStreamStreamOutput(processStdin);
-        try {
-            args.writeTo(out);
-            out.flush();
-        } catch (IOException ignore) {
-            // A failure to write here means the process has problems, and it will die anyway. We let this fall through
-            // so the pump thread can complete, writing out the actual error. All we get here is the failure to write to
-            // the process pipe, which isn't helpful to print.
-        }
-    }
-
     private void sendShutdownMarker() {
         try {
             OutputStream os = jvmProcess.getOutputStream();
@@ -191,80 +99,4 @@ public class ServerProcess {
             // process is already effectively dead, fall through to wait for it, or should we SIGKILL?
         }
     }
-
-    private static Process createProcess(
-        ServerArgs args,
-        ProcessInfo processInfo,
-        Path configDir,
-        OptionsBuilder optionsBuilder,
-        ProcessStarter processStarter
-    ) throws InterruptedException, IOException, UserException {
-        Map<String, String> envVars = new HashMap<>(processInfo.envVars());
-        Path tempDir = setupTempDir(processInfo, envVars.remove("ES_TMPDIR"));
-        if (envVars.containsKey("LIBFFI_TMPDIR") == false) {
-            envVars.put("LIBFFI_TMPDIR", tempDir.toString());
-        }
-
-        List<String> jvmOptions = optionsBuilder.getJvmOptions(args, configDir, tempDir, envVars.remove("ES_JAVA_OPTS"));
-        // also pass through distribution type
-        jvmOptions.add("-Des.distribution.type=" + processInfo.sysprops().get("es.distribution.type"));
-
-        Path esHome = processInfo.workingDir();
-        Path javaHome = PathUtils.get(processInfo.sysprops().get("java.home"));
-        List<String> command = new ArrayList<>();
-        boolean isWindows = processInfo.sysprops().get("os.name").startsWith("Windows");
-        command.add(javaHome.resolve("bin").resolve("java" + (isWindows ? ".exe" : "")).toString());
-        command.addAll(jvmOptions);
-        command.add("--module-path");
-        command.add(esHome.resolve("lib").toString());
-        // Special circumstances require some modules (not depended on by the main server module) to be explicitly added:
-        command.add("--add-modules=jdk.net"); // needed to reflectively set extended socket options
-        // we control the module path, which may have additional modules not required by server
-        command.add("--add-modules=ALL-MODULE-PATH");
-        command.add("-m");
-        command.add("org.elasticsearch.server/org.elasticsearch.bootstrap.Elasticsearch");
-
-        var builder = new ProcessBuilder(command);
-        builder.environment().putAll(envVars);
-        builder.redirectOutput(ProcessBuilder.Redirect.INHERIT);
-
-        return processStarter.start(builder);
-    }
-
-    /**
-     * Returns the java.io.tmpdir Elasticsearch should use, creating it if necessary.
-     *
-     * <p> On non-Windows OS, this will be created as a subdirectory of the default temporary directory.
-     * Note that this causes the created temporary directory to be a private temporary directory.
-     */
-    private static Path setupTempDir(ProcessInfo processInfo, String tmpDirOverride) throws UserException, IOException {
-        final Path path;
-        if (tmpDirOverride != null) {
-            path = Paths.get(tmpDirOverride);
-            if (Files.exists(path) == false) {
-                throw new UserException(ExitCodes.CONFIG, "Temporary directory [" + path + "] does not exist or is not accessible");
-            }
-            if (Files.isDirectory(path) == false) {
-                throw new UserException(ExitCodes.CONFIG, "Temporary directory [" + path + "] is not a directory");
-            }
-        } else {
-            if (processInfo.sysprops().get("os.name").startsWith("Windows")) {
-                /*
-                 * On Windows, we avoid creating a unique temporary directory per invocation lest
-                 * we pollute the temporary directory. On other operating systems, temporary directories
-                 * will be cleaned automatically via various mechanisms (e.g., systemd, or restarts).
-                 */
-                path = Paths.get(processInfo.sysprops().get("java.io.tmpdir"), "elasticsearch");
-                Files.createDirectories(path);
-            } else {
-                path = createTempDirectory("elasticsearch-");
-            }
-        }
-        return path;
-    }
-
-    @SuppressForbidden(reason = "Files#createTempDirectory(String, FileAttribute...)")
-    private static Path createTempDirectory(final String prefix, final FileAttribute<?>... attrs) throws IOException {
-        return Files.createTempDirectory(prefix, attrs);
-    }
 }

+ 208 - 0
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java

@@ -0,0 +1,208 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.server.cli;
+
+import org.elasticsearch.bootstrap.ServerArgs;
+import org.elasticsearch.cli.ProcessInfo;
+import org.elasticsearch.cli.Terminal;
+import org.elasticsearch.cli.UserException;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
+import org.elasticsearch.core.PathUtils;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.UncheckedIOException;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Stream;
+
+/**
+ * This class is used to create a {@link ServerProcess}.
+ * Each ServerProcessBuilder instance manages a collection of process attributes. The {@link ServerProcessBuilder#start()} method creates
+ * a new {@link ServerProcess} instance with those attributes.
+ *
+ * Each process builder manages these process attributes:
+ * - a temporary directory
+ * - process info to pass through to the new Java subprocess
+ * - the command line arguments to run Elasticsearch
+ * - a list of JVM options to be passed to the Elasticsearch Java process
+ * - a {@link Terminal} to read input and write output from/to the cli console
+ */
+public class ServerProcessBuilder {
+    private Path tempDir;
+    private ServerArgs serverArgs;
+    private ProcessInfo processInfo;
+    private List<String> jvmOptions;
+    private Terminal terminal;
+
+    // this allows mocking the process building by tests
+    interface ProcessStarter {
+        Process start(ProcessBuilder pb) throws IOException;
+    }
+
+    /**
+     * Specifies the temporary directory to be used by the server process
+     */
+    public ServerProcessBuilder withTempDir(Path tempDir) {
+        this.tempDir = tempDir;
+        return this;
+    }
+
+    /**
+     * Specifies the process info to pass through to the new Java subprocess
+     */
+    public ServerProcessBuilder withProcessInfo(ProcessInfo processInfo) {
+        this.processInfo = processInfo;
+        return this;
+    }
+
+    /**
+     * Specifies the command line arguments to run Elasticsearch
+     */
+    public ServerProcessBuilder withServerArgs(ServerArgs serverArgs) {
+        this.serverArgs = serverArgs;
+        return this;
+    }
+
+    /**
+     * Specifies the JVM options to be passed to the Elasticsearch Java process
+     */
+    public ServerProcessBuilder withJvmOptions(List<String> jvmOptions) {
+        this.jvmOptions = jvmOptions;
+        return this;
+    }
+
+    /**
+     * Specifies the {@link Terminal} to use for reading input and writing output from/to the cli console
+     */
+    public ServerProcessBuilder withTerminal(Terminal terminal) {
+        this.terminal = terminal;
+        return this;
+    }
+
+    private Map<String, String> getEnvironment() {
+        Map<String, String> envVars = new HashMap<>(processInfo.envVars());
+
+        envVars.remove("ES_TMPDIR");
+        if (envVars.containsKey("LIBFFI_TMPDIR") == false) {
+            envVars.put("LIBFFI_TMPDIR", tempDir.toString());
+        }
+        envVars.remove("ES_JAVA_OPTS");
+
+        return envVars;
+    }
+
+    private List<String> getJvmArgs() {
+        Path esHome = processInfo.workingDir();
+        return List.of(
+            "--module-path",
+            esHome.resolve("lib").toString(),
+            // Special circumstances require some modules (not depended on by the main server module) to be explicitly added:
+            "--add-modules=jdk.net", // needed to reflectively set extended socket options
+            // we control the module path, which may have additional modules not required by server
+            "--add-modules=ALL-MODULE-PATH",
+            "-m",
+            "org.elasticsearch.server/org.elasticsearch.bootstrap.Elasticsearch"
+        );
+    }
+
+    private String getCommand() {
+        Path javaHome = PathUtils.get(processInfo.sysprops().get("java.home"));
+
+        boolean isWindows = processInfo.sysprops().get("os.name").startsWith("Windows");
+        return javaHome.resolve("bin").resolve("java" + (isWindows ? ".exe" : "")).toString();
+    }
+
+    /**
+     * Start a server in a new process.
+     *
+     * @return A running server process that is ready for requests
+     * @throws UserException        If the process failed during bootstrap
+     */
+    public ServerProcess start() throws UserException {
+        return start(ProcessBuilder::start);
+    }
+
+    private static void checkRequiredArgument(Object argument, String argumentName) {
+        if (argument == null) {
+            throw new IllegalStateException(
+                Strings.format("'%s' is a required argument and needs to be specified before calling start()", argumentName)
+            );
+        }
+    }
+
+    // package private for testing
+    ServerProcess start(ProcessStarter processStarter) throws UserException {
+        checkRequiredArgument(tempDir, "tempDir");
+        checkRequiredArgument(serverArgs, "serverArgs");
+        checkRequiredArgument(processInfo, "processInfo");
+        checkRequiredArgument(jvmOptions, "jvmOptions");
+        checkRequiredArgument(terminal, "terminal");
+
+        Process jvmProcess = null;
+        ErrorPumpThread errorPump;
+
+        boolean success = false;
+        try {
+            jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), processStarter);
+            errorPump = new ErrorPumpThread(terminal.getErrorWriter(), jvmProcess.getErrorStream());
+            errorPump.start();
+            sendArgs(serverArgs, jvmProcess.getOutputStream());
+
+            String errorMsg = errorPump.waitUntilReady();
+            if (errorMsg != null) {
+                // something bad happened, wait for the process to exit then rethrow
+                int exitCode = jvmProcess.waitFor();
+                throw new UserException(exitCode, errorMsg);
+            }
+            success = true;
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        } finally {
+            if (success == false && jvmProcess != null && jvmProcess.isAlive()) {
+                jvmProcess.destroyForcibly();
+            }
+        }
+
+        return new ServerProcess(jvmProcess, errorPump);
+    }
+
+    private static Process createProcess(
+        String command,
+        List<String> jvmArgs,
+        List<String> jvmOptions,
+        Map<String, String> environment,
+        ProcessStarter processStarter
+    ) throws InterruptedException, IOException {
+
+        var builder = new ProcessBuilder(Stream.concat(Stream.of(command), Stream.concat(jvmOptions.stream(), jvmArgs.stream())).toList());
+        builder.environment().putAll(environment);
+        builder.redirectOutput(ProcessBuilder.Redirect.INHERIT);
+
+        return processStarter.start(builder);
+    }
+
+    private static void sendArgs(ServerArgs args, OutputStream processStdin) {
+        // DO NOT close the underlying process stdin, since we need to be able to write to it to signal exit
+        var out = new OutputStreamStreamOutput(processStdin);
+        try {
+            args.writeTo(out);
+            out.flush();
+        } catch (IOException ignore) {
+            // A failure to write here means the process has problems, and it will die anyway. We let this fall through
+            // so the pump thread can complete, writing out the actual error. All we get here is the failure to write to
+            // the process pipe, which isn't helpful to print.
+        }
+    }
+}

+ 66 - 0
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessUtils.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.server.cli;
+
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.ProcessInfo;
+import org.elasticsearch.cli.UserException;
+import org.elasticsearch.core.SuppressForbidden;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.attribute.FileAttribute;
+
+public class ServerProcessUtils {
+
+    /**
+     * Returns the java.io.tmpdir Elasticsearch should use, creating it if necessary.
+     *
+     * <p> On non-Windows OS, this will be created as a subdirectory of the default temporary directory.
+     * Note that this causes the created temporary directory to be a private temporary directory.
+     */
+    public static Path setupTempDir(ProcessInfo processInfo) throws UserException {
+        final Path path;
+        String tmpDirOverride = processInfo.envVars().get("ES_TMPDIR");
+        if (tmpDirOverride != null) {
+            path = Paths.get(tmpDirOverride);
+            if (Files.exists(path) == false) {
+                throw new UserException(ExitCodes.CONFIG, "Temporary directory [" + path + "] does not exist or is not accessible");
+            }
+            if (Files.isDirectory(path) == false) {
+                throw new UserException(ExitCodes.CONFIG, "Temporary directory [" + path + "] is not a directory");
+            }
+        } else {
+            try {
+                if (processInfo.sysprops().get("os.name").startsWith("Windows")) {
+                    /*
+                     * On Windows, we avoid creating a unique temporary directory per invocation lest
+                     * we pollute the temporary directory. On other operating systems, temporary directories
+                     * will be cleaned automatically via various mechanisms (e.g., systemd, or restarts).
+                     */
+                    path = Paths.get(processInfo.sysprops().get("java.io.tmpdir"), "elasticsearch");
+                    Files.createDirectories(path);
+                } else {
+                    path = createTempDirectory("elasticsearch-");
+                }
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
+        return path;
+    }
+
+    @SuppressForbidden(reason = "Files#createTempDirectory(String, FileAttribute...)")
+    private static Path createTempDirectory(final String prefix, final FileAttribute<?>... attrs) throws IOException {
+        return Files.createTempDirectory(prefix, attrs);
+    }
+}

+ 5 - 2
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java

@@ -12,12 +12,13 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 final class SystemJvmOptions {
 
-    static List<String> systemJvmOptions(Settings nodeSettings) {
+    static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, String> sysprops) {
         return Stream.of(
             /*
              * Cache ttl in seconds for positive DNS lookups noting that this overrides the JDK security property networkaddress.cache.ttl;
@@ -65,7 +66,9 @@ final class SystemJvmOptions {
              */
             "--add-opens=java.base/java.io=org.elasticsearch.preallocate",
             maybeOverrideDockerCgroup(),
-            maybeSetActiveProcessorCount(nodeSettings)
+            maybeSetActiveProcessorCount(nodeSettings),
+            // Pass through distribution type
+            "-Des.distribution.type=" + sysprops.get("es.distribution.type")
         ).filter(e -> e.isEmpty() == false).collect(Collectors.toList());
     }
 

+ 9 - 5
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java

@@ -53,7 +53,6 @@ public class JvmOptionsParserTests extends ESTestCase {
         try (StringReader sr = new StringReader("-Xms1g\n-Xmx1g"); BufferedReader br = new BufferedReader(sr)) {
             assertExpectedJvmOptions(randomIntBetween(8, Integer.MAX_VALUE), br, Arrays.asList("-Xms1g", "-Xmx1g"));
         }
-
     }
 
     public void testSingleVersionOption() throws IOException {
@@ -351,25 +350,30 @@ public class JvmOptionsParserTests extends ESTestCase {
 
     public void testNodeProcessorsActiveCount() {
         {
-            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY);
+            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, Map.of());
             assertThat(jvmOptions, not(hasItem(containsString("-XX:ActiveProcessorCount="))));
         }
         {
             Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 1).build();
-            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings);
+            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, Map.of());
             assertThat(jvmOptions, hasItem("-XX:ActiveProcessorCount=1"));
         }
         {
             // check rounding
             Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 0.2).build();
-            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings);
+            final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, Map.of());
             assertThat(jvmOptions, hasItem("-XX:ActiveProcessorCount=1"));
         }
         {
             // check validation
             Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 10000).build();
-            var e = expectThrows(IllegalArgumentException.class, () -> SystemJvmOptions.systemJvmOptions(nodeSettings));
+            var e = expectThrows(IllegalArgumentException.class, () -> SystemJvmOptions.systemJvmOptions(nodeSettings, Map.of()));
             assertThat(e.getMessage(), containsString("setting [node.processors] must be <="));
         }
     }
+
+    public void testCommandLineDistributionType() {
+        final List<String> jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, Map.of("es.distribution.type", "testdistro"));
+        assertThat(jvmOptions, hasItem("-Des.distribution.type=testdistro"));
+    }
 }

+ 63 - 46
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java

@@ -314,6 +314,21 @@ public class ServerCliTests extends CommandTestCase {
         assertThat(terminal.getErrorOutput(), not(containsString("null")));
     }
 
+    public void testOptionsBuildingInterrupted() throws IOException {
+        Command command = new TestServerCli() {
+            @Override
+            protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws Exception {
+                throw new InterruptedException("interrupted while get jvm options");
+            }
+        };
+        var e = expectThrows(
+            InterruptedException.class,
+            () -> command.main(new String[0], terminal, new ProcessInfo(sysprops, envVars, esHomeDir))
+        );
+        assertThat(e.getMessage(), equalTo("interrupted while get jvm options"));
+        command.close();
+    }
+
     public void testServerExitsNonZero() throws Exception {
         mockServerExitCode = 140;
         int exitCode = executeMain();
@@ -480,63 +495,65 @@ public class ServerCliTests extends CommandTestCase {
         }
     }
 
-    @Override
-    protected Command newCommand() {
-        return new ServerCli() {
-            @Override
-            protected Command loadTool(String toolname, String libs) {
-                if (toolname.equals("auto-configure-node")) {
-                    assertThat(libs, equalTo("modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli"));
-                    return AUTO_CONFIG_CLI;
-                } else if (toolname.equals("sync-plugins")) {
-                    assertThat(libs, equalTo("lib/tools/plugin-cli"));
-                    return SYNC_PLUGINS_CLI;
-                }
-                throw new AssertionError("Unknown tool: " + toolname);
+    private class TestServerCli extends ServerCli {
+        @Override
+        protected Command loadTool(String toolname, String libs) {
+            if (toolname.equals("auto-configure-node")) {
+                assertThat(libs, equalTo("modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli"));
+                return AUTO_CONFIG_CLI;
+            } else if (toolname.equals("sync-plugins")) {
+                assertThat(libs, equalTo("lib/tools/plugin-cli"));
+                return SYNC_PLUGINS_CLI;
             }
+            throw new AssertionError("Unknown tool: " + toolname);
+        }
 
-            @Override
-            Environment autoConfigureSecurity(
-                Terminal terminal,
-                OptionSet options,
-                ProcessInfo processInfo,
-                Environment env,
-                SecureString keystorePassword
-            ) throws Exception {
-                if (mockSecureSettingsLoader != null && mockSecureSettingsLoader.supportsSecurityAutoConfiguration() == false) {
-                    fail("We shouldn't be calling auto configure on loaders that don't support it");
-                }
-                return super.autoConfigureSecurity(terminal, options, processInfo, env, keystorePassword);
+        @Override
+        Environment autoConfigureSecurity(
+            Terminal terminal,
+            OptionSet options,
+            ProcessInfo processInfo,
+            Environment env,
+            SecureString keystorePassword
+        ) throws Exception {
+            if (mockSecureSettingsLoader != null && mockSecureSettingsLoader.supportsSecurityAutoConfiguration() == false) {
+                fail("We shouldn't be calling auto configure on loaders that don't support it");
             }
+            return super.autoConfigureSecurity(terminal, options, processInfo, env, keystorePassword);
+        }
 
-            @Override
-            protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) {
-                if (argsValidator != null) {
-                    argsValidator.accept(args);
-                }
-                mockServer.reset();
-                return mockServer;
+        @Override
+        void syncPlugins(Terminal terminal, Environment env, ProcessInfo processInfo) throws Exception {
+            if (mockSecureSettingsLoader != null && mockSecureSettingsLoader instanceof MockSecureSettingsLoader mock) {
+                mock.verifiedEnv = true;
+                // equals as a pointer, environment shouldn't be changed if autoconfigure is not supported
+                assertFalse(mockSecureSettingsLoader.supportsSecurityAutoConfiguration());
+                assertTrue(mock.environment == env);
             }
 
-            @Override
-            void syncPlugins(Terminal terminal, Environment env, ProcessInfo processInfo) throws Exception {
-                if (mockSecureSettingsLoader != null && mockSecureSettingsLoader instanceof MockSecureSettingsLoader mock) {
-                    mock.verifiedEnv = true;
-                    // equals as a pointer, environment shouldn't be changed if autoconfigure is not supported
-                    assertFalse(mockSecureSettingsLoader.supportsSecurityAutoConfiguration());
-                    assertTrue(mock.environment == env);
-                }
+            super.syncPlugins(terminal, env, processInfo);
+        }
 
-                super.syncPlugins(terminal, env, processInfo);
+        @Override
+        protected SecureSettingsLoader secureSettingsLoader(Environment env) {
+            if (mockSecureSettingsLoader != null) {
+                return mockSecureSettingsLoader;
             }
 
+            return new KeystoreSecureSettingsLoader();
+        }
+    }
+
+    @Override
+    protected Command newCommand() {
+        return new TestServerCli() {
             @Override
-            protected SecureSettingsLoader secureSettingsLoader(Environment env) {
-                if (mockSecureSettingsLoader != null) {
-                    return mockSecureSettingsLoader;
+            protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) {
+                if (argsValidator != null) {
+                    argsValidator.accept(args);
                 }
-
-                return new KeystoreSecureSettingsLoader();
+                mockServer.reset();
+                return mockServer;
             }
         };
     }

+ 51 - 86
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java

@@ -13,7 +13,6 @@ import org.elasticsearch.bootstrap.ServerArgs;
 import org.elasticsearch.cli.ExitCodes;
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.cli.ProcessInfo;
-import org.elasticsearch.cli.UserException;
 import org.elasticsearch.common.io.stream.InputStreamStreamInput;
 import org.elasticsearch.common.settings.KeyStoreWrapper;
 import org.elasticsearch.common.settings.SecureSettings;
@@ -34,7 +33,6 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -47,7 +45,6 @@ import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.elasticsearch.server.cli.ProcessUtil.nonInterruptibleVoid;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasEntry;
@@ -56,7 +53,6 @@ import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.nullValue;
-import static org.hamcrest.Matchers.startsWith;
 
 public class ServerProcessTests extends ESTestCase {
 
@@ -66,7 +62,6 @@ public class ServerProcessTests extends ESTestCase {
     protected final Map<String, String> envVars = new HashMap<>();
     Path esHomeDir;
     Settings.Builder nodeSettings;
-    ServerProcess.OptionsBuilder optionsBuilder;
     ProcessValidator processValidator;
     MainMethod mainCallback;
     MockElasticsearchProcess process;
@@ -81,7 +76,7 @@ public class ServerProcessTests extends ESTestCase {
     }
 
     int runForeground() throws Exception {
-        var server = startProcess(false, false, "");
+        var server = startProcess(false, false);
         return server.waitFor();
     }
 
@@ -94,7 +89,6 @@ public class ServerProcessTests extends ESTestCase {
         envVars.clear();
         esHomeDir = createTempDir();
         nodeSettings = Settings.builder();
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> new ArrayList<>();
         processValidator = null;
         mainCallback = null;
         secrets = KeyStoreWrapper.create();
@@ -193,9 +187,12 @@ public class ServerProcessTests extends ESTestCase {
         }
     }
 
-    ServerProcess startProcess(boolean daemonize, boolean quiet, String keystorePassword) throws Exception {
-        var pinfo = new ProcessInfo(Map.copyOf(sysprops), Map.copyOf(envVars), esHomeDir);
-        var args = new ServerArgs(
+    ProcessInfo createProcessInfo() {
+        return new ProcessInfo(Map.copyOf(sysprops), Map.copyOf(envVars), esHomeDir);
+    }
+
+    ServerArgs createServerArgs(boolean daemonize, boolean quiet) {
+        return new ServerArgs(
             daemonize,
             quiet,
             null,
@@ -204,14 +201,23 @@ public class ServerProcessTests extends ESTestCase {
             esHomeDir.resolve("config"),
             esHomeDir.resolve("logs")
         );
-        ServerProcess.ProcessStarter starter = pb -> {
+    }
+
+    ServerProcess startProcess(boolean daemonize, boolean quiet) throws Exception {
+        var pinfo = createProcessInfo();
+        ServerProcessBuilder.ProcessStarter starter = pb -> {
             if (processValidator != null) {
                 processValidator.validate(pb);
             }
             process = new MockElasticsearchProcess();
             return process;
         };
-        return ServerProcess.start(terminal, pinfo, args, optionsBuilder, starter);
+        var serverProcessBuilder = new ServerProcessBuilder().withTerminal(terminal)
+            .withProcessInfo(pinfo)
+            .withServerArgs(createServerArgs(daemonize, quiet))
+            .withJvmOptions(List.of())
+            .withTempDir(ServerProcessUtils.setupTempDir(pinfo));
+        return serverProcessBuilder.start(starter);
     }
 
     public void testProcessBuilder() throws Exception {
@@ -231,7 +237,7 @@ public class ServerProcessTests extends ESTestCase {
     }
 
     public void testPid() throws Exception {
-        var server = startProcess(true, false, "");
+        var server = startProcess(true, false);
         assertThat(server.pid(), equalTo(12345L));
         server.stop();
     }
@@ -246,18 +252,12 @@ public class ServerProcessTests extends ESTestCase {
         assertThat(terminal.getErrorOutput(), containsString("a bootstrap exception"));
     }
 
-    public void testStartError() throws Exception {
+    public void testStartError() {
         processValidator = pb -> { throw new IOException("something went wrong"); };
-        var e = expectThrows(UncheckedIOException.class, () -> runForeground());
+        var e = expectThrows(UncheckedIOException.class, this::runForeground);
         assertThat(e.getCause().getMessage(), equalTo("something went wrong"));
     }
 
-    public void testOptionsBuildingInterrupted() throws Exception {
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> { throw new InterruptedException("interrupted while get jvm options"); };
-        var e = expectThrows(RuntimeException.class, () -> runForeground());
-        assertThat(e.getCause().getMessage(), equalTo("interrupted while get jvm options"));
-    }
-
     public void testEnvPassthrough() throws Exception {
         envVars.put("MY_ENV", "foo");
         processValidator = pb -> { assertThat(pb.environment(), hasEntry(equalTo("MY_ENV"), equalTo("foo"))); };
@@ -276,83 +276,48 @@ public class ServerProcessTests extends ESTestCase {
         runForeground();
     }
 
-    public void testTempDir() throws Exception {
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> {
-            assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
-            assertThat(tmpDir.getFileName().toString(), startsWith("elasticsearch-"));
-            return new ArrayList<>();
-        };
-        runForeground();
-    }
-
-    public void testTempDirWindows() throws Exception {
-        Path baseTmpDir = createTempDir();
-        sysprops.put("os.name", "Windows 10");
-        sysprops.put("java.io.tmpdir", baseTmpDir.toString());
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> {
-            assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
-            assertThat(tmpDir.getFileName().toString(), equalTo("elasticsearch"));
-            assertThat(tmpDir.getParent().toString(), equalTo(baseTmpDir.toString()));
-            return new ArrayList<>();
-        };
-        runForeground();
-    }
-
-    public void testTempDirOverride() throws Exception {
+    public void testEnvCleared() throws Exception {
         Path customTmpDir = createTempDir();
         envVars.put("ES_TMPDIR", customTmpDir.toString());
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> {
-            assertThat(tmpDir.toString(), equalTo(customTmpDir.toString()));
-            return new ArrayList<>();
-        };
-        processValidator = pb -> assertThat(pb.environment(), not(hasKey("ES_TMPDIR")));
-        runForeground();
-    }
-
-    public void testTempDirOverrideMissing() throws Exception {
-        Path baseDir = createTempDir();
-        envVars.put("ES_TMPDIR", baseDir.resolve("dne").toString());
-        var e = expectThrows(UserException.class, () -> runForeground());
-        assertThat(e.exitCode, equalTo(ExitCodes.CONFIG));
-        assertThat(e.getMessage(), containsString("dne] does not exist"));
-    }
-
-    public void testTempDirOverrideNotADirectory() throws Exception {
-        Path tmpFile = createTempFile();
-        envVars.put("ES_TMPDIR", tmpFile.toString());
-        var e = expectThrows(UserException.class, () -> runForeground());
-        assertThat(e.exitCode, equalTo(ExitCodes.CONFIG));
-        assertThat(e.getMessage(), containsString("is not a directory"));
-    }
-
-    public void testCustomJvmOptions() throws Exception {
         envVars.put("ES_JAVA_OPTS", "-Dmyoption=foo");
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> {
-            assertThat(envOptions, equalTo("-Dmyoption=foo"));
-            return new ArrayList<>();
+
+        processValidator = pb -> {
+            assertThat(pb.environment(), not(hasKey("ES_TMPDIR")));
+            assertThat(pb.environment(), not(hasKey("ES_JAVA_OPTS")));
         };
-        processValidator = pb -> assertThat(pb.environment(), not(hasKey("ES_JAVA_OPTS")));
         runForeground();
     }
 
     public void testCommandLineSysprops() throws Exception {
-        optionsBuilder = (args, configDir, tmpDir, envOptions) -> List.of("-Dfoo1=bar", "-Dfoo2=baz");
-        processValidator = pb -> {
-            assertThat(pb.command(), contains("-Dfoo1=bar"));
-            assertThat(pb.command(), contains("-Dfoo2=bar"));
+        ServerProcessBuilder.ProcessStarter starter = pb -> {
+            assertThat(pb.command(), hasItems("-Dfoo1=bar", "-Dfoo2=baz"));
+            process = new MockElasticsearchProcess();
+            return process;
         };
+        var serverProcessBuilder = new ServerProcessBuilder().withTerminal(terminal)
+            .withProcessInfo(createProcessInfo())
+            .withServerArgs(createServerArgs(false, false))
+            .withJvmOptions(List.of("-Dfoo1=bar", "-Dfoo2=baz"))
+            .withTempDir(Path.of("."));
+        serverProcessBuilder.start(starter).waitFor();
+    }
+
+    public void testServerProcessBuilderMissingArgumentError() throws Exception {
+        ServerProcessBuilder.ProcessStarter starter = pb -> new MockElasticsearchProcess();
+        var serverProcessBuilder = new ServerProcessBuilder().withTerminal(terminal)
+            .withProcessInfo(createProcessInfo())
+            .withServerArgs(createServerArgs(false, false))
+            .withTempDir(Path.of("."));
+        var ex = expectThrows(IllegalStateException.class, () -> serverProcessBuilder.start(starter).waitFor());
+        assertThat(ex.getMessage(), equalTo("'jvmOptions' is a required argument and needs to be specified before calling start()"));
     }
 
     public void testCommandLine() throws Exception {
         String mainClass = "org.elasticsearch.server/org.elasticsearch.bootstrap.Elasticsearch";
-        String distroSysprop = "-Des.distribution.type=testdistro";
         String modulePath = esHomeDir.resolve("lib").toString();
         Path javaBin = Paths.get("javahome").resolve("bin");
-        sysprops.put("es.distribution.type", "testdistro");
         AtomicReference<String> expectedJava = new AtomicReference<>(javaBin.resolve("java").toString());
-        processValidator = pb -> {
-            assertThat(pb.command(), hasItems(expectedJava.get(), distroSysprop, "--module-path", modulePath, "-m", mainClass));
-        };
+        processValidator = pb -> { assertThat(pb.command(), hasItems(expectedJava.get(), "--module-path", modulePath, "-m", mainClass)); };
         runForeground();
 
         sysprops.put("os.name", "Windows 10");
@@ -370,7 +335,7 @@ public class ServerProcessTests extends ESTestCase {
             // will block until stdin closed manually after test
             assertThat(stdin.read(), equalTo(-1));
         };
-        var server = startProcess(true, false, "");
+        var server = startProcess(true, false);
         server.detach();
         assertThat(terminal.getErrorOutput(), containsString("final message"));
         server.stop(); // this should be a noop, and will fail the stdin read assert above if shutdown sent
@@ -384,7 +349,7 @@ public class ServerProcessTests extends ESTestCase {
             nonInterruptibleVoid(mainReady::await);
             stderr.println("final message");
         };
-        var server = startProcess(false, false, "");
+        var server = startProcess(false, false);
         mainReady.countDown();
         server.stop();
         assertThat(process.main.isDone(), is(true)); // stop should have waited
@@ -399,7 +364,7 @@ public class ServerProcessTests extends ESTestCase {
             assertThat(stdin.read(), equalTo((int) BootstrapInfo.SERVER_SHUTDOWN_MARKER));
             stderr.println("final message");
         };
-        var server = startProcess(false, false, "");
+        var server = startProcess(false, false);
         new Thread(() -> {
             // simulate stop run as shutdown hook in another thread, eg from Ctrl-C
             nonInterruptibleVoid(mainReady::await);
@@ -420,7 +385,7 @@ public class ServerProcessTests extends ESTestCase {
             nonInterruptibleVoid(mainExit::await);
             exitCode.set(-9);
         };
-        var server = startProcess(false, false, "");
+        var server = startProcess(false, false);
         mainExit.countDown();
         int exitCode = server.waitFor();
         assertThat(exitCode, equalTo(-9));

+ 82 - 0
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessUtilsTests.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.server.cli;
+
+import org.elasticsearch.cli.ExitCodes;
+import org.elasticsearch.cli.ProcessInfo;
+import org.elasticsearch.cli.UserException;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.startsWith;
+
+public class ServerProcessUtilsTests extends ESTestCase {
+
+    protected final Map<String, String> sysprops = new HashMap<>();
+    protected final Map<String, String> envVars = new HashMap<>();
+
+    @Before
+    public void resetEnv() {
+        sysprops.clear();
+        sysprops.put("os.name", "Linux");
+        sysprops.put("java.home", "javahome");
+        envVars.clear();
+    }
+
+    private ProcessInfo createProcessInfo() {
+        return new ProcessInfo(Map.copyOf(sysprops), Map.copyOf(envVars), Path.of("."));
+    }
+
+    public void testTempDir() throws Exception {
+        var tmpDir = ServerProcessUtils.setupTempDir(createProcessInfo());
+        assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
+        assertThat(tmpDir.getFileName().toString(), startsWith("elasticsearch-"));
+    }
+
+    public void testTempDirWindows() throws Exception {
+        Path baseTmpDir = createTempDir();
+        sysprops.put("os.name", "Windows 10");
+        sysprops.put("java.io.tmpdir", baseTmpDir.toString());
+        var tmpDir = ServerProcessUtils.setupTempDir(createProcessInfo());
+        assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
+        assertThat(tmpDir.getFileName().toString(), equalTo("elasticsearch"));
+        assertThat(tmpDir.getParent().toString(), equalTo(baseTmpDir.toString()));
+    }
+
+    public void testTempDirOverride() throws Exception {
+        Path customTmpDir = createTempDir();
+        envVars.put("ES_TMPDIR", customTmpDir.toString());
+        var tmpDir = ServerProcessUtils.setupTempDir(createProcessInfo());
+        assertThat(tmpDir.toString(), equalTo(customTmpDir.toString()));
+    }
+
+    public void testTempDirOverrideMissing() {
+        Path baseDir = createTempDir();
+        envVars.put("ES_TMPDIR", baseDir.resolve("dne").toString());
+        var e = expectThrows(UserException.class, () -> ServerProcessUtils.setupTempDir(createProcessInfo()));
+        assertThat(e.exitCode, equalTo(ExitCodes.CONFIG));
+        assertThat(e.getMessage(), containsString("dne] does not exist"));
+    }
+
+    public void testTempDirOverrideNotADirectory() throws Exception {
+        Path tmpFile = createTempFile();
+        envVars.put("ES_TMPDIR", tmpFile.toString());
+        var e = expectThrows(UserException.class, () -> ServerProcessUtils.setupTempDir(createProcessInfo()));
+        assertThat(e.exitCode, equalTo(ExitCodes.CONFIG));
+        assertThat(e.getMessage(), containsString("is not a directory"));
+    }
+}

+ 11 - 1
distribution/tools/windows-service-cli/src/main/java/org/elasticsearch/windows/service/WindowsServiceDaemon.java

@@ -17,7 +17,10 @@ import org.elasticsearch.common.cli.EnvironmentAwareCommand;
 import org.elasticsearch.common.settings.KeyStoreWrapper;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.env.Environment;
+import org.elasticsearch.server.cli.JvmOptionsParser;
 import org.elasticsearch.server.cli.ServerProcess;
+import org.elasticsearch.server.cli.ServerProcessBuilder;
+import org.elasticsearch.server.cli.ServerProcessUtils;
 
 /**
  * Starts an Elasticsearch process, but does not wait for it to exit.
@@ -38,7 +41,14 @@ class WindowsServiceDaemon extends EnvironmentAwareCommand {
         // the Windows service daemon doesn't support secure settings implementations other than the keystore
         try (var loadedSecrets = KeyStoreWrapper.bootstrap(env.configFile(), () -> new SecureString(new char[0]))) {
             var args = new ServerArgs(false, true, null, loadedSecrets, env.settings(), env.configFile(), env.logsFile());
-            this.server = ServerProcess.start(terminal, processInfo, args);
+            var tempDir = ServerProcessUtils.setupTempDir(processInfo);
+            var jvmOptions = JvmOptionsParser.determineJvmOptions(args, processInfo, tempDir);
+            var serverProcessBuilder = new ServerProcessBuilder().withTerminal(terminal)
+                .withProcessInfo(processInfo)
+                .withServerArgs(args)
+                .withTempDir(tempDir)
+                .withJvmOptions(jvmOptions);
+            this.server = serverProcessBuilder.start();
             // start does not return until the server is ready, and we do not wait for the process
         }
     }