diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java index 8eae170667a84..5837f5fa8557e 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java @@ -271,8 +271,7 @@ protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, .withProcessInfo(processInfo) .withServerArgs(args) .withTempDir(tempDir) - .withJvmOptions(jvmOptions) - .withWorkingDir(args.logsDir()); + .withJvmOptions(jvmOptions); return serverProcessBuilder.start(); } diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java index 12a8bc00d4209..375a12ea5cc7b 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.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.Terminal; import org.elasticsearch.cli.UserException; @@ -21,6 +22,8 @@ import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; import java.util.List; @@ -44,7 +47,6 @@ public class ServerProcessBuilder { private ServerArgs serverArgs; private ProcessInfo processInfo; private List jvmOptions; - private Path workingDir; private Terminal terminal; // this allows mocking the process building by tests @@ -84,11 +86,6 @@ public ServerProcessBuilder withJvmOptions(List jvmOptions) { return this; } - public ServerProcessBuilder withWorkingDir(Path workingDir) { - this.workingDir = workingDir; - return this; - } - /** * Specifies the {@link Terminal} to use for reading input and writing output from/to the cli console */ @@ -141,6 +138,17 @@ public ServerProcess start() throws UserException { return start(ProcessBuilder::start); } + private void ensureWorkingDirExists() throws UserException { + Path workingDir = serverArgs.logsDir(); + try { + Files.createDirectories(workingDir); + } catch (FileAlreadyExistsException e) { + throw new UserException(ExitCodes.CONFIG, "Logs dir [" + workingDir + "] exists but is not a directory", e); + } catch (IOException e) { + throw new UserException(ExitCodes.CONFIG, "Unable to create logs dir [" + workingDir + "]", e); + } + } + private static void checkRequiredArgument(Object argument, String argumentName) { if (argument == null) { throw new IllegalStateException( @@ -157,12 +165,14 @@ ServerProcess start(ProcessStarter processStarter) throws UserException { checkRequiredArgument(jvmOptions, "jvmOptions"); checkRequiredArgument(terminal, "terminal"); + ensureWorkingDirExists(); + Process jvmProcess = null; ErrorPumpThread errorPump; boolean success = false; try { - jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), workingDir, processStarter); + jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), serverArgs.logsDir(), processStarter); errorPump = new ErrorPumpThread(terminal, jvmProcess.getErrorStream()); errorPump.start(); sendArgs(serverArgs, jvmProcess.getOutputStream()); diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java index c9e233e22dd76..86bc336f0b4ba 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java @@ -65,7 +65,7 @@ public class ServerProcessTests extends ESTestCase { protected final Map sysprops = new HashMap<>(); protected final Map envVars = new HashMap<>(); Path esHomeDir; - Path workingDir; + Path logsDir; Settings.Builder nodeSettings; ProcessValidator processValidator; MainMethod mainCallback; @@ -94,8 +94,8 @@ public void resetEnv() { sysprops.put("os.name", "Linux"); sysprops.put("java.home", "javahome"); sysprops.put("es.path.home", esHomeDir.toString()); + logsDir = esHomeDir.resolve("logs"); envVars.clear(); - workingDir = createTempDir(); nodeSettings = Settings.builder(); processValidator = null; mainCallback = null; @@ -207,15 +207,7 @@ ProcessInfo createProcessInfo() { } ServerArgs createServerArgs(boolean daemonize, boolean quiet) { - return new ServerArgs( - daemonize, - quiet, - null, - secrets, - nodeSettings.build(), - esHomeDir.resolve("config"), - esHomeDir.resolve("logs") - ); + return new ServerArgs(daemonize, quiet, null, secrets, nodeSettings.build(), esHomeDir.resolve("config"), logsDir); } ServerProcess startProcess(boolean daemonize, boolean quiet) throws Exception { @@ -231,8 +223,7 @@ ServerProcess startProcess(boolean daemonize, boolean quiet) throws Exception { .withProcessInfo(pinfo) .withServerArgs(createServerArgs(daemonize, quiet)) .withJvmOptions(List.of()) - .withTempDir(ServerProcessUtils.setupTempDir(pinfo)) - .withWorkingDir(workingDir); + .withTempDir(ServerProcessUtils.setupTempDir(pinfo)); return serverProcessBuilder.start(starter); } @@ -241,7 +232,7 @@ public void testProcessBuilder() throws Exception { assertThat(pb.redirectInput(), equalTo(ProcessBuilder.Redirect.PIPE)); assertThat(pb.redirectOutput(), equalTo(ProcessBuilder.Redirect.INHERIT)); assertThat(pb.redirectError(), equalTo(ProcessBuilder.Redirect.PIPE)); - assertThat(String.valueOf(pb.directory()), equalTo(workingDir.toString())); // leave default, which is working directory + assertThat(String.valueOf(pb.directory()), equalTo(esHomeDir.resolve("logs").toString())); }; mainCallback = (args, stdin, stderr, exitCode) -> { try (PrintStream err = new PrintStream(stderr, true, StandardCharsets.UTF_8)) { @@ -315,8 +306,7 @@ public void testCommandLineSysprops() throws Exception { .withProcessInfo(createProcessInfo()) .withServerArgs(createServerArgs(false, false)) .withJvmOptions(List.of("-Dfoo1=bar", "-Dfoo2=baz")) - .withTempDir(Path.of(".")) - .withWorkingDir(workingDir); + .withTempDir(Path.of(".")); serverProcessBuilder.start(starter).waitFor(); } @@ -433,4 +423,26 @@ public void testProcessDies() throws Exception { int exitCode = server.waitFor(); assertThat(exitCode, equalTo(-9)); } + + public void testLogsDirIsFile() throws Exception { + Files.createFile(logsDir); + var e = expectThrows(UserException.class, this::runForeground); + assertThat(e.getMessage(), containsString("exists but is not a directory")); + } + + public void testLogsDirCreateParents() throws Exception { + Path testDir = createTempDir(); + logsDir = testDir.resolve("subdir/logs"); + processValidator = pb -> assertThat(String.valueOf(pb.directory()), equalTo(logsDir.toString())); + runForeground(); + } + + public void testLogsCreateFailure() throws Exception { + Path testDir = createTempDir(); + Path parentFile = testDir.resolve("exists"); + Files.createFile(parentFile); + logsDir = parentFile.resolve("logs"); + var e = expectThrows(UserException.class, this::runForeground); + assertThat(e.getMessage(), containsString("Unable to create logs dir")); + } }