From 2f04a2fa4c5c3a9ef535ceb2fe2609eaee12cdf5 Mon Sep 17 00:00:00 2001 From: Jae Gangemi Date: Sat, 30 Jul 2016 15:42:01 -0400 Subject: [PATCH] - run 'stopContainer' in a Future Signed-off-by: Jae Gangemi --- doc/changelog.md | 3 + .../maven/docker/service/RunService.java | 104 ++++++++++++------ .../fabric8/maven/docker/util/WaitUtil.java | 23 +++- .../maven/docker/service/RunServiceTest.java | 28 +++-- .../maven/docker/util/WaitUtilTest.java | 57 +++++++--- 5 files changed, 154 insertions(+), 61 deletions(-) diff --git a/doc/changelog.md b/doc/changelog.md index 78f5cf94c..f46f5b1b7 100644 --- a/doc/changelog.md +++ b/doc/changelog.md @@ -1,5 +1,8 @@ # ChangeLog +* **0.15.14** + - Run 'stopContainer' in a Future to short circuit extra waiting ([#518](https://github.com/fabric8io/docker-maven-plugin/issues/518)) + * **0.15.13** (2016-07-29) - Add for running containers in special security contexts (#524) - Add support for multiples network aliases (#466) diff --git a/src/main/java/io/fabric8/maven/docker/service/RunService.java b/src/main/java/io/fabric8/maven/docker/service/RunService.java index 060004fe1..4af055edc 100644 --- a/src/main/java/io/fabric8/maven/docker/service/RunService.java +++ b/src/main/java/io/fabric8/maven/docker/service/RunService.java @@ -18,6 +18,8 @@ */ import java.util.*; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import io.fabric8.maven.docker.model.Container; import io.fabric8.maven.docker.log.LogOutputSpecFactory; @@ -25,6 +27,7 @@ import io.fabric8.maven.docker.config.*; import io.fabric8.maven.docker.model.Network; import io.fabric8.maven.docker.util.*; +import io.fabric8.maven.docker.util.WaitUtil.WaitTimeoutException; /** @@ -116,13 +119,13 @@ public String createAndStartContainer(ImageConfiguration imageConfig, * @param removeVolumes whether to remove volumes after stopping */ public void stopContainer(String containerId, - ImageConfiguration imageConfig, - boolean keepContainer, - boolean removeVolumes) - throws DockerAccessException { + ImageConfiguration imageConfig, + boolean keepContainer, + boolean removeVolumes) + throws DockerAccessException { ContainerTracker.ContainerShutdownDescriptor descriptor = - new ContainerTracker.ContainerShutdownDescriptor(imageConfig,containerId); - shutdown(docker, descriptor, log, keepContainer, removeVolumes); + new ContainerTracker.ContainerShutdownDescriptor(imageConfig, containerId); + shutdown(descriptor, keepContainer, removeVolumes); } /** @@ -140,7 +143,7 @@ public void stopPreviouslyStartedContainer(String containerId, throws DockerAccessException { ContainerTracker.ContainerShutdownDescriptor descriptor = tracker.removeContainer(containerId); if (descriptor != null) { - shutdown(docker, descriptor, log, keepContainer, removeVolumes); + shutdown(descriptor, keepContainer, removeVolumes); } } @@ -159,7 +162,7 @@ public void stopStartedContainers(boolean keepContainer, Set networksToRemove = new HashSet<>(); for (ContainerTracker.ContainerShutdownDescriptor descriptor : tracker.removeShutdownDescriptors(pomLabel)) { collectCustomNetworks(networksToRemove, descriptor, removeCustomNetworks); - shutdown(docker, descriptor, log, keepContainer, removeVolumes); + shutdown(descriptor, keepContainer, removeVolumes); } removeCustomNetworks(networksToRemove); } @@ -399,10 +402,8 @@ private void updateMappedPortsAndAddresses(String containerId, PortMapping mappe } } - private void shutdown(DockerAccess access, - ContainerTracker.ContainerShutdownDescriptor descriptor, - Logger log, boolean keepContainer, boolean removeVolumes) - throws DockerAccessException { + private void shutdown(ContainerTracker.ContainerShutdownDescriptor descriptor, boolean keepContainer, boolean removeVolumes) + throws DockerAccessException { String containerId = descriptor.getContainerId(); if (descriptor.getPreStop() != null) { @@ -412,32 +413,20 @@ private void shutdown(DockerAccess access, log.error("%s", e.getMessage()); } } - // Stop the container - int killGracePeriod = descriptor.getKillGracePeriod(); - int killGracePeriodInSeconds = (killGracePeriod + 500) / 1000; - if (killGracePeriod != 0 && killGracePeriodInSeconds == 0) { - log.warn("A kill grace period of %d ms leads to no wait at all since its rounded to seconds. " + - "Please use at least 500 as value for wait.kill",killGracePeriod); - } - access.stopContainer(containerId, killGracePeriodInSeconds); - if (killGracePeriod > 0) { - log.debug("Shutdown: Wait %d s after stopping and before killing container", killGracePeriodInSeconds); - WaitUtil.sleep(killGracePeriodInSeconds * 1000); - } + + int killGracePeriod = adjustGracePeriod(descriptor.getKillGracePeriod()); + log.debug("shutdown will wait max of %d seconds before removing container", killGracePeriod); + + long waited = shutdownAndWait(containerId, killGracePeriod); + if (!keepContainer) { - int shutdownGracePeriod = descriptor.getShutdownGracePeriod(); - if (shutdownGracePeriod != 0) { - log.debug("Shutdown: Wait %d ms before removing container", shutdownGracePeriod); - WaitUtil.sleep(shutdownGracePeriod); - } - // Remove the container - access.removeContainer(containerId, removeVolumes); + removeContainer(descriptor, removeVolumes, containerId); } - log.info("%s: Stop%s container %s", - descriptor.getDescription(), - (keepContainer ? "" : " and remove"), - containerId.substring(0, 12)); + log.info("%s: Stop%s container %s after %s ms", + descriptor.getDescription(), + (keepContainer ? "" : " and removed"), + containerId.substring(0, 12), waited); } public void createCustomNetworkIfNotExistant(String customNetwork) throws DockerAccessException { @@ -453,4 +442,49 @@ public void removeCustomNetworks(Collection networks) throws DockerAcce docker.removeNetwork(network.getId()); } } + + private int adjustGracePeriod(int gracePeriod) { + int killGracePeriodInSeconds = (gracePeriod + 500) / 1000; + if (gracePeriod != 0 && killGracePeriodInSeconds == 0) { + log.warn("A kill grace period of %d ms leads to no wait at all since its rounded to seconds. " + + "Please use at least 500 as value for wait.kill", gracePeriod); + } + + return killGracePeriodInSeconds; + } + + private void removeContainer(ContainerTracker.ContainerShutdownDescriptor descriptor, boolean removeVolumes, String containerId) + throws DockerAccessException { + int shutdownGracePeriod = descriptor.getShutdownGracePeriod(); + if (shutdownGracePeriod != 0) { + log.debug("Shutdown: Wait %d ms before removing container", shutdownGracePeriod); + WaitUtil.sleep(shutdownGracePeriod); + } + // Remove the container + docker.removeContainer(containerId, removeVolumes); + } + + private long shutdownAndWait(final String containerId, final int killGracePeriodInSeconds) throws DockerAccessException { + long waited = 0; + try { + waited = WaitUtil.wait(killGracePeriodInSeconds, new Callable() { + @Override + public Void call() throws Exception { + docker.stopContainer(containerId, killGracePeriodInSeconds); + return null; + } + }); + } catch (ExecutionException e) { + if (e.getCause() instanceof DockerAccessException) { + throw (DockerAccessException) e.getCause(); + } else { + throw new DockerAccessException(e, "failed to stop container id [%s]", containerId); + } + } catch (WaitTimeoutException e) { + waited = e.getWaited(); + log.warn("Stop container id [%s] timed out after %s ms", containerId, waited); + } + + return waited; + } } diff --git a/src/main/java/io/fabric8/maven/docker/util/WaitUtil.java b/src/main/java/io/fabric8/maven/docker/util/WaitUtil.java index 60ffd78e7..6e7879f3d 100644 --- a/src/main/java/io/fabric8/maven/docker/util/WaitUtil.java +++ b/src/main/java/io/fabric8/maven/docker/util/WaitUtil.java @@ -7,7 +7,7 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; -import java.util.concurrent.TimeoutException; +import java.util.concurrent.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -25,6 +25,8 @@ */ public class WaitUtil { + private static final int DEFAULT_DOCKER_TIMEOUT = 10; + // how long to wait at max when doing a http ping private static final long DEFAULT_MAX_WAIT = 10 * 1000; @@ -48,6 +50,24 @@ public class WaitUtil { private WaitUtil() {} + public static long wait(int wait, Callable callable) throws ExecutionException, WaitTimeoutException { + long now = System.currentTimeMillis(); + wait = (wait == 0) ? DEFAULT_DOCKER_TIMEOUT : wait; + + try { + FutureTask task = new FutureTask<>(callable); + task.run(); + + task.get(wait, TimeUnit.SECONDS); + } catch (@SuppressWarnings("unused") InterruptedException e) { + Thread.currentThread().interrupt(); + } catch (@SuppressWarnings("unused") TimeoutException e) { + throw new WaitTimeoutException("timed out waiting for execution to complete", delta(now)); + } + + return delta(now); + } + public static long wait(int maxWait, WaitChecker ... checkers) throws WaitTimeoutException { return wait(maxWait, Arrays.asList(checkers)); } @@ -89,6 +109,7 @@ public static void sleep(long millis) { Thread.sleep(millis); } catch (InterruptedException e) { // ... + Thread.currentThread().interrupt(); } } diff --git a/src/test/java/io/fabric8/maven/docker/service/RunServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/RunServiceTest.java index 3d181e593..1f9e88db9 100644 --- a/src/test/java/io/fabric8/maven/docker/service/RunServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/RunServiceTest.java @@ -6,6 +6,7 @@ import io.fabric8.maven.docker.log.LogOutputSpec; import io.fabric8.maven.docker.util.Logger; +import io.fabric8.maven.docker.util.WaitUtil; import io.fabric8.maven.docker.access.*; import io.fabric8.maven.docker.config.*; import io.fabric8.maven.docker.log.LogOutputSpecFactory; @@ -106,9 +107,9 @@ public void shutdownWithoutKeepingContainers() throws Exception { } @Test public void killafterAndShutdownWithoutKeepingContainers() throws Exception { + long start = System.currentTimeMillis(); setupForKillWait(); - long start = System.currentTimeMillis(); runService.stopContainer(container, createImageConfig(SHUTDOWN_WAIT, KILL_AFTER), false, false); assertTrue("Waited for at least " + (SHUTDOWN_WAIT + KILL_AFTER) + " ms", System.currentTimeMillis() - start >= SHUTDOWN_WAIT + KILL_AFTER); @@ -116,26 +117,35 @@ public void killafterAndShutdownWithoutKeepingContainers() throws Exception { @Test public void killafterWithoutKeepingContainers() throws Exception { + long start = System.currentTimeMillis(); setupForKillWait(); - long start = System.currentTimeMillis(); runService.stopContainer(container, createImageConfig(0, KILL_AFTER), false, false); assertTrue("Waited for at least " + (KILL_AFTER) + " ms", System.currentTimeMillis() - start >= KILL_AFTER); } private void setupForKillWait() throws DockerAccessException { + // use this to simulate something happened - timers need to be started before this method gets invoked + docker = new MockUp() { + @Mock + public void stopContainer(String contaierId, int wait) { + WaitUtil.sleep(KILL_AFTER); + } + }.getMockInstance(); + new Expectations() {{ - docker.stopContainer(container, (KILL_AFTER + 500) / 1000); - log.debug(anyString, (Object[]) any); minTimes = 1; - docker.removeContainer(container, false); - new LogInfoMatchingExpectations(container, true); + docker.stopContainer(container, (KILL_AFTER + 500) / 1000); + log.debug(anyString, (Object[]) any); minTimes = 1; + docker.removeContainer(container, false); + new LogInfoMatchingExpectations(container, true); }}; } @Test public void shutdownWithoutKeepingContainersAndRemovingVolumes() throws Exception { new Expectations() {{ + docker.stopContainer(container, 0); log.debug(anyString, (Object[]) any); minTimes = 1; docker.removeContainer(container, true); @@ -362,8 +372,10 @@ final class LogInfoMatchingExpectations extends Expectations { LogInfoMatchingExpectations(String container, boolean withRemove) { log.info(withSubstring("Stop"), anyString, - withRemove ? withSubstring("remove") : withNotEqual(" and remiver"), - withSubstring(container.substring(0,12))); + withRemove ? withSubstring("removed") : withNotEqual(" and removed"), + withSubstring(container.substring(0,12)), + anyLong); } } + } diff --git a/src/test/java/io/fabric8/maven/docker/util/WaitUtilTest.java b/src/test/java/io/fabric8/maven/docker/util/WaitUtilTest.java index 0911614d3..899bd18ce 100644 --- a/src/test/java/io/fabric8/maven/docker/util/WaitUtilTest.java +++ b/src/test/java/io/fabric8/maven/docker/util/WaitUtilTest.java @@ -3,18 +3,23 @@ import java.io.IOException; import java.net.*; import java.util.Collections; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.TimeoutException; import com.sun.net.httpserver.*; import org.junit.*; +import io.fabric8.maven.docker.util.WaitUtil.WaitTimeoutException; + import static org.junit.Assert.*; /** * @author roland * @since 18.10.14 */ +@SuppressWarnings("restriction") public class WaitUtilTest { static HttpServer server; @@ -22,24 +27,23 @@ public class WaitUtilTest { static String httpPingUrl; private static String serverMethodToAssert; - @Test(expected = TimeoutException.class) public void httpFail() throws TimeoutException { WaitUtil.HttpPingChecker checker = new WaitUtil.HttpPingChecker("http://127.0.0.1:" + port + "/fake-context/"); - long waited = WaitUtil.wait(500,checker); + long waited = WaitUtil.wait(500, checker); } @Test public void httpSuccess() throws TimeoutException { WaitUtil.HttpPingChecker checker = new WaitUtil.HttpPingChecker(httpPingUrl); - long waited = WaitUtil.wait(700,checker); + long waited = WaitUtil.wait(700, checker); assertTrue("Waited less than 700ms: " + waited, waited < 700); } @Test public void httpSuccessWithStatus() throws TimeoutException { - for (String status : new String[] { "200", "200 ... 300", "200..250"}) { - long waited = WaitUtil.wait(700,new WaitUtil.HttpPingChecker(httpPingUrl,null,status)); + for (String status : new String[] { "200", "200 ... 300", "200..250" }) { + long waited = WaitUtil.wait(700, new WaitUtil.HttpPingChecker(httpPingUrl, null, status)); assertTrue("Waited less than 700ms: " + waited, waited < 700); } } @@ -64,7 +68,7 @@ public void httpSuccessWithGetMethod() throws Exception { @Test public void tcpSuccess() throws TimeoutException { WaitUtil.TcpPortChecker checker = new WaitUtil.TcpPortChecker("localhost", Collections.singletonList(port)); - long waited = WaitUtil.wait(700,checker); + long waited = WaitUtil.wait(700, checker); assertTrue("Waited less than 700ms: " + waited, waited < 700); } @@ -86,6 +90,30 @@ public void cleanupShouldBeCalledAfterFailedExceptation() { } } + @Test + public void waitOnCallable() throws Exception { + long waited = waitOnCallable(1, 500); + + assertTrue(500 <= waited); + assertTrue(1000 > waited); + } + + @Test + public void waitOnCallableFullWait() throws Exception { + long waited = waitOnCallable(1, 1000); + assertTrue(1000 <= waited); + } + + private long waitOnCallable(long wait, final long sleep) throws WaitTimeoutException, ExecutionException { + return WaitUtil.wait(5, new Callable() { + @Override + public Void call() throws Exception { + Thread.sleep(sleep); + return null; + } + }); + } + private static class StubWaitChecker implements WaitUtil.WaitChecker { private final boolean checkResult; @@ -111,12 +139,13 @@ public boolean isCleaned() { } @BeforeClass + public static void createServer() throws IOException { port = getRandomPort(); serverMethodToAssert = "HEAD"; System.out.println("Created HTTP server at port " + port); InetAddress address = InetAddress.getLoopbackAddress(); - InetSocketAddress socketAddress = new InetSocketAddress(address,port); + InetSocketAddress socketAddress = new InetSocketAddress(address, port); server = HttpServer.create(socketAddress, 10); // Prepare executor pool @@ -146,7 +175,7 @@ public static void cleanupServer() { } private static int getRandomPort() throws IOException { - for (int port = 22332; port < 22500;port++) { + for (int port = 22332; port < 22500; port++) { if (trySocket(port)) { return port; } @@ -155,20 +184,14 @@ private static int getRandomPort() throws IOException { } private static boolean trySocket(int port) throws IOException { - InetAddress address = Inet4Address.getByName("localhost"); - ServerSocket s = null; - try { - s = new ServerSocket(); - s.bind(new InetSocketAddress(address,port)); + InetAddress address = InetAddress.getByName("localhost"); + try (ServerSocket s = new ServerSocket()) { + s.bind(new InetSocketAddress(address, port)); return true; } catch (IOException exp) { System.err.println("Port " + port + " already in use, tying next ..."); // exp.printStackTrace(); // next try .... - } finally { - if (s != null) { - s.close(); - } } return false; }