Add docker options for pull retry

snowdrop · Jul 16, 2024 · 3a45bd0 · 3a45bd0
1 parent 27fc32e
commit 3a45bd0
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 37 deletions.
diff --git a/README.md b/README.md
@@ -33,7 +33,9 @@ The [`BuildpackConfig`](client/src/main/java/dev/snowdrop/buildpack/BuildConfig.
 
 - run/build/output Image can be specified
 - docker can be configured with.. 
-    - pull timeout
+    - pull timeout 
+    - pull retry count (will retry image pull on failure)
+    - pull retry timeout increase (increases timeout each time pull is retried)
     - host
     - network
     - docker socket path

diff --git a/client/src/main/java/dev/snowdrop/buildpack/config/DockerConfig.java b/client/src/main/java/dev/snowdrop/buildpack/config/DockerConfig.java
@@ -15,11 +15,13 @@ public static DockerConfigBuilder builder() {
     public static enum PullPolicy {ALWAYS, IF_NOT_PRESENT};
 
     private static final Integer DEFAULT_PULL_TIMEOUT = 60;
+    private static final Integer DEFAULT_PULL_RETRY_INCREASE = 15;
     private static final Integer DEFAULT_PULL_RETRY_COUNT = 3;
     private static final PullPolicy DEFAULT_PULL_POLICY = PullPolicy.IF_NOT_PRESENT;
 
     private Integer pullTimeoutSeconds;
     private Integer pullRetryCount;
+    private Integer pullRetryIncreaseSeconds;
     private PullPolicy pullPolicy;
     private String dockerHost;
     private String dockerSocket;
@@ -30,15 +32,17 @@ public static enum PullPolicy {ALWAYS, IF_NOT_PRESENT};
     public DockerConfig(                   
         Integer pullTimeoutSeconds, 
         Integer pullRetryCount,
+        Integer pullRetryIncreaseSeconds,
         PullPolicy pullPolicy,
         String dockerHost, 
         String dockerSocket,
         String dockerNetwork,
         Boolean useDaemon, 
         DockerClient dockerClient
     ){
-        this.pullTimeoutSeconds = pullTimeoutSeconds != null ? pullTimeoutSeconds : DEFAULT_PULL_TIMEOUT;
-        this.pullRetryCount = pullRetryCount != null ? pullRetryCount : DEFAULT_PULL_RETRY_COUNT;
+        this.pullTimeoutSeconds = pullTimeoutSeconds != null ? Integer.max(0,pullTimeoutSeconds) : DEFAULT_PULL_TIMEOUT;
+        this.pullRetryCount = pullRetryCount != null ? Integer.max(0,pullRetryCount) : DEFAULT_PULL_RETRY_COUNT;
+        this.pullRetryIncreaseSeconds = pullRetryIncreaseSeconds != null ? Integer.max(0,pullRetryIncreaseSeconds) : DEFAULT_PULL_RETRY_INCREASE;
         this.pullPolicy = pullPolicy != null ? pullPolicy : DEFAULT_PULL_POLICY;
         this.dockerHost = dockerHost != null ? dockerHost : DockerClientUtils.getDockerHost();
         this.dockerSocket = dockerSocket != null ? dockerSocket : (this.dockerHost.startsWith("unix://") ? this.dockerHost.substring("unix://".length()) : "/var/run/docker.sock");
@@ -61,6 +65,10 @@ public Integer getPullRetryCount(){
         return this.pullRetryCount;
     }
 
+    public Integer getPullRetryIncrease(){
+        return this.pullRetryIncreaseSeconds;
+    }
+
     public PullPolicy getPullPolicy(){
         return this.pullPolicy;
     }

diff --git a/client/src/main/java/dev/snowdrop/buildpack/docker/ImageUtils.java b/client/src/main/java/dev/snowdrop/buildpack/docker/ImageUtils.java
@@ -9,6 +9,7 @@
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -18,6 +19,7 @@
 import com.github.dockerjava.api.command.PullImageResultCallback;
 import com.github.dockerjava.api.model.Image;
 import com.github.dockerjava.api.exception.DockerClientException;
+import com.github.dockerjava.api.exception.NotFoundException;
 
 import dev.snowdrop.buildpack.config.DockerConfig;
 import dev.snowdrop.buildpack.BuildpackException;
@@ -36,6 +38,7 @@ public static class ImageInfo {
   /**
    * Util method to pull images, configure behavior via dockerconfig.
    */
+  @SuppressWarnings("resource")
   public static void pullImages(DockerConfig config, String... imageNames) {
     Set<String> imageNameSet = new HashSet<>(Arrays.asList(imageNames));
 
@@ -63,7 +66,7 @@ public static void pullImages(DockerConfig config, String... imageNames) {
       }
     }
 
-    int retriesRemaining = Integer.max(config.getPullRetryCount(), 0);
+    int retryCount = 0;
     Map<String,PullImageResultCallback> pircMap = new HashMap<>();
 
     // pull the images still in set.
@@ -75,20 +78,25 @@ public static void pullImages(DockerConfig config, String... imageNames) {
     }
 
     // wait for pulls to complete.
-    DockerClientException lastSeen = null;
-    boolean allDone = true;
-    while(!allDone && retriesRemaining>=0){
+    RuntimeException lastSeen = null;
+    boolean allDone = false;
+    while(!allDone && retryCount<=config.getPullRetryCount()){
       allDone = true;
+      long thisWait = config.getPullTimeout()+(retryCount*config.getPullRetryIncrease());
       for (Entry<String, PullImageResultCallback> e : pircMap.entrySet()) {
         boolean done = false;
         try {
           if(e.getValue()==null) continue;
-          done = e.getValue().awaitCompletion(config.getPullTimeout(), TimeUnit.SECONDS);
+          log.debug("waiting on image "+e.getKey()+" for "+thisWait+" seconds");
+          done = e.getValue().awaitCompletion( thisWait, TimeUnit.SECONDS);
+          log.debug("success for image "+e.getKey());
         } catch (InterruptedException ie) {
           throw BuildpackException.launderThrowable(ie);
         } catch (DockerClientException dce) {
           //error occurred during pull for this pirc, need to pause & retry the pull op
           lastSeen = dce;
+        } catch (NotFoundException nfe) {
+          lastSeen = nfe;
         }
         if(!done){
           String imageName = e.getKey();
@@ -100,10 +108,16 @@ public static void pullImages(DockerConfig config, String... imageNames) {
           e.setValue(null);
         }
       }
-      retriesRemaining-=1;
+      retryCount++;
+      if(retryCount<=config.getPullRetryCount()){
+        if(lastSeen!=null){
+          log.debug("Error during pull "+lastSeen.getMessage());
+        }
+        log.debug("Retrying ("+retryCount+") for "+pircMap.entrySet().stream().filter(e -> e.getValue()!=null).collect(Collectors.toList()));
+      }
     }
 
-    if(lastSeen!=null){
+    if(lastSeen!=null && !allDone){
       throw lastSeen;
     }
   }

diff --git a/client/src/test/java/dev/snowdrop/buildpack/config/DockerConfigTest.java b/client/src/test/java/dev/snowdrop/buildpack/config/DockerConfigTest.java
@@ -6,7 +6,6 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.lenient;
-import static org.mockito.Mockito.when;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -20,21 +19,21 @@
 public class DockerConfigTest {
     @Test
     void checkTimeout() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertEquals(60, dc1.getPullTimeout());
 
-        DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null, null, null);
+        DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null, null, null, null);
         assertEquals(dc2.getPullTimeout(), 245017);
     }
 
     @Test
     void checkDockerHost(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerHost());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, "tcp://stilettos", null, null, null, dockerClient);
         assertEquals("tcp://stilettos", dc2.getDockerHost());
     }
 
@@ -43,62 +42,62 @@ void checkDockerSocket(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
 
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerSocket());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, "unix:///stilettos", null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, "unix:///stilettos", null, null, null, dockerClient);
         assertEquals("/stilettos", dc2.getDockerSocket());
 
-        DockerConfig dc3 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
+        DockerConfig dc3 = new DockerConfig(null, null, null, null, "tcp://stilettos", null, null, null, dockerClient);
         assertEquals("/var/run/docker.sock", dc3.getDockerSocket());
 
-        DockerConfig dc4 = new DockerConfig(null, null, null, null, "fish", null, null, null);
+        DockerConfig dc4 = new DockerConfig(null, null, null, null, null, "fish", null, null, null);
         assertEquals("fish", dc4.getDockerSocket());
     }
 
     @Test
     void checkDockerNetwork() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, "kitten", null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, "kitten", null, null);
         assertEquals("kitten", dc1.getDockerNetwork());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertNull(dc2.getDockerNetwork());
     }
 
     @Test
     void checkUseDaemon() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertTrue(dc1.getUseDaemon());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, true, null);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, true, null);
         assertTrue(dc2.getUseDaemon());
 
