From 156afa8b237294dc7be01b766d13f493f399f6f3 Mon Sep 17 00:00:00 2001 From: Andreas Born Date: Mon, 30 Mar 2020 22:00:44 +0200 Subject: [PATCH 1/4] Remove DiskSpaceHealthIndicator path checks By removing these checks the configuration can be successfully instantiated even if the the path for the health check does not exist (or is not readable) at that point. This is for example useful when the application intends to create the actual configured path itself during startup. --- .../system/DiskSpaceHealthIndicatorProperties.java | 2 -- ...DiskSpaceHealthContributorAutoConfigurationTests.java | 9 +++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java index 75a79304f39f..b740ea79a339 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthIndicatorProperties.java @@ -48,8 +48,6 @@ public File getPath() { } public void setPath(File path) { - Assert.isTrue(path.exists(), () -> "Path '" + path + "' does not exist"); - Assert.isTrue(path.canRead(), () -> "Path '" + path + "' cannot be read"); this.path = path; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java index 88cda1c89b91..6b49b53601ce 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/system/DiskSpaceHealthContributorAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure.system; +import java.util.UUID; + import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.health.HealthContributorAutoConfiguration; @@ -58,6 +60,13 @@ void thresholdCanBeCustomized() { }); } + @Test + void pathIsNotRequiredToExist() { + String randomPath = "IDoNOTeXiST" + UUID.randomUUID().toString(); + this.contextRunner.withPropertyValues("management.health.diskspace.path=" + randomPath) + .run((context) -> assertThat(context).hasSingleBean(DiskSpaceHealthIndicator.class)); + } + @Test void runWhenDisabledShouldNotCreateIndicator() { this.contextRunner.withPropertyValues("management.health.diskspace.enabled:false") From ef4449b59d9c8fdd7a0acd5412f28bbad8008823 Mon Sep 17 00:00:00 2001 From: Andreas Born Date: Mon, 30 Mar 2020 22:07:49 +0200 Subject: [PATCH 2/4] Add exists & canRead/Write/Execute details These new details are meant to provide pointers especially when the DiskSpaceHealthIndicator reports as DOWN. --- .../boot/actuate/system/DiskSpaceHealthIndicator.java | 4 +++- .../actuate/system/DiskSpaceHealthIndicatorTests.java | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java index ae1f38f38bc4..43401fc65581 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java @@ -68,7 +68,9 @@ protected void doHealthCheck(Health.Builder builder) throws Exception { builder.down(); } builder.withDetail("total", this.path.getTotalSpace()).withDetail("free", diskFreeInBytes) - .withDetail("threshold", this.threshold.toBytes()); + .withDetail("threshold", this.threshold.toBytes()).withDetail("exists", this.path.exists()) + .withDetail("canRead", this.path.canRead()).withDetail("canWrite", this.path.canWrite()) + .withDetail("canExecute", this.path.canExecute()); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java index 44b0b0c9fa9f..6046faa2c5eb 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorTests.java @@ -53,6 +53,8 @@ void setUp() { MockitoAnnotations.initMocks(this); given(this.fileMock.exists()).willReturn(true); given(this.fileMock.canRead()).willReturn(true); + given(this.fileMock.canWrite()).willReturn(true); + given(this.fileMock.canExecute()).willReturn(true); this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, THRESHOLD); } @@ -66,6 +68,10 @@ void diskSpaceIsUp() { assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); + assertThat(health.getDetails().get("exists")).isEqualTo(true); + assertThat(health.getDetails().get("canRead")).isEqualTo(true); + assertThat(health.getDetails().get("canWrite")).isEqualTo(true); + assertThat(health.getDetails().get("canExecute")).isEqualTo(true); } @Test @@ -78,6 +84,10 @@ void diskSpaceIsDown() { assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); + assertThat(health.getDetails().get("exists")).isEqualTo(true); + assertThat(health.getDetails().get("canRead")).isEqualTo(true); + assertThat(health.getDetails().get("canWrite")).isEqualTo(true); + assertThat(health.getDetails().get("canExecute")).isEqualTo(true); } } From 8b70460e2b5894f3005c2478347376d918714bd2 Mon Sep 17 00:00:00 2001 From: Andreas Born Date: Mon, 30 Mar 2020 22:18:12 +0200 Subject: [PATCH 3/4] Add tests for DiskSpaceHealthIndicator path --- .../DiskSpaceHealthIndicatorPathTests.java | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java new file mode 100644 index 000000000000..ff4ec8e57fbc --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java @@ -0,0 +1,90 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.system; + +import java.io.File; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import org.springframework.boot.actuate.health.Health; +import org.springframework.boot.actuate.health.HealthIndicator; +import org.springframework.boot.actuate.health.Status; +import org.springframework.util.unit.DataSize; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; + +/** + * Tests for the {@link DiskSpaceHealthIndicator} {@code path} parameter. + * + * @author Andreas Born + */ +class DiskSpaceHealthIndicatorPathTests { + + private static final DataSize THRESHOLD = DataSize.ofKilobytes(1); + + private static final DataSize TOTAL_SPACE = DataSize.ofKilobytes(10); + + @Mock + private File fileMock; + + private HealthIndicator healthIndicator; + + @BeforeEach + void setUp() { + MockitoAnnotations.initMocks(this); + given(this.fileMock.exists()).willReturn(false); + given(this.fileMock.canRead()).willReturn(false); + given(this.fileMock.canWrite()).willReturn(false); + given(this.fileMock.canExecute()).willReturn(false); + this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, THRESHOLD); + } + + @Test + void diskSpaceIsDown() { + Health health = this.healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); + assertThat(health.getDetails().get("free")).isEqualTo(0L); + assertThat(health.getDetails().get("total")).isEqualTo(0L); + assertThat(health.getDetails().get("exists")).isEqualTo(false); + assertThat(health.getDetails().get("canRead")).isEqualTo(false); + assertThat(health.getDetails().get("canWrite")).isEqualTo(false); + assertThat(health.getDetails().get("canExecute")).isEqualTo(false); + } + + @Test + void diskSpaceIsUpWhenPathOnlyExists() { + long freeSpace = THRESHOLD.toBytes() + 10; + given(this.fileMock.getUsableSpace()).willReturn(freeSpace); + given(this.fileMock.getTotalSpace()).willReturn(TOTAL_SPACE.toBytes()); + given(this.fileMock.exists()).willReturn(true); + Health health = this.healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.UP); + assertThat(health.getDetails().get("threshold")).isEqualTo(THRESHOLD.toBytes()); + assertThat(health.getDetails().get("free")).isEqualTo(freeSpace); + assertThat(health.getDetails().get("total")).isEqualTo(TOTAL_SPACE.toBytes()); + assertThat(health.getDetails().get("exists")).isEqualTo(true); + assertThat(health.getDetails().get("canRead")).isEqualTo(false); + assertThat(health.getDetails().get("canWrite")).isEqualTo(false); + assertThat(health.getDetails().get("canExecute")).isEqualTo(false); + } + +} From 1123957224d95b4fed0b38801f137ae49ea4e286 Mon Sep 17 00:00:00 2001 From: Andreas Born Date: Mon, 30 Mar 2020 22:21:33 +0200 Subject: [PATCH 4/4] Indicate DOWN even if the threshold is zero --- .../actuate/system/DiskSpaceHealthIndicator.java | 4 +++- .../DiskSpaceHealthIndicatorPathTests.java | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java index 43401fc65581..50166c32a72d 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicator.java @@ -59,7 +59,9 @@ public DiskSpaceHealthIndicator(File path, DataSize threshold) { @Override protected void doHealthCheck(Health.Builder builder) throws Exception { long diskFreeInBytes = this.path.getUsableSpace(); - if (diskFreeInBytes >= this.threshold.toBytes()) { + // return value of 0L means "the abstract pathname does not name a + // partition" which for our purposes means it's not usable i.e DOWN + if (diskFreeInBytes >= this.threshold.toBytes() && diskFreeInBytes != 0L) { builder.up(); } else { diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java index ff4ec8e57fbc..44bb3b9f4837 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/system/DiskSpaceHealthIndicatorPathTests.java @@ -40,6 +40,8 @@ class DiskSpaceHealthIndicatorPathTests { private static final DataSize THRESHOLD = DataSize.ofKilobytes(1); + private static final DataSize ZERO_THRESHOLD = DataSize.ofBytes(0); + private static final DataSize TOTAL_SPACE = DataSize.ofKilobytes(10); @Mock @@ -70,6 +72,20 @@ void diskSpaceIsDown() { assertThat(health.getDetails().get("canExecute")).isEqualTo(false); } + @Test + void diskSpaceIsDownWhenThresholdIsZero() { + this.healthIndicator = new DiskSpaceHealthIndicator(this.fileMock, ZERO_THRESHOLD); + Health health = this.healthIndicator.health(); + assertThat(health.getStatus()).isEqualTo(Status.DOWN); + assertThat(health.getDetails().get("threshold")).isEqualTo(ZERO_THRESHOLD.toBytes()); + assertThat(health.getDetails().get("free")).isEqualTo(0L); + assertThat(health.getDetails().get("total")).isEqualTo(0L); + assertThat(health.getDetails().get("exists")).isEqualTo(false); + assertThat(health.getDetails().get("canRead")).isEqualTo(false); + assertThat(health.getDetails().get("canWrite")).isEqualTo(false); + assertThat(health.getDetails().get("canExecute")).isEqualTo(false); + } + @Test void diskSpaceIsUpWhenPathOnlyExists() { long freeSpace = THRESHOLD.toBytes() + 10;