Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow retries for pull #994

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/asciidoc/inc/build/_configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ a| Scan the archive specified in `dockerArchive` and find the actual repository
| *shell*
| Shell to be used for the *runCmds*. It contains *arg* elements which are defining the executable and its params.

| *retries*
| If pulling an image is required, how often should a pull be retried before giving up. This useful for flaky registries which tend to return 500 error codes from time to time. The default is 0 which means no retry at all.

| *runCmds*
| Commands to be run during the build process. It contains *run* elements which are passed to the shell. Whitespace is trimmed from each element and empty elements are ignored. The run commands are inserted right after the assembly and after *workdir* into the Dockerfile. This tag is not to be confused with the `<run>` section for this image which specifies the runtime behaviour when starting containers.

Expand Down
5 changes: 4 additions & 1 deletion src/main/asciidoc/inc/start/_configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ In addition to the <<global-configuration>>, this goal supports the following gl
| Default pattern for naming all containers when they are created. See <<container-name, Container Names>> for details.
| `docker.containerNamePattern`

| *retries*
| If pulling an image is required, how often should a pull be retried before giving up. This useful for flaky registries which tend to return 500 error codes from time to time. The default is 0 which means no retry at all.
| `docker.pull.retries`

| *showLogs*
| In order to switch on globally the logs *showLogs* can be used as global configuration (i.e. outside of `<images>`). If set it will print out all standard
output and standard error messages for all containers started. As value the images for which logs should be shown can be given as a comma separated list.
Expand All @@ -18,7 +22,6 @@ In addition to the <<global-configuration>>, this goal supports the following gl
| *startParallel*
| Starts docker images in parallel while dependencies expressed as <<start-links,Link>> or <<start-depends-on,dependsOn>> are respected. This option can significantly reduce the startup time because independent containers do not need to wait for each other.
| `docker.startParallel`

|===

The `<run>` configuration element knows the following sub elements:
Expand Down
18 changes: 17 additions & 1 deletion src/main/asciidoc/inc/watch/_configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,25 @@ below how this can be specified.
| A command which is executed within the container after files are copied into this container when `watchMode` is `copy`. Note that this container must be running.
|

| *keepRunning*
| If set to `true` all container will be kept running after `{plugin}:watch` has been stopped. By default this is set to `false`.
| `docker.keepRunning`

| *keepContainer*
| As for `{plugin}:stop`, if this is set to `true` (and `keepRunning` is disabled) then all container will be removed after they have been stopped. The default is `true`.
| `docker.keepContainer`

| *removeVolumes*
| if set to `true` will remove any volumes associated to the container as well. This option will be ignored if either `keepContainer` or `keepRunning` are `true`.
| `docker.removeVolumes`

| *retries*
| If pulling an image is required, how often should a pull be retried before giving up. This useful for flaky registries which tend to return 500 error codes from time to time. The default is 0 which means no retry at all.
| `docker.pull.retries`

| *watchPostGoal*
| A maven goal which should be called if a rebuild or a restart has been performed. This goal must have the format `<pluginGroupId>:<pluginArtifactId>:<goal>` and the plugin must be configured in the `pom.xml`. For example a post-goal `io.fabric8:fabric8:delete-pods` will trigger the deletion of PODs in Kubernetes which in turn triggers are new start of a POD within the Kubernetes cluster. The value specified here is the the default post goal which can be overridden by `<postGoal>` in a `<watch>` configuration.
|

|===

