Discovery NPE when a zone has 0 nodes in it

Closes #43.
(cherry picked from commit b028910)
(cherry picked from commit fa3e157)

Author: dadoonet

src/main/java/org/elasticsearch/cloud/gce/GceComputeServiceImpl.java

@@ -67,6 +67,9 @@ public List<Instance> apply(String zoneId) {
                     try {
                         Compute.Instances.List list = client().instances().list(project, zoneId);
                         InstanceList instanceList = list.execute();
+                        if (instanceList.isEmpty()) {
+                            return Lists.newArrayList();
+                        }
 
                         return instanceList.getItems();
                     } catch (IOException e) {

src/test/java/org/elasticsearch/discovery/gce/GceComputeEngineTest.java

@@ -24,10 +24,7 @@
 import org.elasticsearch.common.collect.Lists;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.discovery.gce.mock.GceComputeServiceTwoNodesDifferentTagsMock;
-import org.elasticsearch.discovery.gce.mock.GceComputeServiceTwoNodesOneZoneMock;
-import org.elasticsearch.discovery.gce.mock.GceComputeServiceTwoNodesSameTagsMock;
-import org.elasticsearch.discovery.gce.mock.GceComputeServiceTwoNodesTwoZonesMock;
+import org.elasticsearch.discovery.gce.mock.*;
 import org.elasticsearch.plugins.PluginsService;
 import org.elasticsearch.test.ElasticsearchIntegrationTest;
 import org.junit.Ignore;
@@ -186,11 +183,11 @@ public void nodes_with_same_tags_and_two_tags_set() {
     public void multiple_zones_and_two_nodes_in_same_zone() {
         startNode(1,
                 GceComputeServiceTwoNodesOneZoneMock.class,
-                ImmutableSettings.settingsBuilder().put("cloud.gce.zones", Lists.newArrayList("us-central1-a", "us-central1-b",
+                ImmutableSettings.settingsBuilder().put("cloud.gce.zone", Lists.newArrayList("us-central1-a", "us-central1-b",
                         "us-central1-f", "europe-west1-a", "europe-west1-b")).build());
         startNode(2,
                 GceComputeServiceTwoNodesOneZoneMock.class,
-                ImmutableSettings.settingsBuilder().put("cloud.gce.zones", Lists.newArrayList("us-central1-a", "us-central1-b",
+                ImmutableSettings.settingsBuilder().put("cloud.gce.zone", Lists.newArrayList("us-central1-a", "us-central1-b",
                         "us-central1-f", "europe-west1-a", "europe-west1-b")).build());
 
         // We expect having 2 nodes as part of the cluster, let's test that
@@ -201,15 +198,26 @@ public void multiple_zones_and_two_nodes_in_same_zone() {
     public void multiple_zones_and_two_nodes_in_different_zones() {
         startNode(1,
                 GceComputeServiceTwoNodesTwoZonesMock.class,
-                ImmutableSettings.settingsBuilder().put("cloud.gce.zones", Lists.newArrayList("us-central1-a", "us-central1-b",
+                ImmutableSettings.settingsBuilder().put("cloud.gce.zone", Lists.newArrayList("us-central1-a", "us-central1-b",
                         "us-central1-f", "europe-west1-a", "europe-west1-b")).build());
         startNode(2,
                 GceComputeServiceTwoNodesTwoZonesMock.class,
-                ImmutableSettings.settingsBuilder().put("cloud.gce.zones", Lists.newArrayList("us-central1-a", "us-central1-b",
+                ImmutableSettings.settingsBuilder().put("cloud.gce.zone", Lists.newArrayList("us-central1-a", "us-central1-b",
                         "us-central1-f", "europe-west1-a", "europe-west1-b")).build());
 
         // We expect having 2 nodes as part of the cluster, let's test that
         checkNumberOfNodes(2);
     }
 
+    /**
+     * For issue https://github.com/elastic/elasticsearch-cloud-gce/issues/43
+     */
+    @Test @Ignore
+    public void zero_node_43() {
+        startNode(1,
+                GceComputeServiceZeroNodeMock.class,
+                ImmutableSettings.settingsBuilder().put("cloud.gce.zone", Lists.newArrayList("us-central1-a", "us-central1-b",
+                        "us-central1-f", "europe-west1-a", "europe-west1-b")).build());
+    }
+
 }

src/test/java/org/elasticsearch/discovery/gce/mock/GceComputeServiceZeroNodeMock.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.mock;
+
+import com.google.api.services.compute.model.Instance;
+import org.elasticsearch.common.base.Function;
+import org.elasticsearch.common.collect.Iterables;
+import org.elasticsearch.common.collect.Lists;
+import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.settings.Settings;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+/**
+ *
+ */
+public class GceComputeServiceZeroNodeMock extends GceComputeServiceAbstractMock {
+
+    @Override
+    protected List<ArrayList<String>> getTags() {
+        return Lists.newArrayList();
+    }
+
+    @Override
+    protected List<String> getZones() {
+        return Lists.newArrayList();
+    }
+
+    private final List<String> zoneList;
+
+    @Override
+    public Collection<Instance> instances() {
+        logger.debug("get instances for zoneList [{}]", zoneList);
+
+        List<List<Instance>> instanceListByZone = Lists.transform(zoneList, new Function<String, List<Instance>>() {
+            @Override
+            public List<Instance> apply(String zoneId) {
+                // If we return null here we will get a trace as explained in issue 43
+                return Lists.newArrayList();
+            }
+        });
+
+        //Collapse instances from all zones into one neat list
+        List<Instance> instanceList = Lists.newArrayList(Iterables.concat(instanceListByZone));
+
+        if (instanceList.size() == 0) {
+            logger.warn("disabling GCE discovery. Can not get list of nodes");
+        }
+
+        return instanceList;
+    }
+
+    @Inject
+    protected GceComputeServiceZeroNodeMock(Settings settings) {
+        super(settings);
+        String[] zoneList = componentSettings.getAsArray(Fields.ZONE, settings.getAsArray("cloud.gce." + Fields.ZONE));
+        this.zoneList = Lists.newArrayList(zoneList);
+    }
+}