-        DockerConfig dc3 = new DockerConfig(null, null, null, null, null, null, false, null);
+        DockerConfig dc3 = new DockerConfig(null, null, null, null, null, null, null, false, null);
         assertFalse(dc3.getUseDaemon());
     }
 
     @Test
     void checkDockerClient(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerClient());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, null, dockerClient);
         assertEquals(dockerClient, dc2.getDockerClient());
     }
 
     @Test
     void checkPullPolicy(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, dockerClient);
         assertEquals(DockerConfig.PullPolicy.IF_NOT_PRESENT, dc1.getPullPolicy());
 
-        DockerConfig dc2 = new DockerConfig(null, null, DockerConfig.PullPolicy.IF_NOT_PRESENT, null, null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, DockerConfig.PullPolicy.IF_NOT_PRESENT, null, null, null, null, dockerClient);
         assertEquals(DockerConfig.PullPolicy.IF_NOT_PRESENT, dc2.getPullPolicy());
 
-        DockerConfig dc3 = new DockerConfig(null, null, DockerConfig.PullPolicy.ALWAYS, null, null, null, null, dockerClient);
+        DockerConfig dc3 = new DockerConfig(null, null, null, DockerConfig.PullPolicy.ALWAYS, null, null, null, null, dockerClient);
         assertEquals(DockerConfig.PullPolicy.ALWAYS, dc3.getPullPolicy());        
     }  
 
