Avoid race condition when creating port bindings

doc/changelog.md

@@ -13,6 +13,7 @@
   - Add a skipPom parameter, skipping a project if packaging is pom ([1388](https://github.com/fabric8io/docker-maven-plugin/pull/1388))
   - Add support for config to specify isolation technology for container ([1376](https://github.com/fabric8io/docker-maven-plugin/pull/1376))
   - Add Docker build cache friendly example utilizing Spring Boot Layered JAR and Maven Assembly Plugin ([1412](https://github.com/fabric8io/docker-maven-plugin/pull/1412))
+  - Retry port mapping to avoid race condition where complete port information may not be initially available in Docker Engine 20.10.5 ([1447](https://github.com/fabric8io/docker-maven-plugin/pull/1447))
 
 * **0.34.1** (2020-09-27)
   - Fix NPE with "skipPush" and no build configuration given ([#1381](https://github.com/fabric8io/docker-maven-plugin/issues/1381))

pom.xml

@@ -183,6 +183,12 @@
       <artifactId>plexus-interpolation</artifactId>
       <version>1.24</version>
     </dependency>
+
+    <dependency>
+      <groupId>net.jodah</groupId>
+      <artifactId>failsafe</artifactId>
+      <version>2.4.0</version>
+    </dependency>
 
 
     <!-- =============================================================================== -->

src/main/java/io/fabric8/maven/docker/model/ContainerDetails.java

@@ -8,6 +8,8 @@
 import java.util.Set;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
+import com.google.gson.JsonArray;
 import com.google.gson.JsonObject;
 
 
@@ -196,8 +198,12 @@ private Map<String, PortBinding> createPortBindings(JsonObject ports) {
             if (ports.get(port).isJsonNull()) {
                 addPortMapping(port, (PortBinding) null, portBindings);
             } else {
+                JsonArray hostMappings = ports.getAsJsonArray(port);
+                if (hostMappings.isJsonNull() || hostMappings.size() == 0) {
+                    throw new PortBindingException(port, ports);
+                }
                 // use the first entry in the array
-                JsonObject hostConfig = ports.getAsJsonArray(port).get(0).getAsJsonObject();
+                JsonObject hostConfig = hostMappings.get(0).getAsJsonObject();
                 addPortMapping(port, hostConfig, portBindings);
             }
         }

src/main/java/io/fabric8/maven/docker/model/PortBindingException.java

@@ -0,0 +1,9 @@
+package io.fabric8.maven.docker.model;
+
+import com.google.gson.JsonObject;
+
+public class PortBindingException extends RuntimeException {
+    public PortBindingException(String port, JsonObject portDetails) {
+        super(String.format("Failed to create binding for port '%s'. Container ports: %s", port, portDetails));
+    }
+}

src/main/java/io/fabric8/maven/docker/service/RunService.java

@@ -20,6 +20,8 @@
  */
 
 import java.io.File;
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -55,13 +57,16 @@
 import io.fabric8.maven.docker.model.ContainerDetails;
 import io.fabric8.maven.docker.model.ExecDetails;
 import io.fabric8.maven.docker.model.Network;
+import io.fabric8.maven.docker.model.PortBindingException;
 import io.fabric8.maven.docker.util.ContainerNamingUtil;
 import io.fabric8.maven.docker.util.EnvUtil;
 import io.fabric8.maven.docker.util.GavLabel;
 import io.fabric8.maven.docker.util.Logger;
 import io.fabric8.maven.docker.util.StartOrderResolver;
 import io.fabric8.maven.docker.wait.WaitTimeoutException;
 import io.fabric8.maven.docker.wait.WaitUtil;
+import net.jodah.failsafe.Failsafe;
+import net.jodah.failsafe.RetryPolicy;
 
 
 /**
@@ -464,12 +469,23 @@ private void startContainer(ImageConfiguration imageConfig, String id, GavLabel
     }
 
     private void updateMappedPortsAndAddresses(String containerId, PortMapping mappedPorts) throws DockerAccessException {
-        Container container = queryService.getMandatoryContainer(containerId);
-        if (container.isRunning()) {
-            mappedPorts.updateProperties(container.getPortBindings());
-        } else {
-            log.warn("Container %s is not running anymore, can not extract dynamic ports",containerId);
-        }
+        RetryPolicy<Void> retryPolicy = new RetryPolicy<Void>()
+                .withMaxAttempts(20)
+                .withBackoff(10, 100, ChronoUnit.MILLIS)
+                .handle(PortBindingException.class)
+                .onFailedAttempt(f -> log.debug("Failed to update mapped ports for container %s (attempt %d), retrying", 
+                        containerId, f.getAttemptCount()))
+                .onRetriesExceeded(f -> log.warn("Failed to update mapped ports for container %s after %d retries", 
+                        containerId, f.getAttemptCount()));
+
+        Failsafe.with(retryPolicy).run(() -> {
+            Container container = queryService.getMandatoryContainer(containerId);
+            if (container.isRunning()) {
+                mappedPorts.updateProperties(container.getPortBindings());
+            } else {
+                log.warn("Container %s is not running anymore, can not extract dynamic ports", containerId);
+            }
+        });
     }
 
     private void shutdown(ContainerTracker.ContainerShutdownDescriptor descriptor, boolean keepContainer, boolean removeVolumes)

src/test/java/io/fabric8/maven/docker/model/ContainerDetailsTest.java

@@ -133,6 +133,13 @@ public void testCreateContainer() throws Exception {
         thenValidateContainer();
     }
 
+    @Test(expected = PortBindingException.class)
+    public void testCreateContainerWithEmptyPortBindings() throws Exception {
+        givenAContainerWithUnboundPorts();
+        whenCreateContainer();
+        container.getPortBindings();
+    }
+
     private JsonArray createHostIpAndPort(int port, String ip) {
         JsonObject object = new JsonObject();
 
@@ -172,6 +179,15 @@ private void givenAContainerWithMappedPorts() {
     private void givenAContainerWithoutPorts() {
         json.add(ContainerDetails.NETWORK_SETTINGS, new JsonObject());
     }
+
+    private void givenAContainerWithUnboundPorts() {
+        JsonObject ports = new JsonObject();
+        ports.add("80/tcp", new JsonArray());
+        ports.add("52/udp", new JsonArray());
+        JsonObject networkSettings = new JsonObject();
+        networkSettings.add(ContainerDetails.PORTS, ports);
+        json.add(ContainerDetails.NETWORK_SETTINGS, networkSettings);
+    }
 
     private void givenContainerData() {
         json.addProperty(ContainerDetails.CREATED, "2015-01-06T15:47:31.485331387Z");

src/test/java/io/fabric8/maven/docker/service/RunServiceTest.java

@@ -1,10 +1,14 @@
 package io.fabric8.maven.docker.service;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.gson.Gson;
 import com.google.gson.JsonObject;
 
 import io.fabric8.maven.docker.access.ExecException;
 import io.fabric8.maven.docker.config.StopMode;
 import io.fabric8.maven.docker.config.VolumeConfiguration;
+import io.fabric8.maven.docker.model.Container;
+import io.fabric8.maven.docker.model.PortBindingException;
 import io.fabric8.maven.docker.util.GavLabel;
 import org.apache.commons.io.IOUtils;
 import org.apache.maven.execution.MavenSession;
@@ -18,6 +22,7 @@
 import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -42,11 +47,11 @@
 import io.fabric8.maven.docker.util.Logger;
 import mockit.Expectations;
 import mockit.Mocked;
+import mockit.Verifications;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 /**
  * This test need to be refactored. In fact, testing Mojos must be setup correctly at all. Blame on me that there are so
@@ -336,6 +341,42 @@ public void testStopModeWithKill() throws DockerAccessException, ExecException {
         runService.stopContainer(container, createImageConfigWithStopMode(StopMode.kill), false, false);
         assertTrue("No wait", System.currentTimeMillis() - start < SHUTDOWN_WAIT);
     }
+
+    @Test
+    public void retryIfInsufficientPortBindingInformation(
+            @Mocked Container container,
+            @Mocked ImageConfiguration imageConfiguration,
+            @Mocked PortMapping portMapping
+    ) throws DockerAccessException {
+        new Expectations() {{
+            docker.createContainer(withAny(new ContainerCreateConfig("img")), anyString); result = "containerId";
+            portMapping.needsPropertiesUpdate(); result = true;
+            queryService.getMandatoryContainer("containerId"); result = container; times = 2;
+            container.isRunning(); result = true;
+            container.getPortBindings(); result = new PortBindingException("5432/tcp", new Gson().fromJson("{\"5432/tcp\": []}", JsonObject.class));
+            returns(ImmutableMap.of("5432/tcp", new Container.PortBinding(56741, "0.0.0.0")), ImmutableMap.of("5432/tcp", new Container.PortBinding(56741, "0.0.0.0")));
+        }};
+        String containerId = runService.createAndStartContainer(imageConfiguration, portMapping, new GavLabel("Im:A:Test"), properties, getBaseDirectory(), "blah", new Date());
+        new Verifications() {{
+            assertEquals("containerId", containerId);
+        }};
+    }
+
+    @Test(expected = PortBindingException.class)
+    public void failAfterRetryingIfInsufficientPortBindingInformation(
+            @Mocked Container container,
+            @Mocked ImageConfiguration imageConfiguration,
+            @Mocked PortMapping portMapping
+    ) throws DockerAccessException {
+        new Expectations() {{
+            docker.createContainer(withAny(new ContainerCreateConfig("img")), anyString); result = "containerId";
+            portMapping.needsPropertiesUpdate(); result = true;
+            queryService.getMandatoryContainer("containerId"); result = container; times = 20;
+            container.isRunning(); result = true;
+            container.getPortBindings(); result = new PortBindingException("5432/tcp", new Gson().fromJson("{\"5432/tcp\": []}", JsonObject.class));
+        }};
+        runService.createAndStartContainer(imageConfiguration, portMapping, new GavLabel("Im:A:Test"), properties, getBaseDirectory(), "blah", new Date());
+    }
 
     private ImageConfiguration createImageConfig(int wait, int kill) {
         return new ImageConfiguration.Builder()