Add pull policy & retry

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

Hide file tree

Showing 8 changed files with 154 additions and 62 deletions.
diff --git a/client/src/main/java/dev/snowdrop/buildpack/BuilderImage.java b/client/src/main/java/dev/snowdrop/buildpack/BuilderImage.java
@@ -44,9 +44,7 @@ public BuilderImage(DockerConfig dc, PlatformConfig pc, ImageReference runImage,
         image = builderImage;
 
         // pull and inspect the builderImage to obtain builder metadata.
-        ImageUtils.pullImages(dc.getDockerClient(), 
-                              dc.getPullTimeout(), 
-                              builderImage.getReference());
+        ImageUtils.pullImages(dc, builderImage.getReference());
 
         ImageInfo ii = ImageUtils.inspectImage(dc.getDockerClient(), 
                                                builderImage.getReference());

diff --git a/client/src/main/java/dev/snowdrop/buildpack/config/DockerConfig.java b/client/src/main/java/dev/snowdrop/buildpack/config/DockerConfig.java
@@ -12,9 +12,15 @@ public static DockerConfigBuilder builder() {
         return new DockerConfigBuilder();
     }
 
+    public static enum PullPolicy {ALWAYS, IF_NOT_PRESENT};
+
     private static final Integer DEFAULT_PULL_TIMEOUT = 60;
+    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 PullPolicy pullPolicy;
     private String dockerHost;
     private String dockerSocket;
     private String dockerNetwork;
@@ -23,13 +29,17 @@ public static DockerConfigBuilder builder() {
 
     public DockerConfig(                   
         Integer pullTimeoutSeconds, 
+        Integer pullRetryCount,
+        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.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");
         this.dockerNetwork = dockerNetwork;
@@ -47,6 +57,14 @@ public Integer getPullTimeout(){
         return this.pullTimeoutSeconds;
     }
 
+    public Integer getPullRetryCount(){
+        return this.pullRetryCount;
+    }
+
+    public PullPolicy getPullPolicy(){
+        return this.pullPolicy;
+    }
+
     public String getDockerHost(){
         return this.dockerHost;
     }

diff --git a/client/src/main/java/dev/snowdrop/buildpack/docker/ImageUtils.java b/client/src/main/java/dev/snowdrop/buildpack/docker/ImageUtils.java
@@ -2,9 +2,11 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -15,7 +17,9 @@
 import com.github.dockerjava.api.command.InspectImageResponse;
 import com.github.dockerjava.api.command.PullImageResultCallback;
 import com.github.dockerjava.api.model.Image;
-
+import com.github.dockerjava.api.exception.DockerClientException;
+
+import dev.snowdrop.buildpack.config.DockerConfig;
 import dev.snowdrop.buildpack.BuildpackException;
 /**
  * Higher level docker image api
@@ -30,51 +34,81 @@ public static class ImageInfo {
   }
 
   /**
-   * Util method to pull images if they don't exist to the local docker yet.
+   * Util method to pull images, configure behavior via dockerconfig.
    */
-  public static void pullImages(DockerClient dc, int timeoutSeconds, String... imageNames) {
+  public static void pullImages(DockerConfig config, String... imageNames) {
     Set<String> imageNameSet = new HashSet<>(Arrays.asList(imageNames));
 
-    // list the current known images
-    List<Image> li = dc.listImagesCmd().exec();
-    for (Image i : li) {
-      if (i.getRepoTags() == null) {
-        continue;
-      }
-      for (String it : i.getRepoTags()) {
-        if (imageNameSet.contains(it)) {
-          imageNameSet.remove(it);
+    DockerClient dc = config.getDockerClient();
+
+    //if using ifnotpresent, filter set to unknown images.
+    if(config.getPullPolicy() == DockerConfig.PullPolicy.IF_NOT_PRESENT) {
+      // list the current known images
+      List<Image> li = dc.listImagesCmd().exec();
+      for (Image i : li) {
+        if (i.getRepoTags() == null) {
+          continue;
+        }
+        for (String it : i.getRepoTags()) {
+          if (imageNameSet.contains(it)) {
+            imageNameSet.remove(it);
+          }
         }
       }
-    }
 
-    if (imageNameSet.isEmpty()) {
-      // fast exit if all images are already known to the local docker.
-      log.debug("Nothing to pull, all of " + Arrays.asList(imageNames) + " are known");
-      return;
+      if (imageNameSet.isEmpty()) {
+        // fast exit if all images are already known to the local docker.
+        log.debug("Nothing to pull, all of " + Arrays.asList(imageNames) + " are known");
+        return;
+      }
     }
 
-    // pull the images not known
-    List<PullImageResultCallback> pircs = new ArrayList<>();
+    int retriesRemaining = Integer.max(config.getPullRetryCount(), 0);
+    Map<String,PullImageResultCallback> pircMap = new HashMap<>();
+
+    // pull the images still in set.
     for (String stillNeeded : imageNameSet) {
       log.debug("pulling '" + stillNeeded + "'");
       PullImageResultCallback pirc = new PullImageResultCallback();
       dc.pullImageCmd(stillNeeded).exec(pirc);
-      pircs.add(pirc);
+      pircMap.put(stillNeeded,pirc);
     }
 
     // wait for pulls to complete.
-    for (PullImageResultCallback pirc : pircs) {
-      try {
-        pirc.awaitCompletion(timeoutSeconds, TimeUnit.SECONDS);
-      } catch (InterruptedException e) {
-        throw BuildpackException.launderThrowable(e);
+    DockerClientException lastSeen = null;
+    boolean allDone = true;
+    while(!allDone && retriesRemaining>=0){
+      allDone = true;
+      for (Entry<String, PullImageResultCallback> e : pircMap.entrySet()) {
+        boolean done = false;
+        try {
+          if(e.getValue()==null) continue;
+          done = e.getValue().awaitCompletion(config.getPullTimeout(), TimeUnit.SECONDS);
+        } 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;
+        }
+        if(!done){
+          String imageName = e.getKey();
+          PullImageResultCallback newPirc = new PullImageResultCallback();
+          dc.pullImageCmd(imageName).exec(newPirc);
+          e.setValue(newPirc);
+          allDone=false;      
+        }else{
+          e.setValue(null);
+        }
       }
+      retriesRemaining-=1;
     }
 
-    // TODO: progress tracking..
+    if(lastSeen!=null){
+      throw lastSeen;
+    }
   }
 
+
   /**
    * Util method to retrieve info for a given docker image.
    */

diff --git a/client/src/main/java/dev/snowdrop/buildpack/lifecycle/LifecycleExecutor.java b/client/src/main/java/dev/snowdrop/buildpack/lifecycle/LifecycleExecutor.java
@@ -105,7 +105,7 @@ public int execute() {
                         }
 
                         //pull the new image.. 
-                        ImageUtils.pullImages(config.getDockerConfig().getDockerClient(), factory.getDockerConfig().getPullTimeout(), newRunImage);
+                        ImageUtils.pullImages(config.getDockerConfig(), newRunImage);
 
                         //update run image associated with our builder image.
                         factory.getBuilderImage().getRunImages(activePlatformLevel).clear();

diff --git a/client/src/main/java/dev/snowdrop/buildpack/utils/LifecycleMetadata.java b/client/src/main/java/dev/snowdrop/buildpack/utils/LifecycleMetadata.java
@@ -20,9 +20,7 @@ public class LifecycleMetadata {
     public LifecycleMetadata(DockerConfig dc, ImageReference lifecycleImage) throws BuildpackException {
 
         // pull and inspect the builderImage to obtain builder metadata.
-        ImageUtils.pullImages(dc.getDockerClient(), 
-                              dc.getPullTimeout(), 
-                              lifecycleImage.getReference());
+        ImageUtils.pullImages(dc,lifecycleImage.getReference());
 
         ImageInfo ii = ImageUtils.inspectImage(dc.getDockerClient(), 
                                                lifecycleImage.getReference());

diff --git a/client/src/test/java/dev/snowdrop/buildpack/config/DockerConfigTest.java b/client/src/test/java/dev/snowdrop/buildpack/config/DockerConfigTest.java
@@ -20,20 +20,21 @@
 public class DockerConfigTest {
     @Test
     void checkTimeout() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertEquals(60, dc1.getPullTimeout());
 
-        DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null);
+        DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null, null, null);
         assertEquals(dc2.getPullTimeout(), 245017);
     }
 
     @Test
     void checkDockerHost(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
+        lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
+
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerHost());
 
-        when(dockerClient.pingCmd()).thenReturn(pingCmd);
-        DockerConfig dc2 = new DockerConfig(null, "tcp://stilettos", null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
         assertEquals("tcp://stilettos", dc2.getDockerHost());
     }
 
@@ -42,48 +43,77 @@ void checkDockerSocket(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
 
         lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
 
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerSocket());
 
-        DockerConfig dc2 = new DockerConfig(null, "unix:///stilettos", null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(null, null, null, "unix:///stilettos", null, null, null, dockerClient);
         assertEquals("/stilettos", dc2.getDockerSocket());
 
-        DockerConfig dc3 = new DockerConfig(null, "tcp://stilettos", null, null, null, dockerClient);
+        DockerConfig dc3 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
         assertEquals("/var/run/docker.sock", dc3.getDockerSocket());
 
-        DockerConfig dc4 = new DockerConfig(null, null, "fish", null, null, null);
+        DockerConfig dc4 = new DockerConfig(null, null, null, null, "fish", null, null, null);
         assertEquals("fish", dc4.getDockerSocket());
     }
 
     @Test
     void checkDockerNetwork() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, "kitten", null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, "kitten", null, null);
         assertEquals("kitten", dc1.getDockerNetwork());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertNull(dc2.getDockerNetwork());
     }
 
     @Test
     void checkUseDaemon() {
-        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertTrue(dc1.getUseDaemon());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, true, null);
+        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, true, null);
         assertTrue(dc2.getUseDaemon());
 
-        DockerConfig dc3 = new DockerConfig(null, null, null, null, false, null);
+        DockerConfig dc3 = new DockerConfig(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);
+        DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
         assertNotNull(dc1.getDockerClient());
 
-        DockerConfig dc2 = new DockerConfig(null, null, null, null, null, dockerClient);
+        DockerConfig dc2 = new DockerConfig(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);
+        assertEquals(DockerConfig.PullPolicy.IF_NOT_PRESENT, dc1.getPullPolicy());
+
+        DockerConfig dc2 = new DockerConfig(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);
+        assertEquals(DockerConfig.PullPolicy.ALWAYS, dc3.getPullPolicy());        
+    }  
+
+
+    @Test
+    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);
+        assertEquals(3, dc1.getPullRetryCount());
+
+        DockerConfig dc2 = new DockerConfig(null, 5, null, null, null, null, null, dockerClient);
+        assertEquals(5, dc2.getPullRetryCount());
+
+        DockerConfig dc3 = new DockerConfig(null, 0, 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
@@ -6,6 +6,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.lenient;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -26,6 +27,7 @@
 import com.github.dockerjava.api.model.ContainerConfig;
 import com.github.dockerjava.api.model.Image;
 
+import dev.snowdrop.buildpack.config.DockerConfig;
 import dev.snowdrop.buildpack.docker.ImageUtils.ImageInfo;
 
 @ExtendWith(MockitoExtension.class)
@@ -61,39 +63,46 @@ void testInspectImage(@Mock DockerClient dc,
   }
 
   @Test
-  void testPullImageSingleUnknown(@Mock DockerClient dc,
+  void testPullImageSingleUnknown(@Mock DockerConfig config, 
+      @Mock DockerClient dc,
       @Mock ListImagesCmd lic,
       @Mock PullImageCmd pic) throws InterruptedException {
 
     String imageName = "test";
 
-    when(dc.listImagesCmd()).thenReturn(lic);
-    when(lic.exec()).thenReturn(new ArrayList<Image>());
+    lenient().when(config.getDockerClient()).thenReturn(dc);
+    lenient().when(config.getPullPolicy()).thenReturn(DockerConfig.PullPolicy.IF_NOT_PRESENT);
+    lenient().when(dc.listImagesCmd()).thenReturn(lic);
+    lenient().when(lic.exec()).thenReturn(new ArrayList<Image>());
 
     when(dc.pullImageCmd(eq(imageName))).thenReturn(pic);
 
-    ImageUtils.pullImages(dc, 0, imageName);
+    ImageUtils.pullImages(config, imageName);
 
     verify(pic).exec(ArgumentMatchers.any());
   }
 
   @Test
-  void testPullImageSingleKnown(@Mock DockerClient dc,
+  void testPullImageSingleKnown(@Mock DockerConfig config, 
+      @Mock DockerClient dc,
       @Mock ListImagesCmd lic,
       @Mock Image i,
       @Mock PullImageCmd pic) throws InterruptedException {
 
     String imageName = "test";
-
-    when(dc.listImagesCmd()).thenReturn(lic);
+
+    lenient().when(config.getDockerClient()).thenReturn(dc);
+    lenient().when(config.getPullPolicy()).thenReturn(DockerConfig.PullPolicy.IF_NOT_PRESENT);
+    lenient().when(dc.listImagesCmd()).thenReturn(lic);
+
     List<Image> li = new ArrayList<Image>();
     li.add(i);
     when(lic.exec()).thenReturn(li);
     when(i.getRepoTags()).thenReturn(new String[] {imageName});
 
     //when(dc.pullImageCmd(eq(imageName))).thenReturn(pic);
 
-    ImageUtils.pullImages(dc, 0, imageName);
+    ImageUtils.pullImages(config, imageName);
 
     verify(pic, never()).exec(ArgumentMatchers.any());
   }