@@ -107,13 +106,13 @@ void checkPullPolicy(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
     void checkPullRetry(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null, dockerClient);
         assertEquals(3, dc1.getPullRetryCount());
 
-        DockerConfig dc2 = new DockerConfig(null, 5, null, null, null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, 5, null, null, null, null, null, null, dockerClient);
         assertEquals(5, dc2.getPullRetryCount());
 
-        DockerConfig dc3 = new DockerConfig(null, 0, null, null, null, null, null, dockerClient);
+        DockerConfig dc3 = new DockerConfig(null, 0, null, null, null, null, null, null, dockerClient);
         assertEquals(0, dc3.getPullRetryCount());        
     }     
 }
diff --git a/client/src/test/java/dev/snowdrop/buildpack/docker/ImageUtilsTest.java b/client/src/test/java/dev/snowdrop/buildpack/docker/ImageUtilsTest.java
@@ -7,6 +7,7 @@
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.atLeast;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -79,7 +80,7 @@ void testPullImageSingleUnknown(@Mock DockerConfig config,
 
     ImageUtils.pullImages(config, imageName);
 
-    verify(pic).exec(ArgumentMatchers.any());
+    verify(pic, atLeast(1)).exec(ArgumentMatchers.any());
   }
 
   @Test

diff --git a/samples/hello-quarkus/pack.java b/samples/hello-quarkus/pack.java
@@ -20,8 +20,8 @@ public static void main(String... args) {
       System.setProperty("org.slf4j.simpleLogger.log.dev.snowdrop.buildpack.lifecycle.phases","debug");      
 
       int exitCode = BuildConfig.builder()
-                           .withBuilderImage(new ImageReference("paketocommunity/builder-ubi-base:latest"))
-                           .withOutputImage(new ImageReference("snowdrop/hello-quarkus:latest"))
+                           .withBuilderImage(new ImageReference("docker.io/paketocommunity/builder-ubi-base"))
+                           .withOutputImage(new ImageReference("snowdrop/hello-quarkus"))
                            .withNewLogConfig()
                               .withLogger(new SystemLogger())
                               .withLogLevel("debug")

diff --git a/samples/hello-spring/pack.java b/samples/hello-spring/pack.java
@@ -22,8 +22,8 @@ public static void main(String... args) {
         HashMap<String,String> env = new HashMap<>();
 
         int exitCode = BuildConfig.builder()
-                           .withBuilderImage(new ImageReference("paketocommunity/builder-ubi-base:latest"))             
-                           .withOutputImage(new ImageReference("snowdrop/hello-spring:latest"))
+                           .withBuilderImage(new ImageReference("docker.io/paketocommunity/builder-ubi-base"))             
+                           .withOutputImage(new ImageReference("snowdrop/hello-spring"))
                            .withNewLogConfig()
                                 .withLogger(new SystemLogger())
                                 .withLogLevel("debug")