Browse Source

Move pidfile handling to server cli (#86934)

Now that the server cli is in java, we can do more system level things
inside it. This commit moves validating and writing the pidfile into the
server cli. One benefit is we get validation of directory/file problems
up front before even trying to start the ES process.
Ryan Ernst 3 years ago
parent
commit
55d8e60298

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

@@ -30,6 +30,7 @@ import org.elasticsearch.env.Environment;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Locale;
@@ -163,13 +164,26 @@ class ServerCli extends EnvironmentAwareCommand {
             env = createEnv(options, processInfo);
         }
         return env;
+    }
 
+    private void validatePidFile(Path pidFile) throws UserException {
+        Path parent = pidFile.getParent();
+        if (parent != null && Files.exists(parent) && Files.isDirectory(parent) == false) {
+            throw new UserException(ExitCodes.USAGE, "pid file parent [" + parent + "] exists but is not a directory");
+        }
+        if (Files.exists(pidFile) && Files.isRegularFile(pidFile) == false) {
+            throw new UserException(ExitCodes.USAGE, pidFile + " exists but is not a regular file");
+        }
     }
 
-    private ServerArgs createArgs(OptionSet options, Environment env, SecureString keystorePassword) {
+    private ServerArgs createArgs(OptionSet options, Environment env, SecureString keystorePassword) throws UserException {
         boolean daemonize = options.has(daemonizeOption);
         boolean quiet = options.has(quietOption);
-        Path pidFile = options.valueOf(pidfileOption);
+        Path pidFile = null;
+        if (options.has(pidfileOption)) {
+            pidFile = options.valueOf(pidfileOption);
+            validatePidFile(pidFile);
+        }
         return new ServerArgs(daemonize, quiet, pidFile, keystorePassword, env.settings(), env.configFile());
     }
 

+ 7 - 0
distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java

@@ -128,6 +128,13 @@ public class ServerProcess {
         return new ServerProcess(jvmProcess, errorPump);
     }
 
+    /**
+     * Return the process id of the server.
+     */
+    public long pid() {
+        return jvmProcess.pid();
+    }
+
     /**
      * Detaches the server process from the current process, enabling the current process to exit.
      *

+ 19 - 6
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java

@@ -93,15 +93,23 @@ public class ServerCliTests extends CommandTestCase {
         assertUsage(containsString(prefix + "[foo]"), "-E", "foo=bar", "foo", "-E", "baz=qux");
     }
 
-    public void testPidFile() throws Exception {
+    public void assertPidFile(String option) throws Exception {
         Path tmpDir = createTempDir();
         Path pidFileArg = tmpDir.resolve("pid");
-        assertUsage(containsString("Option p/pidfile requires an argument"), "-p");
-        argsValidator = args -> assertThat(args.pidFile().toString(), equalTo(pidFileArg.toString()));
-        terminal.reset();
-        assertOk("-p", pidFileArg.toString());
         terminal.reset();
-        assertOk("--pidfile", pidFileArg.toString());
+        argsValidator = args -> assertThat(args.pidFile().toString(), equalTo(pidFileArg.toString()));
+        assertOk(option, pidFileArg.toString());
+    }
+
+    public void testPidFile() throws Exception {
+        assertPidFile("-p");
+        assertPidFile("--pidfile");
+
+        assertUsage(containsString("Option p/pidfile requires an argument"), "-p");
+        Path pidParentFile = createTempFile();
+        assertUsage(containsString("exists but is not a directory"), "-p", pidParentFile.resolve("pid").toString());
+        assertUsage(containsString("exists but is not a regular file"), "-p", createTempDir().toString());
+
     }
 
     public void assertDaemonized(boolean daemonized, String... args) throws Exception {
@@ -300,6 +308,11 @@ public class ServerCliTests extends CommandTestCase {
             super(null, null);
         }
 
+        @Override
+        public long pid() {
+            return 12345;
+        }
+
         @Override
         public void detach() {
             assert detachCalled == false;

+ 13 - 7
distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java

@@ -79,7 +79,7 @@ public class ServerProcessTests extends ESTestCase {
     }
 
     void runForeground() throws Exception {
-        var server = startProcess(false, false, null, "");
+        var server = startProcess(false, false, "");
         server.waitFor();
     }
 
@@ -190,10 +190,10 @@ public class ServerProcessTests extends ESTestCase {
         }
     }
 
-    ServerProcess startProcess(boolean daemonize, boolean quiet, Path pidFile, String keystorePassword) throws Exception {
+    ServerProcess startProcess(boolean daemonize, boolean quiet, String keystorePassword) throws Exception {
         var pinfo = new ProcessInfo(Map.copyOf(sysprops), Map.copyOf(envVars), esHomeDir);
         SecureString password = new SecureString(keystorePassword.toCharArray());
-        var args = new ServerArgs(daemonize, quiet, pidFile, password, nodeSettings.build(), esHomeDir.resolve("config"));
+        var args = new ServerArgs(daemonize, quiet, null, password, nodeSettings.build(), esHomeDir.resolve("config"));
         ServerProcess.ProcessStarter starter = pb -> {
             if (processValidator != null) {
                 processValidator.validate(pb);
@@ -220,6 +220,12 @@ public class ServerProcessTests extends ESTestCase {
         assertThat(terminal.getErrorOutput(), containsString("stderr message"));
     }
 
+    public void testPid() throws Exception {
+        var server = startProcess(true, false, "");
+        assertThat(server.pid(), equalTo(12345L));
+        server.stop();
+    }
+
     public void testBootstrapError() throws Exception {
         mainCallback = (args, stdin, stderr, exitCode) -> {
             stderr.println(BootstrapInfo.USER_EXCEPTION_MARKER + "a bootstrap exception");
@@ -367,7 +373,7 @@ public class ServerProcessTests extends ESTestCase {
             // will block until stdin closed manually after test
             assertThat(stdin.read(), equalTo(-1));
         };
-        var server = startProcess(true, false, null, "");
+        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
@@ -381,7 +387,7 @@ public class ServerProcessTests extends ESTestCase {
             nonInterruptibleVoid(mainReady::await);
             stderr.println("final message");
         };
-        var server = startProcess(false, false, null, "");
+        var server = startProcess(false, false, "");
         mainReady.countDown();
         server.stop();
         assertThat(process.main.isDone(), is(true)); // stop should have waited
@@ -396,7 +402,7 @@ public class ServerProcessTests extends ESTestCase {
             assertThat(stdin.read(), equalTo((int) BootstrapInfo.SERVER_SHUTDOWN_MARKER));
             stderr.println("final message");
         };
-        var server = startProcess(false, false, null, "");
+        var server = startProcess(false, false, "");
         new Thread(() -> {
             // simulate stop run as shutdown hook in another thread, eg from Ctrl-C
             nonInterruptibleVoid(mainReady::await);
@@ -417,7 +423,7 @@ public class ServerProcessTests extends ESTestCase {
             nonInterruptibleVoid(mainExit::await);
             exitCode.set(-9);
         };
-        var server = startProcess(false, false, null, "");
+        var server = startProcess(false, false, "");
         nonInterruptibleVoid(mainReady::await);
         process.processStderr.close(); // mimic pipe break if cli process dies
         mainExit.countDown();

+ 0 - 3
qa/evil-tests/src/test/java/org/elasticsearch/bootstrap/EvilSecurityTests.java

@@ -77,7 +77,6 @@ public class EvilSecurityTests extends ESTestCase {
         );
         settingsBuilder.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), esHome.resolve("custom").toString());
         settingsBuilder.put(Environment.PATH_LOGS_SETTING.getKey(), esHome.resolve("logs").toString());
-        settingsBuilder.put(Environment.NODE_PIDFILE_SETTING.getKey(), esHome.resolve("test.pid").toString());
         Settings settings = settingsBuilder.build();
 
         Path fakeTmpDir = createTempDir();
@@ -123,8 +122,6 @@ public class EvilSecurityTests extends ESTestCase {
         assertExactPermissions(new FilePermission(environment.logsFile().toString(), "read,readlink,write,delete"), permissions);
         // temp dir: r/w
         assertExactPermissions(new FilePermission(fakeTmpDir.toString(), "read,readlink,write,delete"), permissions);
-        // PID file: delete only (for the shutdown hook)
-        assertExactPermissions(new FilePermission(environment.pidFile().toString(), "delete"), permissions);
     }
 
     public void testDuplicateDataPaths() throws IOException {

+ 3 - 20
server/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -19,7 +19,6 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
 import org.elasticsearch.bootstrap.plugins.PluginsManager;
 import org.elasticsearch.cli.UserException;
-import org.elasticsearch.common.PidFile;
 import org.elasticsearch.common.filesystem.FileSystemNatives;
 import org.elasticsearch.common.inject.CreationException;
 import org.elasticsearch.common.logging.LogConfigurator;
@@ -243,15 +242,11 @@ final class Bootstrap {
     // visible for tests
 
     private static Environment createEnvironment(
-        final Path pidFile,
         final SecureSettings secureSettings,
         final Settings initialSettings,
         final Path configPath
     ) {
         Settings.Builder builder = Settings.builder();
-        if (pidFile != null) {
-            builder.put(Environment.NODE_PIDFILE_SETTING.getKey(), pidFile);
-        }
         builder.put(initialSettings);
         if (secureSettings != null) {
             builder.setSecureSettings(secureSettings);
@@ -287,13 +282,8 @@ final class Bootstrap {
     /**
      * This method is invoked by {@link Elasticsearch#main(String[])} to startup elasticsearch.
      */
-    static void init(
-        final boolean foreground,
-        final Path pidFile,
-        final boolean quiet,
-        final Environment initialEnv,
-        SecureString keystorePassword
-    ) throws BootstrapException, NodeValidationException, UserException {
+    static void init(final boolean foreground, final boolean quiet, final Environment initialEnv, SecureString keystorePassword)
+        throws BootstrapException, NodeValidationException, UserException {
         // force the class initializer for BootstrapInfo to run before
         // the security manager is installed
         BootstrapInfo.init();
@@ -301,7 +291,7 @@ final class Bootstrap {
         INSTANCE = new Bootstrap();
 
         final SecureSettings keystore = BootstrapUtil.loadSecureSettings(initialEnv, keystorePassword);
-        final Environment environment = createEnvironment(pidFile, keystore, initialEnv.settings(), initialEnv.configFile());
+        final Environment environment = createEnvironment(keystore, initialEnv.settings(), initialEnv.configFile());
 
         BootstrapInfo.setConsole(getConsole(environment));
 
@@ -311,13 +301,6 @@ final class Bootstrap {
         } catch (IOException e) {
             throw new BootstrapException(e);
         }
-        if (environment.pidFile() != null) {
-            try {
-                PidFile.create(environment.pidFile(), true);
-            } catch (IOException e) {
-                throw new BootstrapException(e);
-            }
-        }
 
         try {
             // fail if somebody replaced the lucene jars

+ 1 - 3
server/src/main/java/org/elasticsearch/bootstrap/BootstrapException.java

@@ -8,14 +8,12 @@
 
 package org.elasticsearch.bootstrap;
 
-import java.nio.file.Path;
-
 /**
  * Wrapper exception for checked exceptions thrown during the bootstrap process. Methods invoked
  * during bootstrap should explicitly declare the checked exceptions that they can throw, rather
  * than declaring the top-level checked exception {@link Exception}. This exception exists to wrap
  * these checked exceptions so that
- * {@link Bootstrap#init(boolean, Path, boolean, org.elasticsearch.env.Environment, org.elasticsearch.common.settings.SecureString)}
+ * {@link Bootstrap#init(boolean, boolean, org.elasticsearch.env.Environment, org.elasticsearch.common.settings.SecureString)}
  * does not have to declare all of these checked exceptions.
  */
 class BootstrapException extends Exception {

+ 29 - 3
server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -22,6 +22,7 @@ import org.elasticsearch.node.NodeValidationException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.PrintStream;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.Permission;
 import java.security.Security;
@@ -61,9 +62,9 @@ class Elasticsearch {
         try {
             final var in = new InputStreamStreamInput(System.in);
             final ServerArgs serverArgs = new ServerArgs(in);
+            initPidFile(serverArgs.pidFile());
             elasticsearch.init(
                 serverArgs.daemonize(),
-                serverArgs.pidFile(),
                 serverArgs.quiet(),
                 new Environment(serverArgs.nodeSettings(), serverArgs.configDir()),
                 serverArgs.keystorePassword()
@@ -171,6 +172,31 @@ class Elasticsearch {
         }).start();
     }
 
+    /**
+     * Writes the current process id into the given pidfile, if not null. The pidfile is cleaned up on system exit.
+     *
+     * @param pidFile A path to a file, or null of no pidfile should be written
+     */
+    private static void initPidFile(Path pidFile) throws IOException {
+        if (pidFile == null) {
+            return;
+        }
+        Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+            if (Files.exists(pidFile)) {
+                try {
+                    Files.delete(pidFile);
+                } catch (IOException e) {
+                    // ignore, nothing we can do because are shutting down
+                }
+            }
+        }, "elasticsearch[pidfile-cleanup]"));
+
+        if (Files.exists(pidFile.getParent()) == false) {
+            Files.createDirectories(pidFile.getParent());
+        }
+        Files.writeString(pidFile, Long.toString(ProcessHandle.current().pid()));
+    }
+
     private static void overrideDnsCachePolicyProperties() {
         for (final String property : new String[] { "networkaddress.cache.ttl", "networkaddress.cache.negative.ttl" }) {
             final String overrideProperty = "es." + property;
@@ -186,10 +212,10 @@ class Elasticsearch {
         }
     }
 
-    void init(final boolean daemonize, final Path pidFile, final boolean quiet, Environment initialEnv, SecureString keystorePassword)
+    void init(final boolean daemonize, final boolean quiet, Environment initialEnv, SecureString keystorePassword)
         throws NodeValidationException, UserException {
         try {
-            Bootstrap.init(daemonize == false, pidFile, quiet, initialEnv, keystorePassword);
+            Bootstrap.init(daemonize == false, quiet, initialEnv, keystorePassword);
         } catch (BootstrapException | RuntimeException e) {
             // format exceptions to the console in a special way
             // to avoid 2MB stacktraces from guice, etc.

+ 0 - 4
server/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -245,10 +245,6 @@ final class Security {
         for (Path path : environment.repoFiles()) {
             addDirectoryPath(policy, Environment.PATH_REPO_SETTING.getKey(), path, "read,readlink,write,delete", false);
         }
-        if (environment.pidFile() != null) {
-            // we just need permission to remove the file if its elsewhere.
-            addSingleFilePath(policy, environment.pidFile(), "delete");
-        }
     }
 
     /**

+ 0 - 108
server/src/main/java/org/elasticsearch/common/PidFile.java

@@ -1,108 +0,0 @@
-/*
- * 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.common;
-
-import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.monitor.jvm.JvmInfo;
-
-import java.io.IOException;
-import java.io.OutputStream;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardOpenOption;
-
-/**
- * Process ID file abstraction that writes the current pid into a file and optionally
- * removes it on system exit.
- */
-public final class PidFile {
-
-    private final long pid;
-    private final Path path;
-    private final boolean deleteOnExit;
-
-    private PidFile(Path path, boolean deleteOnExit, long pid) throws IOException {
-        this.path = path;
-        this.deleteOnExit = deleteOnExit;
-        this.pid = pid;
-    }
-
-    /**
-     * Creates a new PidFile and writes the current process ID into the provided path
-     *
-     * @param path the path to the pid file. The file is newly created or truncated if it already exists
-     * @param deleteOnExit if <code>true</code> the pid file is deleted with best effort on system exit
-     * @throws IOException if an IOException occurs
-     */
-    public static PidFile create(Path path, boolean deleteOnExit) throws IOException {
-        return create(path, deleteOnExit, JvmInfo.jvmInfo().pid());
-    }
-
-    static PidFile create(Path path, boolean deleteOnExit, long pid) throws IOException {
-        Path parent = path.getParent();
-        if (parent != null) {
-            if (Files.exists(parent) && Files.isDirectory(parent) == false) {
-                throw new IllegalArgumentException(parent + " exists but is not a directory");
-            }
-            if (Files.exists(parent) == false) {
-                // only do this if it doesn't exists we get a better exception further down
-                // if there are security issues etc. this also doesn't work if the parent exists
-                // and is a soft-link like on many linux systems /var/run can be a link and that should
-                // not prevent us from writing the PID
-                Files.createDirectories(parent);
-            }
-        }
-        if (Files.exists(path) && Files.isRegularFile(path) == false) {
-            throw new IllegalArgumentException(path + " exists but is not a regular file");
-        }
-
-        try (OutputStream stream = Files.newOutputStream(path, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING)) {
-            stream.write(Long.toString(pid).getBytes(StandardCharsets.UTF_8));
-        }
-
-        if (deleteOnExit) {
-            addShutdownHook(path);
-        }
-        return new PidFile(path, deleteOnExit, pid);
-    }
-
-    /**
-     * Returns the current process id
-     */
-    public long getPid() {
-        return pid;
-    }
-
-    /**
-     * Returns the process id file path
-     */
-    public Path getPath() {
-        return path;
-    }
-
-    /**
-     * Returns <code>true</code> iff the process id file is deleted on system exit. Otherwise <code>false</code>.
-     */
-    public boolean isDeleteOnExit() {
-        return deleteOnExit;
-    }
-
-    private static void addShutdownHook(final Path path) {
-        Runtime.getRuntime().addShutdownHook(new Thread() {
-            @Override
-            public void run() {
-                try {
-                    Files.deleteIfExists(path);
-                } catch (IOException e) {
-                    throw new ElasticsearchException("Failed to delete pid file " + path, e);
-                }
-            }
-        });
-    }
-}

+ 0 - 1
server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -407,7 +407,6 @@ public final class ClusterSettings extends AbstractScopedSettings {
         Environment.PATH_LOGS_SETTING,
         Environment.PATH_REPO_SETTING,
         Environment.PATH_SHARED_DATA_SETTING,
-        Environment.NODE_PIDFILE_SETTING,
         NodeEnvironment.NODE_ID_SEED_SETTING,
         Node.INITIAL_STATE_TIMEOUT_SETTING,
         DiscoveryModule.DISCOVERY_TYPE_SETTING,

+ 0 - 22
server/src/main/java/org/elasticsearch/env/Environment.java

@@ -54,7 +54,6 @@ public class Environment {
         Property.NodeScope
     );
     public static final Setting<String> PATH_SHARED_DATA_SETTING = Setting.simpleString("path.shared_data", Property.NodeScope);
-    public static final Setting<String> NODE_PIDFILE_SETTING = Setting.simpleString("node.pidfile", Property.NodeScope);
 
     private final Settings settings;
 
@@ -78,9 +77,6 @@ public class Environment {
 
     private final Path logsFile;
 
-    /** Path to the PID file (can be null if no PID file is configured) **/
-    private final Path pidFile;
-
     /** Path to the temporary file directory used by the JDK */
     private final Path tmpFile;
 
@@ -138,12 +134,6 @@ public class Environment {
             logsFile = homeFile.resolve("logs");
         }
 
-        if (NODE_PIDFILE_SETTING.exists(settings)) {
-            pidFile = PathUtils.get(NODE_PIDFILE_SETTING.get(settings)).toAbsolutePath().normalize();
-        } else {
-            pidFile = null;
-        }
-
         binFile = homeFile.resolve("bin");
         libFile = homeFile.resolve("lib");
         modulesFile = homeFile.resolve("modules");
@@ -166,10 +156,6 @@ public class Environment {
             assert sharedDataFile != null;
             finalSettings.put(Environment.PATH_SHARED_DATA_SETTING.getKey(), sharedDataFile.toString());
         }
-        if (NODE_PIDFILE_SETTING.exists(settings)) {
-            assert pidFile != null;
-            finalSettings.put(Environment.NODE_PIDFILE_SETTING.getKey(), pidFile.toString());
-        }
         this.settings = finalSettings.build();
     }
 
@@ -284,13 +270,6 @@ public class Environment {
         return logsFile;
     }
 
-    /**
-     * The PID file location (can be null if no PID file is configured)
-     */
-    public Path pidFile() {
-        return pidFile;
-    }
-
     /** Path to the default temp directory used by the JDK */
     public Path tmpFile() {
         return tmpFile;
@@ -378,7 +357,6 @@ public class Environment {
         assertEquals(actual.libFile(), expected.libFile(), "libFile");
         assertEquals(actual.modulesFile(), expected.modulesFile(), "modulesFile");
         assertEquals(actual.logsFile(), expected.logsFile(), "logsFile");
-        assertEquals(actual.pidFile(), expected.pidFile(), "pidFile");
         assertEquals(actual.tmpFile(), expected.tmpFile(), "tmpFile");
     }
 

+ 0 - 72
server/src/test/java/org/elasticsearch/common/PidFileTests.java

@@ -1,72 +0,0 @@
-/*
- * 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.common;
-
-import org.elasticsearch.test.ESTestCase;
-
-import java.io.BufferedWriter;
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.StandardOpenOption;
-
-import static org.hamcrest.Matchers.containsString;
-
-/**
- * UnitTest for {@link org.elasticsearch.common.PidFile}
- */
-public class PidFileTests extends ESTestCase {
-    public void testParentIsFile() throws IOException {
-        Path dir = createTempDir();
-        Path parent = dir.resolve("foo");
-        try (BufferedWriter stream = Files.newBufferedWriter(parent, StandardCharsets.UTF_8, StandardOpenOption.CREATE_NEW)) {
-            stream.write("foo");
-        }
-
-        try {
-            PidFile.create(parent.resolve("bar.pid"), false);
-            fail("Expected IllegalArgumentException");
-        } catch (IllegalArgumentException e) {
-            assertThat(e.getMessage(), containsString("exists but is not a directory"));
-        }
-    }
-
-    public void testPidFile() throws IOException {
-        Path dir = createTempDir();
-        Path parent = dir.resolve("foo");
-        if (randomBoolean()) {
-            Files.createDirectories(parent);
-            if (randomBoolean()) {
-                try {
-                    Path link = dir.resolve("link_to_real_path");
-                    Files.createSymbolicLink(link, parent.getFileName());
-                    parent = link;
-                } catch (UnsupportedOperationException | IOException | SecurityException ex) {
-                    // fine - no links on this system
-                }
-
-            }
-        }
-        Path pidFile = parent.resolve("foo.pid");
-        long pid = randomLong();
-        if (randomBoolean() && Files.exists(parent)) {
-            try (BufferedWriter stream = Files.newBufferedWriter(pidFile, StandardCharsets.UTF_8, StandardOpenOption.CREATE_NEW)) {
-                stream.write("foo");
-            }
-        }
-
-        final PidFile inst = PidFile.create(pidFile, false, pid);
-        assertEquals(pidFile, inst.getPath());
-        assertEquals(pid, inst.getPid());
-        assertFalse(inst.isDeleteOnExit());
-        assertTrue(Files.exists(pidFile));
-        assertEquals(pid, Long.parseLong(new String(Files.readAllBytes(pidFile), StandardCharsets.UTF_8)));
-    }
-}

+ 0 - 4
server/src/test/java/org/elasticsearch/env/EnvironmentTests.java

@@ -152,7 +152,6 @@ public class EnvironmentTests extends ESTestCase {
             .put(Environment.PATH_LOGS_SETTING.getKey(), "./home/../home/logs")
             .put(Environment.PATH_REPO_SETTING.getKey(), "./home/../home/repo")
             .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), "./home/../home/shared_data")
-            .put(Environment.NODE_PIDFILE_SETTING.getKey(), "./home/../home/pidfile")
             .build();
 
         // the above paths will be treated as relative to the working directory
@@ -177,9 +176,6 @@ public class EnvironmentTests extends ESTestCase {
 
         final String sharedDataPath = Environment.PATH_SHARED_DATA_SETTING.get(environment.settings());
         assertPath(sharedDataPath, home.resolve("shared_data"));
-
-        final String pidFile = Environment.NODE_PIDFILE_SETTING.get(environment.settings());
-        assertPath(pidFile, home.resolve("pidfile"));
     }
 
     public void testSingleDataPathListCheck() {