[GCE Discovery] Correcly handle large zones with 500 or more instances (

elastic#83785) Discovery gce plugin has some issues while running in large pools of vms instances. This pr attempts to solve it. Closes elastic#83783
arteam · Feb 15, 2022 · 136d797 · 136d797
1 parent c142ef1
commit 136d797
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 8 deletions.
diff --git a/docs/changelog/83785.yaml b/docs/changelog/83785.yaml
@@ -0,0 +1,6 @@
+pr: 83785
+summary: '[GCE Discovery] Correcly handle large zones with 500 or more instances'
+area: Distributed
+type: bug
+issues:
+  - 83783
diff --git a/plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/GceInstancesServiceImpl.java b/plugins/discovery-gce/src/main/java/org/elasticsearch/cloud/gce/GceInstancesServiceImpl.java
@@ -69,14 +69,19 @@ public Collection<Instance> instances() {
             try {
                 // hack around code messiness in GCE code
                 // TODO: get this fixed
-                InstanceList instanceList = Access.doPrivilegedIOException(() -> {
-                    Compute.Instances.List list = client().instances().list(project, zoneId);
-                    return list.execute();
+                return Access.doPrivilegedIOException(() -> {
+                    String nextPageToken = null;
+                    List<Instance> zoneInstances = new ArrayList<>();
+                    do {
+                        Compute.Instances.List list = client().instances().list(project, zoneId).setPageToken(nextPageToken);
+                        InstanceList instanceList = list.execute();
+                        nextPageToken = instanceList.getNextPageToken();
+                        if (instanceList.isEmpty() == false && instanceList.getItems() != null) {
+                            zoneInstances.addAll(instanceList.getItems());
+                        }
+                    } while (nextPageToken != null);
+                    return zoneInstances;
                 });
-                // assist type inference
-                return instanceList.isEmpty() || instanceList.getItems() == null
-                    ? Collections.<Instance>emptyList()
-                    : instanceList.getItems();
             } catch (IOException e) {
                 logger.warn((Supplier<?>) () -> new ParameterizedMessage("Problem fetching instance list for zone {}", zoneId), e);
                 logger.debug("Full exception:", e);

diff --git a/plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceDiscoveryTests.java b/plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceDiscoveryTests.java
@@ -272,4 +272,17 @@ public void testMetadataServerValues() {
         List<TransportAddress> dynamicHosts = buildDynamicNodes(mock, nodeSettings);
         assertThat(dynamicHosts, hasSize(1));
     }
+
+    public void testNodesWithPagination() {
+        Settings nodeSettings = Settings.builder()
+            .put(GceInstancesServiceImpl.PROJECT_SETTING.getKey(), projectName)
+            .put(GceInstancesServiceImpl.ZONE_SETTING.getKey(), "europe-west1-b")
+            .putList(GceSeedHostsProvider.TAGS_SETTING.getKey(), "elasticsearch", "dev")
+            .build();
+        mock = new GceInstancesServiceMock(nodeSettings);
+        List<TransportAddress> dynamicHosts = buildDynamicNodes(mock, nodeSettings);
+        assertThat(dynamicHosts, hasSize(2));
+        assertEquals("10.240.79.59", dynamicHosts.get(0).getAddress());
+        assertEquals("10.240.79.60", dynamicHosts.get(1).getAddress());
+    }
 }
diff --git a/plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceMockUtils.java b/plugins/discovery-gce/src/test/java/org/elasticsearch/discovery/gce/GceMockUtils.java
@@ -67,7 +67,7 @@ public static String readGoogleApiJsonResponse(String url) throws IOException {
 
     private static String readJsonResponse(String url, String urlRoot) throws IOException {
         // We extract from the url the mock file path we want to use
-        String mockFileName = Strings.replace(url, urlRoot, "");
+        String mockFileName = Strings.replace(url, urlRoot, "").replace("?", "%3F");
 
         URL resource = GceMockUtils.class.getResource(mockFileName);
         if (resource == null) {

diff --git a/...arch/discovery/gce/compute/v1/projects/nodeswithpagination/zones/europe-west1-b/instances b/...arch/discovery/gce/compute/v1/projects/nodeswithpagination/zones/europe-west1-b/instances
@@ -0,0 +1,37 @@
+{
+  "id": "dummy",
+  "items":[
+    {
+      "description": "ES Node 1",
+      "id": "9309873766428965105",
+      "kind": "compute#instance",
+      "machineType": "n1-standard-1",
+      "name": "test1",
+      "networkInterfaces": [
+        {
+          "accessConfigs": [
+            {
+              "kind": "compute#accessConfig",
+              "name": "External NAT",
+              "natIP": "104.155.13.147",
+              "type": "ONE_TO_ONE_NAT"
+            }
+          ],
+          "name": "nic0",
+          "network": "default",
+          "networkIP": "10.240.79.59"
+        }
+      ],
+      "status": "RUNNING",
+      "tags": {
+        "fingerprint": "xA6QJb-rGtg=",
+        "items": [
+          "elasticsearch",
+          "dev"
+        ]
+      },
+      "zone": "europe-west1-b"
+    }
+  ],
+  "nextPageToken": "next-token"
+}
diff --git a/...ute/v1/projects/nodeswithpagination/zones/europe-west1-b/instances%3FpageToken=next-token b/...ute/v1/projects/nodeswithpagination/zones/europe-west1-b/instances%3FpageToken=next-token
@@ -0,0 +1,36 @@
+{
+  "id": "dummy",
+  "items":[
+    {
+      "description": "ES Node 2",
+      "id": "9309873766428965105",
+      "kind": "compute#instance",
+      "machineType": "n1-standard-1",
+      "name": "test2",
+      "networkInterfaces": [
+        {
+          "accessConfigs": [
+            {
+              "kind": "compute#accessConfig",
+              "name": "External NAT",
+              "natIP": "104.155.13.147",
+              "type": "ONE_TO_ONE_NAT"
+            }
+          ],
+          "name": "nic0",
+          "network": "default",
+          "networkIP": "10.240.79.60"
+        }
+      ],
+      "status": "RUNNING",
+      "tags": {
+        "fingerprint": "xA6QJb-rGtg=",
+        "items": [
+          "elasticsearch",
+          "dev"
+        ]
+      },
+      "zone": "europe-west1-b"
+    }
+  ]
+}