From cd441130bd7a46da412baf327d6040d638fe23a5 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Thu, 16 May 2024 17:37:44 -0500 Subject: [PATCH] Warn when image building workspace directories cannot be deleted When the `buildWorkspace` location in the `spring-boot:build-image` Maven goal or `bootBuildImage` Gradle task is configured to use a local bind source, the location is passed to the CNB lifecycle without further processing by Spring Boot. The lifecycle is in control of creating any files in the specified location. Spring Boot tries to remove the directories at the specified location after an image is successfully created, but should not fail the image build if the lifecycle has created files or directories with permissions that keep them from being deleted successfully. Fixes gh-40760 --- .../buildpack/platform/build/AbstractBuildLog.java | 11 +++++++++++ .../boot/buildpack/platform/build/BuildLog.java | 7 +++++++ .../boot/buildpack/platform/build/Lifecycle.java | 8 ++++---- .../bundling/BootBuildImageIntegrationTests.java | 7 ++----- .../springframework/boot/maven/BuildImageTests.java | 7 ++----- 5 files changed, 26 insertions(+), 14 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/AbstractBuildLog.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/AbstractBuildLog.java index b870d30f9f6..7e4f26b62b5 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/AbstractBuildLog.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/AbstractBuildLog.java @@ -102,6 +102,17 @@ public abstract class AbstractBuildLog implements BuildLog { log(); } + @Override + public void failedCleaningWorkDir(Cache cache, Exception exception) { + StringBuilder message = new StringBuilder("Warning: Working location " + cache + " could not be cleaned"); + if (exception != null) { + message.append(": ").append(exception.getMessage()); + } + log(); + log(message.toString()); + log(); + } + private String getDigest(Image image) { List digests = image.getDigests(); return (digests.isEmpty() ? "" : digests.get(0)); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/BuildLog.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/BuildLog.java index e84054773f4..868a81a2300 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/BuildLog.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/BuildLog.java @@ -114,6 +114,13 @@ public interface BuildLog { */ void taggedImage(ImageReference tag); + /** + * Log that a cache cleanup step was not completed successfully. + * @param cache the cache + * @param exception any exception that caused the failure + */ + void failedCleaningWorkDir(Cache cache, Exception exception); + /** * Factory method that returns a {@link BuildLog} the outputs to {@link System#out}. * @return a build log instance that logs to system out diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Lifecycle.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Lifecycle.java index 781d0aa7782..80322a0230a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Lifecycle.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Lifecycle.java @@ -311,7 +311,7 @@ class Lifecycle implements Closeable { deleteVolume(cache.getVolume().getVolumeName()); } if (cache.getBind() != null) { - deleteBind(cache.getBind().getSource()); + deleteBind(cache.getBind()); } } @@ -319,12 +319,12 @@ class Lifecycle implements Closeable { this.docker.volume().delete(name, true); } - private void deleteBind(String source) { + private void deleteBind(Cache.Bind bind) { try { - FileSystemUtils.deleteRecursively(Path.of(source)); + FileSystemUtils.deleteRecursively(Path.of(bind.getSource())); } catch (IOException ex) { - throw new IllegalStateException("Error cleaning bind mount directory '" + source + "'", ex); + this.log.failedCleaningWorkDir(bind, ex); } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootBuildImageIntegrationTests.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootBuildImageIntegrationTests.java index 5a0706ff108..b20924ad7d6 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootBuildImageIntegrationTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootBuildImageIntegrationTests.java @@ -36,7 +36,6 @@ import org.apache.commons.compress.compressors.gzip.GzipCompressorOutputStream; import org.apache.commons.compress.utils.IOUtils; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.TaskOutcome; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; @@ -301,10 +300,8 @@ class BootBuildImageIntegrationTests { } @TestTemplate - @EnabledOnOs(value = OS.LINUX, - disabledReason = "Works with Docker Engine on Linux but is not reliable with " - + "Docker Desktop on other OSs") - @Disabled("gh-40760") + @EnabledOnOs(value = OS.LINUX, disabledReason = "Works with Docker Engine on Linux but is not reliable with " + + "Docker Desktop on other OSs") void buildsImageWithBindCaches() throws IOException { writeMainClass(); writeLongNameResource(); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java index a5e53a286b7..b4d0fffeb5b 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java @@ -25,7 +25,6 @@ import java.time.OffsetDateTime; import java.util.Random; import java.util.stream.IntStream; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.TestTemplate; import org.junit.jupiter.api.condition.EnabledOnOs; import org.junit.jupiter.api.condition.OS; @@ -402,10 +401,8 @@ class BuildImageTests extends AbstractArchiveIntegrationTests { } @TestTemplate - @EnabledOnOs(value = OS.LINUX, - disabledReason = "Works with Docker Engine on Linux but is not reliable with " - + "Docker Desktop on other OSs") - @Disabled("gh-40760") + @EnabledOnOs(value = OS.LINUX, disabledReason = "Works with Docker Engine on Linux but is not reliable with " + + "Docker Desktop on other OSs") void whenBuildImageIsInvokedWithBindCaches(MavenBuild mavenBuild) { String testBuildId = randomString(); mavenBuild.project("build-image-bind-caches")