Merge branch 'fix/16720-move-gce-settings'

docs/plugins/discovery-gce.asciidoc

@@ -48,6 +48,16 @@ discovery:
 
 The following gce settings (prefixed with `cloud.gce`) are supported:
 
+ `project_id`::
+
+     Your Google project id (mandatory).
+
+ `zone`::
+
+     helps to retrieve instances running in a given zone (mandatory). It should be one of the
+     https://developers.google.com/compute/docs/zones#available[GCE supported zones].
+     See also <<discovery-gce-usage-zones>>.
+
  `retry`::
 
      If set to `true`, client will use
@@ -56,8 +66,14 @@ The following gce settings (prefixed with `cloud.gce`) are supported:
 
  `max_wait`::
 
-     The maximum elapsed time in milliseconds after the client instantiating retry. If the time elapsed goes past the
+     The maximum elapsed time after the client instantiating retry. If the time elapsed goes past the
-     `max_wait`, client stops to retry. Defaults to 15 minutes (900000 milliseconds).
+     `max_wait`, client stops to retry. A negative value means that it will wait indefinitely. Defaults to `0s` (retry
+     indefinitely).
+
+ `refresh_interval`::
+
+     How long the list of hosts is cached to prevent further requests to the GCE API. `0s` disables caching.
+     A negative value will cause infinite caching. Defaults to `0s`.
 
 
 [IMPORTANT]

plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeService.java

@@ -21,21 +21,55 @@ package org.elasticsearch.cloud.gce;
 
 import com.google.api.services.compute.model.Instance;
 import org.elasticsearch.common.component.LifecycleComponent;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 
