Browse Source

Support removing multiple plugins at once in the CLI (#69063)

Closes #66476. Add support for removing multiple plugins at the
same time to `elasticsearch-plugin`. Also change references from
"plugin name" to "plugin id", to align better with the installer
class.
Rory Hunter 4 years ago
parent
commit
0f6ad19f15

+ 67 - 32
distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java

@@ -23,16 +23,19 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
+import java.util.StringJoiner;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static org.elasticsearch.cli.Terminal.Verbosity.VERBOSE;
 
 /**
- * A command for the plugin CLI to remove a plugin from Elasticsearch.
+ * A command for the plugin CLI to remove plugins from Elasticsearch.
  */
 class RemovePluginCommand extends EnvironmentAwareCommand {
 
@@ -44,16 +47,16 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
     private final OptionSpec<String> arguments;
 
     RemovePluginCommand() {
-        super("removes a plugin from Elasticsearch");
+        super("removes plugins from Elasticsearch");
         this.purgeOption = parser.acceptsAll(Arrays.asList("p", "purge"), "Purge plugin configuration files");
-        this.arguments = parser.nonOptions("plugin name");
+        this.arguments = parser.nonOptions("plugin id");
     }
 
     @Override
     protected void execute(final Terminal terminal, final OptionSet options, final Environment env) throws Exception {
-        final String pluginName = arguments.value(options);
+        final List<String> pluginIds = arguments.values(options);
         final boolean purge = options.has(purgeOption);
-        execute(terminal, env, pluginName, purge);
+        execute(terminal, env, pluginIds, purge);
     }
 
     /**
@@ -61,55 +64,91 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
      *
      * @param terminal   the terminal to use for input/output
      * @param env        the environment for the local node
-     * @param pluginName the name of the plugin to remove
+     * @param pluginIds  the IDs of the plugins to remove
      * @param purge      if true, plugin configuration files will be removed but otherwise preserved
      * @throws IOException   if any I/O exception occurs while performing a file operation
-     * @throws UserException if plugin name is null
+     * @throws UserException if pluginIds is null or empty
      * @throws UserException if plugin directory does not exist
      * @throws UserException if the plugin bin directory is not a directory
      */
-    void execute(Terminal terminal, Environment env, String pluginName, boolean purge) throws IOException, UserException {
-        if (pluginName == null) {
-            throw new UserException(ExitCodes.USAGE, "plugin name is required");
+    void execute(Terminal terminal, Environment env, List<String> pluginIds, boolean purge) throws IOException, UserException {
+        if (pluginIds == null || pluginIds.isEmpty()) {
+            throw new UserException(ExitCodes.USAGE, "At least one plugin ID is required");
         }
 
-        // first make sure nothing extends this plugin
-        List<String> usedBy = new ArrayList<>();
+        ensurePluginsNotUsedByOtherPlugins(env, pluginIds);
+
+        for (String pluginId : pluginIds) {
+            checkCanRemove(env, pluginId, purge);
+        }
+
+        for (String pluginId : pluginIds) {
+            removePlugin(env, terminal, pluginId, purge);
+        }
+    }
+
+    private void ensurePluginsNotUsedByOtherPlugins(Environment env, List<String> pluginIds) throws IOException, UserException {
+        // First make sure nothing extends this plugin
+        final Map<String, List<String>> usedBy = new HashMap<>();
         Set<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(env.pluginsFile());
         for (PluginsService.Bundle bundle : bundles) {
             for (String extendedPlugin : bundle.plugin.getExtendedPlugins()) {
-                if (extendedPlugin.equals(pluginName)) {
-                    usedBy.add(bundle.plugin.getName());
+                for (String pluginId : pluginIds) {
+                    if (extendedPlugin.equals(pluginId)) {
+                        usedBy.computeIfAbsent(bundle.plugin.getName(), (_key -> new ArrayList<>())).add(pluginId);
+                    }
                 }
             }
         }
-        if (usedBy.isEmpty() == false) {
-            throw new UserException(
-                PLUGIN_STILL_USED,
-                "plugin [" + pluginName + "] cannot be removed" + " because it is extended by other plugins: " + usedBy
-            );
+        if (usedBy.isEmpty()) {
+            return;
         }
 
-        final Path pluginDir = env.pluginsFile().resolve(pluginName);
-        final Path pluginConfigDir = env.configFile().resolve(pluginName);
-        final Path removing = env.pluginsFile().resolve(".removing-" + pluginName);
+        final StringJoiner message = new StringJoiner("\n");
+        message.add("Cannot remove plugins because the following are extended by other plugins:");
+        usedBy.forEach((key, value) -> {
+            String s = "\t" + key + " used by " + value;
+            message.add(s);
+        });
+
+        throw new UserException(PLUGIN_STILL_USED, message.toString());
+    }
+
+    private void checkCanRemove(Environment env, String pluginId, boolean purge) throws UserException {
+        final Path pluginDir = env.pluginsFile().resolve(pluginId);
+        final Path pluginConfigDir = env.configFile().resolve(pluginId);
+        final Path removing = env.pluginsFile().resolve(".removing-" + pluginId);
 
-        terminal.println("-> removing [" + pluginName + "]...");
         /*
          * If the plugin does not exist and the plugin config does not exist, fail to the user that the plugin is not found, unless there's
          * a marker file left from a previously failed attempt in which case we proceed to clean up the marker file. Or, if the plugin does
          * not exist, the plugin config does, and we are not purging, again fail to the user that the plugin is not found.
          */
-        if ((!Files.exists(pluginDir) && !Files.exists(pluginConfigDir) && !Files.exists(removing))
-            || (!Files.exists(pluginDir) && Files.exists(pluginConfigDir) && !purge)) {
+        if ((Files.exists(pluginDir) == false && Files.exists(pluginConfigDir) == false && Files.exists(removing) == false)
+            || (Files.exists(pluginDir) == false && Files.exists(pluginConfigDir) && purge == false)) {
             final String message = String.format(
                 Locale.ROOT,
                 "plugin [%s] not found; run 'elasticsearch-plugin list' to get list of installed plugins",
-                pluginName
+                pluginId
             );
             throw new UserException(ExitCodes.CONFIG, message);
         }
 
+        final Path pluginBinDir = env.binFile().resolve(pluginId);
+        if (Files.exists(pluginBinDir)) {
+            if (Files.isDirectory(pluginBinDir) == false) {
+                throw new UserException(ExitCodes.IO_ERROR, "bin dir for " + pluginId + " is not a directory");
+            }
+        }
+    }
+
+    private void removePlugin(Environment env, Terminal terminal, String pluginId, boolean purge) throws IOException {
+        final Path pluginDir = env.pluginsFile().resolve(pluginId);
+        final Path pluginConfigDir = env.configFile().resolve(pluginId);
+        final Path removing = env.pluginsFile().resolve(".removing-" + pluginId);
+
+        terminal.println("-> removing [" + pluginId + "]...");
+
         final List<Path> pluginPaths = new ArrayList<>();
 
         /*
@@ -123,11 +162,8 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
             terminal.println(VERBOSE, "removing [" + pluginDir + "]");
         }
 
-        final Path pluginBinDir = env.binFile().resolve(pluginName);
+        final Path pluginBinDir = env.binFile().resolve(pluginId);
         if (Files.exists(pluginBinDir)) {
-            if (Files.isDirectory(pluginBinDir) == false) {
-                throw new UserException(ExitCodes.IO_ERROR, "bin dir for " + pluginName + " is not a directory");
-            }
             try (Stream<Path> paths = Files.list(pluginBinDir)) {
                 pluginPaths.addAll(paths.collect(Collectors.toList()));
             }
@@ -180,7 +216,6 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
         // finally, add the marker file
         pluginPaths.add(removing);
 
-        IOUtils.rm(pluginPaths.toArray(new Path[pluginPaths.size()]));
+        IOUtils.rm(pluginPaths.toArray(new Path[0]));
     }
-
 }

+ 32 - 6
distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/RemovePluginCommandTests.java

@@ -26,8 +26,10 @@ import java.io.StringReader;
 import java.nio.file.DirectoryStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.List;
 import java.util.Map;
 
+import static java.util.Collections.emptyList;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.Matchers.equalTo;
@@ -96,10 +98,14 @@ public class RemovePluginCommandTests extends ESTestCase {
         );
     }
 
-    static MockTerminal removePlugin(String name, Path home, boolean purge) throws Exception {
+    static MockTerminal removePlugin(String pluginId, Path home, boolean purge) throws Exception {
+        return removePlugin(List.of(pluginId), home, purge);
+    }
+
+    static MockTerminal removePlugin(List<String> pluginIds, Path home, boolean purge) throws Exception {
         Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
         MockTerminal terminal = new MockTerminal();
-        new MockRemovePluginCommand(env).execute(terminal, env, name, purge);
+        new MockRemovePluginCommand(env).execute(terminal, env, pluginIds, purge);
         return terminal;
     }
 
@@ -130,6 +136,23 @@ public class RemovePluginCommandTests extends ESTestCase {
         assertRemoveCleaned(env);
     }
 
+    /** Check that multiple plugins can be removed at the same time. */
+    public void testRemoveMultiple() throws Exception {
+        createPlugin("fake");
+        Files.createFile(env.pluginsFile().resolve("fake").resolve("plugin.jar"));
+        Files.createDirectory(env.pluginsFile().resolve("fake").resolve("subdir"));
+
+        createPlugin("other");
+        Files.createFile(env.pluginsFile().resolve("other").resolve("plugin.jar"));
+        Files.createDirectory(env.pluginsFile().resolve("other").resolve("subdir"));
+
+        removePlugin("fake", home, randomBoolean());
+        removePlugin("other", home, randomBoolean());
+        assertFalse(Files.exists(env.pluginsFile().resolve("fake")));
+        assertFalse(Files.exists(env.pluginsFile().resolve("other")));
+        assertRemoveCleaned(env);
+    }
+
     public void testRemoveOldVersion() throws Exception {
         Version previous = VersionUtils.getPreviousVersion();
         if (previous.before(Version.CURRENT.minimumIndexCompatibilityVersion())) {
@@ -236,7 +259,6 @@ public class RemovePluginCommandTests extends ESTestCase {
             BufferedReader reader = new BufferedReader(new StringReader(terminal.getOutput()));
             BufferedReader errorReader = new BufferedReader(new StringReader(terminal.getErrorOutput()))
         ) {
-            assertEquals("-> removing [fake]...", reader.readLine());
             assertEquals(
                 "ERROR: plugin [fake] not found; run 'elasticsearch-plugin list' to get list of installed plugins",
                 errorReader.readLine()
@@ -246,10 +268,14 @@ public class RemovePluginCommandTests extends ESTestCase {
         }
     }
 
-    public void testMissingPluginName() throws Exception {
-        UserException e = expectThrows(UserException.class, () -> removePlugin(null, home, randomBoolean()));
+    public void testMissingPluginName() {
+        UserException e = expectThrows(UserException.class, () -> removePlugin((List<String>) null, home, randomBoolean()));
+        assertEquals(ExitCodes.USAGE, e.exitCode);
+        assertEquals("At least one plugin ID is required", e.getMessage());
+
+        e = expectThrows(UserException.class, () -> removePlugin(emptyList(), home, randomBoolean()));
         assertEquals(ExitCodes.USAGE, e.exitCode);
-        assertEquals("plugin name is required", e.getMessage());
+        assertEquals("At least one plugin ID is required", e.getMessage());
     }
 
     public void testRemoveWhenRemovingMarker() throws Exception {

+ 10 - 0
docs/plugins/plugin-script.asciidoc

@@ -182,6 +182,16 @@ purge the configuration files while removing a plugin, use `-p` or `--purge`.
 This can option can be used after a plugin is removed to remove any lingering
 configuration files.
 
+[[removing-multiple-plugins]]
+=== Removing multiple plugins
+
+Multiple plugins can be removed in one invocation as follows:
+
+[source,shell]
+-----------------------------------
+sudo bin/elasticsearch-plugin remove [pluginname] [pluginname] ... [pluginname]
+-----------------------------------
+
 [discrete]
 === Updating plugins