From 1da5d292173ac26bb7820f8c9fbd2b9fcfacfa82 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Mon, 7 Sep 2020 19:05:44 +0530 Subject: [PATCH] Fix #1242: docker.container..ip property is no longer set (#1323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix #1242: docker.container..ip property is no longer set * Refactored property handling, made tests more robust * cleaned up Co-authored-by: Roland Huß --- doc/changelog.md | 1 + .../io/fabric8/maven/docker/StartMojo.java | 5 +- .../maven/docker/service/WatchService.java | 7 +-- .../helper/StartContainerExecutor.java | 17 ++--- .../maven/docker/util/AuthConfigFactory.java | 3 +- .../helper/StartContainerExecutorTest.java | 63 +++++++++++++++++++ .../docker/util/AuthConfigFactoryTest.java | 34 +++++----- 7 files changed, 99 insertions(+), 31 deletions(-) diff --git a/doc/changelog.md b/doc/changelog.md index 551edcb5b..3682ded6e 100644 --- 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..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 index f4df8fc83..a2909ecd1 100644 --- 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 index ad89fa7a8..e4a13e8b2 100644 --- 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 defaultContainerRestartTask() { .buildTimestamp(watcher.watchContext.buildTimestamp) .build(); - String containerId = helper.startContainers(); - + String containerId = helper.startContainer(); watcher.setContainerId(containerId); }; } @@ -594,4 +593,4 @@ public WatchContext build() { } } } -} \ No newline at end of file +} 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 index fa9ce09dd..fe5deeda3 100644 --- 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 nets = container.getCustomNetworkIpAddresses(); if (nets != null) { for (Map.Entry 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; } } -} \ No newline at end of file +} diff --git a/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java b/src/main/java/io/fabric8/maven/docker/util/AuthConfigFactory.java index 4552ac050..5fa842ef4 100644 --- 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 index 9fdd16041..630496212 100644 --- 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 index 38bfc3426..b072c1699 100644 --- 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 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(); }