Remove unused plugins dir var from server CLI (#88917)

Split out of #88443. Remove the now-unused plugins directory variable from the
server CLI code.

Author: pugnascotia

distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java

@@ -69,16 +69,15 @@ SortedMap<Integer, String> invalidLines() {
      * variable.
      *
      * @param configDir the ES config dir
-     * @param pluginsDir the ES plugins dir
      * @param tmpDir the directory that should be passed to {@code -Djava.io.tmpdir}
      * @param envOptions the options passed through the ES_JAVA_OPTS env var
      * @return the list of options to put on the Java command line
      * @throws InterruptedException if the java subprocess is interrupted
      * @throws IOException if there is a problem reading any of the files
-     * @throws UserException if there is a problem parsing the jvm.options file or jvm.options.d files
+     * @throws UserException if there is a problem parsing the `jvm.options` file or `jvm.options.d` files
      */
-    static List<String> determineJvmOptions(Path configDir, Path pluginsDir, Path tmpDir, String envOptions) throws InterruptedException,
-        IOException, UserException {
+    static List<String> determineJvmOptions(Path configDir, Path tmpDir, String envOptions) throws InterruptedException, IOException,
+        UserException {
 
         final JvmOptionsParser parser = new JvmOptionsParser();
 
@@ -87,7 +86,7 @@ static List<String> determineJvmOptions(Path configDir, Path pluginsDir, Path tm
         substitutions.put("ES_PATH_CONF", configDir.toString());
 
         try {
-            return parser.jvmOptions(configDir, pluginsDir, envOptions, substitutions);
+            return parser.jvmOptions(configDir, envOptions, substitutions);
         } catch (final JvmOptionsFileParserException e) {
             final String errorMessage = String.format(
                 Locale.ROOT,
@@ -116,7 +115,7 @@ static List<String> determineJvmOptions(Path configDir, Path pluginsDir, Path tm
         }
     }
 
-    private List<String> jvmOptions(final Path config, Path plugins, final String esJavaOpts, final Map<String, String> substitutions)
+    private List<String> jvmOptions(final Path config, final String esJavaOpts, final Map<String, String> substitutions)
         throws InterruptedException, IOException, JvmOptionsFileParserException {
 
         final List<String> jvmOptions = readJvmOptionsFiles(config);

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

@@ -13,8 +13,6 @@
 import joptsimple.OptionSpecBuilder;
 import joptsimple.util.PathConverter;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 import org.elasticsearch.Build;
 import org.elasticsearch.bootstrap.ServerArgs;
 import org.elasticsearch.cli.CliToolProvider;
@@ -40,8 +38,6 @@
  */
 class ServerCli extends EnvironmentAwareCommand {
 
-    private static final Logger logger = LogManager.getLogger(ServerCli.class);
-
     private final OptionSpecBuilder versionOption;
     private final OptionSpecBuilder daemonizeOption;
     private final OptionSpec<Path> pidfileOption;
@@ -52,7 +48,7 @@ class ServerCli extends EnvironmentAwareCommand {
 
     // visible for testing
     ServerCli() {
-        super("Starts Elasticsearch"); // we configure logging later so we override the base class from configuring logging
+        super("Starts Elasticsearch"); // we configure logging later, so we override the base class from configuring logging
         versionOption = parser.acceptsAll(Arrays.asList("V", "version"), "Prints Elasticsearch version information and exits");
         daemonizeOption = parser.acceptsAll(Arrays.asList("d", "daemonize"), "Starts Elasticsearch in the background")
             .availableUnless(versionOption);
@@ -88,7 +84,7 @@ public void execute(Terminal terminal, OptionSet options, Environment env, Proce
         syncPlugins(terminal, env, processInfo);
 
         ServerArgs args = createArgs(options, env, keystorePassword, processInfo);
-        this.server = startServer(terminal, processInfo, args, env.pluginsFile());
+        this.server = startServer(terminal, processInfo, args);
 
         if (options.has(daemonizeOption)) {
             server.detach();
@@ -162,7 +158,7 @@ private Environment autoConfigureSecurity(
         } catch (UserException e) {
             boolean okCode = switch (e.exitCode) {
                 // these exit codes cover the cases where auto-conf cannot run but the node should NOT be prevented from starting as usual
-                // eg the node is restarted, is already configured in an incompatible way, or the file system permissions do not allow it
+                // e.g. the node is restarted, is already configured in an incompatible way, or the file system permissions do not allow it
                 case ExitCodes.CANT_CREATE, ExitCodes.CONFIG, ExitCodes.NOOP -> true;
                 default -> false;
             };
@@ -230,7 +226,7 @@ protected Command loadTool(String toolname, String libs) {
     }
 
     // protected to allow tests to override
-    protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args, Path pluginsDir) throws UserException {
-        return ServerProcess.start(terminal, processInfo, args, pluginsDir);
+    protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws UserException {
+        return ServerProcess.start(terminal, processInfo, args);
     }
 }

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

@@ -36,7 +36,7 @@
 /**
  * A helper to control a {@link Process} running the main Elasticsearch server.
  *
- * <p> The process can be started by calling {@link #start(Terminal, ProcessInfo, ServerArgs, Path)}.
+ * <p> The process can be started by calling {@link #start(Terminal, ProcessInfo, ServerArgs)}.
  * The process is controlled by internally sending arguments and control signals on stdin,
  * and receiving control signals on stderr. The start method does not return until the
  * server is ready to process requests and has exited the bootstrap thread.
@@ -66,8 +66,7 @@ public class ServerProcess {
 
     // this allows mocking the process building by tests
     interface OptionsBuilder {
-        List<String> getJvmOptions(Path configDir, Path pluginsDir, Path tmpDir, String envOptions) throws InterruptedException,
-            IOException, UserException;
+        List<String> getJvmOptions(Path configDir, Path tmpDir, String envOptions) throws InterruptedException, IOException, UserException;
     }
 
     // this allows mocking the process building by tests
@@ -81,20 +80,18 @@ interface ProcessStarter {
      * @param terminal A terminal to connect the standard inputs and outputs to for the new process.
      * @param processInfo Info about the current process, for passing through to the subprocess.
      * @param args Arguments to the server process.
-     * @param pluginsDir The directory in which plugins can be found
      * @return A running server process that is ready for requests
      * @throws UserException If the process failed during bootstrap
      */
-    public static ServerProcess start(Terminal terminal, ProcessInfo processInfo, ServerArgs args, Path pluginsDir) throws UserException {
-        return start(terminal, processInfo, args, pluginsDir, JvmOptionsParser::determineJvmOptions, ProcessBuilder::start);
+    public static ServerProcess start(Terminal terminal, ProcessInfo processInfo, ServerArgs args) throws UserException {
+        return start(terminal, processInfo, args, JvmOptionsParser::determineJvmOptions, ProcessBuilder::start);
     }
 
     // package private so tests can mock options building and process starting
     static ServerProcess start(
         Terminal terminal,
         ProcessInfo processInfo,
         ServerArgs args,
-        Path pluginsDir,
         OptionsBuilder optionsBuilder,
         ProcessStarter processStarter
     ) throws UserException {
@@ -103,7 +100,7 @@ static ServerProcess start(
 
         boolean success = false;
         try {
-            jvmProcess = createProcess(processInfo, args.configDir(), pluginsDir, optionsBuilder, processStarter);
+            jvmProcess = createProcess(processInfo, args.configDir(), optionsBuilder, processStarter);
             errorPump = new ErrorPumpThread(terminal.getErrorWriter(), jvmProcess.getErrorStream());
             errorPump.start();
             sendArgs(args, jvmProcess.getOutputStream());
@@ -138,7 +135,7 @@ public long pid() {
     /**
      * Detaches the server process from the current process, enabling the current process to exit.
      *
-     * @throws IOException If an I/O error occured while reading stderr or closing any of the standard streams
+     * @throws IOException If an I/O error occurred while reading stderr or closing any of the standard streams
      */
     public synchronized void detach() throws IOException {
         errorPump.drain();
@@ -178,7 +175,7 @@ private static void sendArgs(ServerArgs args, OutputStream processStdin) {
             args.writeTo(out);
             out.flush();
         } catch (IOException ignore) {
-            // A failure to write here means the process has problems, and it will die anyways. We let this fall through
+            // A failure to write here means the process has problems, and it will die anyway. We let this fall through
             // so the pump thread can complete, writing out the actual error. All we get here is the failure to write to
             // the process pipe, which isn't helpful to print.
         }
@@ -198,7 +195,6 @@ private void sendShutdownMarker() {
     private static Process createProcess(
         ProcessInfo processInfo,
         Path configDir,
-        Path pluginsDir,
         OptionsBuilder optionsBuilder,
         ProcessStarter processStarter
     ) throws InterruptedException, IOException, UserException {
@@ -208,7 +204,7 @@ private static Process createProcess(
             envVars.put("LIBFFI_TMPDIR", tempDir.toString());
         }
 
-        List<String> jvmOptions = optionsBuilder.getJvmOptions(configDir, pluginsDir, tempDir, envVars.remove("ES_JAVA_OPTS"));
+        List<String> jvmOptions = optionsBuilder.getJvmOptions(configDir, tempDir, envVars.remove("ES_JAVA_OPTS"));
         // also pass through distribution type
         jvmOptions.add("-Des.distribution.type=" + processInfo.sysprops().get("es.distribution.type"));
 
@@ -233,7 +229,7 @@ private static Process createProcess(
     /**
      * Returns the java.io.tmpdir Elasticsearch should use, creating it if necessary.
      *
-     * <p> On non-Windows OS, this will be created as a sub-directory of the default temporary directory.
+     * <p> On non-Windows OS, this will be created as a subdirectory of the default temporary directory.
      * Note that this causes the created temporary directory to be a private temporary directory.
      */
     private static Path setupTempDir(ProcessInfo processInfo, String tmpDirOverride) throws UserException, IOException {

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

@@ -436,7 +436,7 @@ protected Command loadTool(String toolname, String libs) {
             }
 
             @Override
-            protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args, Path pluginsDir) {
+            protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, ServerArgs args) {
                 if (argsValidator != null) {
                     argsValidator.accept(args);
                 }

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

@@ -92,7 +92,7 @@ public void resetEnv() {
         envVars.clear();
         esHomeDir = createTempDir();
         nodeSettings = Settings.builder();
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> new ArrayList<>();
+        optionsBuilder = (configDir, tmpDir, envOptions) -> new ArrayList<>();
         processValidator = null;
         mainCallback = null;
     }
@@ -201,7 +201,7 @@ ServerProcess startProcess(boolean daemonize, boolean quiet, String keystorePass
             process = new MockElasticsearchProcess();
             return process;
         };
-        return ServerProcess.start(terminal, pinfo, args, esHomeDir.resolve("plugins"), optionsBuilder, starter);
+        return ServerProcess.start(terminal, pinfo, args, optionsBuilder, starter);
     }
 
     public void testProcessBuilder() throws Exception {
@@ -253,9 +253,7 @@ public void testStartError() throws Exception {
     }
 
     public void testOptionsBuildingInterrupted() throws Exception {
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> {
-            throw new InterruptedException("interrupted while get jvm options");
-        };
+        optionsBuilder = (configDir, tmpDir, envOptions) -> { throw new InterruptedException("interrupted while get jvm options"); };
         var e = expectThrows(RuntimeException.class, () -> runForeground());
         assertThat(e.getCause().getMessage(), equalTo("interrupted while get jvm options"));
     }
@@ -279,7 +277,7 @@ public void testLibffiEnv() throws Exception {
     }
 
     public void testTempDir() throws Exception {
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> {
+        optionsBuilder = (configDir, tmpDir, envOptions) -> {
             assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
             assertThat(tmpDir.getFileName().toString(), startsWith("elasticsearch-"));
             return new ArrayList<>();
@@ -291,7 +289,7 @@ public void testTempDirWindows() throws Exception {
         Path baseTmpDir = createTempDir();
         sysprops.put("os.name", "Windows 10");
         sysprops.put("java.io.tmpdir", baseTmpDir.toString());
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> {
+        optionsBuilder = (configDir, tmpDir, envOptions) -> {
             assertThat(tmpDir.toString(), Files.exists(tmpDir), is(true));
             assertThat(tmpDir.getFileName().toString(), equalTo("elasticsearch"));
             assertThat(tmpDir.getParent().toString(), equalTo(baseTmpDir.toString()));
@@ -303,7 +301,7 @@ public void testTempDirWindows() throws Exception {
     public void testTempDirOverride() throws Exception {
         Path customTmpDir = createTempDir();
         envVars.put("ES_TMPDIR", customTmpDir.toString());
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> {
+        optionsBuilder = (configDir, tmpDir, envOptions) -> {
             assertThat(tmpDir.toString(), equalTo(customTmpDir.toString()));
             return new ArrayList<>();
         };
@@ -329,7 +327,7 @@ public void testTempDirOverrideNotADirectory() throws Exception {
 
     public void testCustomJvmOptions() throws Exception {
         envVars.put("ES_JAVA_OPTS", "-Dmyoption=foo");
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> {
+        optionsBuilder = (configDir, tmpDir, envOptions) -> {
             assertThat(envOptions, equalTo("-Dmyoption=foo"));
             return new ArrayList<>();
         };
@@ -338,7 +336,7 @@ public void testCustomJvmOptions() throws Exception {
     }
 
     public void testCommandLineSysprops() throws Exception {
-        optionsBuilder = (configDir, pluginsDir, tmpDir, envOptions) -> List.of("-Dfoo1=bar", "-Dfoo2=baz");
+        optionsBuilder = (configDir, tmpDir, envOptions) -> List.of("-Dfoo1=bar", "-Dfoo2=baz");
         processValidator = pb -> {
             assertThat(pb.command(), contains("-Dfoo1=bar"));
             assertThat(pb.command(), contains("-Dfoo2=bar"));

distribution/tools/windows-service-cli/src/main/java/org/elasticsearch/windows/service/WindowsServiceDaemon.java

@@ -20,9 +20,9 @@
 
 /**
  * Starts an Elasticsearch process, but does not wait for it to exit.
- *
- * This class is expected to be run via Apache Procrun in a long lived JVM that will call close
- * when the server should shutdown.
+ * <p>
+ * This class is expected to be run via Apache Procrun in a long-lived JVM that will call close
+ * when the server should shut down.
  */
 class WindowsServiceDaemon extends EnvironmentAwareCommand {
 
@@ -35,7 +35,7 @@ class WindowsServiceDaemon extends EnvironmentAwareCommand {
     @Override
     public void execute(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws Exception {
         var args = new ServerArgs(false, true, null, new SecureString(""), env.settings(), env.configFile());
-        this.server = ServerProcess.start(terminal, processInfo, args, env.pluginsFile());
+        this.server = ServerProcess.start(terminal, processInfo, args);
         // start does not return until the server is ready, and we do not wait for the process
     }