Skip to content

Ensure logs dir exists before using as working dir #126566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 17, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,9 +19,11 @@
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.core.SuppressForbidden;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.List;
Expand All @@ -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
Expand Down Expand Up @@ -84,11 +86,6 @@ public ServerProcessBuilder withJvmOptions(List<String> 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
*/
Expand Down Expand Up @@ -138,9 +135,21 @@ private String getCommand() {
* @throws UserException If the process failed during bootstrap
*/
public ServerProcess start() throws UserException {
ensureWorkingDirExists();
return start(ProcessBuilder::start);
}

private void ensureWorkingDirExists() throws UserException {
Path workingDir = serverArgs.logsDir();
try {
Files.createDirectories(workingDir);
} catch (FileNotFoundException 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(
Expand All @@ -162,7 +171,7 @@ ServerProcess start(ProcessStarter processStarter) throws UserException {

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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ 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;
Settings.Builder nodeSettings;
ProcessValidator processValidator;
MainMethod mainCallback;
Expand Down Expand Up @@ -95,7 +94,6 @@ public void resetEnv() {
sysprops.put("java.home", "javahome");
sysprops.put("es.path.home", esHomeDir.toString());
envVars.clear();
workingDir = createTempDir();
nodeSettings = Settings.builder();
processValidator = null;
mainCallback = null;
Expand Down Expand Up @@ -231,8 +229,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);
}

Expand All @@ -241,7 +238,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)) {
Expand Down Expand Up @@ -315,8 +312,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();
}

Expand Down