Fix disabling of healthchecks
When a specific healthcheck is disabled, the HTTP status
code should be 200.
Make sure that disabled tests are checked during integration
tests.
Bug: Issue 10574
Change-Id: Ia4e46e49873279f63fcbab925f49b68b70d230a6
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
index b92588b..4e4fab1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
@@ -60,7 +60,7 @@
}
@VisibleForTesting
- public HealthCheckConfig(String configText) {
+ HealthCheckConfig(String configText) {
config = new Config();
if (!Strings.isNullOrEmpty(configText)) {
try {
@@ -73,6 +73,11 @@
allUsersName = new AllUsersName("All-Users");
}
+ @VisibleForTesting
+ void fromText(String configText) throws ConfigInvalidException {
+ config.fromText(configText);
+ }
+
public long getTimeout() {
return getTimeout(null);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
index 26a2042..c508222 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/api/HealthCheckStatusEndpoint.java
@@ -51,7 +51,7 @@
.filter(
res ->
res instanceof HealthCheck.StatusSummary
- && ((HealthCheck.StatusSummary) res).result != HealthCheck.Result.PASSED)
+ && ((HealthCheck.StatusSummary) res).isFailure())
.findFirst()
.isPresent()) {
return HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
index e8aeb95..a6eae79 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AbstractHealthCheck.java
@@ -29,14 +29,14 @@
private final String name;
private final ListeningExecutorService executor;
protected StatusSummary latestStatus;
- protected boolean enabled;
+ protected HealthCheckConfig config;
protected AbstractHealthCheck(
ListeningExecutorService executor, HealthCheckConfig config, String name) {
this.executor = executor;
this.name = name;
this.timeout = config.getTimeout(name);
- this.enabled = config.healthCheckEnabled(name);
+ this.config = config;
this.latestStatus = StatusSummary.INITIAL_STATUS;
}
@@ -47,6 +47,7 @@
@Override
public StatusSummary run() {
+ boolean enabled = config.healthCheckEnabled(name);
final long ts = System.currentTimeMillis();
ListenableFuture<StatusSummary> resultFuture =
executor.submit(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
index 858e142..de571d1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/HealthCheck.java
@@ -39,7 +39,7 @@
public final long ts;
public final long elapsed;
public static final Set<Result> failingResults =
- new HashSet(Arrays.asList(Result.FAILED, Result.TIMEOUT));
+ new HashSet<>(Arrays.asList(Result.FAILED, Result.TIMEOUT));
public StatusSummary(Result result, long ts, long elapsed) {
this.result = result;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
index 9f481ff..b244b65 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -16,6 +16,7 @@
import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.AUTH;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.JGIT;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES;
import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.REVIEWDB;
@@ -24,12 +25,11 @@
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.TestPlugin;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gson.Gson;
import com.google.gson.JsonObject;
-import com.google.inject.AbstractModule;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames;
import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
import org.junit.Before;
import org.junit.Test;
@@ -44,17 +44,7 @@
public void setUp() throws Exception {
super.setUp();
- config =
- server
- .getTestInjector()
- .createChildInjector(
- new AbstractModule() {
- @Override
- protected void configure() {
- bind(String.class).annotatedWith(PluginName.class).toInstance("healthcheck");
- }
- })
- .getInstance(HealthCheckConfig.class);
+ config = plugin.getSysInjector().getInstance(HealthCheckConfig.class);
int numChanges = config.getLimit(HealthCheckNames.QUERYCHANGES);
for (int i = 0; i < numChanges; i++) {
createChange("refs/for/master");
@@ -75,21 +65,52 @@
@Test
public void shouldReturnReviewDbCheck() throws Exception {
RestResponse resp = getHealthCheckStatus();
+
resp.assertOK();
+ assertCheckResult(getResponseJson(resp), REVIEWDB, "passed");
+ }
- JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
+ @Test
+ public void shouldReturnReviewDbCheckAsDisabled() throws Exception {
+ disableCheck(REVIEWDB);
+ RestResponse resp = getHealthCheckStatus();
- assertCheckResult(respPayload, REVIEWDB, "passed");
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), REVIEWDB, "disabled");
}
@Test
public void shouldReturnJGitCheck() throws Exception {
RestResponse resp = getHealthCheckStatus();
+
resp.assertOK();
+ assertCheckResult(getResponseJson(resp), JGIT, "passed");
+ }
- JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
+ @Test
+ public void shouldReturnJGitCheckAsDisabled() throws Exception {
+ disableCheck(JGIT);
+ RestResponse resp = getHealthCheckStatus();
- assertCheckResult(respPayload, JGIT, "passed");
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), JGIT, "disabled");
+ }
+
+ @Test
+ public void shouldReturnAuthCheck() throws Exception {
+ RestResponse resp = getHealthCheckStatus();
+
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), AUTH, "passed");
+ }
+
+ @Test
+ public void shouldReturnAuthCheckAsDisabled() throws Exception {
+ disableCheck(AUTH);
+ RestResponse resp = getHealthCheckStatus();
+
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), AUTH, "disabled");
}
@Test
@@ -98,9 +119,16 @@
RestResponse resp = getHealthCheckStatus();
resp.assertOK();
- JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
+ assertCheckResult(getResponseJson(resp), QUERYCHANGES, "passed");
+ }
- assertCheckResult(respPayload, QUERYCHANGES, "passed");
+ @Test
+ public void shouldReturnQueryChangesCheckAsDisabled() throws Exception {
+ disableCheck(QUERYCHANGES);
+ RestResponse resp = getHealthCheckStatus();
+
+ resp.assertOK();
+ assertCheckResult(getResponseJson(resp), QUERYCHANGES, "disabled");
}
@Test
@@ -110,9 +138,7 @@
RestResponse resp = getHealthCheckStatus();
resp.assertOK();
- JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
-
- assertCheckResult(respPayload, QUERYCHANGES, "passed");
+ assertCheckResult(getResponseJson(resp), QUERYCHANGES, "passed");
}
private RestResponse getHealthCheckStatus() throws IOException {
@@ -125,4 +151,13 @@
assertThat(reviewDbStatus.has("result")).isTrue();
assertThat(reviewDbStatus.get("result").getAsString()).isEqualTo(result);
}
+
+ private void disableCheck(String check) throws ConfigInvalidException {
+ config.fromText(String.format("[healthcheck \"%s\"]\n" + "enabled = false", check));
+ }
+
+ private JsonObject getResponseJson(RestResponse resp) throws IOException {
+ JsonObject respPayload = gson.fromJson(resp.getReader(), JsonObject.class);
+ return respPayload;
+ }
}
