Merge pull request #22736 from anshlykov

* gh-22736:
  Polish "Add pullPolicy option for image building"
  Add pullPolicy option for image building

Closes gh-22736

Author: scottfrederick

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

@@ -30,6 +30,7 @@
  *
  * @author Phillip Webb
  * @author Scott Frederick
+ * @author Andrey Shlykov
  * @since 2.3.0
  */
 public abstract class AbstractBuildLog implements BuildLog {
@@ -41,23 +42,37 @@ public void start(BuildRequest request) {
 	}
 
 	@Override
+	@Deprecated
 	public Consumer<TotalProgressEvent> pullingBuilder(BuildRequest request, ImageReference imageReference) {
-		return getProgressConsumer(" > Pulling builder image '" + imageReference + "'");
+		return pullingImage(imageReference, ImageType.BUILDER);
 	}
 
 	@Override
+	@Deprecated
 	public void pulledBuilder(BuildRequest request, Image image) {
-		log(" > Pulled builder image '" + getDigest(image) + "'");
+		pulledImage(image, ImageType.BUILDER);
 	}
 
 	@Override
+	@Deprecated
 	public Consumer<TotalProgressEvent> pullingRunImage(BuildRequest request, ImageReference imageReference) {
-		return getProgressConsumer(" > Pulling run image '" + imageReference + "'");
+		return pullingImage(imageReference, ImageType.RUNNER);
 	}
 
 	@Override
+	@Deprecated
 	public void pulledRunImage(BuildRequest request, Image image) {
-		log(" > Pulled run image '" + getDigest(image) + "'");
+		pulledImage(image, ImageType.RUNNER);
+	}
+
+	@Override
+	public Consumer<TotalProgressEvent> pullingImage(ImageReference imageReference, ImageType imageType) {
+		return getProgressConsumer(String.format(" > Pulling %s '%s'", imageType.getDescription(), imageReference));
+	}
+
+	@Override
+	public void pulledImage(Image image, ImageType imageType) {
+		log(String.format(" > Pulled %s '%s'", imageType.getDescription(), getDigest(image)));
 	}
 
 	@Override

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

@@ -30,6 +30,7 @@
  *
  * @author Phillip Webb
  * @author Scott Frederick
+ * @author Andrey Shlykov
  * @since 2.3.0
  * @see #toSystemOut()
  */
@@ -46,31 +47,56 @@ public interface BuildLog {
 	 * @param request the build request
 	 * @param imageReference the builder image reference
 	 * @return a consumer for progress update events
+	 * @deprecated since 2.4.0 in favor of
+	 * {@link #pullingImage(ImageReference, ImageType)}
 	 */
+	@Deprecated
 	Consumer<TotalProgressEvent> pullingBuilder(BuildRequest request, ImageReference imageReference);
 
 	/**
 	 * Log that the builder image has been pulled.
 	 * @param request the build request
 	 * @param image the builder image that was pulled
+	 * @deprecated since 2.4.0 in favor of {@link #pulledImage(Image, ImageType)}
 	 */
+	@Deprecated
 	void pulledBuilder(BuildRequest request, Image image);
 
 	/**
 	 * Log that a run image is being pulled.
 	 * @param request the build request
 	 * @param imageReference the run image reference
 	 * @return a consumer for progress update events
+	 * @deprecated since 2.4.0 in favor of
+	 * {@link #pullingImage(ImageReference, ImageType)}
 	 */
+	@Deprecated
 	Consumer<TotalProgressEvent> pullingRunImage(BuildRequest request, ImageReference imageReference);
 
 	/**
 	 * Log that a run image has been pulled.
 	 * @param request the build request
 	 * @param image the run image that was pulled
+	 * @deprecated since 2.4.0 in favor of {@link #pulledImage(Image, ImageType)}
 	 */
+	@Deprecated
 	void pulledRunImage(BuildRequest request, Image image);
 
+	/**
+	 * Log that an image is being pulled.
+	 * @param imageReference the image reference
+	 * @param imageType the image type
+	 * @return a consumer for progress update events
+	 */
+	Consumer<TotalProgressEvent> pullingImage(ImageReference imageReference, ImageType imageType);
+
+	/**
+	 * Log that an image has been pulled.
+	 * @param image the builder image that was pulled
+	 * @param imageType the image type that was pulled
+	 */
+	void pulledImage(Image image, ImageType imageType);
+
 	/**
 	 * Log that the lifecycle is executing.
 	 * @param request the build request

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

@@ -32,6 +32,7 @@
  *
  * @author Phillip Webb
  * @author Scott Frederick
+ * @author Andrey Shlykov
  * @since 2.3.0
  */
 public class BuildRequest {
@@ -56,6 +57,8 @@ public class BuildRequest {
 
 	private final boolean verboseLogging;
 
+	private final PullPolicy pullPolicy;
+
 	BuildRequest(ImageReference name, Function<Owner, TarArchive> applicationContent) {
 		Assert.notNull(name, "Name must not be null");
 		Assert.notNull(applicationContent, "ApplicationContent must not be null");
@@ -66,12 +69,13 @@ public class BuildRequest {
 		this.env = Collections.emptyMap();
 		this.cleanCache = false;
 		this.verboseLogging = false;
+		this.pullPolicy = PullPolicy.ALWAYS;
 		this.creator = Creator.withVersion("");
 	}
 
 	BuildRequest(ImageReference name, Function<Owner, TarArchive> applicationContent, ImageReference builder,
 			ImageReference runImage, Creator creator, Map<String, String> env, boolean cleanCache,
-			boolean verboseLogging) {
+			boolean verboseLogging, PullPolicy pullPolicy) {
 		this.name = name;
 		this.applicationContent = applicationContent;
 		this.builder = builder;
@@ -80,6 +84,7 @@ public class BuildRequest {
 		this.env = env;
 		this.cleanCache = cleanCache;
 		this.verboseLogging = verboseLogging;
+		this.pullPolicy = pullPolicy;
 	}
 
 	/**
@@ -90,7 +95,7 @@ public class BuildRequest {
 	public BuildRequest withBuilder(ImageReference builder) {
 		Assert.notNull(builder, "Builder must not be null");
 		return new BuildRequest(this.name, this.applicationContent, builder.inTaggedOrDigestForm(), this.runImage,
-				this.creator, this.env, this.cleanCache, this.verboseLogging);
+				this.creator, this.env, this.cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -100,7 +105,7 @@ public BuildRequest withBuilder(ImageReference builder) {
 	 */
 	public BuildRequest withRunImage(ImageReference runImageName) {
 		return new BuildRequest(this.name, this.applicationContent, this.builder, runImageName.inTaggedOrDigestForm(),
-				this.creator, this.env, this.cleanCache, this.verboseLogging);
+				this.creator, this.env, this.cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -111,7 +116,7 @@ public BuildRequest withRunImage(ImageReference runImageName) {
 	public BuildRequest withCreator(Creator creator) {
 		Assert.notNull(creator, "Creator must not be null");
 		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, creator, this.env,
-				this.cleanCache, this.verboseLogging);
+				this.cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -126,7 +131,7 @@ public BuildRequest withEnv(String name, String value) {
 		Map<String, String> env = new LinkedHashMap<>(this.env);
 		env.put(name, value);
 		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, this.creator,
-				Collections.unmodifiableMap(env), this.cleanCache, this.verboseLogging);
+				Collections.unmodifiableMap(env), this.cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -139,7 +144,7 @@ public BuildRequest withEnv(Map<String, String> env) {
 		Map<String, String> updatedEnv = new LinkedHashMap<>(this.env);
 		updatedEnv.putAll(env);
 		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, this.creator,
-				Collections.unmodifiableMap(updatedEnv), this.cleanCache, this.verboseLogging);
+				Collections.unmodifiableMap(updatedEnv), this.cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -149,7 +154,7 @@ public BuildRequest withEnv(Map<String, String> env) {
 	 */
 	public BuildRequest withCleanCache(boolean cleanCache) {
 		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, this.creator, this.env,
-				cleanCache, this.verboseLogging);
+				cleanCache, this.verboseLogging, this.pullPolicy);
 	}
 
 	/**
@@ -159,7 +164,17 @@ public BuildRequest withCleanCache(boolean cleanCache) {
 	 */
 	public BuildRequest withVerboseLogging(boolean verboseLogging) {
 		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, this.creator, this.env,
-				this.cleanCache, verboseLogging);
+				this.cleanCache, verboseLogging, this.pullPolicy);
+	}
+
+	/**
+	 * Return a new {@link BuildRequest} with the updated image pull policy.
+	 * @param pullPolicy image pull policy {@link PullPolicy}
+	 * @return an updated build request
+	 */
+	public BuildRequest withPullPolicy(PullPolicy pullPolicy) {
+		return new BuildRequest(this.name, this.applicationContent, this.builder, this.runImage, this.creator, this.env,
+				this.cleanCache, this.verboseLogging, pullPolicy);
 	}
 
 	/**
@@ -229,6 +244,14 @@ public boolean isVerboseLogging() {
 		return this.verboseLogging;
 	}
 
+	/**
+	 * Return the image {@link PullPolicy} that the builder should use.
+	 * @return image pull policy
+	 */
+	public PullPolicy getPullPolicy() {
+		return this.pullPolicy;
+	}
+
 	/**
 	 * Factory method to create a new {@link BuildRequest} from a JAR file.
 	 * @param jarFile the source jar file

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

@@ -35,6 +35,7 @@
  *
  * @author Phillip Webb
  * @author Scott Frederick
+ * @author Andrey Shlykov
  * @since 2.3.0
  */
 public class Builder {
@@ -60,7 +61,7 @@ public Builder(BuildLog log) {
 	public void build(BuildRequest request) throws DockerEngineException, IOException {
 		Assert.notNull(request, "Request must not be null");
 		this.log.start(request);
-		Image builderImage = pullBuilder(request);
+		Image builderImage = getImage(request, ImageType.BUILDER);
 		BuilderMetadata builderMetadata = BuilderMetadata.fromImage(builderImage);
 		BuildOwner buildOwner = BuildOwner.fromEnv(builderImage.getConfig().getEnv());
 		request = determineRunImage(request, builderImage, builderMetadata.getStack());
@@ -75,22 +76,13 @@ public void build(BuildRequest request) throws DockerEngineException, IOExceptio
 		}
 	}
 
-	private Image pullBuilder(BuildRequest request) throws IOException {
-		ImageReference builderImageReference = request.getBuilder();
-		Consumer<TotalProgressEvent> progressConsumer = this.log.pullingBuilder(request, builderImageReference);
-		TotalProgressPullListener listener = new TotalProgressPullListener(progressConsumer);
-		Image builderImage = this.docker.image().pull(builderImageReference, listener);
-		this.log.pulledBuilder(request, builderImage);
-		return builderImage;
-	}
-
 	private BuildRequest determineRunImage(BuildRequest request, Image builderImage, Stack builderStack)
 			throws IOException {
 		if (request.getRunImage() == null) {
 			ImageReference runImage = getRunImageReferenceForStack(builderStack);
 			request = request.withRunImage(runImage);
 		}
-		Image runImage = pullRunImage(request);
+		Image runImage = getImage(request, ImageType.RUNNER);
 		assertStackIdsMatch(runImage, builderImage);
 		return request;
 	}
@@ -101,12 +93,31 @@ private ImageReference getRunImageReferenceForStack(Stack stack) {
 		return ImageReference.of(name).inTaggedOrDigestForm();
 	}
 
-	private Image pullRunImage(BuildRequest request) throws IOException {
-		ImageReference runImage = request.getRunImage();
-		Consumer<TotalProgressEvent> progressConsumer = this.log.pullingRunImage(request, runImage);
+	private Image getImage(BuildRequest request, ImageType imageType) throws IOException {
+		ImageReference imageReference = (imageType == ImageType.BUILDER) ? request.getBuilder() : request.getRunImage();
+
+		if (request.getPullPolicy() == PullPolicy.ALWAYS) {
+			return pullImage(imageReference, imageType);
+		}
+
+		try {
+			return this.docker.image().inspect(imageReference);
+		}
+		catch (DockerEngineException exception) {
+			if (request.getPullPolicy() == PullPolicy.IF_NOT_PRESENT && exception.getStatusCode() == 404) {
+				return pullImage(imageReference, imageType);
+			}
+			else {
+				throw exception;
+			}
+		}
+	}
+
+	private Image pullImage(ImageReference reference, ImageType imageType) throws IOException {
+		Consumer<TotalProgressEvent> progressConsumer = this.log.pullingImage(reference, imageType);
 		TotalProgressPullListener listener = new TotalProgressPullListener(progressConsumer);
-		Image image = this.docker.image().pull(runImage, listener);
-		this.log.pulledRunImage(request, image);
+		Image image = this.docker.image().pull(reference, listener);
+		this.log.pulledImage(image, imageType);
 		return image;
 	}
 

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/ImageType.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;
+
+/**
+ * Image types.
+ *
+ * @author Andrey Shlykov
+ */
+enum ImageType {
+
+	/**
+	 * Builder image.
+	 */
+	BUILDER("builder image"),
+
+	/**
+	 * Run image.
+	 */
+	RUNNER("run image");
+
+	private final String description;
+
+	ImageType(String description) {
+		this.description = description;
+	}
+
+	String getDescription() {
+		return this.description;
+	}
+
+}