Stop image building on error from builder

Previously, the image builder used by the build tool plugins ignored
errors from lifecycle phases and continued with subsequent phases.

This commit inspects the status of the builder container after each
lifecycle phase and aborts the image building process if the exit
status of the container after any phase is non-zero.

Fixes #19949

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/BuilderException.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2012-2020 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.buildpack.platform.build;
+
+/**
+ * Exception thrown to indicate a Builder error.
+ *
+ * @author Scott Frederick
+ * @since 2.3.0
+ */
+public class BuilderException extends RuntimeException {
+
+	private final String operation;
+
+	private final int statusCode;
+
+	BuilderException(String operation, int statusCode) {
+		super(buildMessage(operation, statusCode));
+		this.operation = operation;
+		this.statusCode = statusCode;
+	}
+
+	/**
+	 * Return the Builder operation that failed.
+	 * @return the operation description
+	 */
+	public String getOperation() {
+		return this.operation;
+	}
+
+	/**
+	 * Return the status code returned from a Builder operation.
+	 * @return the statusCode the status code
+	 */
+	public int getStatusCode() {
+		return this.statusCode;
+	}
+
+	private static String buildMessage(String operation, int statusCode) {
+		StringBuilder message = new StringBuilder("Builder");
+		if (operation != null && !operation.isEmpty()) {
+			message.append(" lifecycle '").append(operation).append("'");
+		}
+		message.append(" failed with status code ").append(statusCode);
+		return message.toString();
+	}
+
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Lifecycle.java

@@ -25,6 +25,7 @@ import org.springframework.boot.buildpack.platform.docker.LogUpdateEvent;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerConfig;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerContent;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerReference;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerStatus;
 import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
 import org.springframework.boot.buildpack.platform.docker.type.VolumeName;
 import org.springframework.boot.buildpack.platform.io.TarArchive;
@@ -202,6 +203,11 @@ class Lifecycle implements Closeable {
 		try {
 			this.docker.container().start(reference);
 			this.docker.container().logs(reference, logConsumer::accept);
+
+			ContainerStatus status = this.docker.container().wait(reference);
+			if (status.getStatusCode() != 0) {
+				throw new BuilderException(phase.getName(), status.getStatusCode());
+			}
 		}
 		finally {
 			this.docker.container().remove(reference, true);

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/DockerApi.java

@@ -30,6 +30,7 @@ import org.springframework.boot.buildpack.platform.docker.Http.Response;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerConfig;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerContent;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerReference;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerStatus;
 import org.springframework.boot.buildpack.platform.docker.type.Image;
 import org.springframework.boot.buildpack.platform.docker.type.ImageArchive;
 import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
@@ -270,6 +271,19 @@ public class DockerApi {
 			}
 		}
 
+		/**
+		 * Wait for a container to stop and retrieve the status.
+		 * @param reference the container reference
+		 * @return a {@link ContainerStatus} indicating the exit status of the container
+		 * @throws IOException on IO error
+		 */
+		public ContainerStatus wait(ContainerReference reference) throws IOException {
+			Assert.notNull(reference, "Reference must not be null");
+			URI uri = buildUrl("/containers/" + reference + "/wait");
+			Response response = http().post(uri);
+			return ContainerStatus.of(response.getContent());
+		}
+
 		/**
 		 * Remove a specific container.
 		 * @param reference the container to remove

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/type/ContainerStatus.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2012-2020 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.buildpack.platform.docker.type;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.lang.invoke.MethodHandles;
+
+import com.fasterxml.jackson.databind.JsonNode;
+
+import org.springframework.boot.buildpack.platform.json.MappedObject;
+
+/**
+ * Status details returned from {@code Docker container wait}.
+ *
+ * @author Scott Frederick
+ * @since 2.3.0
+ */
+public class ContainerStatus extends MappedObject {
+
+	private final int statusCode;
+
+	private final String waitingErrorMessage;
+
+	ContainerStatus(int statusCode, String waitingErrorMessage) {
+		super(null, null);
+		this.statusCode = statusCode;
+		this.waitingErrorMessage = waitingErrorMessage;
+	}
+
+	ContainerStatus(JsonNode node) {
+		super(node, MethodHandles.lookup());
+		this.statusCode = valueAt("/StatusCode", Integer.class);
+		this.waitingErrorMessage = valueAt("/Error/Message", String.class);
+	}
+
+	/**
+	 * Return the container exit status code.
+	 * @return the exit status code
+	 */
+	public int getStatusCode() {
+		return this.statusCode;
+	}
+
+	/**
+	 * Return a message indicating an error waiting for a container to stop.
+	 * @return the waiting error message
+	 */
+	public String getWaitingErrorMessage() {
+		return this.waitingErrorMessage;
+	}
+
+	public static ContainerStatus of(InputStream content) throws IOException {
+		return of(content, ContainerStatus::new);
+	}
+
+	public static ContainerStatus of(int statusCode, String errorMessage) {
+		return new ContainerStatus(statusCode, errorMessage);
+	}
+
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/BuilderExceptionTests.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2012-2020 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.buildpack.platform.build;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link BuilderException}.
+ *
+ * @author Scott Frederick
+ */
+class BuilderExceptionTests {
+
+	@Test
+	void create() {
+		BuilderException exception = new BuilderException("detector", 1);
+		assertThat(exception.getOperation()).isEqualTo("detector");
+		assertThat(exception.getStatusCode()).isEqualTo(1);
+		assertThat(exception.getMessage()).isEqualTo("Builder lifecycle 'detector' failed with status code 1");
+	}
+
+	@Test
+	void createWhenOperationIsNull() {
+		BuilderException exception = new BuilderException(null, 1);
+		assertThat(exception.getOperation()).isNull();
+		assertThat(exception.getStatusCode()).isEqualTo(1);
+		assertThat(exception.getMessage()).isEqualTo("Builder failed with status code 1");
+	}
+
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/BuilderTests.java

@@ -29,12 +29,15 @@ import org.springframework.boot.buildpack.platform.docker.DockerApi.ContainerApi
 import org.springframework.boot.buildpack.platform.docker.DockerApi.ImageApi;
 import org.springframework.boot.buildpack.platform.docker.DockerApi.VolumeApi;
 import org.springframework.boot.buildpack.platform.docker.TotalProgressPullListener;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerReference;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerStatus;
 import org.springframework.boot.buildpack.platform.docker.type.Image;
 import org.springframework.boot.buildpack.platform.docker.type.ImageArchive;
 import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
 import org.springframework.boot.buildpack.platform.io.TarArchive;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.mockito.ArgumentMatchers.any;
@@ -63,7 +66,7 @@ class BuilderTests {
 	}
 
 	@Test
-	void buildInvokesBuildpack() throws Exception {
+	void buildInvokesBuilder() throws Exception {
 		TestPrintStream out = new TestPrintStream();
 		DockerApi docker = mockDockerApi();
 		Image builderImage = loadImage("image.json");
@@ -103,14 +106,53 @@ class BuilderTests {
 				"Run image stack 'org.cloudfoundry.stacks.cfwindowsfs3' does not match builder stack 'org.cloudfoundry.stacks.cflinuxfs3'");
 	}
 
-	private DockerApi mockDockerApi() {
+	@Test
-		DockerApi docker = mock(DockerApi.class);
+	void buildWhenBuilderReturnsErrorThrowsException() throws Exception {
+		TestPrintStream out = new TestPrintStream();
+		DockerApi docker = mockDockerApiLifecycleError();
+		Image builderImage = loadImage("image.json");
+		Image runImage = loadImage("run-image.json");
+		given(docker.image().pull(eq(ImageReference.of("docker.io/cloudfoundry/cnb:0.0.43-bionic")), any()))
+				.willAnswer(withPulledImage(builderImage));
+		given(docker.image().pull(eq(ImageReference.of("docker.io/cloudfoundry/run:full-cnb")), any()))
+				.willAnswer(withPulledImage(runImage));
+		Builder builder = new Builder(BuildLog.to(out), docker);
+		BuildRequest request = getTestRequest();
+		assertThatExceptionOfType(BuilderException.class).isThrownBy(() -> builder.build(request))
+				.withMessage("Builder lifecycle 'detector' failed with status code 9");
+	}
+
+	private DockerApi mockDockerApi() throws IOException {
+		ContainerApi containerApi = mock(ContainerApi.class);
+		ContainerReference reference = ContainerReference.of("container-ref");
+		given(containerApi.create(any(), any())).willReturn(reference);
+		given(containerApi.wait(eq(reference))).willReturn(ContainerStatus.of(0, null));
+
 		ImageApi imageApi = mock(ImageApi.class);
+		VolumeApi volumeApi = mock(VolumeApi.class);
+
+		DockerApi docker = mock(DockerApi.class);
+		given(docker.image()).willReturn(imageApi);
+		given(docker.container()).willReturn(containerApi);
+		given(docker.volume()).willReturn(volumeApi);
+
+		return docker;
+	}
+
+	private DockerApi mockDockerApiLifecycleError() throws IOException {
 		ContainerApi containerApi = mock(ContainerApi.class);
+		ContainerReference reference = ContainerReference.of("container-ref");
+		given(containerApi.create(any(), any())).willReturn(reference);
+		given(containerApi.wait(eq(reference))).willReturn(ContainerStatus.of(9, null));
+
+		ImageApi imageApi = mock(ImageApi.class);
 		VolumeApi volumeApi = mock(VolumeApi.class);
+
+		DockerApi docker = mock(DockerApi.class);
 		given(docker.image()).willReturn(imageApi);
 		given(docker.container()).willReturn(containerApi);
 		given(docker.volume()).willReturn(volumeApi);
+
 		return docker;
 	}
 

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/build/LifecycleTests.java

@@ -26,7 +26,6 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import org.junit.jupiter.api.BeforeEach;
@@ -40,6 +39,7 @@ import org.springframework.boot.buildpack.platform.docker.DockerApi.VolumeApi;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerConfig;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerContent;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerReference;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerStatus;
 import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
 import org.springframework.boot.buildpack.platform.docker.type.VolumeName;
 import org.springframework.boot.buildpack.platform.io.IOConsumer;
@@ -48,6 +48,7 @@ import org.springframework.boot.buildpack.platform.json.SharedObjectMapper;
 import org.springframework.util.FileCopyUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -95,6 +96,7 @@ class LifecycleTests {
 	void executeExecutesPhases() throws Exception {
 		given(this.docker.container().create(any())).willAnswer(answerWithGeneratedContainerId());
 		given(this.docker.container().create(any(), any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().wait(any())).willReturn(ContainerStatus.of(0, null));
 		this.lifecycle.execute();
 		assertPhaseWasRun("detector", withExpectedConfig("lifecycle-detector.json"));
 		assertPhaseWasRun("restorer", withExpectedConfig("lifecycle-restorer.json"));
@@ -109,6 +111,7 @@ class LifecycleTests {
 	void executeOnlyUploadsContentOnce() throws Exception {
 		given(this.docker.container().create(any())).willAnswer(answerWithGeneratedContainerId());
 		given(this.docker.container().create(any(), any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().wait(any())).willReturn(ContainerStatus.of(0, null));
 		this.lifecycle.execute();
 		assertThat(this.content).hasSize(1);
 	}
@@ -117,15 +120,26 @@ class LifecycleTests {
 	void executeWhenAleadyRunThrowsException() throws Exception {
 		given(this.docker.container().create(any())).willAnswer(answerWithGeneratedContainerId());
 		given(this.docker.container().create(any(), any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().wait(any())).willReturn(ContainerStatus.of(0, null));
 		this.lifecycle.execute();
 		assertThatIllegalStateException().isThrownBy(this.lifecycle::execute)
 				.withMessage("Lifecycle has already been executed");
 	}
 
+	@Test
+	void executeWhenBuilderReturnsErrorThrowsException() throws Exception {
+		given(this.docker.container().create(any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().create(any(), any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().wait(any())).willReturn(ContainerStatus.of(9, null));
+		assertThatExceptionOfType(BuilderException.class).isThrownBy(() -> this.lifecycle.execute())
+				.withMessage("Builder lifecycle 'detector' failed with status code 9");
+	}
+
 	@Test
 	void executeWhenCleanCacheClearsCache() throws Exception {
 		given(this.docker.container().create(any())).willAnswer(answerWithGeneratedContainerId());
 		given(this.docker.container().create(any(), any())).willAnswer(answerWithGeneratedContainerId());
+		given(this.docker.container().wait(any())).willReturn(ContainerStatus.of(0, null));
 		BuildRequest request = getTestRequest().withCleanCache(true);
 		createLifecycle(request).execute();
 		VolumeName name = VolumeName.of("pack-cache-b35197ac41ea.build");
@@ -174,7 +188,7 @@ class LifecycleTests {
 		};
 	}
 
-	private ArrayNode getCommand(ContainerConfig config) throws JsonProcessingException, JsonMappingException {
+	private ArrayNode getCommand(ContainerConfig config) throws JsonProcessingException {
 		JsonNode node = SharedObjectMapper.get().readTree(config.toString());
 		return (ArrayNode) node.at("/Cmd");
 	}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/DockerApiTests.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.buildpack.platform.docker;
 
 import java.io.ByteArrayOutputStream;
-import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URI;
@@ -38,6 +37,7 @@ import org.springframework.boot.buildpack.platform.docker.Http.Response;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerConfig;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerContent;
 import org.springframework.boot.buildpack.platform.docker.type.ContainerReference;
+import org.springframework.boot.buildpack.platform.docker.type.ContainerStatus;
 import org.springframework.boot.buildpack.platform.docker.type.Image;
 import org.springframework.boot.buildpack.platform.docker.type.ImageArchive;
 import org.springframework.boot.buildpack.platform.docker.type.ImageReference;
@@ -87,19 +87,19 @@ class DockerApiTests {
 		return this.httpClient;
 	}
 
-	private Response emptyResponse() throws IOException {
+	private Response emptyResponse() {
 		return responseOf(null);
 	}
 
-	private Response responseOf(String name) throws IOException {
+	private Response responseOf(String name) {
 		return new Response() {
 
 			@Override
-			public void close() throws IOException {
+			public void close() {
 			}
 
 			@Override
-			public InputStream getContent() throws IOException {
+			public InputStream getContent() {
 				if (name == null) {
 					return null;
 				}
@@ -322,7 +322,22 @@ class DockerApiTests {
 		}
 
 		@Test
-		void removeWhenReferenceIsNulllThrowsException() {
+		void waitWhenReferenceIsNullThrowsException() {
+			assertThatIllegalArgumentException().isThrownBy(() -> this.api.wait(null))
+					.withMessage("Reference must not be null");
+		}
+
+		@Test
+		void waitReturnsStatus() throws Exception {
+			ContainerReference reference = ContainerReference.of("e90e34656806");
+			URI waitUri = new URI(CONTAINERS_URL + "/e90e34656806/wait");
+			given(httpClient().post(waitUri)).willReturn(responseOf("container-wait-response.json"));
+			ContainerStatus status = this.api.wait(reference);
+			assertThat(status.getStatusCode()).isEqualTo(1);
+		}
+
+		@Test
+		void removeWhenReferenceIsNullThrowsException() {
 			assertThatIllegalArgumentException().isThrownBy(() -> this.api.remove(null, true))
 					.withMessage("Reference must not be null");
 		}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/type/ContainerStatusTests.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2012-2020 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.buildpack.platform.docker.type;
+
+import java.io.IOException;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests for {@link ContainerStatus}.
+ *
+ * @author Scott Frederick
+ */
+class ContainerStatusTests {
+
+	@Test
+	void ofCreatesFromJson() throws IOException {
+		ContainerStatus status = ContainerStatus.of(getClass().getResourceAsStream("container-status-error.json"));
+		assertThat(status.getStatusCode()).isEqualTo(1);
+		assertThat(status.getWaitingErrorMessage()).isEqualTo("error detail");
+	}
+
+	@Test
+	void ofCreatesFromValues() {
+		ContainerStatus status = ContainerStatus.of(1, "error detail");
+		assertThat(status.getStatusCode()).isEqualTo(1);
+		assertThat(status.getWaitingErrorMessage()).isEqualTo("error detail");
+	}
+
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/container-wait-response.json

@@ -0,0 +1,3 @@
+{
+	"StatusCode": 1
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/type/container-status-error.json

@@ -0,0 +1,6 @@
+{
+	"StatusCode": 1,
+	"Error": {
+		"Message": "error detail"
+	}
+}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/type/container-status-success.json

@@ -0,0 +1,3 @@
+{
+	"StatusCode": 0
+}