Image specific watch configuration goes into an extra image-level `<watch>` section (i.e. `+<image><watch>...</watch></image>+`). The following parameters are recognized:
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/io/fabric8/maven/docker/BuildMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ public class BuildMojo extends AbstractBuildSupportMojo {
@Parameter(property = "docker.skip.build", defaultValue = "false")
protected boolean skipBuild;

@Parameter(property = "docker.pull.retries", defaultValue = "0")
private int retries;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we please rename this in pullRetries for the member configuration ? "retries" alone is a bit unspecific.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for the other MoJos please and the documentation.


@Parameter(property = "docker.skip.pom", defaultValue = "false")
protected boolean skipPom;

Expand Down Expand Up @@ -101,8 +104,9 @@ private void proceedWithJibBuild(ServiceHub hub, BuildService.BuildContext build
private void proceedWithDockerBuild(BuildService buildService, BuildService.BuildContext buildContext, ImageConfiguration imageConfig, ImagePullManager pullManager) throws MojoExecutionException, IOException {
File buildArchiveFile = buildService.buildArchive(imageConfig, buildContext, resolveBuildArchiveParameter());
if (Boolean.FALSE.equals(shallBuildArchiveOnly())) {
buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile);
buildService.buildImage(imageConfig, pullManager, buildContext, retries, buildArchiveFile);
}

if (!skipTag) {
buildService.tagImage(imageConfig);
}
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/io/fabric8/maven/docker/StartMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ public class StartMojo extends AbstractDockerMojo {
@Parameter(property = "docker.startParallel", defaultValue = "false")
private boolean startParallel;

@Parameter(property = "docker.pull.retries", defaultValue = "0")
private int retries;

// whether to block during to start. Set it via System property docker.follow
private boolean follow;

Expand Down Expand Up @@ -338,7 +341,7 @@ private Queue<ImageConfiguration> prepareStart(ServiceHub hub, QueryService quer
ImagePullManager pullManager = getImagePullManager(determinePullPolicy(runConfig), autoPull);

hub.getRegistryService().pullImageWithPolicy(imageConfig.getName(), pullManager, registryConfig,
queryService.hasImage(imageConfig.getName()));
queryService.hasImage(imageConfig.getName()), retries);

NetworkConfig config = runConfig.getNetworkingConfig();
List<String> bindMounts = extractBindMounts(runConfig.getVolumeConfiguration());
Expand Down
5 changes: 4 additions & 1 deletion src/main/java/io/fabric8/maven/docker/WatchMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,17 @@ public class WatchMojo extends AbstractBuildSupportMojo {
@Parameter(property = "docker.autoCreateCustomNetworks", defaultValue = "false")
protected boolean autoCreateCustomNetworks;

@Parameter(property = "docker.pull.retries", defaultValue = "0")
private int retries;

@Override
protected synchronized void executeInternal(ServiceHub hub) throws IOException,
MojoExecutionException {

BuildService.BuildContext buildContext = getBuildContext();
WatchService.WatchContext watchContext = getWatchContext(hub);

hub.getWatchService().watch(watchContext, buildContext, getResolvedImages());
hub.getWatchService().watch(watchContext, buildContext, getResolvedImages(), retries);
}

protected WatchService.WatchContext getWatchContext(ServiceHub hub) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ void copyArchive(String containerId, File archive, String targetPath)
* @param registry an optional registry from where to pull the image. Can be null.
* @throws DockerAccessException if the image couldn't be pulled.
*/
void pullImage(String image, AuthConfig authConfig, String registry) throws DockerAccessException;
void pullImage(String image, AuthConfig authConfig, String registry, int retries) throws DockerAccessException;

/**
* Push an image to a registry. An registry can be specified which is used as target
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,14 +469,13 @@ public void loadImage(String image, File tarArchive) throws DockerAccessExceptio
}

@Override
public void pullImage(String image, AuthConfig authConfig, String registry)
public void pullImage(String image, AuthConfig authConfig, String registry, int retries)
throws DockerAccessException {
ImageName name = new ImageName(image);
String pullUrl = urlBuilder.pullImage(name, registry);

try {
delegate.post(pullUrl, null, createAuthHeader(authConfig),
createPullOrPushResponseHandler(), HTTP_OK);
doPushPullImage(pullUrl, createAuthHeader(authConfig), createBuildResponseHandler(), HTTP_OK, retries);
} catch (IOException e) {
throw new DockerAccessException(e, "Unable to pull '%s'%s", image, (registry != null) ? " from registry '" + registry + "'" : "");
}
Expand All @@ -490,7 +489,7 @@ public void pushImage(String image, AuthConfig authConfig, String registry, int
TemporaryImageHandler temporaryImageHandler = tagTemporaryImage(name, registry);
DockerAccessException dae = null;
try {
doPushImage(pushUrl, createAuthHeader(authConfig), createPullOrPushResponseHandler(), HTTP_OK, retries);
doPushPullImage(pushUrl, createAuthHeader(authConfig), createPullOrPushResponseHandler(), HTTP_OK, retries);
} catch (IOException e) {
dae = new DockerAccessException(e, "Unable to push '%s'%s", image, (registry != null) ? " to registry '" + registry + "'" : "");
throw dae;
Expand Down Expand Up @@ -695,16 +694,16 @@ private boolean isRetryableErrorCode(int errorCode) {
return errorCode == HTTP_INTERNAL_ERROR;
}

private void doPushImage(String url, Map<String, String> header, HcChunkedResponseHandlerWrapper handler, int status,
private void doPushPullImage(String url, Map<String, String> header, HcChunkedResponseHandlerWrapper handler, int status,
int retries) throws IOException {
// 0: The original attemp, 1..retry: possible retries.
// 0: The original attempt, 1..retry: possible retries.
for (int i = 0; i <= retries; i++) {
try {
delegate.post(url, null, header, handler, HTTP_OK);
delegate.post(url, null, header, handler, status);
return;
} catch (HttpResponseException e) {
if (isRetryableErrorCode(e.getStatusCode()) && i != retries) {
log.warn("failed to push image to [{}], retrying...", url);
log.warn("failed to push/pull image to/from [%s], retrying...", url);
} else {
throw e;
}
Expand Down
15 changes: 9 additions & 6 deletions src/main/java/io/fabric8/maven/docker/service/BuildService.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,19 @@ public class BuildService {
*
* @param imageConfig the image configuration
* @param buildContext the build context
* @param pullRetries the number of times to retry if pulling an image fails
* @throws DockerAccessException
* @throws MojoExecutionException
*/
public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, File buildArchiveFile)

public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, int retries, File buildArchiveFile)
throws DockerAccessException, MojoExecutionException {

if (imagePullManager != null) {
autoPullBaseImage(imageConfig, imagePullManager, buildContext);
autoPullBaseImage(imageConfig, imagePullManager, buildContext, pullRetries);
}

buildImage(imageConfig, buildContext.getMojoParameters(), checkForNocache(imageConfig), checkForSquash(imageConfig), addBuildArgs(buildContext), buildArchiveFile);
buildImage(imageConfig, buildContext.getMojoParameters(), checkForNocache(imageConfig), checkForSquash(imageConfig), addBuildArgs(buildContext), retries, buildArchiveFile);
}

/**
Expand Down Expand Up @@ -136,7 +138,7 @@ public void tagImage(ImageConfiguration imageConfig) throws DockerAccessExceptio
* @throws DockerAccessException
* @throws MojoExecutionException
*/
protected void buildImage(ImageConfiguration imageConfig, MojoParameters params, boolean noCache, boolean squash, Map<String, String> buildArgs, File dockerArchive)
protected void buildImage(ImageConfiguration imageConfig, MojoParameters params, boolean noCache, boolean squash, Map<String, String> buildArgs, int retries, File dockerArchive)
throws DockerAccessException, MojoExecutionException {

String imageName = imageConfig.getName();
Expand Down Expand Up @@ -362,7 +364,7 @@ private Map<String, String> addBuildArgsFromDockerConfig() {
return buildArgs;
}

private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext)
private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, int retries)
throws DockerAccessException, MojoExecutionException {
BuildImageConfiguration buildConfig = imageConfig.getBuildConfiguration();

Expand All @@ -381,9 +383,10 @@ private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager
fromImages.add(extractBaseFromConfiguration(buildConfig));
}
}

