Allow installing multiple plugins as a transaction (#50924)

jasontedor · jasontedor · commit ca9ca68cbebc · 2020-01-14T12:20:54.000-05:00
This commit allows the plugin installer to install multiple plugins in a
single invocation. The installation will be treated as a transaction, so
that all of the plugins are install successfully, or none of the plugins
are installed.
diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java
@@ -212,24 +212,50 @@ protected void printAdditionalHelp(Terminal terminal) {
 
     @Override
     protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
-        String pluginId = arguments.value(options);
+        List<String> pluginId = arguments.values(options);
         final boolean isBatch = options.has(batchOption);
         execute(terminal, pluginId, isBatch, env);
     }
 
     // pkg private for testing
-    void execute(Terminal terminal, String pluginId, boolean isBatch, Environment env) throws Exception {
-        if (pluginId == null) {
-            throw new UserException(ExitCodes.USAGE, "plugin id is required");
+    void execute(Terminal terminal, List<String> pluginIds, boolean isBatch, Environment env) throws Exception {
+        if (pluginIds.isEmpty()) {
+            throw new UserException(ExitCodes.USAGE, "at least one plugin id is required");
         }
 
-        if ("x-pack".equals(pluginId)) {
-            handleInstallXPack(buildFlavor());
+        final Set<String> uniquePluginIds = new HashSet<>();
+        for (final String pluginId : pluginIds) {
+            if (uniquePluginIds.add(pluginId) == false) {
+                throw new UserException(ExitCodes.USAGE, "duplicate plugin id [" + pluginId + "]");
+            }
+        }
+
+        final List<Path> deleteOnFailure = new ArrayList<>();
+        final Set<PluginInfo> pluginInfos = new HashSet<>();
+        for (final String pluginId : pluginIds) {
+            try {
+                if ("x-pack".equals(pluginId)) {
+                    handleInstallXPack(buildFlavor());
+                }
+
+                final Path pluginZip = download(terminal, pluginId, env.tmpFile(), isBatch);
+                final Path extractedZip = unzip(pluginZip, env.pluginsFile());
+                deleteOnFailure.add(extractedZip);
+                final PluginInfo pluginInfo = installPlugin(terminal, isBatch, extractedZip, env, deleteOnFailure);
+                pluginInfos.add(pluginInfo);
+            } catch (final Exception installProblem) {
+                try {
+                    IOUtils.rm(deleteOnFailure.toArray(new Path[0]));
+                } catch (final IOException exceptionWhileRemovingFiles) {
+                    installProblem.addSuppressed(exceptionWhileRemovingFiles);
+                }
+                throw installProblem;
+            }
         }
 
-        Path pluginZip = download(terminal, pluginId, env.tmpFile(), isBatch);
-        Path extractedZip = unzip(pluginZip, env.pluginsFile());
-        install(terminal, isBatch, extractedZip, env);
+        for (final PluginInfo pluginInfo : pluginInfos) {
+            terminal.println("-> Installed " + pluginInfo.getName());
+        }
     }
 
     Build.Flavor buildFlavor() {
@@ -779,26 +805,11 @@ void jarHellCheck(PluginInfo candidateInfo, Path candidateDir, Path pluginsDir,
         // TODO: verify the classname exists in one of the jars!
     }
 
-    private void install(Terminal terminal, boolean isBatch, Path tmpRoot, Environment env) throws Exception {
-        List<Path> deleteOnFailure = new ArrayList<>();
-        deleteOnFailure.add(tmpRoot);
-        try {
-            installPlugin(terminal, isBatch, tmpRoot, env, deleteOnFailure);
-        } catch (Exception installProblem) {
-            try {
-                IOUtils.rm(deleteOnFailure.toArray(new Path[0]));
-            } catch (IOException exceptionWhileRemovingFiles) {
-                installProblem.addSuppressed(exceptionWhileRemovingFiles);
-            }
-            throw installProblem;
-        }
-    }
-
     /**
      * Installs the plugin from {@code tmpRoot} into the plugins dir.
      * If the plugin has a bin dir and/or a config dir, those are moved.
      */
-    private void installPlugin(Terminal terminal, boolean isBatch, Path tmpRoot,
+    private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoot,
                                Environment env, List<Path> deleteOnFailure) throws Exception {
         final PluginInfo info = loadPluginInfo(terminal, tmpRoot, env);
         // read optional security policy (extra permissions), if it exists, confirm or warn the user
@@ -817,7 +828,7 @@ private void installPlugin(Terminal terminal, boolean isBatch, Path tmpRoot,
         installPluginSupportFiles(info, tmpRoot, env.binFile().resolve(info.getName()),
                                   env.configFile().resolve(info.getName()), deleteOnFailure);
         movePlugin(tmpRoot, destination);
-        terminal.println("-> Installed " + info.getName());
+        return info;
     }
 
     /** Moves bin and config directories from the plugin if they exist */
diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java
@@ -64,6 +64,7 @@
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringReader;
@@ -93,6 +94,7 @@
 import java.security.NoSuchProviderException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -280,9 +282,17 @@ void installPlugin(String pluginUrl, Path home) throws Exception {
         installPlugin(pluginUrl, home, skipJarHellCommand);
     }
 
+    void installPlugins(final List<String> pluginUrls, final Path home) throws Exception {
+        installPlugins(pluginUrls, home, skipJarHellCommand);
+    }
+
     void installPlugin(String pluginUrl, Path home, InstallPluginCommand command) throws Exception {
-        Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
-        command.execute(terminal, pluginUrl, false, env);
+        installPlugins(pluginUrl == null ? Collections.emptyList() : Collections.singletonList(pluginUrl), home, command);
+    }
+
+    void installPlugins(final List<String> pluginUrls, final Path home, final InstallPluginCommand command) throws Exception {
+        final Environment env = TestEnvironment.newEnvironment(Settings.builder().put("path.home", home).build());
+        command.execute(terminal, pluginUrls, false, env);
     }
 
     void assertPlugin(String name, Path original, Environment env) throws IOException {
@@ -382,7 +392,7 @@ void assertInstallCleaned(Environment env) throws IOException {
     public void testMissingPluginId() throws IOException {
         final Tuple<Path, Environment> env = createEnv(fs, temp);
         final UserException e = expectThrows(UserException.class, () -> installPlugin(null, env.v1()));
-        assertTrue(e.getMessage(), e.getMessage().contains("plugin id is required"));
+        assertTrue(e.getMessage(), e.getMessage().contains("at least one plugin id is required"));
     }
 
     public void testSomethingWorks() throws Exception {
@@ -393,6 +403,38 @@ public void testSomethingWorks() throws Exception {
         assertPlugin("fake", pluginDir, env.v2());
     }
 
+    public void testMultipleWorks() throws Exception {
+        Tuple<Path, Environment> env = createEnv(fs, temp);
+        Path pluginDir = createPluginDir(temp);
+        String fake1PluginZip = createPluginUrl("fake1", pluginDir);
+        String fake2PluginZip = createPluginUrl("fake2", pluginDir);
+        installPlugins(Arrays.asList(fake1PluginZip, fake2PluginZip), env.v1());
+        assertPlugin("fake1", pluginDir, env.v2());
+        assertPlugin("fake2", pluginDir, env.v2());
+    }
+
+    public void testDuplicateInstall() throws Exception {
+        Tuple<Path, Environment> env = createEnv(fs, temp);
+        Path pluginDir = createPluginDir(temp);
+        String pluginZip = createPluginUrl("fake", pluginDir);
+        final UserException e = expectThrows(UserException.class, () -> installPlugins(Arrays.asList(pluginZip, pluginZip), env.v1()));
+        assertThat(e, hasToString(containsString("duplicate plugin id [" + pluginZip + "]")));
+    }
+
+    public void testTransaction() throws Exception {
+        Tuple<Path, Environment> env = createEnv(fs, temp);
+        Path pluginDir = createPluginDir(temp);
+        String pluginZip = createPluginUrl("fake", pluginDir);
+        final FileNotFoundException e = expectThrows(
+            FileNotFoundException.class,
+            () -> installPlugins(Arrays.asList(pluginZip, pluginZip + "does-not-exist"), env.v1()));
+        assertThat(e, hasToString(containsString("does-not-exist")));
+        final Path fakeInstallPath = env.v2().pluginsFile().resolve("fake");
+        // fake should have been removed when the file not found exception occurred
+        assertFalse(Files.exists(fakeInstallPath));
+        assertInstallCleaned(env.v2());
+    }
+
     public void testInstallFailsIfPreviouslyRemovedPluginFailed() throws Exception {
         Tuple<Path, Environment> env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);
@@ -769,7 +811,8 @@ Build.Flavor buildFlavor() {
         };
 
         final Environment environment = createEnv(fs, temp).v2();
-        final T exception = expectThrows(clazz, () -> flavorCommand.execute(terminal, "x-pack", false, environment));
+        final T exception =
+            expectThrows(clazz, () -> flavorCommand.execute(terminal, Collections.singletonList("x-pack"), false, environment));
         assertThat(exception, hasToString(containsString(expectedMessage)));
     }
 
@@ -830,7 +873,7 @@ private void installPlugin(MockTerminal terminal, boolean isBatch) throws Except
             writePluginSecurityPolicy(pluginDir, "setFactory");
         }
         String pluginZip = createPlugin("fake", pluginDir).toUri().toURL().toString();
-        skipJarHellCommand.execute(terminal, pluginZip, isBatch, env.v2());
+        skipJarHellCommand.execute(terminal, Collections.singletonList(pluginZip), isBatch, env.v2());
     }
 
     void assertInstallPluginFromUrl(
diff --git a/docs/plugins/plugin-script.asciidoc b/docs/plugins/plugin-script.asciidoc
@@ -106,6 +106,32 @@ sudo ES_JAVA_OPTS="-Djavax.net.ssl.trustStore=/path/to/trustStore.jks" bin/elast
 -----------------------------------
 --
 
+[[installing-multiple-plugins]]
+=== Installing multiple plugins
+
+Multiple plugins can be installed in one invocation as follows:
+
+[source,shell]
+-----------------------------------
+sudo bin/elasticsearch-plugin install [plugin_id] [plugin_id] ... [plugin_id]
+-----------------------------------
+
+Each `plugin_id` can be any valid form for installing a single plugin (e.g., the
+name of a core plugin, or a custom URL).
+
+For instance, to install the core <<analysis-icu,ICU plugin>>, and
+<<repository-s3,S3 repository plugin>> run the following command:
+
+[source,shell]
+-----------------------------------
+sudo bin/elasticsearch-plugin install analysis-icu repository-s3
+-----------------------------------
+
+This command will install the versions of the plugins that matches your
+Elasticsearch version. The installation will be treated as a transaction, so
+that all the plugins will be installed, or none of the plugins will be installed
+if any installation fails.
+
 [[mandatory-plugins]]
 === Mandatory Plugins
 
