From dc393c83d1874d41f92d9297e657552a56bc243d Mon Sep 17 00:00:00 2001
From: David Turner <david.turner@elastic.co>
Date: Wed, 13 Jun 2018 11:25:26 +0100
Subject: [PATCH] Ignore numeric shard count if waiting for ALL (#31265)

Today, if GET /_cluster/health?wait_for_active_shards=all does not immediately
succeed then it throws an exception due to an erroneous and unnecessary call to
ActiveShardCount#enoughShardsActive(). This commit fixes this logic.

Fixes #31151
---
 .../health/TransportClusterHealthAction.java      | 10 +++++-----
 .../health/TransportClusterHealthActionTests.java | 15 +++++++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java
index bd5912b9853ec..c67e2da2b9237 100644
--- a/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java
+++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java
@@ -234,11 +234,11 @@ static int prepareResponse(final ClusterHealthRequest request, final ClusterHeal
             ActiveShardCount waitForActiveShards = request.waitForActiveShards();
             assert waitForActiveShards.equals(ActiveShardCount.DEFAULT) == false :
                 "waitForActiveShards must not be DEFAULT on the request object, instead it should be NONE";
-            if (waitForActiveShards.equals(ActiveShardCount.ALL)
-                    && response.getUnassignedShards() == 0
-                    && response.getInitializingShards() == 0) {
-                // if we are waiting for all shards to be active, then the num of unassigned and num of initializing shards must be 0
-                waitForCounter++;
+            if (waitForActiveShards.equals(ActiveShardCount.ALL)) {
+                if (response.getUnassignedShards() == 0 && response.getInitializingShards() == 0) {
+                    // if we are waiting for all shards to be active, then the num of unassigned and num of initializing shards must be 0
+                    waitForCounter++;
+                }
             } else if (waitForActiveShards.enoughShardsActive(response.getActiveShards())) {
                 // there are enough active shards to meet the requirements of the request
                 waitForCounter++;
diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java
index cac5bed4033ac..8601687b04a23 100644
--- a/server/src/test/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java
+++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.health;
 
 import org.elasticsearch.Version;
+import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -61,6 +62,20 @@ public void testWaitForInitializingShards() throws Exception {
         assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
     }
 
+    public void testWaitForAllShards() {
+        final String[] indices = {"test"};
+        final ClusterHealthRequest request = new ClusterHealthRequest();
+        request.waitForActiveShards(ActiveShardCount.ALL);
+
+        ClusterState clusterState = randomClusterStateWithInitializingShards("test", 1);
+        ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
+        assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
+
+        clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
+        response = new ClusterHealthResponse("", indices, clusterState);
+        assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
+    }
+
     ClusterState randomClusterStateWithInitializingShards(String index, final int initializingShards) {
         final IndexMetaData indexMetaData = IndexMetaData
             .builder(index)