From 70b4bbe58b335f8054afce8c262584e6f20fa375 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Apr 2025 07:07:11 -0700 Subject: [PATCH 1/2] Use logs dir as working directory (#124966) In the unexpected case that Elasticsearch dies due to a segfault or other similar native issue, a core dump is useful in diagnosing the problem. Yet core dumps are written to the working directory, which is read-only for most installations of Elasticsearch. This commit changes the working directory to the logs dir which should always be writeable. --- .../LegacyYamlRestTestPluginFuncTest.groovy | 6 +- .../testclusters/ElasticsearchNode.java | 53 +++++++++++++---- distribution/build.gradle | 18 ------ distribution/src/config/jvm.options | 6 +- .../launcher/CliToolLauncher.java | 2 +- .../elasticsearch/server/cli/JvmOption.java | 14 ++++- .../server/cli/JvmOptionsParser.java | 6 +- .../elasticsearch/server/cli/ServerCli.java | 12 ++-- .../server/cli/ServerProcessBuilder.java | 16 ++++- .../server/cli/SystemJvmOptions.java | 10 ++-- .../server/cli/JvmOptionsParserTests.java | 21 +++---- .../server/cli/ServerCliTests.java | 3 +- .../server/cli/ServerProcessTests.java | 14 +++-- docs/changelog/124966.yaml | 5 ++ .../elasticsearch/cli/CliToolProvider.java | 7 ++- .../bootstrap/EntitlementBootstrap.java | 3 +- .../nativeaccess/lib/LoaderHelper.java | 3 +- .../local/AbstractLocalClusterFactory.java | 59 +++++++++++++++---- .../TimestampFormatFinderTests.java | 2 +- 19 files changed, 173 insertions(+), 87 deletions(-) create mode 100644 docs/changelog/124966.yaml diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy index ce5c1519fe11f..90ac5369f5df4 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy @@ -179,9 +179,9 @@ echo "Running elasticsearch \$0" file(distProjectFolder, 'src/config/elasticsearch.properties') << "some propes" file(distProjectFolder, 'src/config/jvm.options') << """ --Xlog:gc*,gc+age=trace,safepoint:file=logs/gc.log:utctime,level,pid,tags:filecount=32,filesize=64m --XX:ErrorFile=logs/hs_err_pid%p.log --XX:HeapDumpPath=data +-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m +-XX:ErrorFile=hs_err_pid%p.log +# -XX:HeapDumpPath=/heap/dump/path """ file(distProjectFolder, 'build.gradle') << """ import org.gradle.api.internal.artifacts.ArtifactAttributes; diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index 705fda530256c..2ada0b74f169e 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1437,12 +1437,17 @@ private void tweakJvmOptions(Path configFileRoot) { Path jvmOptions = configFileRoot.resolve("jvm.options"); try { String content = new String(Files.readAllBytes(jvmOptions)); - Map expansions = jvmOptionExpansions(); - for (String origin : expansions.keySet()) { - if (content.contains(origin) == false) { - throw new IOException("template property " + origin + " not found in template."); + Map expansions = jvmOptionExpansions(); + for (var entry : expansions.entrySet()) { + ReplacementKey replacement = entry.getKey(); + String key = replacement.key(); + if (content.contains(key) == false) { + key = replacement.fallback(); + if (content.contains(key) == false) { + throw new IOException("Template property '" + replacement + "' not found in template:\n" + content); + } } - content = content.replace(origin, expansions.get(origin)); + content = content.replace(key, entry.getValue()); } Files.write(jvmOptions, content.getBytes()); } catch (IOException ioException) { @@ -1450,17 +1455,39 @@ private void tweakJvmOptions(Path configFileRoot) { } } - private Map jvmOptionExpansions() { - Map expansions = new HashMap<>(); + private record ReplacementKey(String key, String fallback) {} + + private Map jvmOptionExpansions() { + Map expansions = new HashMap<>(); Version version = getVersion(); - String heapDumpOrigin = getVersion().onOrAfter("6.3.0") ? "-XX:HeapDumpPath=data" : "-XX:HeapDumpPath=/heap/dump/path"; - expansions.put(heapDumpOrigin, "-XX:HeapDumpPath=" + confPathLogs); - if (version.onOrAfter("6.2.0")) { - expansions.put("logs/gc.log", confPathLogs.resolve("gc.log").toString()); + + ReplacementKey heapDumpPathSub; + if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { + heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", null); + } else { + // temporarily fall back to the old substitution so both old and new work during backport + heapDumpPathSub = new ReplacementKey("# -XX:HeapDumpPath=/heap/dump/path", "-XX:HeapDumpPath=data"); } - if (getVersion().getMajor() >= 7) { - expansions.put("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log")); + expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); + + ReplacementKey gcLogSub; + if (version.before("8.19.0") && version.onOrAfter("6.2.0")) { + gcLogSub = new ReplacementKey("logs/gc.log", null); + } else { + // temporarily check the old substitution first so both old and new work during backport + gcLogSub = new ReplacementKey("logs/gc.log", "gc.log"); } + expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); + + ReplacementKey errorFileSub; + if (version.before("8.19.0") && version.getMajor() >= 7) { + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", null); + } else { + // temporarily check the old substitution first so both old and new work during backport + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=hs_err_pid%p.log"); + } + expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log")); + return expansions; } diff --git a/distribution/build.gradle b/distribution/build.gradle index 687b684d5106f..1606bef9798a0 100644 --- a/distribution/build.gradle +++ b/distribution/build.gradle @@ -534,7 +534,6 @@ subprojects { final String packagingPathData = "path.data: /var/lib/elasticsearch" final String pathLogs = "/var/log/elasticsearch" final String packagingPathLogs = "path.logs: ${pathLogs}" - final String packagingLoggc = "${pathLogs}/gc.log" String licenseText if (isTestDistro) { @@ -579,23 +578,6 @@ subprojects { 'rpm': packagingPathLogs, 'def': '#path.logs: /path/to/logs' ], - 'loggc': [ - 'deb': packagingLoggc, - 'rpm': packagingLoggc, - 'def': 'logs/gc.log' - ], - - 'heap.dump.path': [ - 'deb': "-XX:HeapDumpPath=/var/lib/elasticsearch", - 'rpm': "-XX:HeapDumpPath=/var/lib/elasticsearch", - 'def': "-XX:HeapDumpPath=data" - ], - - 'error.file': [ - 'deb': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log", - 'rpm': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log", - 'def': "-XX:ErrorFile=logs/hs_err_pid%p.log" - ], 'scripts.footer': [ /* Debian needs exit 0 on these scripts so we add it here and preserve diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index 340baedded785..8106e0f956b62 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -74,10 +74,10 @@ # specify an alternative path for heap dumps; ensure the directory exists and # has sufficient space -@heap.dump.path@ +# -XX:HeapDumpPath=/heap/dump/path # specify an alternative path for JVM fatal error logs -@error.file@ +-XX:ErrorFile=hs_err_pid%p.log ## GC logging --Xlog:gc*,gc+age=trace,safepoint:file=@loggc@:utctime,level,pid,tags:filecount=32,filesize=64m +-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m diff --git a/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java b/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java index a3ef768c30100..7ffc9276e2f40 100644 --- a/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java +++ b/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java @@ -58,7 +58,7 @@ public static void main(String[] args) throws Exception { String toolname = getToolName(pinfo.sysprops()); String libs = pinfo.sysprops().getOrDefault("cli.libs", ""); - command = CliToolProvider.load(toolname, libs).create(); + command = CliToolProvider.load(pinfo.sysprops(), toolname, libs).create(); Terminal terminal = Terminal.DEFAULT; Runtime.getRuntime().addShutdownHook(createShutdownHook(terminal, command)); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java index 0fa3e9463fc48..03aa7db72d142 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java @@ -10,12 +10,14 @@ package org.elasticsearch.server.cli; import org.elasticsearch.common.Strings; +import org.elasticsearch.core.SuppressForbidden; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.List; import java.util.Locale; @@ -106,7 +108,9 @@ private static List flagsFinal(final List userDefinedJvmOptions) userDefinedJvmOptions.stream(), Stream.of("-XX:+PrintFlagsFinal", "-version") ).flatMap(Function.identity()).toList(); - final Process process = new ProcessBuilder().command(command).start(); + final ProcessBuilder builder = new ProcessBuilder().command(command); + setWorkingDir(builder); + final Process process = builder.start(); final List output = readLinesFromInputStream(process.getInputStream()); final List error = readLinesFromInputStream(process.getErrorStream()); final int status = process.waitFor(); @@ -124,6 +128,14 @@ private static List flagsFinal(final List userDefinedJvmOptions) } } + @SuppressForbidden(reason = "ProcessBuilder takes File") + private static void setWorkingDir(ProcessBuilder builder) throws IOException { + // The real ES process uses the logs dir as the working directory. Since we don't + // have the logs dir yet, here we use a temp directory for calculating jvm options. + final Path tmpDir = Files.createTempDirectory("final-flags"); + builder.directory(tmpDir.toFile()); + } + private static List readLinesFromInputStream(final InputStream is) throws IOException { try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) { return br.lines().toList(); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java index d2ac1549efd1a..e97dc4c06f651 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java @@ -269,13 +269,13 @@ interface InvalidLineConsumer { * and the following JVM options will not be accepted: *
    *
  • - * {@code 18:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m} + * {@code 18:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m} *
    • *
  • - * {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m} + * {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m} *
    • *
  • - * {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m} + * {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m} *
    • *
* 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 be454350133eb..8eae170667a84 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 @@ -33,6 +33,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.Locale; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -168,7 +169,7 @@ Environment autoConfigureSecurity( assert secureSettingsLoader(env) instanceof KeyStoreLoader; String autoConfigLibs = "modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli"; - Command cmd = loadTool("auto-configure-node", autoConfigLibs); + Command cmd = loadTool(processInfo.sysprops(), "auto-configure-node", autoConfigLibs); assert cmd instanceof EnvironmentAwareCommand; @SuppressWarnings("raw") var autoConfigNode = (EnvironmentAwareCommand) cmd; @@ -210,7 +211,7 @@ Environment autoConfigureSecurity( // package private for testing void syncPlugins(Terminal terminal, Environment env, ProcessInfo processInfo) throws Exception { String pluginCliLibs = "lib/tools/plugin-cli"; - Command cmd = loadTool("sync-plugins", pluginCliLibs); + Command cmd = loadTool(processInfo.sysprops(), "sync-plugins", pluginCliLibs); assert cmd instanceof EnvironmentAwareCommand; @SuppressWarnings("raw") var syncPlugins = (EnvironmentAwareCommand) cmd; @@ -258,8 +259,8 @@ protected ServerProcess getServer() { } // protected to allow tests to override - protected Command loadTool(String toolname, String libs) { - return CliToolProvider.load(toolname, libs).create(); + protected Command loadTool(Map sysprops, String toolname, String libs) { + return CliToolProvider.load(sysprops, toolname, libs).create(); } // protected to allow tests to override @@ -270,7 +271,8 @@ protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, .withProcessInfo(processInfo) .withServerArgs(args) .withTempDir(tempDir) - .withJvmOptions(jvmOptions); + .withJvmOptions(jvmOptions) + .withWorkingDir(args.logsDir()); 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 adebf6be9842b..12a8bc00d4209 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 @@ -16,6 +16,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.core.PathUtils; +import org.elasticsearch.core.SuppressForbidden; import java.io.IOException; import java.io.OutputStream; @@ -43,6 +44,7 @@ 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 @@ -82,6 +84,11 @@ 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 */ @@ -155,7 +162,7 @@ ServerProcess start(ProcessStarter processStarter) throws UserException { boolean success = false; try { - jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), processStarter); + jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), workingDir, processStarter); errorPump = new ErrorPumpThread(terminal, jvmProcess.getErrorStream()); errorPump.start(); sendArgs(serverArgs, jvmProcess.getOutputStream()); @@ -185,16 +192,23 @@ private static Process createProcess( List jvmArgs, List jvmOptions, Map environment, + Path workingDir, 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); + setWorkingDir(builder, workingDir); builder.redirectOutput(ProcessBuilder.Redirect.INHERIT); return processStarter.start(builder); } + @SuppressForbidden(reason = "ProcessBuilder takes File") + private static void setWorkingDir(ProcessBuilder builder, Path path) { + builder.directory(path.toFile()); + } + 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); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java index 1dd2e8fb8ace6..128604a9db8fc 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java @@ -24,6 +24,7 @@ final class SystemJvmOptions { static List systemJvmOptions(Settings nodeSettings, final Map sysprops) { + Path esHome = Path.of(sysprops.get("es.path.home")); String distroType = sysprops.get("es.distribution.type"); String javaType = sysprops.get("es.java.type"); boolean isHotspot = sysprops.getOrDefault("sun.management.compiler", "").contains("HotSpot"); @@ -67,7 +68,8 @@ static List systemJvmOptions(Settings nodeSettings, final Map TEST_SYSPROPS = Map.of("os.name", "Linux", "os.arch", "aarch64"); - - private static final Path ENTITLEMENTS_LIB_DIR = Path.of("lib", "entitlement-bridge"); + private static Map testSysprops; @BeforeClass public static void beforeClass() throws IOException { - Files.createDirectories(ENTITLEMENTS_LIB_DIR); - Files.createTempFile(ENTITLEMENTS_LIB_DIR, "mock-entitlements-bridge", ".jar"); + Path homeDir = createTempDir(); + Path entitlementLibDir = homeDir.resolve("lib/entitlement-bridge"); + Files.createDirectories(entitlementLibDir); + Files.createTempFile(entitlementLibDir, "mock-entitlements-bridge", ".jar"); + testSysprops = Map.of("os.name", "Linux", "os.arch", "aarch64", "es.path.home", homeDir.toString()); } @AfterClass @@ -367,30 +368,30 @@ public void accept(final int lineNumber, final String line) { public void testNodeProcessorsActiveCount() { { - final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, testSysprops); assertThat(jvmOptions, not(hasItem(containsString("-XX:ActiveProcessorCount=")))); } { Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 1).build(); - final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops); assertThat(jvmOptions, hasItem("-XX:ActiveProcessorCount=1")); } { // check rounding Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 0.2).build(); - final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops); 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, TEST_SYSPROPS)); + var e = expectThrows(IllegalArgumentException.class, () -> SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops)); assertThat(e.getMessage(), containsString("setting [node.processors] must be <=")); } } public void testCommandLineDistributionType() { - var sysprops = new HashMap<>(TEST_SYSPROPS); + var sysprops = new HashMap<>(testSysprops); sysprops.put("es.distribution.type", "testdistro"); final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, sysprops); assertThat(jvmOptions, hasItem("-Des.distribution.type=testdistro")); diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java index 41ed786690328..2792dcd04986f 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java @@ -36,6 +36,7 @@ import java.nio.file.Path; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Optional; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CyclicBarrier; @@ -558,7 +559,7 @@ private class TestServerCli extends ServerCli { boolean startServerCalled = false; @Override - protected Command loadTool(String toolname, String libs) { + protected Command loadTool(Map sysprops, 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; 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 d4c05937f35b7..c9e233e22dd76 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 @@ -57,7 +57,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; public class ServerProcessTests extends ESTestCase { @@ -66,6 +65,7 @@ public class ServerProcessTests extends ESTestCase { protected final Map sysprops = new HashMap<>(); protected final Map envVars = new HashMap<>(); Path esHomeDir; + Path workingDir; Settings.Builder nodeSettings; ProcessValidator processValidator; MainMethod mainCallback; @@ -88,12 +88,14 @@ int runForeground() throws Exception { @Before public void resetEnv() { + esHomeDir = createTempDir(); terminal.reset(); sysprops.clear(); sysprops.put("os.name", "Linux"); sysprops.put("java.home", "javahome"); + sysprops.put("es.path.home", esHomeDir.toString()); envVars.clear(); - esHomeDir = createTempDir(); + workingDir = createTempDir(); nodeSettings = Settings.builder(); processValidator = null; mainCallback = null; @@ -229,7 +231,8 @@ ServerProcess startProcess(boolean daemonize, boolean quiet) throws Exception { .withProcessInfo(pinfo) .withServerArgs(createServerArgs(daemonize, quiet)) .withJvmOptions(List.of()) - .withTempDir(ServerProcessUtils.setupTempDir(pinfo)); + .withTempDir(ServerProcessUtils.setupTempDir(pinfo)) + .withWorkingDir(workingDir); return serverProcessBuilder.start(starter); } @@ -238,7 +241,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(pb.directory(), nullValue()); // leave default, which is working directory + assertThat(String.valueOf(pb.directory()), equalTo(workingDir.toString())); // leave default, which is working directory }; mainCallback = (args, stdin, stderr, exitCode) -> { try (PrintStream err = new PrintStream(stderr, true, StandardCharsets.UTF_8)) { @@ -312,7 +315,8 @@ public void testCommandLineSysprops() throws Exception { .withProcessInfo(createProcessInfo()) .withServerArgs(createServerArgs(false, false)) .withJvmOptions(List.of("-Dfoo1=bar", "-Dfoo2=baz")) - .withTempDir(Path.of(".")); + .withTempDir(Path.of(".")) + .withWorkingDir(workingDir); serverProcessBuilder.start(starter).waitFor(); } diff --git a/docs/changelog/124966.yaml b/docs/changelog/124966.yaml new file mode 100644 index 0000000000000..7e962a795a485 --- /dev/null +++ b/docs/changelog/124966.yaml @@ -0,0 +1,5 @@ +pr: 124966 +summary: Use logs dir as working directory +area: Infra/CLI +type: enhancement +issues: [] diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java b/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java index 99138f2321475..6d42d3f764df1 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java @@ -19,6 +19,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.ServiceLoader; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -43,14 +44,14 @@ public interface CliToolProvider { /** * Loads a tool provider from the Elasticsearch distribution. * + * @param sysprops the system properties of the CLI process * @param toolname the name of the tool to load * @param libs the library directories to load, relative to the Elasticsearch homedir * @return the instance of the loaded tool * @throws AssertionError if the given toolname cannot be found or there are more than one tools found with the same name */ - static CliToolProvider load(String toolname, String libs) { - // the ES homedir is always our working dir - Path homeDir = Paths.get("").toAbsolutePath(); + static CliToolProvider load(Map sysprops, String toolname, String libs) { + Path homeDir = Paths.get(sysprops.get("es.path.home")).toAbsolutePath(); final ClassLoader cliLoader; if (libs.isBlank()) { cliLoader = ClassLoader.getSystemClassLoader(); diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java index 02037214b1b1b..928b953c295fe 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java @@ -140,7 +140,8 @@ static String findAgentJar() { return propertyValue; } - Path dir = Path.of("lib", "entitlement-agent"); + Path esHome = Path.of(System.getProperty("es.path.home")); + Path dir = esHome.resolve("lib/entitlement-agent"); if (Files.exists(dir) == false) { throw new IllegalStateException("Directory for entitlement jar does not exist: " + dir); } diff --git a/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java b/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java index d9c725f5a8d3b..98887381e5d40 100644 --- a/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java +++ b/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java @@ -26,7 +26,8 @@ private static Path findPlatformLibDir() { return Paths.get(path); } - Path platformDir = Paths.get("lib", "platform"); + Path homeDir = Paths.get(System.getProperty("es.path.home")); + Path platformDir = homeDir.resolve("lib/platform"); String osname = System.getProperty("os.name"); String os; diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index 78e790afb5a6e..10fd1de940d00 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -441,12 +441,17 @@ private void writeConfiguration() { // Patch jvm.options file to update paths String content = Files.readString(jvmOptionsFile); - Map expansions = getJvmOptionsReplacements(); - for (String key : expansions.keySet()) { + Map expansions = getJvmOptionsReplacements(); + for (var entry : expansions.entrySet()) { + ReplacementKey replacement = entry.getKey(); + String key = replacement.key(); if (content.contains(key) == false) { - throw new IOException("Template property '" + key + "' not found in template."); + key = replacement.fallback(); + if (content.contains(key) == false) { + throw new IOException("Template property '" + replacement + "' not found in template."); + } } - content = content.replace(key, expansions.get(key)); + content = content.replace(key, entry.getValue()); } Files.writeString(jvmOptionsFile, content); } catch (IOException e) { @@ -912,15 +917,43 @@ private Map getEnvironmentVariables() { return environment; } - private Map getJvmOptionsReplacements() { - return Map.of( - "-XX:HeapDumpPath=data", - "-XX:HeapDumpPath=" + logsDir, - "logs/gc.log", - logsDir.resolve("gc.log").toString(), - "-XX:ErrorFile=logs/hs_err_pid%p.log", - "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log") - ); + private record ReplacementKey(String key, String fallback) { + ReplacementKey { + assert fallback == null || fallback.isEmpty() == false; // no empty fallback, which would match anything + } + } + + private Map getJvmOptionsReplacements() { + var expansions = new HashMap(); + var version = spec.getVersion(); + + ReplacementKey heapDumpPathSub; + if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { + heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", null); + } else { + // temporarily fall back to the old substitution so both old and new work during backport + heapDumpPathSub = new ReplacementKey("# -XX:HeapDumpPath=/heap/dump/path", "-XX:HeapDumpPath=data"); + } + expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); + + ReplacementKey gcLogSub; + if (version.before("8.19.0") && version.onOrAfter("6.2.0")) { + gcLogSub = new ReplacementKey("logs/gc.log", null); + } else { + // temporarily check the old substitution first so both old and new work during backport + gcLogSub = new ReplacementKey("logs/gc.log", "gc.log"); + } + expansions.put(gcLogSub, logsDir.resolve("gc.log").toString()); + + ReplacementKey errorFileSub; + if (version.before("8.19.0") && version.getMajor() >= 7) { + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", null); + } else { + // temporarily check the old substitution first so both old and new work during backport + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=hs_err_pid%p.log"); + } + expansions.put(errorFileSub, "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log")); + return expansions; } private void runToolScript(String tool, String input, String... args) { diff --git a/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java b/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java index 87bb63f442acf..048a4c2e6e512 100644 --- a/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java +++ b/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java @@ -1590,7 +1590,7 @@ public void testSelectBestMatchGivenAllSame() { [2018-06-27T11:59:22,202][INFO ][o.e.e.NodeEnvironment ] [node-0] heap size [494.9mb], compressed ordinary object pointers [true] [2018-06-27T11:59:22,204][INFO ][o.e.n.Node ] [node-0] node name [node-0], node ID [Ha1gD8nNSDqjd6PIyu3DJA] [2018-06-27T11:59:22,204][INFO ][o.e.n.Node ] [node-0] version[6.4.0-SNAPSHOT], pid[2785], build[default/zip/3c60efa/2018-06-26T14:55:15.206676Z], OS[Mac OS X/10.12.6/x86_64], JVM["Oracle Corporation"/Java HotSpot(TM) 64-Bit Server VM/10/10+46] - [2018-06-27T11:59:22,205][INFO ][o.e.n.Node ] [node-0] JVM arguments [-Xms1g, -Xmx1g, -XX:+UseConcMarkSweepGC, -XX:CMSInitiatingOccupancyFraction=75, -XX:+UseCMSInitiatingOccupancyOnly, -XX:+AlwaysPreTouch, -Xss1m, -Djava.awt.headless=true, -Dfile.encoding=UTF-8, -Djna.nosys=true, -XX:-OmitStackTraceInFastThrow, -Dio.netty.noUnsafe=true, -Dio.netty.noKeySetOptimization=true, -Dio.netty.recycler.maxCapacityPerThread=0, -Dlog4j.shutdownHookEnabled=false, -Dlog4j2.disable.jmx=true, -Djava.io.tmpdir=/var/folders/k5/5sqcdlps5sg3cvlp783gcz740000h0/T/elasticsearch.nFUyeMH1, -XX:+HeapDumpOnOutOfMemoryError, -XX:HeapDumpPath=data, -XX:ErrorFile=logs/hs_err_pid%p.log, -Xlog:gc*,gc+age=trace,safepoint:file=logs/gc.log:utctime,level,pid,tags:filecount=32,filesize=64m, -Djava.locale.providers=COMPAT, -Dio.netty.allocator.type=unpooled, -ea, -esa, -Xms512m, -Xmx512m, -Des.path.home=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT, -Des.path.conf=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT/config, -Des.distribution.flavor=default, -Des.distribution.type=zip] + [2018-06-27T11:59:22,205][INFO ][o.e.n.Node ] [node-0] JVM arguments [-Xms1g, -Xmx1g, -XX:+UseConcMarkSweepGC, -XX:CMSInitiatingOccupancyFraction=75, -XX:+UseCMSInitiatingOccupancyOnly, -XX:+AlwaysPreTouch, -Xss1m, -Djava.awt.headless=true, -Dfile.encoding=UTF-8, -Djna.nosys=true, -XX:-OmitStackTraceInFastThrow, -Dio.netty.noUnsafe=true, -Dio.netty.noKeySetOptimization=true, -Dio.netty.recycler.maxCapacityPerThread=0, -Dlog4j.shutdownHookEnabled=false, -Dlog4j2.disable.jmx=true, -Djava.io.tmpdir=/var/folders/k5/5sqcdlps5sg3cvlp783gcz740000h0/T/elasticsearch.nFUyeMH1, -XX:+HeapDumpOnOutOfMemoryError, -XX:ErrorFile=hs_err_pid%p.log, -Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m, -Djava.locale.providers=COMPAT, -Dio.netty.allocator.type=unpooled, -ea, -esa, -Xms512m, -Xmx512m, -Des.path.home=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT, -Des.path.conf=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT/config, -Des.distribution.flavor=default, -Des.distribution.type=zip] [2018-06-27T11:59:22,205][WARN ][o.e.n.Node ] [node-0] version [6.4.0-SNAPSHOT] is a pre-release version of Elasticsearch and is not suitable for production [2018-06-27T11:59:23,585][INFO ][o.e.p.PluginsService ] [node-0] loaded module [aggs-matrix-stats] [2018-06-27T11:59:23,586][INFO ][o.e.p.PluginsService ] [node-0] loaded module [analysis-common] From 7f6ba85991b9e8442873bd1a75b7801f0fa35b86 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 17 Apr 2025 12:59:47 -0700 Subject: [PATCH 2/2] 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 --- .../elasticsearch/server/cli/ServerCli.java | 3 +- .../server/cli/ServerProcessBuilder.java | 24 +++++++--- .../server/cli/ServerProcessTests.java | 44 ++++++++++++------- 3 files changed, 46 insertions(+), 25 deletions(-) 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")); + } }