Disable git space check by default Since the healthcheck is used also in the high-availability config using a shared filesystem, this check should be disabled by default. System administrators should evaluate whether to enable this check based on their system architecture and the plugins in use. Change-Id: I159b4f6ca1b78047ef1669fd1cb14b987a5db3e9
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 c6b5b43..6f98b21 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java +++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfig.java
@@ -17,11 +17,13 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS; import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.CHANGES_INDEX; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.GITSPACE; import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.gerrit.entities.Project; import com.google.gerrit.server.config.AllProjectsName; @@ -50,7 +52,6 @@ private static final String USERNAME_DEFAULT = "healthcheck"; private static final String PASSWORD_DEFAULT = ""; private static final String FAIL_FILE_FLAG_DEFAULT = "data/healthcheck/fail"; - private static final boolean HEALTH_CHECK_ENABLED_DEFAULT = true; private final AllProjectsName allProjectsName; private final AllUsersName allUsersName; @@ -60,6 +61,9 @@ private static final Set<String> HEALTH_CHECK_DISABLED_FOR_REPLICAS = ImmutableSet.of(CHANGES_INDEX, QUERYCHANGES); + private static final ImmutableList<String> HEALTH_CHECKS_DISABLED_BY_DEFAULT = + ImmutableList.of(GITSPACE); + @Inject public HealthCheckConfig( PluginConfigFactory configFactory, @@ -150,8 +154,13 @@ if (isReplica && HEALTH_CHECK_DISABLED_FOR_REPLICAS.contains(healthCheckName)) { return false; } - return config.getBoolean( - HEALTHCHECK, checkNotNull(healthCheckName), "enabled", HEALTH_CHECK_ENABLED_DEFAULT); + + String notNullHealthcheckName = checkNotNull(healthCheckName); + boolean defaultValue = + HEALTH_CHECKS_DISABLED_BY_DEFAULT.stream() + .noneMatch(hc -> hc.equals(notNullHealthcheckName)); + + return config.getBoolean(HEALTHCHECK, notNullHealthcheckName, "enabled", defaultValue); } public String[] getListOfBlockedThreadsThresholds() {
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md index c2c7dd3..bf07650 100644 --- a/src/main/resources/Documentation/config.md +++ b/src/main/resources/Documentation/config.md
@@ -49,18 +49,19 @@ The following check names are available: -- `querychanges`: check the ability to query changes -- `jgit` : check connectivity to the filesystem and ability to open a JGit ref and object -- `gitspace`: Checks that sufficient disk space is available on the volume where Git repositories are stored -- `projectslist` : check the ability to list projects with their descriptions -- `auth`: check the ability to authenticate with username and password -- `activeworkers`: check the number of active worker threads and the ability to create a new one -- `httpactiveworkers`: check the number of active HTTP worker threads and the ability +- `querychanges`: check the ability to query changes. `Enabled` by default. +- `jgit` : check connectivity to the filesystem and ability to open a JGit ref and object. `Enabled` by default. +- `gitspace`: Checks that sufficient disk space is available on the volume where Git repositories are stored. `Disabled` + by default. +- `projectslist` : check the ability to list projects with their descriptions. `Enabled` by default. +- `auth`: check the ability to authenticate with username and password. `Enabled` by default. +- `activeworkers`: check the number of active worker threads and the ability to create a new one. `Enabled` by default. +- `httpactiveworkers`: check the number of active HTTP worker threads and the ability. `Enabled` by default. to create a new one -- `deadlock` : check if Java deadlocks are reported by the JVM -- `blockedthreads` : check the number of blocked threads +- `deadlock` : check if Java deadlocks are reported by the JVM. `Enabled` by default. +- `blockedthreads` : check the number of blocked threads. `Enabled` by default. - `changesindex` : check if the lucene based changes indexes (open and closed) are operable - (examines index lock files) + (examines index lock files). `Enabled` by default. Each check name can be disabled by setting the `enabled` parameter to **false**, by default this parameter is set to **true**
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfigTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfigTest.java index 264db94..48b3e85 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfigTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckConfigTest.java
@@ -16,6 +16,16 @@ import static com.google.common.truth.Truth.assertThat; import static com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig.DEFAULT_CONFIG; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.ACTIVEWORKERS; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.AUTH; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.BLOCKEDTHREADS; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.CHANGES_INDEX; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.DEADLOCK; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.GITSPACE; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.HTTPACTIVEWORKERS; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.JGIT; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.PROJECTSLIST; +import static com.googlesource.gerrit.plugins.healthcheck.check.HealthCheckNames.QUERYCHANGES; import org.junit.Test; @@ -85,4 +95,34 @@ assertThat(config.healthCheckEnabled("barCheck")).isEqualTo(true); assertThat(config.healthCheckEnabled("bazCheck")).isEqualTo(true); } + + @Test + public void shouldHonourDefaultEnabledValue() { + HealthCheckConfig config = new HealthCheckConfig("[healthcheck \"fooCheck\"]"); + + assertThat(config.healthCheckEnabled(GITSPACE)).isEqualTo(false); + + assertThat(config.healthCheckEnabled(JGIT)).isEqualTo(true); + assertThat(config.healthCheckEnabled(PROJECTSLIST)).isEqualTo(true); + assertThat(config.healthCheckEnabled(QUERYCHANGES)).isEqualTo(true); + assertThat(config.healthCheckEnabled(AUTH)).isEqualTo(true); + assertThat(config.healthCheckEnabled(ACTIVEWORKERS)).isEqualTo(true); + assertThat(config.healthCheckEnabled(HTTPACTIVEWORKERS)).isEqualTo(true); + assertThat(config.healthCheckEnabled(DEADLOCK)).isEqualTo(true); + assertThat(config.healthCheckEnabled(BLOCKEDTHREADS)).isEqualTo(true); + assertThat(config.healthCheckEnabled(CHANGES_INDEX)).isEqualTo(true); + } + + @Test + public void shouldOverrideDisabledByDefault() { + HealthCheckConfig config = + new HealthCheckConfig( + """ + [healthcheck "%s"] + enabled=true + """ + .formatted(GITSPACE)); + + assertThat(config.healthCheckEnabled(GITSPACE)).isEqualTo(true); + } }