cloud-gce plugin should check discovery.type

GCE plugin tries to start immediately gce discovery even if we don't
set discovery.type. This commmit adds check `discovery.type` and
other required parameters before loading gce plugin.

closes #13614

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

@@ -20,7 +20,6 @@
 package org.elasticsearch.cloud.gce;
 
 import com.google.api.services.compute.model.Instance;
-import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.component.LifecycleComponent;
 
 import java.util.Collection;

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

@@ -21,8 +21,6 @@ package org.elasticsearch.discovery.gce;
 
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterService;
-import org.elasticsearch.cluster.settings.ClusterDynamicSettings;
-import org.elasticsearch.cluster.settings.DynamicSettings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.discovery.DiscoverySettings;
@@ -38,6 +36,8 @@ import org.elasticsearch.transport.TransportService;
  */
 public class GceDiscovery extends ZenDiscovery {
 
+    public static final String GCE = "gce";
+
     @Inject
     public GceDiscovery(Settings settings, ClusterName clusterName, ThreadPool threadPool, TransportService transportService,
                         ClusterService clusterService, NodeSettingsService nodeSettingsService, ZenPingService pingService,

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

@@ -83,10 +83,6 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas
         this.project = settings.get(Fields.PROJECT);
         this.zones = settings.getAsArray(Fields.ZONE);
 
-        // Check that we have all needed properties
-        checkProperty(Fields.PROJECT, project);
-        checkProperty(Fields.ZONE, zones);
-
         this.tags = settings.getAsArray(Fields.TAGS);
         if (logger.isDebugEnabled()) {
             logger.debug("using tags {}", Arrays.asList(this.tags));
@@ -250,16 +246,4 @@ public class GceUnicastHostsProvider extends AbstractComponent implements Unicas
 
         return cachedDiscoNodes;
     }
-
-    private void checkProperty(String name, String value) {
-        if (!Strings.hasText(value)) {
-            logger.warn("{} is not set.", name);
-        }
-    }
-
-    private void checkProperty(String name, String[] values) {
-        if (values == null || values.length == 0) {
-            logger.warn("{} is not set.", name);
-        }
-    }
 }

plugins/cloud-gce/src/main/java/org/elasticsearch/plugin/cloud/gce/CloudGcePlugin.java

@@ -19,9 +19,13 @@
 
 package org.elasticsearch.plugin.cloud.gce;
 
+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.discovery.DiscoveryModule;
 import org.elasticsearch.discovery.gce.GceDiscovery;
@@ -38,6 +42,7 @@ import java.util.List;
 public class CloudGcePlugin extends Plugin {
 
     private final Settings settings;
+    protected final ESLogger logger = Loggers.getLogger(CloudGcePlugin.class);
 
     public CloudGcePlugin(Settings settings) {
         this.settings = settings;
@@ -56,7 +61,7 @@ public class CloudGcePlugin extends Plugin {
     @Override
     public Collection<Module> nodeModules() {
         List<Module> modules = new ArrayList<>();
-        if (settings.getAsBoolean("cloud.enabled", true)) {
+        if (isDiscoveryAlive(settings, logger)) {
             modules.add(new GceModule());
         }
         return modules;
@@ -65,15 +70,59 @@ public class CloudGcePlugin extends Plugin {
     @Override
     public Collection<Class<? extends LifecycleComponent>> nodeServices() {
         Collection<Class<? extends LifecycleComponent>> services = new ArrayList<>();
-        if (settings.getAsBoolean("cloud.enabled", true)) {
+        if (isDiscoveryAlive(settings, logger)) {
             services.add(GceModule.getComputeServiceImpl());
         }
         return services;
     }
 
     public void onModule(DiscoveryModule discoveryModule) {
-        discoveryModule.addDiscoveryType("gce", GceDiscovery.class);
+        if (isDiscoveryAlive(settings, logger)) {
-        discoveryModule.addUnicastHostProvider(GceUnicastHostsProvider.class);
+            discoveryModule.addDiscoveryType("gce", GceDiscovery.class);
+            discoveryModule.addUnicastHostProvider(GceUnicastHostsProvider.class);
+        }
+    }
+
+    /**
+     * Check if discovery is meant to start
+     *
+     * @return true if we can start gce discovery features
+     */
+    public static boolean isDiscoveryAlive(Settings settings, ESLogger logger) {
+        // User set discovery.type: gce
+        if (GceDiscovery.GCE.equalsIgnoreCase(settings.get("discovery.type")) == false) {
+            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/cloud-gce/src/test/java/org/elasticsearch/discovery/gce/GceDiscoverySettingsTests.java

@@ -0,0 +1,78 @@
+/*
+ * 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.cloud.gce.GceModule;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.plugin.cloud.gce.CloudGcePlugin;
+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 = CloudGcePlugin.isDiscoveryAlive(settings, logger);
+        assertThat(discoveryReady, is(true));
+    }
+
+    public void testDiscoveryNotReady() {
+        Settings settings = Settings.EMPTY;
+        boolean discoveryReady = CloudGcePlugin.isDiscoveryAlive(settings, logger);
+        assertThat(discoveryReady, is(false));
+
+        settings = Settings.builder()
+                .put("discovery.type", "gce")
+                .build();
+
+        discoveryReady = CloudGcePlugin.isDiscoveryAlive(settings, logger);
+        assertThat(discoveryReady, is(false));
+
+        settings = Settings.builder()
+                .put("discovery.type", "gce")
+                .put("cloud.gce.project_id", "gce_id")
+                .build();
+
+        discoveryReady = CloudGcePlugin.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 = CloudGcePlugin.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 = CloudGcePlugin.isDiscoveryAlive(settings, logger);
+        assertThat(discoveryReady, is(false));
+    }
+}