Skip to content

Commit 27fc32e

Browse files
BarDwellerBarDweller
committed
Add pull policy & retry
1 parent fb6f150 commit 27fc32e

File tree

8 files changed

+154
-62
lines changed

8 files changed

+154
-62
lines changed

‎client/src/main/java/dev/snowdrop/buildpack/BuilderImage.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ public BuilderImage(DockerConfig dc, PlatformConfig pc, ImageReference runImage,
4444
image = builderImage;
4545

4646
// pull and inspect the builderImage to obtain builder metadata.
47-
ImageUtils.pullImages(dc.getDockerClient(),
48-
dc.getPullTimeout(),
49-
builderImage.getReference());
47+
ImageUtils.pullImages(dc, builderImage.getReference());
5048

5149
ImageInfo ii = ImageUtils.inspectImage(dc.getDockerClient(),
5250
builderImage.getReference());

‎client/src/main/java/dev/snowdrop/buildpack/config/DockerConfig.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,15 @@ public static DockerConfigBuilder builder() {
1212
return new DockerConfigBuilder();
1313
}
1414

15+
public static enum PullPolicy {ALWAYS, IF_NOT_PRESENT};
16+
1517
private static final Integer DEFAULT_PULL_TIMEOUT = 60;
18+
private static final Integer DEFAULT_PULL_RETRY_COUNT = 3;
19+
private static final PullPolicy DEFAULT_PULL_POLICY = PullPolicy.IF_NOT_PRESENT;
1620

1721
private Integer pullTimeoutSeconds;
22+
private Integer pullRetryCount;
23+
private PullPolicy pullPolicy;
1824
private String dockerHost;
1925
private String dockerSocket;
2026
private String dockerNetwork;
@@ -23,13 +29,17 @@ public static DockerConfigBuilder builder() {
2329

2430
public DockerConfig(
2531
Integer pullTimeoutSeconds,
32+
Integer pullRetryCount,
33+
PullPolicy pullPolicy,
2634
String dockerHost,
2735
String dockerSocket,
2836
String dockerNetwork,
2937
Boolean useDaemon,
3038
DockerClient dockerClient
3139
){
3240
this.pullTimeoutSeconds = pullTimeoutSeconds != null ? pullTimeoutSeconds : DEFAULT_PULL_TIMEOUT;
41+
this.pullRetryCount = pullRetryCount != null ? pullRetryCount : DEFAULT_PULL_RETRY_COUNT;
42+
this.pullPolicy = pullPolicy != null ? pullPolicy : DEFAULT_PULL_POLICY;
3343
this.dockerHost = dockerHost != null ? dockerHost : DockerClientUtils.getDockerHost();
3444
this.dockerSocket = dockerSocket != null ? dockerSocket : (this.dockerHost.startsWith("unix://") ? this.dockerHost.substring("unix://".length()) : "/var/run/docker.sock");
3545
this.dockerNetwork = dockerNetwork;
@@ -47,6 +57,14 @@ public Integer getPullTimeout(){
4757
return this.pullTimeoutSeconds;
4858
}
4959

60+
public Integer getPullRetryCount(){
61+
return this.pullRetryCount;
62+
}
63+
64+
public PullPolicy getPullPolicy(){
65+
return this.pullPolicy;
66+
}
67+
5068
public String getDockerHost(){
5169
return this.dockerHost;
5270
}

‎client/src/main/java/dev/snowdrop/buildpack/docker/ImageUtils.java

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
import java.util.ArrayList;
44
import java.util.Arrays;
5+
import java.util.HashMap;
56
import java.util.HashSet;
67
import java.util.List;
78
import java.util.Map;
9+
import java.util.Map.Entry;
810
import java.util.Set;
911
import java.util.concurrent.TimeUnit;
1012

@@ -15,7 +17,9 @@
1517
import com.github.dockerjava.api.command.InspectImageResponse;
1618
import com.github.dockerjava.api.command.PullImageResultCallback;
1719
import com.github.dockerjava.api.model.Image;
18-
20+
import com.github.dockerjava.api.exception.DockerClientException;
21+
22+
import dev.snowdrop.buildpack.config.DockerConfig;
1923
import dev.snowdrop.buildpack.BuildpackException;
2024
/**
2125
* Higher level docker image api
@@ -30,51 +34,81 @@ public static class ImageInfo {
3034
}
3135

3236
/**
33-
* Util method to pull images if they don't exist to the local docker yet.
37+
* Util method to pull images, configure behavior via dockerconfig.
3438
*/
35-
public static void pullImages(DockerClient dc, int timeoutSeconds, String... imageNames) {
39+
public static void pullImages(DockerConfig config, String... imageNames) {
3640
Set<String> imageNameSet = new HashSet<>(Arrays.asList(imageNames));
3741

38-
// list the current known images
39-
List<Image> li = dc.listImagesCmd().exec();
40-
for (Image i : li) {
41-
if (i.getRepoTags() == null) {
42-
continue;
43-
}
44-
for (String it : i.getRepoTags()) {
45-
if (imageNameSet.contains(it)) {
46-
imageNameSet.remove(it);
42+
DockerClient dc = config.getDockerClient();
43+
44+
//if using ifnotpresent, filter set to unknown images.
45+
if(config.getPullPolicy() == DockerConfig.PullPolicy.IF_NOT_PRESENT) {
46+
// list the current known images
47+
List<Image> li = dc.listImagesCmd().exec();
48+
for (Image i : li) {
49+
if (i.getRepoTags() == null) {
50+
continue;
51+
}
52+
for (String it : i.getRepoTags()) {
53+
if (imageNameSet.contains(it)) {
54+
imageNameSet.remove(it);
55+
}
4756
}
4857
}
49-
}
5058

51-
if (imageNameSet.isEmpty()) {
52-
// fast exit if all images are already known to the local docker.
53-
log.debug("Nothing to pull, all of " + Arrays.asList(imageNames) + " are known");
54-
return;
59+
if (imageNameSet.isEmpty()) {
60+
// fast exit if all images are already known to the local docker.
61+
log.debug("Nothing to pull, all of " + Arrays.asList(imageNames) + " are known");
62+
return;
63+
}
5564
}
5665

57-
// pull the images not known
58-
List<PullImageResultCallback> pircs = new ArrayList<>();
66+
int retriesRemaining = Integer.max(config.getPullRetryCount(), 0);
67+
Map<String,PullImageResultCallback> pircMap = new HashMap<>();
68+
69+
// pull the images still in set.
5970
for (String stillNeeded : imageNameSet) {
6071
log.debug("pulling '" + stillNeeded + "'");
6172
PullImageResultCallback pirc = new PullImageResultCallback();
6273
dc.pullImageCmd(stillNeeded).exec(pirc);
63-
pircs.add(pirc);
74+
pircMap.put(stillNeeded,pirc);
6475
}
6576

6677
// wait for pulls to complete.
67-
for (PullImageResultCallback pirc : pircs) {
68-
try {
69-
pirc.awaitCompletion(timeoutSeconds, TimeUnit.SECONDS);
70-
} catch (InterruptedException e) {
71-
throw BuildpackException.launderThrowable(e);
78+
DockerClientException lastSeen = null;
79+
boolean allDone = true;
80+
while(!allDone && retriesRemaining>=0){
81+
allDone = true;
82+
for (Entry<String, PullImageResultCallback> e : pircMap.entrySet()) {
83+
boolean done = false;
84+
try {
85+
if(e.getValue()==null) continue;
86+
done = e.getValue().awaitCompletion(config.getPullTimeout(), TimeUnit.SECONDS);
87+
} catch (InterruptedException ie) {
88+
throw BuildpackException.launderThrowable(ie);
89+
} catch (DockerClientException dce) {
90+
//error occurred during pull for this pirc, need to pause & retry the pull op
91+
lastSeen = dce;
92+
}
93+
if(!done){
94+
String imageName = e.getKey();
95+
PullImageResultCallback newPirc = new PullImageResultCallback();
96+
dc.pullImageCmd(imageName).exec(newPirc);
97+
e.setValue(newPirc);
98+
allDone=false;
99+
}else{
100+
e.setValue(null);
101+
}
72102
}
103+
retriesRemaining-=1;
73104
}
74105

75-
// TODO: progress tracking..
106+
if(lastSeen!=null){
107+
throw lastSeen;
108+
}
76109
}
77110

111+
78112
/**
79113
* Util method to retrieve info for a given docker image.
80114
*/

‎client/src/main/java/dev/snowdrop/buildpack/lifecycle/LifecycleExecutor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public int execute() {
105105
}
106106

107107
//pull the new image..
108-
ImageUtils.pullImages(config.getDockerConfig().getDockerClient(), factory.getDockerConfig().getPullTimeout(), newRunImage);
108+
ImageUtils.pullImages(config.getDockerConfig(), newRunImage);
109109

110110
//update run image associated with our builder image.
111111
factory.getBuilderImage().getRunImages(activePlatformLevel).clear();

‎client/src/main/java/dev/snowdrop/buildpack/utils/LifecycleMetadata.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ public class LifecycleMetadata {
2020
public LifecycleMetadata(DockerConfig dc, ImageReference lifecycleImage) throws BuildpackException {
2121

2222
// pull and inspect the builderImage to obtain builder metadata.
23-
ImageUtils.pullImages(dc.getDockerClient(),
24-
dc.getPullTimeout(),
25-
lifecycleImage.getReference());
23+
ImageUtils.pullImages(dc,lifecycleImage.getReference());
2624

2725
ImageInfo ii = ImageUtils.inspectImage(dc.getDockerClient(),
2826
lifecycleImage.getReference());

‎client/src/test/java/dev/snowdrop/buildpack/config/DockerConfigTest.java

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,21 @@
2020
public class DockerConfigTest {
2121
@Test
2222
void checkTimeout() {
23-
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
23+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
2424
assertEquals(60, dc1.getPullTimeout());
2525

26-
DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null);
26+
DockerConfig dc2 = new DockerConfig(245017, null, null, null, null, null, null, null);
2727
assertEquals(dc2.getPullTimeout(), 245017);
2828
}
2929

3030
@Test
3131
void checkDockerHost(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
32-
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
32+
lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
33+
34+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
3335
assertNotNull(dc1.getDockerHost());
3436

35-
when(dockerClient.pingCmd()).thenReturn(pingCmd);
36-
DockerConfig dc2 = new DockerConfig(null, "tcp://stilettos", null, null, null, dockerClient);
37+
DockerConfig dc2 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
3738
assertEquals("tcp://stilettos", dc2.getDockerHost());
3839
}
3940

@@ -42,48 +43,77 @@ void checkDockerSocket(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd) {
4243

4344
lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
4445

45-
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
46+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
4647
assertNotNull(dc1.getDockerSocket());
4748

48-
DockerConfig dc2 = new DockerConfig(null, "unix:///stilettos", null, null, null, dockerClient);
49+
DockerConfig dc2 = new DockerConfig(null, null, null, "unix:///stilettos", null, null, null, dockerClient);
4950
assertEquals("/stilettos", dc2.getDockerSocket());
5051

51-
DockerConfig dc3 = new DockerConfig(null, "tcp://stilettos", null, null, null, dockerClient);
52+
DockerConfig dc3 = new DockerConfig(null, null, null, "tcp://stilettos", null, null, null, dockerClient);
5253
assertEquals("/var/run/docker.sock", dc3.getDockerSocket());
5354

54-
DockerConfig dc4 = new DockerConfig(null, null, "fish", null, null, null);
55+
DockerConfig dc4 = new DockerConfig(null, null, null, null, "fish", null, null, null);
5556
assertEquals("fish", dc4.getDockerSocket());
5657
}
5758

5859
@Test
5960
void checkDockerNetwork() {
60-
DockerConfig dc1 = new DockerConfig(null, null, null, "kitten", null, null);
61+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, "kitten", null, null);
6162
assertEquals("kitten", dc1.getDockerNetwork());
6263

63-
DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null);
64+
DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, null);
6465
assertNull(dc2.getDockerNetwork());
6566
}
6667

6768
@Test
6869
void checkUseDaemon() {
69-
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
70+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
7071
assertTrue(dc1.getUseDaemon());
7172

72-
DockerConfig dc2 = new DockerConfig(null, null, null, null, true, null);
73+
DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, true, null);
7374
assertTrue(dc2.getUseDaemon());
7475

75-
DockerConfig dc3 = new DockerConfig(null, null, null, null, false, null);
76+
DockerConfig dc3 = new DockerConfig(null, null, null, null, null, null, false, null);
7677
assertFalse(dc3.getUseDaemon());
7778
}
7879

7980
@Test
8081
void checkDockerClient(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
8182
lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
8283

83-
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null);
84+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, null);
8485
assertNotNull(dc1.getDockerClient());
8586

86-
DockerConfig dc2 = new DockerConfig(null, null, null, null, null, dockerClient);
87+
DockerConfig dc2 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
8788
assertEquals(dockerClient, dc2.getDockerClient());
8889
}
90+
91+
@Test
92+
void checkPullPolicy(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
93+
lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
94+
95+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
96+
assertEquals(DockerConfig.PullPolicy.IF_NOT_PRESENT, dc1.getPullPolicy());
97+
98+
DockerConfig dc2 = new DockerConfig(null, null, DockerConfig.PullPolicy.IF_NOT_PRESENT, null, null, null, null, dockerClient);
99+
assertEquals(DockerConfig.PullPolicy.IF_NOT_PRESENT, dc2.getPullPolicy());
100+
101+
DockerConfig dc3 = new DockerConfig(null, null, DockerConfig.PullPolicy.ALWAYS, null, null, null, null, dockerClient);
102+
assertEquals(DockerConfig.PullPolicy.ALWAYS, dc3.getPullPolicy());
103+
}
104+
105+
106+
@Test
107+
void checkPullRetry(@Mock DockerClient dockerClient, @Mock PingCmd pingCmd){
108+
lenient().when(dockerClient.pingCmd()).thenReturn(pingCmd);
109+
110+
DockerConfig dc1 = new DockerConfig(null, null, null, null, null, null, null, dockerClient);
111+
assertEquals(3, dc1.getPullRetryCount());
112+
113+
DockerConfig dc2 = new DockerConfig(null, 5, null, null, null, null, null, dockerClient);
114+
assertEquals(5, dc2.getPullRetryCount());
115+
116+
DockerConfig dc3 = new DockerConfig(null, 0, null, null, null, null, null, dockerClient);
117+
assertEquals(0, dc3.getPullRetryCount());
118+
}
89119
}

‎client/src/test/java/dev/snowdrop/buildpack/docker/ImageUtilsTest.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import static org.mockito.Mockito.verify;
77
import static org.mockito.Mockito.when;
88
import static org.mockito.Mockito.never;
9+
import static org.mockito.Mockito.lenient;
910

1011
import java.util.ArrayList;
1112
import java.util.HashMap;
@@ -26,6 +27,7 @@
2627
import com.github.dockerjava.api.model.ContainerConfig;
2728
import com.github.dockerjava.api.model.Image;
2829

30+
import dev.snowdrop.buildpack.config.DockerConfig;
2931
import dev.snowdrop.buildpack.docker.ImageUtils.ImageInfo;
3032

3133
@ExtendWith(MockitoExtension.class)
@@ -61,39 +63,46 @@ void testInspectImage(@Mock DockerClient dc,
6163
}
6264

6365
@Test
64-
void testPullImageSingleUnknown(@Mock DockerClient dc,
66+
void testPullImageSingleUnknown(@Mock DockerConfig config,
67+
@Mock DockerClient dc,
6568
@Mock ListImagesCmd lic,
6669
@Mock PullImageCmd pic) throws InterruptedException {
6770

6871
String imageName = "test";
6972

70-
when(dc.listImagesCmd()).thenReturn(lic);
71-
when(lic.exec()).thenReturn(new ArrayList<Image>());
73+
lenient().when(config.getDockerClient()).thenReturn(dc);
74+
lenient().when(config.getPullPolicy()).thenReturn(DockerConfig.PullPolicy.IF_NOT_PRESENT);
75+
lenient().when(dc.listImagesCmd()).thenReturn(lic);
76+
lenient().when(lic.exec()).thenReturn(new ArrayList<Image>());
7277

7378
when(dc.pullImageCmd(eq(imageName))).thenReturn(pic);
7479

75-
ImageUtils.pullImages(dc, 0, imageName);
80+
ImageUtils.pullImages(config, imageName);
7681

7782
verify(pic).exec(ArgumentMatchers.any());
7883
}
7984

8085
@Test
81-
void testPullImageSingleKnown(@Mock DockerClient dc,
86+
void testPullImageSingleKnown(@Mock DockerConfig config,
87+
@Mock DockerClient dc,
8288
@Mock ListImagesCmd lic,
8389
@Mock Image i,
8490
@Mock PullImageCmd pic) throws InterruptedException {
8591

8692
String imageName = "test";
87-
88-
when(dc.listImagesCmd()).thenReturn(lic);
93+
94+
lenient().when(config.getDockerClient()).thenReturn(dc);
95+
lenient().when(config.getPullPolicy()).thenReturn(DockerConfig.PullPolicy.IF_NOT_PRESENT);
96+
lenient().when(dc.listImagesCmd()).thenReturn(lic);
97+
8998
List<Image> li = new ArrayList<Image>();
9099
li.add(i);
91100
when(lic.exec()).thenReturn(li);
92101
when(i.getRepoTags()).thenReturn(new String[] {imageName});
93102

94103
//when(dc.pullImageCmd(eq(imageName))).thenReturn(pic);
95104

96-
ImageUtils.pullImages(dc, 0, imageName);
105+
ImageUtils.pullImages(config, imageName);
97106

98107
verify(pic, never()).exec(ArgumentMatchers.any());
99108
}

0 commit comments

Comments
 (0)