plugin manager: new `timeout` option

When testing plugin manager with real downloads, it could happen that the test run forever. Fortunately, test suite will be interrupted after 20 minutes, but it could be useful not to fail the whole test suite but only warn in that case.

By default, plugin manager still wait indefinitely but it can be modified using new `--timeout` option:

```sh
bin/plugin --install elasticsearch/kibana --timeout 30s

bin/plugin --install elasticsearch/kibana --timeout 1h
```

Closes #4603.
Closes #4600.

docs/reference/modules/plugins.asciidoc

@@ -123,6 +123,24 @@ bin/plugin --install mobz/elasticsearch-head --verbose
 plugin --remove head --silent
 -----------------------------------
 
+[float]
+==== Timeout settings
+
+By default, the `plugin` script will wait indefinitely when downloading before failing.
+The timeout parameter can be used to explicitly specify how long it waits. Here is some examples of setting it to
+different values:
+
+[source,shell]
+-----------------------------------
+# Wait for 30 seconds before failing
+bin/plugin --install mobz/elasticsearch-head --timeout 30s
+
+# Wait for 1 minute before failing
+bin/plugin --install mobz/elasticsearch-head --timeout 1m
+
+# Wait forever (default)
+bin/plugin --install mobz/elasticsearch-head --timeout 0
+-----------------------------------
 
 [float]
 [[known-plugins]]

src/main/java/org/elasticsearch/common/http/client/HttpDownloadHelper.java

@@ -19,7 +19,10 @@
 
 package org.elasticsearch.common.http.client;
 
+import org.apache.lucene.util.IOUtils;
+import org.elasticsearch.ElasticSearchTimeoutException;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.unit.TimeValue;
 
 import java.io.*;
 import java.net.HttpURLConnection;