+import java.util.Arrays;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
 public interface GceComputeService extends LifecycleComponent<GceComputeService> {
-    final class Fields {
-        public static final String PROJECT = "cloud.gce.project_id";
-        public static final String ZONE = "cloud.gce.zone";
-        public static final String REFRESH = "cloud.gce.refresh_interval";
-        public static final String TAGS = "discovery.gce.tags";
-        public static final String VERSION = "Elasticsearch/GceCloud/1.0";
 
-        public static final String RETRY = "cloud.gce.retry";
+    /**
-        public static final String MAXWAIT = "cloud.gce.max_wait";
+     * GCE API Version: Elasticsearch/GceCloud/1.0
-    }
+     */
+    String VERSION = "Elasticsearch/GceCloud/1.0";
+
+    // cloud.gce settings
+
+    /**
+     * cloud.gce.project_id: Google project id
+     */
+    Setting<String> PROJECT_SETTING = Setting.simpleString("cloud.gce.project_id", false, Setting.Scope.CLUSTER);
+
+    /**
+     * cloud.gce.zone: Google Compute Engine zones
+     */
+    Setting<List<String>> ZONE_SETTING =
+        Setting.listSetting("cloud.gce.zone", Collections.emptyList(), s -> s, false, Setting.Scope.CLUSTER);
+
+    /**
+     * cloud.gce.refresh_interval: How long the list of hosts is cached to prevent further requests to the AWS API. 0 disables caching.
+     * A negative value will cause infinite caching. Defaults to 0s.
+     */
+    Setting<TimeValue> REFRESH_SETTING =
+        Setting.timeSetting("cloud.gce.refresh_interval", TimeValue.timeValueSeconds(0), false, Setting.Scope.CLUSTER);
+
+    /**
+     * cloud.gce.retry: Should we retry calling GCE API in case of error? Defaults to true.
+     */
+    Setting<Boolean> RETRY_SETTING = Setting.boolSetting("cloud.gce.retry", true, false, Setting.Scope.CLUSTER);
+
+    /**
+     * cloud.gce.max_wait: How long exponential backoff should retry before definitely failing.
+     * It's a total time since the the initial call is made.
+     * A negative value will retry indefinitely. Defaults to `-1s` (retry indefinitely).
+     */
+    Setting<TimeValue> MAX_WAIT_SETTING =
+        Setting.timeSetting("cloud.gce.max_wait", TimeValue.timeValueSeconds(-1), false, Setting.Scope.CLUSTER);
 
     /**
      * Return a collection of running instances within the same GCE project

plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java

@@ -48,7 +48,6 @@ import java.security.PrivilegedAction;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -157,9 +156,8 @@ public class GceComputeServiceImpl extends AbstractLifecycleComponent<GceCompute
     @Inject
     public GceComputeServiceImpl(Settings settings, NetworkService networkService) {
         super(settings);
-        this.project = settings.get(Fields.PROJECT);
+        this.project = PROJECT_SETTING.get(settings);
-        String[] zoneList = settings.getAsArray(Fields.ZONE);
+        this.zones = ZONE_SETTING.get(settings);
-        this.zones = Arrays.asList(zoneList);
         networkService.addCustomNameResolver(new GceNameResolver(settings, this));
     }
 
@@ -207,15 +205,13 @@ public class GceComputeServiceImpl extends AbstractLifecycleComponent<GceCompute
                 refreshInterval = TimeValue.timeValueSeconds(credential.getExpiresInSeconds() - 1);
             }
 
-            boolean ifRetry = settings.getAsBoolean(Fields.RETRY, true);
+            Compute.Builder builder = new Compute.Builder(getGceHttpTransport(), gceJsonFactory, null).setApplicationName(VERSION);
-            Compute.Builder builder = new Compute.Builder(getGceHttpTransport(), gceJsonFactory, null)
-                    .setApplicationName(Fields.VERSION);
 
-            if (ifRetry) {
+            if (RETRY_SETTING.exists(settings)) {
-                int maxWait = settings.getAsInt(Fields.MAXWAIT, -1);
+                TimeValue maxWait = MAX_WAIT_SETTING.get(settings);
                 RetryHttpInitializerWrapper retryHttpInitializerWrapper;
 
-                if (maxWait > 0) {
+                if (maxWait.getMillis() > 0) {
                     retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential, maxWait);
                 } else {
                     retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential);

plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceDiscovery.java

@@ -23,6 +23,7 @@ import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.discovery.DiscoverySettings;
 import org.elasticsearch.discovery.zen.ZenDiscovery;
@@ -31,6 +32,9 @@ import org.elasticsearch.discovery.zen.ping.ZenPingService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.util.Collections;
+import java.util.List;
+
 /**
  *
  */
@@ -38,6 +42,12 @@ public class GceDiscovery extends ZenDiscovery {
 
     public static final String GCE = "gce";
 
+    /**
+     * discovery.gce.tags: The gce discovery can filter machines to include in the cluster based on tags.
+     */
+    public static final Setting<List<String>> TAGS_SETTING =
+        Setting.listSetting("discovery.gce.tags", Collections.emptyList(), s -> s, false, Setting.Scope.CLUSTER);
+
     @Inject
     public GceDiscovery(Settings settings, ClusterName clusterName, ThreadPool threadPool, TransportService transportService,
                         ClusterService clusterService, ClusterSettings clusterSettings, ZenPingService pingService,

plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/GceUnicastHostsProvider.java

@@ -39,12 +39,9 @@ import org.elasticsearch.transport.TransportService;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
-import static org.elasticsearch.cloud.gce.GceComputeService.Fields;
-
 /**
  *
  */
@@ -60,8 +57,8 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas
 
     private final Version version;
     private final String project;
-    private final String[] zones;
+    private final List<String> zones;
-    private final String[] tags;
+    private final List<String> tags;
 
     private final TimeValue refreshInterval;
     private long lastRefresh;
@@ -78,24 +75,29 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas
         this.networkService = networkService;
         this.version = version;
 
-        this.refreshInterval = settings.getAsTime(Fields.REFRESH, TimeValue.timeValueSeconds(0));
+        this.refreshInterval = GceComputeService.REFRESH_SETTING.get(settings);
-        this.project = settings.get(Fields.PROJECT);
+        this.project = GceComputeService.PROJECT_SETTING.get(settings);
-        this.zones = settings.getAsArray(Fields.ZONE);
+        this.zones = GceComputeService.ZONE_SETTING.get(settings);
 
-        this.tags = settings.getAsArray(Fields.TAGS);
+        this.tags = GceDiscovery.TAGS_SETTING.get(settings);
         if (logger.isDebugEnabled()) {
-            logger.debug("using tags {}", Arrays.asList(this.tags));
+            logger.debug("using tags {}", this.tags);
         }
     }
 
     /**
      * We build the list of Nodes from GCE Management API
-     * Information can be cached using `plugins.refresh_interval` property if needed.
+     * Information can be cached using `cloud.gce.refresh_interval` property if needed.
-     * Setting `plugins.refresh_interval` to `-1` will cause infinite caching.
-     * Setting `plugins.refresh_interval` to `0` will disable caching (default).
      */
     @Override
     public List<DiscoveryNode> buildDynamicNodes() {
+        // We check that needed properties have been set
+        if (this.project == null || this.project.isEmpty() || this.zones == null || this.zones.isEmpty()) {
+            throw new IllegalArgumentException("one or more gce discovery settings are missing. " +
+                "Check elasticsearch.yml file. Should have [" + GceComputeService.PROJECT_SETTING.getKey() +
+                "] and [" + GceComputeService.ZONE_SETTING.getKey() + "].");
+        }
+
         if (refreshInterval.millis() != 0) {
             if (cachedDiscoNodes != null &&
                     (refreshInterval.millis() < 0 || (System.currentTimeMillis() - lastRefresh) < refreshInterval.millis())) {
@@ -142,7 +144,7 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas
 
                 // see if we need to filter by tag
                 boolean filterByTag = false;
-                if (tags.length > 0) {
+                if (tags.isEmpty() == false) {
                     logger.trace("start filtering instance {} with tags {}.", name, tags);
                     if (instance.getTags() == null || instance.getTags().isEmpty()
                             || instance.getTags().getItems() == null || instance.getTags().getItems().isEmpty()) {

plugins/discovery-gce/src/main/java/org/elasticsearch/discovery/gce/RetryHttpInitializerWrapper.java

@@ -32,6 +32,7 @@ import com.google.api.client.util.Sleeper;
 import org.elasticsearch.SpecialPermission;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.ESLoggerFactory;
+import org.elasticsearch.common.unit.TimeValue;
 
 import java.io.IOException;
 import java.security.AccessController;
@@ -40,7 +41,7 @@ import java.util.Objects;
 
 public class RetryHttpInitializerWrapper implements HttpRequestInitializer {
 
-    private int maxWait;
+    private TimeValue maxWait;
 
     private static final ESLogger logger =
             ESLoggerFactory.getLogger(RetryHttpInitializerWrapper.class.getName());
@@ -55,16 +56,16 @@ public class RetryHttpInitializerWrapper implements HttpRequestInitializer {
     private final Sleeper sleeper;
 
     public RetryHttpInitializerWrapper(Credential wrappedCredential) {
-        this(wrappedCredential, Sleeper.DEFAULT, ExponentialBackOff.DEFAULT_MAX_ELAPSED_TIME_MILLIS);
+        this(wrappedCredential, Sleeper.DEFAULT, TimeValue.timeValueMillis(ExponentialBackOff.DEFAULT_MAX_ELAPSED_TIME_MILLIS));
     }
 
-    public RetryHttpInitializerWrapper(Credential wrappedCredential, int maxWait) {
+    public RetryHttpInitializerWrapper(Credential wrappedCredential, TimeValue maxWait) {
         this(wrappedCredential, Sleeper.DEFAULT, maxWait);
     }
 
     // Use only for testing.
     RetryHttpInitializerWrapper(
-            Credential wrappedCredential, Sleeper sleeper, int maxWait) {
+            Credential wrappedCredential, Sleeper sleeper, TimeValue maxWait) {
         this.wrappedCredential = Objects.requireNonNull(wrappedCredential);
         this.sleeper = sleeper;
         this.maxWait = maxWait;
@@ -77,12 +78,7 @@ public class RetryHttpInitializerWrapper implements HttpRequestInitializer {
         if (sm != null) {
             sm.checkPermission(new SpecialPermission());
         }
-        return AccessController.doPrivileged(new PrivilegedAction<MockGoogleCredential.Builder>() {
+        return AccessController.doPrivileged((PrivilegedAction<MockGoogleCredential.Builder>) () -> new MockGoogleCredential.Builder());
-            @Override
-            public MockGoogleCredential.Builder run() {
-                return new MockGoogleCredential.Builder();
-            }
-        });
     }
 
     @Override
@@ -90,7 +86,7 @@ public class RetryHttpInitializerWrapper implements HttpRequestInitializer {
         final HttpUnsuccessfulResponseHandler backoffHandler =
                 new HttpBackOffUnsuccessfulResponseHandler(
                         new ExponentialBackOff.Builder()
-                                .setMaxElapsedTimeMillis(maxWait)
+                                .setMaxElapsedTimeMillis(((int) maxWait.getMillis()))
                                 .build())
                         .setSleeper(sleeper);
 
@@ -122,7 +118,7 @@ public class RetryHttpInitializerWrapper implements HttpRequestInitializer {
         httpRequest.setIOExceptionHandler(
                 new HttpBackOffIOExceptionHandler(
                         new ExponentialBackOff.Builder()
-                                .setMaxElapsedTimeMillis(maxWait)
+                                .setMaxElapsedTimeMillis(((int) maxWait.getMillis()))
                                 .build())
                         .setSleeper(sleeper)
         );

plugins/discovery-gce/src/main/java/org/elasticsearch/plugin/discovery/gce/GceDiscoveryPlugin.java

@@ -25,12 +25,12 @@ import com.google.api.client.util.ClassInfo;
 import org.elasticsearch.SpecialPermission;
 import org.elasticsearch.cloud.gce.GceComputeService;
 import org.elasticsearch.cloud.gce.GceModule;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.component.LifecycleComponent;
 import org.elasticsearch.common.inject.Module;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.settings.SettingsModule;
 import org.elasticsearch.discovery.DiscoveryModule;
 import org.elasticsearch.discovery.gce.GceDiscovery;
 import org.elasticsearch.discovery.gce.GceUnicastHostsProvider;
@@ -38,9 +38,8 @@ import org.elasticsearch.plugins.Plugin;
 
 import java.security.AccessController;
 import java.security.PrivilegedAction;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.List;
+import java.util.Collections;
 
 public class GceDiscoveryPlugin extends Plugin {
     static {
@@ -84,70 +83,30 @@ public class GceDiscoveryPlugin extends Plugin {
 
     @Override
     public Collection<Module> nodeModules() {
-        List<Module> modules = new ArrayList<>();
+        return Collections.singletonList(new GceModule());
-        if (isDiscoveryAlive(settings, logger)) {
-            modules.add(new GceModule());
-        }
-        return modules;
     }
 
     @Override
     @SuppressWarnings("rawtypes") // Supertype uses raw type
     public Collection<Class<? extends LifecycleComponent>> nodeServices() {
-        Collection<Class<? extends LifecycleComponent>> services = new ArrayList<>();
+        return Collections.singletonList(GceModule.getComputeServiceImpl());
-        if (isDiscoveryAlive(settings, logger)) {
-            services.add(GceModule.getComputeServiceImpl());
-        }
-        return services;
     }
 
     public void onModule(DiscoveryModule discoveryModule) {
-        if (isDiscoveryAlive(settings, logger)) {
+        discoveryModule.addDiscoveryType("gce", GceDiscovery.class);
-            discoveryModule.addDiscoveryType("gce", GceDiscovery.class);
+        // If discovery.type: gce, we add Gce as a unicast provider
+        if (GceDiscovery.GCE.equalsIgnoreCase(DiscoveryModule.DISCOVERY_TYPE_SETTING.get(settings))) {
             discoveryModule.addUnicastHostProvider(GceUnicastHostsProvider.class);
         }
     }
 
-    /**
+    public void onModule(SettingsModule settingsModule) {
-     * Check if discovery is meant to start
+        // Register GCE settings
-     *
+        settingsModule.registerSetting(GceComputeService.PROJECT_SETTING);
-     * @return true if we can start gce discovery features
+        settingsModule.registerSetting(GceComputeService.ZONE_SETTING);
-     */
+        settingsModule.registerSetting(GceDiscovery.TAGS_SETTING);
-    public static boolean isDiscoveryAlive(Settings settings, ESLogger logger) {
+        settingsModule.registerSetting(GceComputeService.REFRESH_SETTING);
-        // User set discovery.type: gce
+        settingsModule.registerSetting(GceComputeService.RETRY_SETTING);
-        if (GceDiscovery.GCE.equalsIgnoreCase(DiscoveryModule.DISCOVERY_TYPE_SETTING.get(settings)) == false) {
+        settingsModule.registerSetting(GceComputeService.MAX_WAIT_SETTING);
-            logger.debug("discovery.type not set to {}", GceDiscovery.GCE);
-            return false;
-        }
-
-        if (checkProperty(GceComputeService.Fields.PROJECT, settings.get(GceComputeService.Fields.PROJECT), logger) == false ||
-                checkProperty(GceComputeService.Fields.ZONE, settings.getAsArray(GceComputeService.Fields.ZONE), logger) == false) {
-            logger.debug("one or more gce discovery settings are missing. " +
-                            "Check elasticsearch.yml file. Should have [{}] and [{}].",
-                    GceComputeService.Fields.PROJECT,
-                    GceComputeService.Fields.ZONE);
-            return false;
-        }
-
-        logger.trace("all required properties for gce discovery are set!");
-
-        return true;
     }
-
-    private static boolean checkProperty(String name, String value, ESLogger logger) {
-        if (!Strings.hasText(value)) {
-            logger.warn("{} is not set.", name);
-            return false;
-        }
-        return true;
-    }
-
-    private static boolean checkProperty(String name, String[] values, ESLogger logger) {
-        if (values == null || values.length == 0) {
-            logger.warn("{} is not set.", name);
-            return false;
-        }
-        return true;
-    }
-
 }

plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceDiscoverySettingsTests.java

@@ -1,77 +0,0 @@
-/*
- * Licensed to Elasticsearch under one or more contributor
- * license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright
- * ownership. Elasticsearch licenses this file to you under
- * the Apache License, Version 2.0 (the "License"); you may
- * not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.elasticsearch.discovery.gce;
-
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.plugin.discovery.gce.GceDiscoveryPlugin;
-import org.elasticsearch.test.ESTestCase;
-
-import static org.hamcrest.Matchers.is;
-
-public class GceDiscoverySettingsTests extends ESTestCase {
-    public void testDiscoveryReady() {
-        Settings settings = Settings.builder()
-                .put("discovery.type", "gce")
-                .put("cloud.gce.project_id", "gce_id")
-                .putArray("cloud.gce.zone", "gce_zones_1", "gce_zones_2")
-                .build();
-
-        boolean discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(true));
-    }
-
-    public void testDiscoveryNotReady() {
-        Settings settings = Settings.EMPTY;
-        boolean discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(false));
-
-        settings = Settings.builder()
-                .put("discovery.type", "gce")
-                .build();
-
-        discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(false));
-
-        settings = Settings.builder()
-                .put("discovery.type", "gce")
-                .put("cloud.gce.project_id", "gce_id")
-                .build();
-
-        discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(false));
-
-
-        settings = Settings.builder()
-                .put("discovery.type", "gce")
-                .putArray("cloud.gce.zone", "gce_zones_1", "gce_zones_2")
-                .build();
-
-        discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(false));
-
-        settings = Settings.builder()
-                .put("cloud.gce.project_id", "gce_id")
-                .putArray("cloud.gce.zone", "gce_zones_1", "gce_zones_2")
-                .build();
-
-        discoveryReady = GceDiscoveryPlugin.isDiscoveryAlive(settings, logger);
-        assertThat(discoveryReady, is(false));
-    }
-}

plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceDiscoveryTests.java

@@ -35,6 +35,7 @@ import org.junit.BeforeClass;
 import java.util.List;
 import java.util.Locale;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 
@@ -51,8 +52,8 @@ import static org.hamcrest.Matchers.is;
  * For example, if you create a test `myNewAwesomeTest` with following settings:
  *
  * Settings nodeSettings = Settings.builder()
- *  .put(GceComputeService.Fields.PROJECT, projectName)
+ *  .put(GceComputeService.PROJECT, projectName)
- *  .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+ *  .put(GceComputeService.ZONE, "europe-west1-b")
  *  .build();
  *
  *  You need to create a file under `src/test/resources/org/elasticsearch/discovery/gce/` named:
@@ -118,8 +119,8 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithDifferentTagsAndNoTagSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -128,9 +129,9 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithDifferentTagsAndOneTagSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
-                .putArray(GceComputeService.Fields.TAGS, "elasticsearch")
+                .putArray(GceDiscovery.TAGS_SETTING.getKey(), "elasticsearch")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -140,9 +141,9 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithDifferentTagsAndTwoTagSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
-                .putArray(GceComputeService.Fields.TAGS, "elasticsearch", "dev")
+                .putArray(GceDiscovery.TAGS_SETTING.getKey(), "elasticsearch", "dev")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -152,8 +153,8 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithSameTagsAndNoTagSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -162,9 +163,9 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithSameTagsAndOneTagSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
-                .putArray(GceComputeService.Fields.TAGS, "elasticsearch")
+                .putArray(GceDiscovery.TAGS_SETTING.getKey(), "elasticsearch")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -173,9 +174,9 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testNodesWithSameTagsAndTwoTagsSet() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .put(GceComputeService.Fields.ZONE, "europe-west1-b")
+                .put(GceComputeService.ZONE_SETTING.getKey(), "europe-west1-b")
-                .putArray(GceComputeService.Fields.TAGS, "elasticsearch", "dev")
+                .putArray(GceDiscovery.TAGS_SETTING.getKey(), "elasticsearch", "dev")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -184,8 +185,8 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testMultipleZonesAndTwoNodesInSameZone() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .putArray(GceComputeService.Fields.ZONE, "us-central1-a", "europe-west1-b")
+                .putArray(GceComputeService.ZONE_SETTING.getKey(), "us-central1-a", "europe-west1-b")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -194,8 +195,8 @@ public class GceDiscoveryTests extends ESTestCase {
 
     public void testMultipleZonesAndTwoNodesInDifferentZones() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .putArray(GceComputeService.Fields.ZONE, "us-central1-a", "europe-west1-b")
+                .putArray(GceComputeService.ZONE_SETTING.getKey(), "us-central1-a", "europe-west1-b")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
@@ -207,11 +208,48 @@ public class GceDiscoveryTests extends ESTestCase {
      */
     public void testZeroNode43() {
         Settings nodeSettings = Settings.builder()
-                .put(GceComputeService.Fields.PROJECT, projectName)
+                .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
-                .putArray(GceComputeService.Fields.ZONE, "us-central1-a", "us-central1-b")
+                .putArray(GceComputeService.ZONE_SETTING.getKey(), "us-central1-a", "us-central1-b")
                 .build();
         mock = new GceComputeServiceMock(nodeSettings, networkService);
         List<DiscoveryNode> discoveryNodes = buildDynamicNodes(mock, nodeSettings);
         assertThat(discoveryNodes, hasSize(0));
     }
+
+    public void testIllegalSettingsMissingAllRequired() {
+        Settings nodeSettings = Settings.EMPTY;
+        mock = new GceComputeServiceMock(Settings.EMPTY, networkService);
+        try {
+            buildDynamicNodes(mock, nodeSettings);
+            fail("We expect an IllegalArgumentException for incomplete settings");
+        } catch (IllegalArgumentException expected) {
+            assertThat(expected.getMessage(), containsString("one or more gce discovery settings are missing."));
+        }
+    }
+
+    public void testIllegalSettingsMissingProject() {
+        Settings nodeSettings = Settings.builder()
+            .putArray(GceComputeService.ZONE_SETTING.getKey(), "us-central1-a", "us-central1-b")
+            .build();
+        mock = new GceComputeServiceMock(nodeSettings, networkService);
+        try {
+            buildDynamicNodes(mock, nodeSettings);
+            fail("We expect an IllegalArgumentException for incomplete settings");
+        } catch (IllegalArgumentException expected) {
+            assertThat(expected.getMessage(), containsString("one or more gce discovery settings are missing."));
+        }
+    }
+
+    public void testIllegalSettingsMissingZone() {
+        Settings nodeSettings = Settings.builder()
+            .put(GceComputeService.PROJECT_SETTING.getKey(), projectName)
+            .build();
+        mock = new GceComputeServiceMock(nodeSettings, networkService);
+        try {
+            buildDynamicNodes(mock, nodeSettings);
+            fail("We expect an IllegalArgumentException for incomplete settings");
+        } catch (IllegalArgumentException expected) {
+            assertThat(expected.getMessage(), containsString("one or more gce discovery settings are missing."));
+        }
+    }
 }

plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/RetryHttpInitializerWrapperTests.java

@@ -34,6 +34,7 @@ import com.google.api.client.testing.http.MockLowLevelHttpRequest;
 import com.google.api.client.testing.http.MockLowLevelHttpResponse;
 import com.google.api.client.testing.util.MockSleeper;
 import com.google.api.services.compute.Compute;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
@@ -100,7 +101,8 @@ public class RetryHttpInitializerWrapperTests extends ESTestCase {
                 .build();
         MockSleeper mockSleeper = new MockSleeper();
 
-        RetryHttpInitializerWrapper retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential, mockSleeper, 5000);
+        RetryHttpInitializerWrapper retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential, mockSleeper,
+            TimeValue.timeValueSeconds(5));
 
         Compute client = new Compute.Builder(fakeTransport, new JacksonFactory(), null)
                 .setHttpRequestInitializer(retryHttpInitializerWrapper)
@@ -115,7 +117,7 @@ public class RetryHttpInitializerWrapperTests extends ESTestCase {
     }
 
     public void testRetryWaitTooLong() throws Exception {
-        int maxWaitTime = 10;
+        TimeValue maxWaitTime = TimeValue.timeValueMillis(10);
         int maxRetryTimes = 50;
 
         FailThenSuccessBackoffTransport fakeTransport =
@@ -127,7 +129,7 @@ public class RetryHttpInitializerWrapperTests extends ESTestCase {
         MockSleeper oneTimeSleeper = new MockSleeper() {
             @Override
             public void sleep(long millis) throws InterruptedException {
-                Thread.sleep(maxWaitTime);
+                Thread.sleep(maxWaitTime.getMillis());
                 super.sleep(0); // important number, use this to get count
             }
         };
@@ -157,7 +159,8 @@ public class RetryHttpInitializerWrapperTests extends ESTestCase {
         MockGoogleCredential credential = RetryHttpInitializerWrapper.newMockCredentialBuilder()
                 .build();
         MockSleeper mockSleeper = new MockSleeper();
-        RetryHttpInitializerWrapper retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential, mockSleeper, 500);
+        RetryHttpInitializerWrapper retryHttpInitializerWrapper = new RetryHttpInitializerWrapper(credential, mockSleeper,
+            TimeValue.timeValueMillis(500));
 
         Compute client = new Compute.Builder(fakeTransport, new JacksonFactory(), null)
                 .setHttpRequestInitializer(retryHttpInitializerWrapper)