Ensure logs dir exists before using as working dir (#126566)

With the change to using the logs dir as the working dir of the
Elasticsearch process we need to ensure the logs dir exists within the
CLI instead of later during startup.

relates #124966
This commit is contained in:
Ryan Ernst 2025-04-17 12:59:47 -07:00 committed by GitHub
parent 7e62862eab
commit 42dc870ece
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 46 additions and 25 deletions

View file

@ -271,8 +271,7 @@ class ServerCli extends EnvironmentAwareCommand {
.withProcessInfo(processInfo)
.withServerArgs(args)
.withTempDir(tempDir)
.withJvmOptions(jvmOptions)
.withWorkingDir(args.logsDir());
.withJvmOptions(jvmOptions);
return serverProcessBuilder.start();
}

View file

@ -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 org.elasticsearch.core.SuppressForbidden;
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<String> jvmOptions;
private Path workingDir;
private Terminal terminal;
// this allows mocking the process building by tests
@ -84,11 +86,6 @@ public class ServerProcessBuilder {
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 class ServerProcessBuilder {
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 @@ public class ServerProcessBuilder {
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());

View file

@ -65,7 +65,7 @@ public class ServerProcessTests extends ESTestCase {
protected final Map<String, String> sysprops = new HashMap<>();
protected final Map<String, String> envVars = new HashMap<>();
Path esHomeDir;
Path workingDir;
Path logsDir;
Settings.Builder nodeSettings;
ProcessValidator processValidator;
MainMethod mainCallback;
@ -94,8 +94,8 @@ public class ServerProcessTests extends ESTestCase {
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 @@ public class ServerProcessTests extends ESTestCase {
}
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 @@ public class ServerProcessTests extends ESTestCase {
.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 class ServerProcessTests extends ESTestCase {
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 class ServerProcessTests extends ESTestCase {
.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 class ServerProcessTests extends ESTestCase {
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"));
}
}