@@ -33,9 +36,8 @@ public class HttpDownloadHelper {
 
     private boolean useTimestamp = false;
     private boolean skipExisting = false;
-    private long maxTime = 0;
 
-    public boolean download(URL source, File dest, @Nullable DownloadProgress progress) throws IOException {
+    public boolean download(URL source, File dest, @Nullable DownloadProgress progress, TimeValue timeout) throws Exception {
         if (dest.exists() && skipExisting) {
             return true;
         }
@@ -55,19 +57,20 @@ public class HttpDownloadHelper {
         }
 
         GetThread getThread = new GetThread(source, dest, hasTimestamp, timestamp, progress);
-        getThread.setDaemon(true);
-        getThread.start();
-        try {
-            getThread.join(maxTime * 1000);
-        } catch (InterruptedException ie) {
-            // ignore
-        }
 
-        if (getThread.isAlive()) {
+        try {
-            String msg = "The GET operation took longer than " + maxTime
+            getThread.setDaemon(true);
-                    + " seconds, stopping it.";
+            getThread.start();
+            getThread.join(timeout.millis());
+
+            if (getThread.isAlive()) {
+                throw new ElasticSearchTimeoutException("The GET operation took longer than " + timeout + ", stopping it.");
+            }
+        }
+        catch (InterruptedException ie) {
+            return false;
+        } finally {
             getThread.closeStreams();
-            throw new IOException(msg);
         }
 
         return getThread.wasSuccessful();
@@ -329,16 +332,7 @@ public class HttpDownloadHelper {
                 }
                 finished = !isInterrupted();
             } finally {
-                try {
+                IOUtils.close(os, is);
-                    os.close();
-                } catch (IOException e) {
-                    // ignore
-                }
-                try {
-                    is.close();
-                } catch (IOException e) {
-                    // ignore
-                }
 
                 // we have started to (over)write dest, but failed.
                 // Try to delete the garbage we'd otherwise leave
@@ -374,20 +368,15 @@ public class HttpDownloadHelper {
          * Closes streams, interrupts the download, may delete the
          * output file.
          */
-        void closeStreams() {
+        void closeStreams() throws IOException {
             interrupt();
-            try {
+            if (success) {
-                os.close();
+                IOUtils.close(is, os);
-            } catch (IOException e) {
+            } else {
-                // ignore
+                IOUtils.closeWhileHandlingException(is, os);
-            }
+                if (dest != null && dest.exists()) {
-            try {
+                    dest.delete();
-                is.close();
+                }
-            } catch (IOException e) {
-                // ignore
-            }
-            if (!success && dest.exists()) {
-                dest.delete();
             }
         }
     }

src/main/java/org/elasticsearch/plugins/PluginManager.java

@@ -21,12 +21,14 @@ package org.elasticsearch.plugins;
 
 import com.google.common.base.Strings;
 import org.elasticsearch.ElasticSearchIllegalArgumentException;
+import org.elasticsearch.ElasticSearchTimeoutException;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.http.client.HttpDownloadHelper;
 import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.io.Streams;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
 
@@ -60,15 +62,20 @@ public class PluginManager {
         DEFAULT, SILENT, VERBOSE
     }
 
+    // By default timeout is 0 which means no timeout
+    public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueMillis(0);
+
     private final Environment environment;
 
     private String url;
     private OutputMode outputMode;
+    private TimeValue timeout;
 
-    public PluginManager(Environment environment, String url, OutputMode outputMode) {
+    public PluginManager(Environment environment, String url, OutputMode outputMode, TimeValue timeout) {
         this.environment = environment;
         this.url = url;
         this.outputMode = outputMode;
+        this.timeout = timeout;
 
         TrustManager[] trustAllCerts = new TrustManager[]{
                 new X509TrustManager() {
@@ -124,9 +131,11 @@ public class PluginManager {
             URL pluginUrl = new URL(url);
             log("Trying " + pluginUrl.toExternalForm() + "...");
             try {
-                downloadHelper.download(pluginUrl, pluginFile, progress);
+                downloadHelper.download(pluginUrl, pluginFile, progress, this.timeout);
                 downloaded = true;
-            } catch (IOException e) {
+            } catch (ElasticSearchTimeoutException e) {
+                throw e;
+            } catch (Exception e) {
                 // ignore
                 log("Failed: " + ExceptionsHelper.detailedMessage(e));
             }
@@ -137,9 +146,11 @@ public class PluginManager {
             for (URL url : pluginHandle.urls()) {
                 log("Trying " + url.toExternalForm() + "...");
                 try {
-                    downloadHelper.download(url, pluginFile, progress);
+                    downloadHelper.download(url, pluginFile, progress, this.timeout);
                     downloaded = true;
                     break;
+                } catch (ElasticSearchTimeoutException e) {
+                    throw e;
                 } catch (Exception e) {
                     debug("Failed: " + ExceptionsHelper.detailedMessage(e));
                 }
@@ -308,6 +319,7 @@ public class PluginManager {
         String url = null;
         OutputMode outputMode = OutputMode.DEFAULT;
         String pluginName = null;
+        TimeValue timeout = DEFAULT_TIMEOUT;
         int action = ACTION.NONE;
 
         if (args.length < 1) {
@@ -332,7 +344,9 @@ public class PluginManager {
                         || command.equals("install") || command.equals("-install")) {
                     pluginName = args[++c];
                     action = ACTION.INSTALL;
-
+                } else if (command.equals("-t") || command.equals("--timeout")
+                        || command.equals("timeout") || command.equals("-timeout")) {
+                    timeout = TimeValue.parseTimeValue(args[++c], DEFAULT_TIMEOUT);
                 } else if (command.equals("-r") || command.equals("--remove")
                         // Deprecated commands
                         || command.equals("remove") || command.equals("-remove")) {
@@ -357,7 +371,7 @@ public class PluginManager {
 
         if (action > ACTION.NONE) {
             int exitCode = EXIT_CODE_ERROR; // we fail unless it's reset
-            PluginManager pluginManager = new PluginManager(initialSettings.v2(), url, outputMode);
+            PluginManager pluginManager = new PluginManager(initialSettings.v2(), url, outputMode, timeout);
             switch (action) {
                 case ACTION.INSTALL:
                     try {
@@ -413,6 +427,7 @@ public class PluginManager {
         System.out.println("Usage:");
         System.out.println("    -u, --url     [plugin location]   : Set exact URL to download the plugin from");
         System.out.println("    -i, --install [plugin name]       : Downloads and installs listed plugins [*]");
+        System.out.println("    -t, --timeout [duration]          : Timeout setting: 30s, 1m, 1h... (infinite by default)");
         System.out.println("    -r, --remove  [plugin name]       : Removes listed plugins");
         System.out.println("    -l, --list                        : List installed plugins");
         System.out.println("    -v, --verbose                     : Prints verbose messages");

src/test/java/org/elasticsearch/plugin/PluginManagerTests.java

@@ -19,11 +19,13 @@
 package org.elasticsearch.plugin;
 
 import org.elasticsearch.ElasticSearchIllegalArgumentException;
+import org.elasticsearch.ElasticSearchTimeoutException;
 import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.http.HttpServerTransport;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
@@ -123,13 +125,17 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
         downloadAndExtract(pluginName, "file://" + url.getFile());
     }
 
+    /**
+     * We build a plugin manager instance which wait only for 30 seconds before
+     * raising an ElasticSearchTimeoutException
+     */
     private static PluginManager pluginManager(String pluginUrl) {
         Tuple<Settings, Environment> initialSettings = InternalSettingsPreparer.prepareSettings(
                 ImmutableSettings.settingsBuilder().build(), false);
         if (!initialSettings.v2().pluginsFile().exists()) {
             FileSystemUtils.mkdirs(initialSettings.v2().pluginsFile());
         }
-        return new PluginManager(initialSettings.v2(), pluginUrl, PluginManager.OutputMode.SILENT);
+        return new PluginManager(initialSettings.v2(), pluginUrl, PluginManager.OutputMode.SILENT, TimeValue.timeValueSeconds(30));
     }
 
     private static void downloadAndExtract(String pluginName, String pluginUrl) throws IOException {
@@ -208,16 +214,20 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
 
     private void singlePluginInstallAndRemove(String pluginShortName, String pluginCoordinates) throws IOException {
         PluginManager pluginManager = pluginManager(pluginCoordinates);
-        pluginManager.downloadAndExtract(pluginShortName);
+        try {
-        File[] plugins = pluginManager.getListInstalledPlugins();
+            pluginManager.downloadAndExtract(pluginShortName);
-        assertThat(plugins, notNullValue());
+            File[] plugins = pluginManager.getListInstalledPlugins();
-        assertThat(plugins.length, is(1));
+            assertThat(plugins, notNullValue());
+            assertThat(plugins.length, is(1));
 
-        // We remove it
+            // We remove it
-        pluginManager.removePlugin(pluginShortName);
+            pluginManager.removePlugin(pluginShortName);
-        plugins = pluginManager.getListInstalledPlugins();
+            plugins = pluginManager.getListInstalledPlugins();
-        assertThat(plugins, notNullValue());
+            assertThat(plugins, notNullValue());
-        assertThat(plugins.length, is(0));
+            assertThat(plugins.length, is(0));
+        } catch (ElasticSearchTimeoutException e) {
+            logger.warn("--> timeout exception raised while downloading plugin [{}]. Skipping test.", pluginShortName);
+        }
     }
 
     /**