SONAR-9741 harden ES status check when no node can be contacted

or any other Exception occuring while checking ES cluster status
SonarSource · Sep 13, 2017 · b82b35a · b82b35a
1 parent a444ad1
commit b82b35a
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 20 deletions.
diff --git a/server/sonar-server/src/main/java/org/sonar/server/health/EsStatusCheck.java b/server/sonar-server/src/main/java/org/sonar/server/health/EsStatusCheck.java
@@ -20,11 +20,15 @@
 package org.sonar.server.health;
 
 import org.elasticsearch.cluster.health.ClusterHealthStatus;
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
 import org.sonar.server.es.EsClient;
 
 import static org.sonar.server.health.Health.newHealthCheckBuilder;
 
-public abstract class EsStatusCheck {
+abstract class EsStatusCheck {
+  private static final Logger LOG = Loggers.get(EsStatusCheck.class);
+
   private static final Health YELLOW_HEALTH = newHealthCheckBuilder()
     .setStatus(Health.Status.YELLOW)
     .addCause("Elasticsearch status is YELLOW")
@@ -33,23 +37,33 @@ public abstract class EsStatusCheck {
     .setStatus(Health.Status.RED)
     .addCause("Elasticsearch status is RED")
     .build();
-  protected final EsClient esClient;
+  private static final Health RED_HEALTH_EXCEPTION_OCCURED = newHealthCheckBuilder()
+    .setStatus(Health.Status.RED)
+    .addCause("Elasticsearch status is RED (unavailable)")
+    .build();
+
+  private final EsClient esClient;
 
   EsStatusCheck(EsClient esClient) {
     this.esClient = esClient;
   }
 
   Health checkEsStatus() {
-    ClusterHealthStatus esStatus = esClient.prepareClusterStats().get().getStatus();
-    switch (esStatus) {
-      case GREEN:
-        return Health.GREEN;
-      case YELLOW:
-        return YELLOW_HEALTH;
-      case RED:
-        return RED_HEALTH;
-      default:
-        throw new IllegalArgumentException("Unsupported Elasticsearch status " + esStatus);
+    try {
+      ClusterHealthStatus esStatus = esClient.prepareClusterStats().get().getStatus();
+      switch (esStatus) {
+        case GREEN:
+          return Health.GREEN;
+        case YELLOW:
+          return YELLOW_HEALTH;
+        case RED:
+          return RED_HEALTH;
+        default:
+          throw new IllegalArgumentException("Unsupported Elasticsearch status " + esStatus);
+      }
+    } catch (Exception e) {
+      LOG.error("Failed to query ES status", e);
+      return RED_HEALTH_EXCEPTION_OCCURED;
     }
   }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/health/EsStatusClusterCheckTest.java b/server/sonar-server/src/test/java/org/sonar/server/health/EsStatusClusterCheckTest.java
@@ -25,24 +25,47 @@
 import java.util.stream.IntStream;
 import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.sonar.cluster.health.NodeDetails;
 import org.sonar.cluster.health.NodeHealth;
+import org.sonar.server.es.EsClient;
 import org.sonar.server.es.EsTester;
 
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
 
 public class EsStatusClusterCheckTest {
 
   @Rule
   public EsTester esTester = new EsTester();
 
+  private final Random random = new Random();
   private EsStatusClusterCheck underTest = new EsStatusClusterCheck(esTester.client());
 
+  @Test
+  public void check_ignores_NodeHealth_arg_and_returns_RED_with_cause_if_an_exception_occurs_checking_ES_cluster_status() {
+    Set<NodeHealth> nodeHealths = randomNodeHealths();
+    EsClient esClient = Mockito.mock(EsClient.class);
+    when(esClient.prepareClusterStats()).thenThrow(new RuntimeException("Faking an exception occuring while using the EsClient"));
+
+    Health health = new EsStatusClusterCheck(esClient).check(nodeHealths);
+
+    assertThat(health.getStatus()).isEqualTo(Health.Status.RED);
+    assertThat(health.getCauses()).containsOnly("Elasticsearch status is RED (unavailable)");
+  }
+
   @Test
   public void check_ignores_NodeHealth_arg_and_returns_GREEN_without_cause_if_ES_cluster_status_is_GREEN() {
-    Random random = new Random();
-    Set<NodeHealth> nodeHealths = IntStream.range(0, random.nextInt(20))
+    Set<NodeHealth> nodeHealths = randomNodeHealths();
+
+    Health health = underTest.check(nodeHealths);
+
+    assertThat(health).isEqualTo(Health.GREEN);
+  }
+
+  private Set<NodeHealth> randomNodeHealths() {
+    return IntStream.range(0, random.nextInt(20))
       .mapToObj(i -> NodeHealth.newNodeHealthBuilder()
         .setStatus(NodeHealth.Status.values()[random.nextInt(NodeHealth.Status.values().length)])
         .setDetails(NodeDetails.newNodeDetailsBuilder()
@@ -54,9 +77,6 @@ public void check_ignores_NodeHealth_arg_and_returns_GREEN_without_cause_if_ES_c
           .build())
         .build())
       .collect(Collectors.toSet());
-    Health health = underTest.check(nodeHealths);
-
-    assertThat(health).isEqualTo(Health.GREEN);
   }
 
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/health/EsStatusNodeCheckTest.java b/server/sonar-server/src/test/java/org/sonar/server/health/EsStatusNodeCheckTest.java
@@ -21,9 +21,12 @@
 
 import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.Mockito;
+import org.sonar.server.es.EsClient;
 import org.sonar.server.es.EsTester;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.when;
 
 public class EsStatusNodeCheckTest {
 
@@ -32,6 +35,17 @@ public class EsStatusNodeCheckTest {
 
   private EsStatusNodeCheck underTest = new EsStatusNodeCheck(esTester.client());
 
+  @Test
+  public void check_ignores_NodeHealth_arg_and_returns_RED_with_cause_if_an_exception_occurs_checking_ES_cluster_status() {
+    EsClient esClient = Mockito.mock(EsClient.class);
+    when(esClient.prepareClusterStats()).thenThrow(new RuntimeException("Faking an exception occuring while using the EsClient"));
+
+    Health health = new EsStatusNodeCheck(esClient).check();
+
+    assertThat(health.getStatus()).isEqualTo(Health.Status.RED);
+    assertThat(health.getCauses()).containsOnly("Elasticsearch status is RED (unavailable)");
+  }
+
   @Test
   public void check_returns_GREEN_without_cause_if_ES_cluster_status_is_GREEN() {
     Health health = underTest.check();

diff --git a/tests/src/test/java/org/sonarqube/tests/cluster/ClusterTest.java b/tests/src/test/java/org/sonarqube/tests/cluster/ClusterTest.java
@@ -30,7 +30,6 @@
 import java.util.stream.IntStream;
 import org.apache.commons.io.FileUtils;
 import org.junit.BeforeClass;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.DisableOnDebug;
@@ -249,7 +248,6 @@ public void restarting_all_application_nodes_elects_a_new_startup_leader() throw
   }
 
   @Test
-  @Ignore("WS api/system/health returns 500")
   public void health_becomes_RED_when_all_search_nodes_go_down() throws Exception {
     try (Cluster cluster = newCluster(2, 1)) {
       cluster.getNodes().forEach(Node::start);
@@ -261,7 +259,7 @@ public void health_becomes_RED_when_all_search_nodes_go_down() throws Exception
 
       app.waitForHealth(WsSystem.Health.RED);
       assertThat(app.getHealth().get().getCausesList()).extracting(WsSystem.Cause::getMessage)
-        .contains("Elasticsearch status is RED");
+        .contains("Elasticsearch status is RED (unavailable)");
     }
   }