From e823b558cdca97a51b5da4e0f219cc237404dc79 Mon Sep 17 00:00:00 2001 From: Ben Kelley Date: Tue, 20 Feb 2018 12:08:13 +1100 Subject: [PATCH 1/2] Allow the "retries" parameter to apply to pull as well as push. Signed-off-by: Ben Kelley --- .../asciidoc/inc/build/_configuration.adoc | 3 + .../asciidoc/inc/start/_configuration.adoc | 5 +- .../asciidoc/inc/watch/_configuration.adoc | 16 ++++ .../maven/docker/AbstractDockerMojo.java | 7 +- .../io/fabric8/maven/docker/BuildMojo.java | 4 +- .../io/fabric8/maven/docker/StartMojo.java | 2 + .../io/fabric8/maven/docker/WatchMojo.java | 5 +- .../maven/docker/access/DockerAccess.java | 2 +- .../access/hc/DockerAccessWithHcClient.java | 13 ++- .../maven/docker/service/BuildService.java | 15 ++-- .../maven/docker/service/RegistryService.java | 4 +- .../maven/docker/service/WatchService.java | 12 ++- src/test/java/integration/DockerAccessIT.java | 2 +- .../java/integration/DockerAccessWinIT.java | 2 +- .../fabric8/maven/docker/BuildMojoTest.java | 2 +- .../io/fabric8/maven/docker/CopyMojoTest.java | 2 +- .../hc/DockerAccessWithHcClientTest.java | 81 ++++++++++++++++--- .../docker/service/BuildServiceTest.java | 10 +-- .../docker/service/RegistryServiceTest.java | 8 +- .../docker/service/WatchServiceTest.java | 2 +- 20 files changed, 141 insertions(+), 56 deletions(-) diff --git a/src/main/asciidoc/inc/build/_configuration.adoc b/src/main/asciidoc/inc/build/_configuration.adoc index 9c67403c2..0c1e76326 100644 --- a/src/main/asciidoc/inc/build/_configuration.adoc +++ b/src/main/asciidoc/inc/build/_configuration.adoc @@ -120,6 +120,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 `` section for this image which specifies the runtime behaviour when starting containers. diff --git a/src/main/asciidoc/inc/start/_configuration.adoc b/src/main/asciidoc/inc/start/_configuration.adoc index 6966d7673..42fa5fe39 100644 --- a/src/main/asciidoc/inc/start/_configuration.adoc +++ b/src/main/asciidoc/inc/start/_configuration.adoc @@ -9,6 +9,10 @@ In addition to the <>, this goal supports the following gl | Default pattern for naming all containers when they are created. See <> 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 ``). 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. @@ -18,7 +22,6 @@ In addition to the <>, this goal supports the following gl | *startParallel* | Starts docker images in parallel while dependencies expressed as <> or <> are respected. This option can significantly reduce the startup time because independent containers do not need to wait for each other. | `docker.startParallel` - |=== The `` configuration element knows the following sub elements: diff --git a/src/main/asciidoc/inc/watch/_configuration.adoc b/src/main/asciidoc/inc/watch/_configuration.adoc index 99ee7e2b6..1f8be74fb 100644 --- a/src/main/asciidoc/inc/watch/_configuration.adoc +++ b/src/main/asciidoc/inc/watch/_configuration.adoc @@ -49,6 +49,22 @@ below how this can be specified. | *watchPostGoal* | A maven goal which should be called if a rebuild or a restart has been performed. This goal must have the format `::` 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 `` in a `` configuration. | + +| *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` |=== Image specific watch configuration goes into an extra image-level `` section (i.e. `+...+`). The following parameters are recognized: diff --git a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java index cde5d49d3..4175db720 100644 --- a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java +++ b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java @@ -121,6 +121,9 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context @Parameter(property = "docker.removeVolumes", defaultValue = "false") protected boolean removeVolumes; + @Parameter(property = "docker.pull.retries", defaultValue = "0") + protected int retries; + @Parameter(property = "docker.apiVersion") private String apiVersion; @@ -577,7 +580,7 @@ protected void pullImage(RegistryService registryService, ImageConfiguration ima RunImageConfiguration runConfiguration = imageConfig.getRunConfiguration(); ImagePullManager pullManager = getImagePullManager(determinePullPolicy(runConfiguration), autoPull); RegistryConfig registryConfig = getRegistryConfig(pullRegistry); - registryService.pullImageWithPolicy(imageName, pullManager, registryConfig, imageConfig.getBuildConfiguration()); + registryService.pullImageWithPolicy(imageName, pullManager, registryConfig, imageConfig.getBuildConfiguration(), retries); } protected boolean shouldSkipPom() { @@ -601,7 +604,7 @@ private boolean containerMatchesPattern(Container container, Matcher imageNameMa } } - private String determinePullPolicy(RunImageConfiguration runConfig) { + protected String determinePullPolicy(RunImageConfiguration runConfig) { return runConfig.getImagePullPolicy() != null ? runConfig.getImagePullPolicy() : imagePullPolicy; } diff --git a/src/main/java/io/fabric8/maven/docker/BuildMojo.java b/src/main/java/io/fabric8/maven/docker/BuildMojo.java index 84b15667c..edae9a72b 100644 --- a/src/main/java/io/fabric8/maven/docker/BuildMojo.java +++ b/src/main/java/io/fabric8/maven/docker/BuildMojo.java @@ -64,6 +64,8 @@ public class BuildMojo extends AbstractBuildSupportMojo { */ @Parameter(property = "docker.skip.tag", defaultValue = "false") protected boolean skipTag; + @Parameter(property = "docker.pull.retries", defaultValue = "0") + private int retries; @Override protected void executeInternal(ServiceHub hub) throws IOException, MojoExecutionException { @@ -111,7 +113,7 @@ private void proceedWithDockerBuild(ServiceHub hub, BuildService.BuildContext bu if (imageConfig.isBuildX()) { hub.getBuildXService().build(createProjectPaths(), imageConfig, null, getAuthConfig(imageConfig), buildArchiveFile); } else { - buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile, retries); if (!skipTag) { buildService.tagImage(imageConfig); } diff --git a/src/main/java/io/fabric8/maven/docker/StartMojo.java b/src/main/java/io/fabric8/maven/docker/StartMojo.java index e19249e18..8a3de1abd 100644 --- a/src/main/java/io/fabric8/maven/docker/StartMojo.java +++ b/src/main/java/io/fabric8/maven/docker/StartMojo.java @@ -34,6 +34,7 @@ import io.fabric8.maven.docker.config.RunVolumeConfiguration; import io.fabric8.maven.docker.config.VolumeConfiguration; import io.fabric8.maven.docker.log.LogDispatcher; +import io.fabric8.maven.docker.service.ImagePullManager; import io.fabric8.maven.docker.service.QueryService; import io.fabric8.maven.docker.service.RegistryService; import io.fabric8.maven.docker.service.RunService; @@ -336,6 +337,7 @@ private Queue prepareStart(ServiceHub hub, QueryService quer pullImage(registryService, imageConfig, pullRegistry); RunImageConfiguration runConfig = imageConfig.getRunConfiguration(); + NetworkConfig config = runConfig.getNetworkingConfig(); List bindMounts = extractBindMounts(runConfig.getVolumeConfiguration()); List volumes = getVolumes(); diff --git a/src/main/java/io/fabric8/maven/docker/WatchMojo.java b/src/main/java/io/fabric8/maven/docker/WatchMojo.java index 04d4c0d64..dff5415ac 100644 --- a/src/main/java/io/fabric8/maven/docker/WatchMojo.java +++ b/src/main/java/io/fabric8/maven/docker/WatchMojo.java @@ -73,6 +73,9 @@ 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 { @@ -80,7 +83,7 @@ protected synchronized void executeInternal(ServiceHub hub) throws IOException, 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 { diff --git a/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java b/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java index 8fc199275..8e76112c0 100644 --- a/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java +++ b/src/main/java/io/fabric8/maven/docker/access/DockerAccess.java @@ -241,7 +241,7 @@ void copyArchiveFromContainer(String containerId, String containerPath, File arc * @param options additional query arguments to add when creating the image. Can be null. * @throws DockerAccessException if the image couldn't be pulled. */ - void pullImage(String image, AuthConfig authConfig, String registry, CreateImageOptions options) throws DockerAccessException; + void pullImage(String image, AuthConfig authConfig, String registry, CreateImageOptions options, int retries) throws DockerAccessException; /** * Push an image to a registry. A registry can be specified which is used as target diff --git a/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java b/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java index 319343fbc..f23d0d024 100644 --- a/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java +++ b/src/main/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClient.java @@ -535,13 +535,12 @@ public void loadImage(String image, File tarArchive) throws DockerAccessExceptio } @Override - public void pullImage(String image, AuthConfig authConfig, String registry, CreateImageOptions options) + public void pullImage(String image, AuthConfig authConfig, String registry, CreateImageOptions options, int retries) throws DockerAccessException { String pullUrl = urlBuilder.pullImage(options); log.verbose(Logger.LogVerboseCategory.API, API_LOG_FORMAT_POST, pullUrl); 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 + "'" : ""); } @@ -556,7 +555,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; @@ -764,16 +763,16 @@ private boolean isRetryableErrorCode(int errorCode) { return errorCode == HTTP_INTERNAL_ERROR; } - private void doPushImage(String url, Map header, HcChunkedResponseHandlerWrapper handler, int status, + private void doPushPullImage(String url, Map header, HcChunkedResponseHandlerWrapper handler, int status, int retries) throws IOException { // 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; } diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildService.java b/src/main/java/io/fabric8/maven/docker/service/BuildService.java index fe550aad9..9a1255b14 100644 --- a/src/main/java/io/fabric8/maven/docker/service/BuildService.java +++ b/src/main/java/io/fabric8/maven/docker/service/BuildService.java @@ -57,15 +57,16 @@ 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, File buildArchiveFile, int pullRetries) throws DockerAccessException, MojoExecutionException { if (imagePullManager != null) { - autoPullBaseImage(imageConfig, imagePullManager, buildContext); - autoPullCacheFromImage(imageConfig, imagePullManager, buildContext); + autoPullBaseImage(imageConfig, imagePullManager, buildContext, pullRetries); + autoPullCacheFromImage(imageConfig, imagePullManager, buildContext, pullRetries); } buildImage(imageConfig, buildContext.getMojoParameters(), checkForNocache(imageConfig), checkForSquash(imageConfig), addBuildArgs(buildContext), buildArchiveFile); @@ -356,7 +357,7 @@ private Map addBuildArgsFromDockerConfig() { return buildArgs; } - private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext) + private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, int pullRetries) throws DockerAccessException, MojoExecutionException { BuildImageConfiguration buildConfig = imageConfig.getBuildConfiguration(); CleanupMode cleanupMode = buildConfig.cleanupMode(); @@ -383,7 +384,7 @@ private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager oldImageId = queryService.getImageId(fromImage); } - registryService.pullImageWithPolicy(fromImage, imagePullManager, buildContext.getRegistryConfig(), buildConfig); + registryService.pullImageWithPolicy(fromImage, imagePullManager, buildContext.getRegistryConfig(), buildConfig, pullRetries); String newImageId = queryService.getImageId(fromImage); @@ -392,7 +393,7 @@ private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager } } - private void autoPullCacheFromImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext) throws DockerAccessException, MojoExecutionException { + private void autoPullCacheFromImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, int pullRetries) throws DockerAccessException, MojoExecutionException { if (imageConfig.getBuildConfiguration().getCacheFrom() == null) { return; } @@ -407,7 +408,7 @@ private void autoPullCacheFromImage(ImageConfiguration imageConfig, ImagePullMan } try { - registryService.pullImageWithPolicy(cacheFromImage, imagePullManager, buildContext.getRegistryConfig(), buildConfig); + registryService.pullImageWithPolicy(cacheFromImage, imagePullManager, buildContext.getRegistryConfig(), buildConfig, pullRetries); } catch (DockerAccessException e) { log.warn("Could not pull cacheFrom image: '%s'. Reason: %s", cacheFromImage, e.getMessage()); } diff --git a/src/main/java/io/fabric8/maven/docker/service/RegistryService.java b/src/main/java/io/fabric8/maven/docker/service/RegistryService.java index 30633f91b..fe0391747 100644 --- a/src/main/java/io/fabric8/maven/docker/service/RegistryService.java +++ b/src/main/java/io/fabric8/maven/docker/service/RegistryService.java @@ -103,7 +103,7 @@ private void dockerPush(int retries, boolean skipTag, BuildImageConfiguration bu * @throws DockerAccessException in case of error in contacting docker daemon * @throws MojoExecutionException in case of any other misc failure */ - public void pullImageWithPolicy(String image, ImagePullManager pullManager, RegistryConfig registryConfig, BuildImageConfiguration buildImageConfiguration) + public void pullImageWithPolicy(String image, ImagePullManager pullManager, RegistryConfig registryConfig, BuildImageConfiguration buildImageConfiguration, int retries) throws DockerAccessException, MojoExecutionException { // Already pulled, so we don't need to take care @@ -125,7 +125,7 @@ public void pullImageWithPolicy(String image, ImagePullManager pullManager, Regi docker.pullImage(imageName.getFullName(), createAuthConfig(false, null, actualRegistry, registryConfig), - actualRegistry, createImageOptions); + actualRegistry, createImageOptions, retries); log.info("Pulled %s in %s", imageName.getFullName(), EnvUtil.formatDurationTill(pullStartTime)); pullManager.pulled(image); diff --git a/src/main/java/io/fabric8/maven/docker/service/WatchService.java b/src/main/java/io/fabric8/maven/docker/service/WatchService.java index 3af214a16..c2bba5a70 100644 --- a/src/main/java/io/fabric8/maven/docker/service/WatchService.java +++ b/src/main/java/io/fabric8/maven/docker/service/WatchService.java @@ -55,7 +55,7 @@ public WatchService(ArchiveService archiveService, BuildService buildService, Do this.log = log; } - public synchronized void watch(WatchContext context, BuildService.BuildContext buildContext, List images) throws DockerAccessException, + public synchronized void watch(WatchContext context, BuildService.BuildContext buildContext, List images, int pullRetries) throws DockerAccessException, MojoExecutionException { // Important to be be a single threaded scheduler since watch jobs must run serialized @@ -86,7 +86,7 @@ public synchronized void watch(WatchContext context, BuildService.BuildContext b } if (watcher.isBuild()) { - schedule(executor, createBuildWatchTask(watcher, assemblyConfiguration.getName(), context.getMojoParameters(), watchMode == WatchMode.both, buildContext), interval); + schedule(executor, createBuildWatchTask(watcher, assemblyConfiguration.getName(), context.getMojoParameters(), watchMode == WatchMode.both, buildContext, pullRetries), interval); tasks.add("rebuilding"); } } @@ -153,10 +153,8 @@ private void callPostExec(ImageWatcher watcher) throws DockerAccessException, Ex } } - private Runnable createBuildWatchTask(final ImageWatcher watcher, - final String assemblyName, - final MojoParameters mojoParameters, final boolean doRestart, final BuildService.BuildContext buildContext) - throws MojoExecutionException { + private Runnable createBuildWatchTask(final ImageWatcher watcher, final String assemblyName, + 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, assemblyName, mojoParameters); if (files.isEmpty()) { @@ -177,7 +175,7 @@ public void run() { watcher.getWatchContext().getImageCustomizer().execute(imageConfig); } - buildService.buildImage(imageConfig, null, buildContext, buildService.buildArchive(imageConfig, buildContext, "false")); + buildService.buildImage(imageConfig, null, buildContext, buildService.buildArchive(imageConfig, buildContext, "false"), pullRetries); String name = imageConfig.getName(); watcher.setImageId(queryService.getImageId(name)); diff --git a/src/test/java/integration/DockerAccessIT.java b/src/test/java/integration/DockerAccessIT.java index a61b3ac43..b0ab7b228 100644 --- a/src/test/java/integration/DockerAccessIT.java +++ b/src/test/java/integration/DockerAccessIT.java @@ -122,7 +122,7 @@ private void testDoesNotHave() throws DockerAccessException { } private void testPullImage() throws DockerAccessException { - dockerClient.pullImage(IMAGE, null, null, null); + dockerClient.pullImage(IMAGE, null, null, null, 0); Assertions.assertTrue(hasImage(IMAGE)); } diff --git a/src/test/java/integration/DockerAccessWinIT.java b/src/test/java/integration/DockerAccessWinIT.java index 4fa8873e0..e77744d5a 100644 --- a/src/test/java/integration/DockerAccessWinIT.java +++ b/src/test/java/integration/DockerAccessWinIT.java @@ -120,7 +120,7 @@ private void testDoesNotHave() throws DockerAccessException { } private void testPullImage() throws DockerAccessException { - dockerClient.pullImage(IMAGE, null, null, null); + dockerClient.pullImage(IMAGE, null, null, null, 0); Assertions.assertTrue(hasImage(IMAGE)); } diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java index 185b1d62b..430634a57 100644 --- a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java @@ -285,7 +285,7 @@ private void thenBuildNotRun() throws DockerAccessException, MojoExecutionExcept private void verifyBuild(int wantedNumberOfInvocations) throws DockerAccessException, MojoExecutionException { Mockito.verify(buildService, Mockito.times(wantedNumberOfInvocations)) - .buildImage(Mockito.any(ImageConfiguration.class), Mockito.any(ImagePullManager.class), Mockito.any(BuildService.BuildContext.class), Mockito.any()); + .buildImage(Mockito.any(ImageConfiguration.class), Mockito.any(ImagePullManager.class), Mockito.any(BuildService.BuildContext.class), Mockito.any(), Mockito.anyInt()); } private void thenBuildxRun(String relativeConfigFile, String contextDir, diff --git a/src/test/java/io/fabric8/maven/docker/CopyMojoTest.java b/src/test/java/io/fabric8/maven/docker/CopyMojoTest.java index 93b53981f..21cd80e75 100644 --- a/src/test/java/io/fabric8/maven/docker/CopyMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/CopyMojoTest.java @@ -611,7 +611,7 @@ private void thenImageIsPulled(ImageConfiguration image, String pullRegistry, bo ArgumentCaptor pulledImage = ArgumentCaptor.forClass(String.class); ArgumentCaptor registryCapture = ArgumentCaptor.forClass(RegistryConfig.class); Mockito.verify(registryService) - .pullImageWithPolicy(pulledImage.capture(), Mockito.any(ImagePullManager.class), registryCapture.capture(), Mockito.eq(image.getBuildConfiguration())); + .pullImageWithPolicy(pulledImage.capture(), Mockito.any(ImagePullManager.class), registryCapture.capture(), Mockito.eq(image.getBuildConfiguration()), Mockito.anyInt()); Assertions.assertEquals(image.getName(), pulledImage.getValue()); RegistryConfig registryConfig = registryCapture.getValue(); diff --git a/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java b/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java index 1c49e4133..e16749be8 100644 --- a/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java +++ b/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java @@ -33,6 +33,10 @@ import java.util.UUID; import static java.net.HttpURLConnection.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.isNull; @ExtendWith(MockitoExtension.class) class DockerAccessWithHcClientTest { @@ -57,7 +61,7 @@ class DockerAccessWithHcClientTest { @Mock private Logger mockLogger; - private int pushRetries; + private int pushPullRetries; private String registry; @@ -118,7 +122,8 @@ void testPushImage_imageOfTheSameTagDoesNotExist() throws Exception { @Test void testPushFails_noRetry() throws Exception { givenAnImageName("test"); - givenThePushWillFail(1); + givenThePushOrPullWillFail(0,false); + whenPushImage(); thenImageWasNotPushed(); } @@ -127,7 +132,8 @@ void testPushFails_noRetry() throws Exception { void testRetryPush() throws Exception { givenAnImageName("test"); givenANumberOfRetries(1); - givenThePushWillFailAndEventuallySucceed(1); + givenThePushOrPullWillFail(1, true); + whenPushImage(); thenImageWasPushed(); } @@ -136,7 +142,7 @@ void testRetryPush() throws Exception { void testRetriesExceeded() throws Exception { givenAnImageName("test"); givenANumberOfRetries(1); - givenThePushWillFail(1); + givenThePushOrPullWillFail(1, false); whenPushImage(); thenImageWasNotPushed(); } @@ -185,6 +191,32 @@ void testLoadImage() { thenNoException(); } + @Test + void testPullFailes_noRetry() throws Exception { + givenAnImageName("test"); + givenThePushOrPullWillFail(0,false); + whenPullImage(); + thenImageWasNotPulled(); + } + + @Test + void testRetryPull() throws Exception { + givenAnImageName("test"); + givenANumberOfRetries(1); + givenThePushOrPullWillFail(1, true); + whenPullImage(); + thenImageWasPulled(2); + } + + @Test + void testPullRetriesExceeded() throws Exception { + givenAnImageName("test"); + givenANumberOfRetries(1); + givenThePushOrPullWillFail(1, false); + whenPullImage(); + thenImageWasNotPulled(); + } + @Test void testLoadImageFail() throws IOException { givenAnImageName("test"); @@ -217,14 +249,15 @@ void testSaveImageFail() throws IOException { void testPullImage() throws Exception { givenAnImageName("test"); whenPullImage(); - thenImageWasPulled(); + thenImageWasPulled(1); } @Test void testPullImageThrowsException() throws Exception { givenAnImageName("test"); givenPostCallThrowsException(); - DockerAccessException dae = Assertions.assertThrows(DockerAccessException.class, this::whenPullImage); + whenPullImage(); + DockerAccessException dae = (DockerAccessException) thrownException; Assertions.assertTrue(dae.getMessage().contains("Unable to pull 'test' from registry 'registry'")); Assertions.assertTrue(dae.getMessage().contains("Problem with images/create")); } @@ -285,7 +318,7 @@ private void givenRegistry(String registry) { } private void givenANumberOfRetries(int retries) { - this.pushRetries = retries; + this.pushPullRetries = retries; } private void givenArchiveFile(String archiveFile) { @@ -309,7 +342,7 @@ private void givenContainerIdImagePairs(Pair... idNamePairs) thr array.add(idNameObject); } - Mockito.doReturn(array.toString()).when(mockDelegate).get(Mockito.anyString(), Mockito.eq(200)); + Mockito.doReturn(array.toString()).when(mockDelegate).get(anyString(), Mockito.eq(200)); } private void givenImageIdRepoTagPairs(Pair... idRepoTagPairs) throws IOException { @@ -434,9 +467,20 @@ private void whenListImages() { } } - private void whenPushImage() throws IOException { + private void whenPushImage() { + try { + client.pushImage(imageName, authConfig, registry, pushPullRetries); + } catch (Exception e) { + thrownException = e; + } + } + private void thenImageWasNotPulled() { + Assertions.assertNotNull(thrownException); + } + + private void whenPullImage() { try { - client.pushImage(imageName, authConfig, registry, pushRetries); + client.pullImage("test", null, "registry", new CreateImageOptions().tag("1.1"), pushPullRetries); } catch (Exception e) { thrownException = e; } @@ -506,7 +550,7 @@ private void givenPostCallThrowsException() throws IOException { .post(Mockito.anyString(), Mockito.any(), Mockito.anyMap(), Mockito.any(ResponseHandler.class), Mockito.eq(HTTP_OK)); } - private void thenImageWasPulled() throws IOException { + private void thenImageWasPulled(int pushPullRetries) throws IOException { ArgumentCaptor urlCapture = ArgumentCaptor.forClass(String.class); Mockito.verify(mockDelegate) .post(urlCapture.capture(), Mockito.isNull(), Mockito.anyMap(), Mockito.any(ResponseHandler.class), Mockito.eq(HTTP_OK)); @@ -516,7 +560,18 @@ private void thenImageWasPulled() throws IOException { Assertions.assertEquals("tcp://1.2.3.4:2375/v1.40/images/create?tag=1.1", postUrl); } - private void whenPullImage() throws DockerAccessException { - client.pullImage("test", null, "registry", new CreateImageOptions().tag("1.1")); + private void givenThePushOrPullWillFail(final int pushPullRetries, final boolean succeedAtEnd) throws IOException { + if (pushPullRetries == 1 && succeedAtEnd) { + Mockito.when(mockDelegate.post(anyString(), isNull(), anyMap(), any(ResponseHandler.class), Mockito.eq(HTTP_OK))) + .thenThrow(new HttpResponseException(HTTP_INTERNAL_ERROR, "error")) + .thenReturn(new Object()); + } else if (pushPullRetries == 1) { + Mockito.when(mockDelegate.post(anyString(), isNull(), anyMap(), any(ResponseHandler.class), Mockito.eq(HTTP_OK))) + .thenThrow(new HttpResponseException(HTTP_INTERNAL_ERROR, "error")) + .thenThrow(new HttpResponseException(HTTP_INTERNAL_ERROR, "error")); + } else { + Mockito.when(mockDelegate.post(anyString(), isNull(), anyMap(), any(ResponseHandler.class), Mockito.eq(HTTP_OK))) + .thenThrow(new HttpResponseException(HTTP_INTERNAL_ERROR, "error")); + } } } diff --git a/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java index 2c454fb56..d33033bd0 100644 --- a/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java @@ -141,7 +141,7 @@ void testMultiStageBuild() throws Exception { mockMavenProject(); File buildArchive = buildService.buildArchive(imageConfig, buildContext, ""); - buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive, 0); //verify that tries to pull both images verifyImagePull(buildConfig, pullManager, buildContext, "fabric8/s2i-java"); @@ -151,7 +151,7 @@ void testMultiStageBuild() throws Exception { private void verifyImagePull(BuildImageConfiguration buildConfig, ImagePullManager pullManager, BuildService.BuildContext buildContext, String image) throws DockerAccessException, MojoExecutionException { Mockito.verify(registryService). - pullImageWithPolicy(image, pullManager, buildContext.getRegistryConfig(), buildConfig); + pullImageWithPolicy(image, pullManager, buildContext.getRegistryConfig(), buildConfig, 0); } private void mockMavenProject() { @@ -184,7 +184,7 @@ void testBuildImageWithCacheFrom_ShouldPullImage() throws Exception { mockMavenProject(); File buildArchive = buildService.buildArchive(imageConfig, buildContext, ""); - buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive, 0); //verify that tries to pull both images verifyImagePull(buildConfig, pullManager, buildContext, "fabric8/s2i-java"); @@ -211,7 +211,7 @@ void testBuildImagePullsDefaultImageWhenNoFromImage() throws Exception { mockMavenProject(); File buildArchive = buildService.buildArchive(imageConfig, buildContext, ""); - buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive, 0); verifyImagePull(buildConfig, pullManager, buildContext, DockerAssemblyManager.DEFAULT_DATA_BASE_IMAGE); } @@ -324,7 +324,7 @@ void testBuildArgsFromDifferentSources() throws MojoExecutionException, DockerAc .build(); File buildArchive = buildService.buildArchive(imageConfig, buildContext, ""); - buildService.buildImage(imageConfig, null, buildContext, buildArchive); + buildService.buildImage(imageConfig, null, buildContext, buildArchive, 0); Mockito.verify(docker).buildImage(Mockito.any(), Mockito.any(), Mockito.argThat((BuildOptions options) -> options.getOptions().get("buildargs").equals("{\"http_proxy\":\"http://system-props.com\"}"))); } diff --git a/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java index 15e2a532e..95389b600 100644 --- a/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/RegistryServiceTest.java @@ -228,7 +228,7 @@ void pullImageWithPolicy_pullPolicyAlwaysAndBuildConfiguration_shouldPull() thro ArgumentCaptor pulledImage = ArgumentCaptor.forClass(String.class); ArgumentCaptor imageCapture = ArgumentCaptor.forClass(CreateImageOptions.class); - Mockito.verify(docker).pullImage(pulledImage.capture(), Mockito.any(), Mockito.anyString(), imageCapture.capture()); + Mockito.verify(docker).pullImage(pulledImage.capture(), Mockito.any(), Mockito.anyString(), imageCapture.capture(), Mockito.anyInt()); Assertions.assertEquals("myregistry.com/user/test:1.0.1", pulledImage.getValue()); CreateImageOptions createImageOptions = imageCapture.getValue(); @@ -415,7 +415,7 @@ private void thenNoExceptionThrown() { } private void thenImageHasNotBeenPulled() throws DockerAccessException { - Mockito.verify(docker, Mockito.never()).pullImage(Mockito.anyString(), Mockito.any(AuthConfig.class), Mockito.anyString(), Mockito.any(CreateImageOptions.class)); + Mockito.verify(docker, Mockito.never()).pullImage(Mockito.anyString(), Mockito.any(AuthConfig.class), Mockito.anyString(), Mockito.any(CreateImageOptions.class), Mockito.anyInt()); } private void thenImageHasNotBeenPushed() throws DockerAccessException { @@ -475,7 +475,7 @@ private void thenImageHasBeenPulled() throws DockerAccessException { } private void thenImageHasBeenPulledWithRegistry(final String registry) throws DockerAccessException { - Mockito.verify(docker).pullImage(Mockito.eq(imageName), Mockito.any(), Mockito.eq(registry), Mockito.any(CreateImageOptions.class)); + Mockito.verify(docker).pullImage(Mockito.eq(imageName), Mockito.any(), Mockito.eq(registry), Mockito.any(CreateImageOptions.class), Mockito.anyInt()); Assertions.assertNotNull(cacheStore.get(imageName)); } @@ -492,7 +492,7 @@ private void whenAutoPullImage() { if (registry != null) { registryConfigBuilder.registry(registry); } - registryService.pullImageWithPolicy(imageName, pullManager, registryConfigBuilder.build(), imageConfiguration.getBuildConfiguration()); + registryService.pullImageWithPolicy(imageName, pullManager, registryConfigBuilder.build(), imageConfiguration.getBuildConfiguration(), 0); } catch (Exception e) { this.actualException = e; diff --git a/src/test/java/io/fabric8/maven/docker/service/WatchServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/WatchServiceTest.java index ed95b1eba..97e226287 100644 --- a/src/test/java/io/fabric8/maven/docker/service/WatchServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/WatchServiceTest.java @@ -79,7 +79,7 @@ void testWatchCallsGetAllAssemblyConfigurations() { // ACT Executors.newSingleThreadExecutor().submit(() -> { try { - watchService.watch(watchContext, buildContext, imageConfigurations); + watchService.watch(watchContext, buildContext, imageConfigurations, 0); } catch (Exception ignored) {} }); From 9b6f68928642a7ac17ba89c19380ea9ea396c305 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Tue, 21 Mar 2023 12:59:30 +0530 Subject: [PATCH 2/2] fix : Introduce pullRetries and pushRetries fields Signed-off-by: Rohan Kumar --- .../asciidoc/inc/build/_configuration.adoc | 3 --- .../asciidoc/inc/start/_configuration.adoc | 2 +- .../asciidoc/inc/watch/_configuration.adoc | 4 +-- .../maven/docker/AbstractDockerMojo.java | 24 ++++++++++++++++-- .../io/fabric8/maven/docker/BuildMojo.java | 4 +-- .../io/fabric8/maven/docker/PushMojo.java | 7 ++---- .../io/fabric8/maven/docker/StartMojo.java | 2 -- .../io/fabric8/maven/docker/WatchMojo.java | 5 +--- .../fabric8/maven/docker/BuildMojoTest.java | 25 +++++++++++++++++++ .../io/fabric8/maven/docker/PushMojoTest.java | 25 +++++++++++++++++++ .../hc/DockerAccessWithHcClientTest.java | 3 ++- 11 files changed, 81 insertions(+), 23 deletions(-) diff --git a/src/main/asciidoc/inc/build/_configuration.adoc b/src/main/asciidoc/inc/build/_configuration.adoc index 0c1e76326..9c67403c2 100644 --- a/src/main/asciidoc/inc/build/_configuration.adoc +++ b/src/main/asciidoc/inc/build/_configuration.adoc @@ -120,9 +120,6 @@ 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 `` section for this image which specifies the runtime behaviour when starting containers. diff --git a/src/main/asciidoc/inc/start/_configuration.adoc b/src/main/asciidoc/inc/start/_configuration.adoc index 42fa5fe39..6708899c3 100644 --- a/src/main/asciidoc/inc/start/_configuration.adoc +++ b/src/main/asciidoc/inc/start/_configuration.adoc @@ -9,7 +9,7 @@ In addition to the <>, this goal supports the following gl | Default pattern for naming all containers when they are created. See <> for details. | `docker.containerNamePattern` -| *retries* +| *pullRetries* | 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` diff --git a/src/main/asciidoc/inc/watch/_configuration.adoc b/src/main/asciidoc/inc/watch/_configuration.adoc index 1f8be74fb..dee3d215f 100644 --- a/src/main/asciidoc/inc/watch/_configuration.adoc +++ b/src/main/asciidoc/inc/watch/_configuration.adoc @@ -55,14 +55,14 @@ below how this can be specified. | `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`. +| 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 `false`. | `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* +| *pullRetries* | 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` |=== diff --git a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java index 4175db720..a8141c8c0 100644 --- a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java +++ b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java @@ -121,9 +121,15 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context @Parameter(property = "docker.removeVolumes", defaultValue = "false") protected boolean removeVolumes; - @Parameter(property = "docker.pull.retries", defaultValue = "0") + @Parameter(property = "docker.retries", defaultValue = "0") protected int retries; + @Parameter(property = "docker.pull.retries", defaultValue = "0") + protected int pullRetries; + + @Parameter(property = "docker.push.retries", defaultValue = "0") + protected int pushRetries; + @Parameter(property = "docker.apiVersion") private String apiVersion; @@ -580,7 +586,7 @@ protected void pullImage(RegistryService registryService, ImageConfiguration ima RunImageConfiguration runConfiguration = imageConfig.getRunConfiguration(); ImagePullManager pullManager = getImagePullManager(determinePullPolicy(runConfiguration), autoPull); RegistryConfig registryConfig = getRegistryConfig(pullRegistry); - registryService.pullImageWithPolicy(imageName, pullManager, registryConfig, imageConfig.getBuildConfiguration(), retries); + registryService.pullImageWithPolicy(imageName, pullManager, registryConfig, imageConfig.getBuildConfiguration(), getPullRetries()); } protected boolean shouldSkipPom() { @@ -611,4 +617,18 @@ protected String determinePullPolicy(RunImageConfiguration runConfig) { protected ProjectPaths createProjectPaths() { return new ProjectPaths(project.getBasedir(), outputDirectory); } + + protected int getPullRetries() { + if (pullRetries > 0) { + return pullRetries; + } + return retries; + } + + protected int getPushRetries() { + if (pushRetries > 0) { + return pushRetries; + } + return retries; + } } diff --git a/src/main/java/io/fabric8/maven/docker/BuildMojo.java b/src/main/java/io/fabric8/maven/docker/BuildMojo.java index edae9a72b..b8db6c873 100644 --- a/src/main/java/io/fabric8/maven/docker/BuildMojo.java +++ b/src/main/java/io/fabric8/maven/docker/BuildMojo.java @@ -64,8 +64,6 @@ public class BuildMojo extends AbstractBuildSupportMojo { */ @Parameter(property = "docker.skip.tag", defaultValue = "false") protected boolean skipTag; - @Parameter(property = "docker.pull.retries", defaultValue = "0") - private int retries; @Override protected void executeInternal(ServiceHub hub) throws IOException, MojoExecutionException { @@ -113,7 +111,7 @@ private void proceedWithDockerBuild(ServiceHub hub, BuildService.BuildContext bu if (imageConfig.isBuildX()) { hub.getBuildXService().build(createProjectPaths(), imageConfig, null, getAuthConfig(imageConfig), buildArchiveFile); } else { - buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile, retries); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile, getPullRetries()); if (!skipTag) { buildService.tagImage(imageConfig); } diff --git a/src/main/java/io/fabric8/maven/docker/PushMojo.java b/src/main/java/io/fabric8/maven/docker/PushMojo.java index 0797a6e3e..b2a3e426b 100644 --- a/src/main/java/io/fabric8/maven/docker/PushMojo.java +++ b/src/main/java/io/fabric8/maven/docker/PushMojo.java @@ -30,9 +30,6 @@ public class PushMojo extends AbstractDockerMojo { */ @Parameter(property = "docker.skip.tag", defaultValue = "false") private boolean skipTag; - - @Parameter(property = "docker.push.retries", defaultValue = "0") - private int retries; /** * {@inheritDoc} @@ -51,14 +48,14 @@ public void executeInternal(ServiceHub hub) throws DockerAccessException, MojoEx } private void executeDockerPush(ServiceHub hub) throws MojoExecutionException, DockerAccessException { - hub.getRegistryService().pushImages(createProjectPaths(), getResolvedImages(), retries, getRegistryConfig(pushRegistry), skipTag); + hub.getRegistryService().pushImages(createProjectPaths(), getResolvedImages(), getPushRetries(), getRegistryConfig(pushRegistry), skipTag); } private void executeJibPush(ServiceHub hub) throws MojoExecutionException { log.info("Pushing Container image with [[B]]JIB(Java Image Builder)[[B]] mode"); JibBuildService jibBuildService = new JibBuildService(hub, new MojoParameters(session, project, null, null, null, settings, sourceDirectory, outputDirectory, null), log); - jibBuildService.push(getResolvedImages(), retries, getRegistryConfig(pushRegistry), skipTag); + jibBuildService.push(getResolvedImages(), getPushRetries(), getRegistryConfig(pushRegistry), skipTag); } } diff --git a/src/main/java/io/fabric8/maven/docker/StartMojo.java b/src/main/java/io/fabric8/maven/docker/StartMojo.java index 8a3de1abd..e19249e18 100644 --- a/src/main/java/io/fabric8/maven/docker/StartMojo.java +++ b/src/main/java/io/fabric8/maven/docker/StartMojo.java @@ -34,7 +34,6 @@ import io.fabric8.maven.docker.config.RunVolumeConfiguration; import io.fabric8.maven.docker.config.VolumeConfiguration; import io.fabric8.maven.docker.log.LogDispatcher; -import io.fabric8.maven.docker.service.ImagePullManager; import io.fabric8.maven.docker.service.QueryService; import io.fabric8.maven.docker.service.RegistryService; import io.fabric8.maven.docker.service.RunService; @@ -337,7 +336,6 @@ private Queue prepareStart(ServiceHub hub, QueryService quer pullImage(registryService, imageConfig, pullRegistry); RunImageConfiguration runConfig = imageConfig.getRunConfiguration(); - NetworkConfig config = runConfig.getNetworkingConfig(); List bindMounts = extractBindMounts(runConfig.getVolumeConfiguration()); List volumes = getVolumes(); diff --git a/src/main/java/io/fabric8/maven/docker/WatchMojo.java b/src/main/java/io/fabric8/maven/docker/WatchMojo.java index dff5415ac..15b658306 100644 --- a/src/main/java/io/fabric8/maven/docker/WatchMojo.java +++ b/src/main/java/io/fabric8/maven/docker/WatchMojo.java @@ -73,9 +73,6 @@ 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 { @@ -83,7 +80,7 @@ protected synchronized void executeInternal(ServiceHub hub) throws IOException, BuildService.BuildContext buildContext = getBuildContext(); WatchService.WatchContext watchContext = getWatchContext(hub); - hub.getWatchService().watch(watchContext, buildContext, getResolvedImages(), retries); + hub.getWatchService().watch(watchContext, buildContext, getResolvedImages(), getPullRetries()); } protected WatchService.WatchContext getWatchContext(ServiceHub hub) throws IOException { diff --git a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java index 430634a57..e2628dea4 100644 --- a/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/BuildMojoTest.java @@ -32,6 +32,8 @@ import java.util.List; import java.util.Map; +import static org.junit.jupiter.api.Assertions.assertEquals; + @ExtendWith(MockitoExtension.class) class BuildMojoTest extends MojoTestBase { private static final String NON_NATIVE_PLATFORM = "linux/amd64"; @@ -241,6 +243,29 @@ void buildUsingBuildxWithMultipleAuth() throws IOException, MojoExecutionExcepti thenAuthContainsRegistry("custom-registry.org"); } + @Test + void getPullRetries_whenPullRetriesConfigured_thenUsePullRetries() { + givenMavenProject(buildMojo); + buildMojo.pullRetries = 2; + + assertEquals(2, buildMojo.getPullRetries()); + } + + @Test + void getPullRetries_whenRetriesConfigured_thenUseRetries() { + givenMavenProject(buildMojo); + buildMojo.retries = 2; + + assertEquals(2, buildMojo.getPullRetries()); + } + + @Test + void getPullRetries_whenNothingConfigured_thenReturnDefaultValue() { + givenMavenProject(buildMojo); + + assertEquals(0, buildMojo.getPullRetries()); + } + private void givenBuildXService() { BuildXService buildXService = new BuildXService(dockerAccess, dockerAssemblyManager, log, exec); diff --git a/src/test/java/io/fabric8/maven/docker/PushMojoTest.java b/src/test/java/io/fabric8/maven/docker/PushMojoTest.java index 47e78c3b7..0177ad1c1 100644 --- a/src/test/java/io/fabric8/maven/docker/PushMojoTest.java +++ b/src/test/java/io/fabric8/maven/docker/PushMojoTest.java @@ -12,6 +12,8 @@ import java.io.IOException; +import static org.junit.jupiter.api.Assertions.assertEquals; + @ExtendWith(MockitoExtension.class) class PushMojoTest extends MojoTestBase { @InjectMocks @@ -71,6 +73,29 @@ void executeInternal_whenSkipDisabled_thenImageIsPushed() throws MojoExecutionEx thenImagePushed(); } + @Test + void getPushRetries_whenPushRetriesConfigured_thenUsePushRetries() { + givenMavenProject(pushMojo); + pushMojo.pushRetries = 2; + + assertEquals(2, pushMojo.getPushRetries()); + } + + @Test + void getPushRetries_whenRetriesConfigured_thenUseRetries() { + givenMavenProject(pushMojo); + pushMojo.retries = 2; + + assertEquals(2, pushMojo.getPushRetries()); + } + + @Test + void getPushRetries_whenNothingConfigured_thenReturnDefaultValue() { + givenMavenProject(pushMojo); + + assertEquals(0, pushMojo.getPushRetries()); + } + private void thenImagePushed() throws MojoExecutionException, DockerAccessException { verifyPush(1); } diff --git a/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java b/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java index e16749be8..2983db00f 100644 --- a/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java +++ b/src/test/java/io/fabric8/maven/docker/access/hc/DockerAccessWithHcClientTest.java @@ -37,6 +37,7 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.times; @ExtendWith(MockitoExtension.class) class DockerAccessWithHcClientTest { @@ -552,7 +553,7 @@ private void givenPostCallThrowsException() throws IOException { private void thenImageWasPulled(int pushPullRetries) throws IOException { ArgumentCaptor urlCapture = ArgumentCaptor.forClass(String.class); - Mockito.verify(mockDelegate) + Mockito.verify(mockDelegate, times(pushPullRetries)) .post(urlCapture.capture(), Mockito.isNull(), Mockito.anyMap(), Mockito.any(ResponseHandler.class), Mockito.eq(HTTP_OK)); String postUrl = urlCapture.getValue();