fix : Introduce pullRetries and pushRetries fields

Signed-off-by: Rohan Kumar <[email protected]>

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 `<run>` section for this image which specifies the runtime behaviour when starting containers.
 

src/main/asciidoc/inc/start/_configuration.adoc

@@ -9,7 +9,7 @@ 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*
+| *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`
 

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`
 |===

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;
+    }
 }

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);
                 }

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);
     }
 
 }

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;

src/main/java/io/fabric8/maven/docker/WatchMojo.java

@@ -73,17 +73,14 @@ 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(), retries);
+        hub.getWatchService().watch(watchContext, buildContext, getResolvedImages(), getPullRetries());
     }
 
     protected WatchService.WatchContext getWatchContext(ServiceHub hub) throws IOException {

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);
 

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);
   }

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<String> 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();