for (String fromImage : fromImages) {
if (fromImage != null && !DockerAssemblyManager.SCRATCH_IMAGE.equals(fromImage)) {
registryService.pullImageWithPolicy(fromImage, imagePullManager, buildContext.getRegistryConfig(), queryService.hasImage(fromImage));
registryService.pullImageWithPolicy(fromImage, imagePullManager, buildContext.getRegistryConfig(), queryService.hasImage(fromImage), retries);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void pushImages(Collection<ImageConfiguration> imageConfigs,
* @throws DockerAccessException
* @throws MojoExecutionException
*/
public void pullImageWithPolicy(String image, ImagePullManager pullManager, RegistryConfig registryConfig, boolean hasImage)
public void pullImageWithPolicy(String image, ImagePullManager pullManager, RegistryConfig registryConfig, boolean hasImage, int retries)
throws DockerAccessException, MojoExecutionException {

// Already pulled, so we don't need to take care
Expand All @@ -102,7 +102,7 @@ public void pullImageWithPolicy(String image, ImagePullManager pullManager, Regi
imageName.getRegistry(),
registryConfig.getRegistry());
docker.pullImage(imageName.getFullName(),
createAuthConfig(false, imageName.getUser(), actualRegistry, registryConfig), actualRegistry);
createAuthConfig(false, imageName.getUser(), actualRegistry, registryConfig), actualRegistry, retries);
log.info("Pulled %s in %s", imageName.getFullName(), EnvUtil.formatDurationTill(time));
pullManager.pulled(image);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public WatchService(ArchiveService archiveService, BuildService buildService, Do
this.log = log;
}

public synchronized void watch(WatchContext context, BuildService.BuildContext buildContext, List<ImageConfiguration> images) throws DockerAccessException,
public synchronized void watch(WatchContext context, BuildService.BuildContext buildContext, List<ImageConfiguration> images, int pullRetries) throws DockerAccessException,
MojoExecutionException {

// Important to be be a single threaded scheduler since watch jobs must run serialized
Expand Down Expand Up @@ -85,7 +85,7 @@ public synchronized void watch(WatchContext context, BuildService.BuildContext b
}

if (watcher.isBuild()) {
schedule(executor, createBuildWatchTask(watcher, context.getMojoParameters(), watchMode == WatchMode.both, buildContext), interval);
schedule(executor, createBuildWatchTask(watcher, context.getMojoParameters(), watchMode == WatchMode.both, buildContext, pullRetries), interval);
tasks.add("rebuilding");
}
}
Expand Down Expand Up @@ -151,7 +151,7 @@ private void callPostExec(ImageWatcher watcher) throws DockerAccessException, Ex
}

private Runnable createBuildWatchTask(final ImageWatcher watcher,
final MojoParameters mojoParameters, final boolean doRestart, final BuildService.BuildContext buildContext)
final MojoParameters mojoParameters, final boolean doRestart, final BuildService.BuildContext buildContext, final int pullRetries)
throws MojoExecutionException {
final ImageConfiguration imageConfig = watcher.getImageConfiguration();
final AssemblyFiles files = archiveService.getAssemblyFiles(imageConfig, mojoParameters);
Expand All @@ -173,7 +173,7 @@ public void run() {
watcher.getWatchContext().getImageCustomizer().execute(imageConfig);
}

buildService.buildImage(imageConfig, null, buildContext, buildService.buildArchive(imageConfig, buildContext, "false"));
buildService.buildImage(imageConfig, null, buildContext, pullRetries, buildService.buildArchive(imageConfig, buildContext, "false"));

String name = imageConfig.getName();
watcher.setImageId(queryService.getImageId(name));
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/integration/DockerAccessIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private void testDoesNotHave() throws DockerAccessException {
}

private void testPullImage() throws DockerAccessException {
dockerClient.pullImage(IMAGE, null, null);
dockerClient.pullImage(IMAGE, null, null, 0);
assertTrue(hasImage(IMAGE));
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/integration/DockerAccessWinIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private void testDoesNotHave() throws DockerAccessException {
}

private void testPullImage() throws DockerAccessException {
dockerClient.pullImage(IMAGE, null, null);
dockerClient.pullImage(IMAGE, null, null, 0);
assertTrue(hasImage(IMAGE));
}

Expand Down
Loading