Fix #1242: docker.container.<alias>.ip property is no longer set (#1323)

* Fix #1242: docker.container.<alias>.ip property is no longer set * Refactored property handling, made tests more robust * cleaned up Co-authored-by: Roland Huß <roland@ro14nd.de>
fabric8io · Sep 7, 2020 · 1da5d29 · 1da5d29
1 parent 493aa8b
commit 1da5d29
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 31 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -7,6 +7,7 @@
     The handling of Y changes when the week straddle the New year ([Stack Overflow](https://stackoverflow.com/questions/26431882/difference-between-year-of-era-and-week-based-year))
   - Fix JSON error when parsin tafs (#1354)
   - Add `skipPush` option to build image configuration ([#1243](https://github.com/fabric8io/docker-maven-plugin/issues/1243))
+  - docker.container.<alias>.ip property is no longer set ([#1242](https://github.com/fabric8io/docker-maven-plugin/issues/1242))
   - Support `squash` in build options to squash newly built layers into a single layer ([#785](https://github.com/fabric8io/docker-maven-plugin/issues/785)) 
 
 * **0.33.0** (2020-01-21)

diff --git a/src/main/java/io/fabric8/maven/docker/StartMojo.java b/src/main/java/io/fabric8/maven/docker/StartMojo.java
@@ -178,7 +178,6 @@ public synchronized void executeInternal(final ServiceHub hub) throws DockerAcce
                     waitForStartedContainer(containerStartupService, startedContainerAliases, imagesStarting);
                 }
             }
-
             portMappingPropertyWriteHelper.write();
 
             if (follow) {
@@ -297,13 +296,11 @@ private void startImage(final ImageConfiguration imageConfig,
 
         startingContainers.submit(() -> {
 
-            String containerId = startExecutor.startContainers();
+            String containerId = startExecutor.startContainer();
 
             // Update port-mapping writer
             portMappingPropertyWriteHelper.add(portMapping, runConfig.getPortPropertyFile());
 
-
-
             return new StartedContainer(imageConfig, containerId);
         });
     }

diff --git a/src/main/java/io/fabric8/maven/docker/service/WatchService.java b/src/main/java/io/fabric8/maven/docker/service/WatchService.java
@@ -21,9 +21,9 @@
 import io.fabric8.maven.docker.config.WatchMode;
 import io.fabric8.maven.docker.log.LogDispatcher;
 import io.fabric8.maven.docker.service.helper.StartContainerExecutor;
+import io.fabric8.maven.docker.util.GavLabel;
 import io.fabric8.maven.docker.util.Logger;
 import io.fabric8.maven.docker.util.MojoParameters;
-import io.fabric8.maven.docker.util.GavLabel;
 import io.fabric8.maven.docker.util.StartOrderResolver;
 import io.fabric8.maven.docker.util.Task;
 import org.apache.maven.plugin.MojoExecutionException;
@@ -252,8 +252,7 @@ private Task<ImageWatcher> defaultContainerRestartTask() {
                     .buildTimestamp(watcher.watchContext.buildTimestamp)
                     .build();
 
-            String containerId = helper.startContainers();
-
+            String containerId = helper.startContainer();
             watcher.setContainerId(containerId);
         };
     }
@@ -594,4 +593,4 @@ public WatchContext build() {
             }
         }
     }
-}
+}
diff --git a/src/main/java/io/fabric8/maven/docker/service/helper/StartContainerExecutor.java b/src/main/java/io/fabric8/maven/docker/service/helper/StartContainerExecutor.java
@@ -41,39 +41,42 @@ public class StartContainerExecutor {
 
     private StartContainerExecutor(){}
 
-    public String startContainers() throws IOException, ExecException {
+    public String startContainer() throws IOException, ExecException {
         final Properties projProperties = projectProperties;
 
         final String containerId = hub.getRunService().createAndStartContainer(imageConfig, portMapping, gavLabel, projProperties, basedir, containerNamePattern, buildDate);
 
         showLogsIfRequested(containerId);
-        exposeContainerProps(containerId);
+        Properties exposedProps = queryContainerProperties(containerId);
+        projProperties.putAll(exposedProps);
         waitAndPostExec(containerId, projProperties);
 
         return containerId;
     }
 
-    private void exposeContainerProps(String containerId)
+    public Properties queryContainerProperties(String containerId)
         throws DockerAccessException {
         String propKey = getExposedPropertyKeyPart();
+        Properties exposedProperties = new Properties();
 
         if (StringUtils.isNotEmpty(exposeContainerProps) && StringUtils.isNotEmpty(propKey)) {
             Container container = hub.getQueryService().getMandatoryContainer(containerId);
 
             String prefix = addDot(exposeContainerProps) + addDot(propKey);
-            projectProperties.put(prefix + "id", containerId);
+            exposedProperties.put(prefix + "id", containerId);
             String ip = container.getIPAddress();
             if (StringUtils.isNotEmpty(ip)) {
-                projectProperties.put(prefix + "ip", ip);
+                exposedProperties.put(prefix + "ip", ip);
             }
 
             Map<String, String> nets = container.getCustomNetworkIpAddresses();
             if (nets != null) {
                 for (Map.Entry<String, String> entry : nets.entrySet()) {
-                    projectProperties.put(prefix + addDot("net") + addDot(entry.getKey()) + "ip", entry.getValue());
+                    exposedProperties.put(prefix + addDot("net") + addDot(entry.getKey()) + "ip", entry.getValue());
                 }
             }
         }
+        return exposedProperties;
     }
 
     String getExposedPropertyKeyPart() {
@@ -219,4 +222,4 @@ public StartContainerExecutor build() {
             return helper;
         }
     }
-}
+}
diff --git a/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java b/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java
@@ -353,7 +353,8 @@ private AuthConfig getAuthConfigFromTaskRole() throws IOException {
         // get temporary credentials
         log.debug("Getting temporary security credentials from: %s", uri);
         try (CloseableHttpClient client = HttpClients.custom().useSystemProperties().build()) {
-            RequestConfig conf = RequestConfig.custom().setConnectionRequestTimeout(1000).setConnectTimeout(1000)
+            RequestConfig conf =
+                RequestConfig.custom().setConnectionRequestTimeout(1000).setConnectTimeout(1000)
                     .setSocketTimeout(1000).build();
             HttpGet request = new HttpGet(uri);
             request.setConfig(conf);

diff --git a/src/test/java/io/fabric8/maven/docker/service/helper/StartContainerExecutorTest.java b/src/test/java/io/fabric8/maven/docker/service/helper/StartContainerExecutorTest.java
@@ -1,13 +1,35 @@
 package io.fabric8.maven.docker.service.helper;
 
+import io.fabric8.maven.docker.access.ContainerCreateConfig;
+import io.fabric8.maven.docker.access.DockerAccess;
+import io.fabric8.maven.docker.access.ExecException;
+import io.fabric8.maven.docker.access.PortMapping;
+import io.fabric8.maven.docker.log.LogOutputSpecFactory;
+import io.fabric8.maven.docker.model.ContainerDetails;
+import io.fabric8.maven.docker.service.ContainerTracker;
+import io.fabric8.maven.docker.service.QueryService;
+import io.fabric8.maven.docker.service.RunService;
+import io.fabric8.maven.docker.service.ServiceHub;
+import io.fabric8.maven.docker.util.GavLabel;
+import io.fabric8.maven.docker.util.JsonFactory;
+import io.fabric8.maven.docker.util.Logger;
+import mockit.Expectations;
+import mockit.Mocked;
 import org.junit.Test;
 
 import io.fabric8.maven.docker.config.ImageConfiguration;
 import io.fabric8.maven.docker.config.LogConfiguration;
 import io.fabric8.maven.docker.config.RunImageConfiguration;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Properties;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 public class StartContainerExecutorTest {
@@ -226,4 +248,45 @@ public void showLogs_withShowLogsMatchImage() {
     // Then
     assertTrue(actual);
   }
+
+  @Test
+  public void testStartContainers(@Mocked ServiceHub hub, @Mocked DockerAccess dockerAccess, @Mocked ContainerTracker containerTracker, @Mocked Logger log) throws IOException, ExecException {
+    // Given
+    new Expectations() {{
+      dockerAccess.createContainer((ContainerCreateConfig) any, anyString);
+      result = "container-name";dockerAccess.getContainer(anyString);
+      result = new ContainerDetails(JsonFactory.newJsonObject("{\"NetworkSettings\":{\"IPAddress\":\"192.168.1.2\"}}"));
+
+      QueryService queryService = new QueryService(dockerAccess);
+      hub.getQueryService();
+      result = queryService;
+
+      hub.getRunService();
+      result = new RunService(dockerAccess, queryService, containerTracker, new LogOutputSpecFactory(true, true, null), log);
+    }};
+    Properties projectProps = new Properties();
+    StartContainerExecutor startContainerExecutor = new StartContainerExecutor.Builder()
+            .serviceHub(hub)
+            .projectProperties(projectProps)
+            .portMapping(new PortMapping(Collections.emptyList(), projectProps))
+            .gavLabel(new GavLabel("io.fabric8:test:0.1.0"))
+            .basedir(new File("/tmp/foo"))
+            .containerNamePattern("test-")
+            .buildTimestamp(new Date())
+            .exposeContainerProps("docker.container")
+            .imageConfig(new ImageConfiguration.Builder()
+                    .name("name")
+                    .alias("alias")
+                    .runConfig(new RunImageConfiguration.Builder()
+                            .build())
+                    .build())
+            .build();
+
+    // When
+    String containerId = startContainerExecutor.startContainer();
+    // Then
+    assertEquals("container-name", containerId);
+    assertEquals("container-name", projectProps.getProperty("docker.container.alias.id"));
+    assertEquals("192.168.1.2", projectProps.getProperty("docker.container.alias.ip"));
+  }
 }
diff --git a/src/test/java/io/fabric8/maven/docker/util/AuthConfigFactoryTest.java b/src/test/java/io/fabric8/maven/docker/util/AuthConfigFactoryTest.java
@@ -4,6 +4,7 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.Writer;
+import java.net.InetAddress;
 import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -291,6 +292,7 @@ public void exec(File homeDir) throws IOException, MojoExecutionException {
 
     private void executeWithTempHomeDir(HomeDirExecutor executor) throws IOException, MojoExecutionException {
         String userHome = System.getProperty("user.home");
+        environmentVariables.clear("KUBECONFIG");
         try {
             File tempDir = Files.createTempDirectory("d-m-p").toFile();
             System.setProperty("user.home", tempDir.getAbsolutePath());
@@ -426,7 +428,7 @@ public void exec(File homeDir) throws IOException, MojoExecutionException {
                 Map<String,String> authConfigMap = new HashMap<>();
                 authConfigMap.put("useOpenShiftAuth","true");
                 expectedException.expect(MojoExecutionException.class);
-                expectedException.expectMessage(containsString("~/.kube/config"));
+                expectedException.expectMessage(containsString(".kube/config"));
                 factory.createAuthConfig(isPush,false,authConfigMap,settings,"roland",null);
             }
         });
@@ -632,20 +634,22 @@ private Server create(String id, String user, String password, String email) {
     }
 
     private void givenEcsMetadataService(String containerCredentialsUri, String accessKeyId, String secretAccessKey, String sessionToken) throws IOException {
-        httpServer = ServerBootstrap.bootstrap()
-                .registerHandler("*", (request, response, context) -> {
-                    System.out.println("REQUEST: " + request.getRequestLine());
-                    if (containerCredentialsUri.matches(request.getRequestLine().getUri())) {
-                        response.setEntity(new StringEntity(gsonBuilder.create().toJson(ImmutableMap.of(
-                                "AccessKeyId", accessKeyId,
-                                "SecretAccessKey", secretAccessKey,
-                                "Token", sessionToken
-                        ))));
-                    } else {
-                        response.setStatusCode(SC_NOT_FOUND);
-                    }
-                })
-                .create();
+        httpServer =
+            ServerBootstrap.bootstrap()
+                           .setLocalAddress(InetAddress.getLoopbackAddress())
+                           .registerHandler("*", (request, response, context) -> {
+                               System.out.println("REQUEST: " + request.getRequestLine());
+                               if (containerCredentialsUri.matches(request.getRequestLine().getUri())) {
+                                   response.setEntity(new StringEntity(gsonBuilder.create().toJson(ImmutableMap.of(
+                                       "AccessKeyId", accessKeyId,
+                                       "SecretAccessKey", secretAccessKey,
+                                       "Token", sessionToken
+                                                                                                                  ))));
+                               } else {
+                                   response.setStatusCode(SC_NOT_FOUND);
+                               }
+                           })
+                           .create();
         httpServer.start();
     }