From 06aa3d30c1858179d1be880aedf8133dadc4c0c1 Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 6 Aug 2019 14:18:54 +0100 Subject: [PATCH 1/4] Reinstate retries for image pulls --- .../images/RemoteDockerImage.java | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java index 7aff7918f6d..e7324633c89 100644 --- a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java +++ b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java @@ -29,6 +29,7 @@ public class RemoteDockerImage extends LazyFuture { */ @Deprecated public static final Set AVAILABLE_IMAGE_NAME_CACHE = new HashSet<>(); + private static final int PULL_ATTEMPTS = 3; private DockerImageName imageName; @@ -73,23 +74,29 @@ protected final String resolve() { return imageName.toString(); } + // The image is not available locally - pull it logger.info("Pulling docker image: {}. Please be patient; this may take some time but only needs to be done once.", imageName); - // The image is not available locally - pull it - try { - final PullImageResultCallback callback = new TimeLimitedLoggedPullImageResultCallback(logger); - dockerClient - .pullImageCmd(imageName.getUnversionedPart()) - .withTag(imageName.getVersionPart()) - .exec(callback); - callback.awaitCompletion(); - AVAILABLE_IMAGE_NAME_CACHE.add(imageName); - } catch (Exception e) { - logger.error("Failed to pull image: {}. Please check output of `docker pull {}`", imageName, imageName); - throw new ContainerFetchException("Failed to pull image: " + imageName, e); + Exception lastFailure = null; + for (int i = 1; i <= PULL_ATTEMPTS; i++) { + try { + final PullImageResultCallback callback = new TimeLimitedLoggedPullImageResultCallback(logger); + dockerClient + .pullImageCmd(imageName.getUnversionedPart()) + .withTag(imageName.getVersionPart()) + .exec(callback); + callback.awaitCompletion(); + AVAILABLE_IMAGE_NAME_CACHE.add(imageName); + + return imageName.toString(); + } catch (Exception e) { + logger.debug("Retrying pull for image: {}", imageName); + lastFailure = e; + } } + logger.error("Failed to pull image: {}. Please check output of `docker pull {}`", imageName, imageName, lastFailure); - return imageName.toString(); + throw new ContainerFetchException("Failed to pull image: " + imageName, lastFailure); } catch (DockerClientException e) { throw new ContainerFetchException("Failed to get Docker client for " + imageName, e); } From f4c03842a55d48e7644e63938a11c24964edf9a5 Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 6 Aug 2019 21:23:15 +0100 Subject: [PATCH 2/4] Adopt time limit for retries instead of a fixed number of retries --- .../testcontainers/images/RemoteDockerImage.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java index e7324633c89..aed79995f21 100644 --- a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java +++ b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java @@ -14,6 +14,8 @@ import org.testcontainers.utility.DockerLoggerFactory; import org.testcontainers.utility.LazyFuture; +import java.time.Duration; +import java.time.Instant; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -21,6 +23,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.time.Duration.between; +import static java.time.Instant.now; + @ToString public class RemoteDockerImage extends LazyFuture { @@ -29,7 +34,7 @@ public class RemoteDockerImage extends LazyFuture { */ @Deprecated public static final Set AVAILABLE_IMAGE_NAME_CACHE = new HashSet<>(); - private static final int PULL_ATTEMPTS = 3; + private static final Duration PULL_RETRY_TIME_LIMIT = Duration.ofMinutes(2); private DockerImageName imageName; @@ -78,7 +83,9 @@ protected final String resolve() { logger.info("Pulling docker image: {}. Please be patient; this may take some time but only needs to be done once.", imageName); Exception lastFailure = null; - for (int i = 1; i <= PULL_ATTEMPTS; i++) { + final Instant lastRetryAllowed = now().plus(PULL_RETRY_TIME_LIMIT); + + while (now().isBefore(lastRetryAllowed)) { try { final PullImageResultCallback callback = new TimeLimitedLoggedPullImageResultCallback(logger); dockerClient @@ -90,8 +97,10 @@ protected final String resolve() { return imageName.toString(); } catch (Exception e) { - logger.debug("Retrying pull for image: {}", imageName); lastFailure = e; + logger.warn("Retrying pull for image: {} ({}s remaining)", + imageName, + between(now(), lastRetryAllowed).getSeconds()); } } logger.error("Failed to pull image: {}. Please check output of `docker pull {}`", imageName, imageName, lastFailure); From 7beaeb2d1eac95e101af26f82ee322d039f5efca Mon Sep 17 00:00:00 2001 From: Richard North Date: Tue, 6 Aug 2019 23:06:27 +0100 Subject: [PATCH 3/4] Only retry image pull failures that could potentially succeed next time --- .../java/org/testcontainers/images/RemoteDockerImage.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java index aed79995f21..4173888ff00 100644 --- a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java +++ b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java @@ -3,6 +3,7 @@ import com.github.dockerjava.api.DockerClient; import com.github.dockerjava.api.command.ListImagesCmd; import com.github.dockerjava.api.exception.DockerClientException; +import com.github.dockerjava.api.exception.InternalServerErrorException; import com.github.dockerjava.api.model.Image; import com.github.dockerjava.core.command.PullImageResultCallback; import lombok.NonNull; @@ -96,7 +97,8 @@ protected final String resolve() { AVAILABLE_IMAGE_NAME_CACHE.add(imageName); return imageName.toString(); - } catch (Exception e) { + } catch (InterruptedException | InternalServerErrorException e) { + // these classes of exception often relate to timeout/connection errors so should be retried lastFailure = e; logger.warn("Retrying pull for image: {} ({}s remaining)", imageName, From b9b2a22d2baf73bece52b84bc304c762eb764a56 Mon Sep 17 00:00:00 2001 From: Richard North Date: Wed, 21 Aug 2019 20:20:12 +0100 Subject: [PATCH 4/4] Qualify usages of `Instant` and `Duration` --- .../org/testcontainers/images/RemoteDockerImage.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java index 4173888ff00..257bacc9961 100644 --- a/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java +++ b/core/src/main/java/org/testcontainers/images/RemoteDockerImage.java @@ -24,9 +24,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static java.time.Duration.between; -import static java.time.Instant.now; - @ToString public class RemoteDockerImage extends LazyFuture { @@ -84,9 +81,9 @@ protected final String resolve() { logger.info("Pulling docker image: {}. Please be patient; this may take some time but only needs to be done once.", imageName); Exception lastFailure = null; - final Instant lastRetryAllowed = now().plus(PULL_RETRY_TIME_LIMIT); + final Instant lastRetryAllowed = Instant.now().plus(PULL_RETRY_TIME_LIMIT); - while (now().isBefore(lastRetryAllowed)) { + while (Instant.now().isBefore(lastRetryAllowed)) { try { final PullImageResultCallback callback = new TimeLimitedLoggedPullImageResultCallback(logger); dockerClient @@ -102,7 +99,7 @@ protected final String resolve() { lastFailure = e; logger.warn("Retrying pull for image: {} ({}s remaining)", imageName, - between(now(), lastRetryAllowed).getSeconds()); + Duration.between(Instant.now(), lastRetryAllowed).getSeconds()); } } logger.error("Failed to pull image: {}. Please check output of `docker pull {}`", imageName, imageName, lastFailure);