From f1ce2b139233da7e6932778944f376ef6e6e7743 Mon Sep 17 00:00:00 2001 From: Richard North Date: Sun, 10 Dec 2017 08:28:36 +0000 Subject: [PATCH] Wait robustness (#504) Refactor port awaits for improved robustness: * We now allow a list of multiple startup liveness check ports to be defined for a container. This helps, e.g. for the browser containers, and lets us wait for both Selenium and VNC ports to be listening. I think this will help eliminate random flapping tests in this area and should fix #466 * For exposed/bound ports, we now check that the port is accepting connections both (1) from within the container, and (2) from the testcontainers host. Previously, the internal check was done for Docker on Mac/Win and the external check for Linux. Now both are used for all environments, which simplifies the logic and gives a solid guarantee that the port truly is listening. (For reference, the internal check was done due to issues with the Docker networking stack opening a listening socket before the containerized service itself was listening). * Also, we now have a WaitAllStrategy that lets more than one wait strategy be used. For browser containers, we now wait for (a) a log message, and (b) the listening ports to be available. * Broken out some aspects of the wait strategies/port detection into separate classes and used this to help improve test coverage. * Refactored/tidied some code in network link configuration that was redundant, using Docker API filters instead of streamed filtering of containers --- CHANGELOG.md | 4 + .../containers/GenericContainer.java | 91 +++++++++++++++---- .../VncRecordingSidekickContainer.java | 9 +- .../containers/wait/HostPortWaitStrategy.java | 90 +++++------------- .../containers/wait/WaitAllStrategy.java | 38 ++++++++ .../internal/ExternalPortListeningCheck.java | 32 +++++++ .../InternalCommandPortListeningCheck.java | 49 ++++++++++ .../containers/wait/WaitAllStrategyTest.java | 69 ++++++++++++++ .../ExternalPortListeningCheckTest.java | 71 +++++++++++++++ ...InternalCommandPortListeningCheckTest.java | 33 +++++++ .../containers/JdbcDatabaseContainer.java | 3 - .../containers/MySQLContainer.java | 10 +- .../containers/NginxContainer.java | 8 +- .../containers/PostgreSQLContainer.java | 8 +- .../containers/BrowserWebDriverContainer.java | 23 +++-- .../junit/FirefoxWebDriverContainerTest.java | 6 +- 16 files changed, 434 insertions(+), 110 deletions(-) create mode 100644 core/src/main/java/org/testcontainers/containers/wait/WaitAllStrategy.java create mode 100644 core/src/main/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheck.java create mode 100644 core/src/main/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheck.java create mode 100644 core/src/test/java/org/testcontainers/containers/wait/WaitAllStrategyTest.java create mode 100644 core/src/test/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheckTest.java create mode 100644 core/src/test/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheckTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 795c7fb1ee7..9a0d764da7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file. - Allowing `addExposedPort` to be used after ports have been specified with `withExposedPorts` (#453) - Stopping creation of temporary directory prior to creating temporary file (#443) - Ensure that temp files are created in a temp directory (#423) +- Added `WaitAllStrategy` as a mechanism for composing multiple startup `WaitStrategy` objects together +- Changed `BrowserWebDriverContainer` to use improved wait strategies, to eliminate race conditions when starting VNC recording containers. This should lead to far fewer 'error' messages logged when starting up selenium containers, and less exposure to race related bugs (fixes #466). ### Changed - Make Network instances reusable (i.e. work with `@ClassRule`) ([\#469](https://github.com/testcontainers/testcontainers-java/issues/469)) @@ -16,6 +18,8 @@ All notable changes to this project will be documented in this file. - Use Visible Assertions 2.1.0 for pre-flight test output (eliminating Jansi/JNR-POSIX dependencies for lower likelihood of conflict. JNA is now used internally by Visible Assertions instead). - Mark all links functionality as deprecated. This is pending removal in a later release. Please see [\#465](https://github.com/testcontainers/testcontainers-java/issues/465). {@link Network} features should be used instead. - Added support for copying files to/from running containers ([\#378](https://github.com/testcontainers/testcontainers-java/issues/378)) +- Add `getLivenessCheckPorts` as an eventual replacement for `getLivenessCheckPort`; this allows multiple ports to be included in post-startup wait strategies. +- Refactor wait strategy port checking and improve test coverage. - Added support for customising the recording file name ([\#500](https://github.com/testcontainers/testcontainers-java/issues/500)) ## [1.4.3] - 2017-10-14 diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java index f9d96e85ef9..d8ebb36ec93 100644 --- a/core/src/main/java/org/testcontainers/containers/GenericContainer.java +++ b/core/src/main/java/org/testcontainers/containers/GenericContainer.java @@ -9,8 +9,9 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import lombok.*; -import org.apache.commons.compress.utils.IOUtils; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.utils.IOUtils; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.runner.Description; import org.rnorth.ducttape.ratelimits.RateLimiter; @@ -44,6 +45,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; +import java.util.stream.Stream; import static com.google.common.collect.Lists.newArrayList; import static org.testcontainers.containers.output.OutputFrame.OutputType.STDERR; @@ -96,7 +98,6 @@ public class GenericContainer> @NonNull private List binds = new ArrayList<>(); - @NonNull private boolean privilegedMode; @NonNull @@ -326,8 +327,11 @@ protected void containerIsStarted(InspectContainerResponse containerInfo) { /** * @return the port on which to check if the container is ready + * @deprecated see {@link GenericContainer#getLivenessCheckPorts()} for replacement */ + @Deprecated protected Integer getLivenessCheckPort() { + // legacy implementation for backwards compatibility if (exposedPorts.size() > 0) { return getMappedPort(exposedPorts.get(0)); } else if (portBindings.size() > 0) { @@ -337,6 +341,39 @@ protected Integer getLivenessCheckPort() { } } + /** + * @return the ports on which to check if the container is ready + */ + @NotNull + @NonNull + protected Set getLivenessCheckPorts() { + final Set result = new HashSet<>(); + result.addAll(getExposedPortNumbers()); + result.addAll(getBoundPortNumbers()); + + // for backwards compatibility + if (this.getLivenessCheckPort() != null) { + result.add(this.getLivenessCheckPort()); + } + + return result; + } + + private List getExposedPortNumbers() { + return exposedPorts.stream() + .map(this::getMappedPort) + .collect(Collectors.toList()); + } + + private List getBoundPortNumbers() { + return portBindings.stream() + .map(PortBinding::parse) + .map(PortBinding::getBinding) + .map(Ports.Binding::getHostPortSpec) + .map(Integer::valueOf) + .collect(Collectors.toList()); + } + private void applyConfiguration(CreateContainerCmd createCommand) { // Set up exposed ports (where there are no host port bindings defined) @@ -376,31 +413,16 @@ private void applyConfiguration(CreateContainerCmd createCommand) { String alias = linkEntries.getKey(); LinkableContainer linkableContainer = linkEntries.getValue(); - Set links = dockerClient.listContainersCmd().exec().stream() - .filter(container -> container.getNames()[0].endsWith(linkableContainer.getContainerName())) - .map(container -> new Link(container.getNames()[0], alias)) - .collect(Collectors.toSet()); + Set links = findLinksFromThisContainer(alias, linkableContainer); allLinks.addAll(links); - boolean linkableContainerIsRunning = dockerClient.listContainersCmd().exec().stream() - .filter(container -> container.getNames()[0].endsWith(linkableContainer.getContainerName())) - .map(com.github.dockerjava.api.model.Container::getId) - .map(id -> dockerClient.inspectContainerCmd(id).exec()) - .anyMatch(linkableContainerInspectResponse -> linkableContainerInspectResponse.getState().getRunning()); - - if (!linkableContainerIsRunning) { + if (allLinks.size() == 0) { throw new ContainerLaunchException("Aborting attempt to link to container " + linkableContainer.getContainerName() + " as it is not running"); } - Set linkedContainerNetworks = dockerClient.listContainersCmd().exec().stream() - .filter(container -> container.getNames()[0].endsWith(linkableContainer.getContainerName())) - .filter(container -> container.getNetworkSettings() != null && - container.getNetworkSettings().getNetworks() != null) - .flatMap(container -> container.getNetworkSettings().getNetworks().keySet().stream()) - .distinct() - .collect(Collectors.toSet()); + Set linkedContainerNetworks = findAllNetworksForLinkedContainers(linkableContainer); allLinkedContainerNetworks.addAll(linkedContainerNetworks); } @@ -443,6 +465,26 @@ private void applyConfiguration(CreateContainerCmd createCommand) { createCommand.withLabels(Collections.singletonMap("org.testcontainers", "true")); } + private Set findLinksFromThisContainer(String alias, LinkableContainer linkableContainer) { + return dockerClient.listContainersCmd() + .withStatusFilter("running") + .exec().stream() + .flatMap(container -> Stream.of(container.getNames())) + .filter(name -> name.endsWith(linkableContainer.getContainerName())) + .map(name -> new Link(name, alias)) + .collect(Collectors.toSet()); + } + + private Set findAllNetworksForLinkedContainers(LinkableContainer linkableContainer) { + return dockerClient.listContainersCmd().exec().stream() + .filter(container -> container.getNames()[0].endsWith(linkableContainer.getContainerName())) + .filter(container -> container.getNetworkSettings() != null && + container.getNetworkSettings().getNetworks() != null) + .flatMap(container -> container.getNetworkSettings().getNetworks().keySet().stream()) + .distinct() + .collect(Collectors.toSet()); + } + /** * {@inheritDoc} */ @@ -1015,11 +1057,20 @@ protected Logger logger() { /** * @return the port on which to check if the container is ready + * @deprecated see {@link AbstractWaitStrategy#getLivenessCheckPorts()} */ + @Deprecated protected Integer getLivenessCheckPort() { return container.getLivenessCheckPort(); } + /** + * @return the ports on which to check if the container is ready + */ + protected Set getLivenessCheckPorts() { + return container.getLivenessCheckPorts(); + } + /** * @return the rate limiter to use */ diff --git a/core/src/main/java/org/testcontainers/containers/VncRecordingSidekickContainer.java b/core/src/main/java/org/testcontainers/containers/VncRecordingSidekickContainer.java index 959ae80e46f..04f01c33623 100644 --- a/core/src/main/java/org/testcontainers/containers/VncRecordingSidekickContainer.java +++ b/core/src/main/java/org/testcontainers/containers/VncRecordingSidekickContainer.java @@ -1,6 +1,7 @@ package org.testcontainers.containers; import com.github.dockerjava.api.command.InspectContainerResponse; +import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.traits.LinkableContainer; import org.testcontainers.containers.traits.VncService; import org.testcontainers.utility.TestcontainersConfiguration; @@ -10,6 +11,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.Set; + +import static java.util.Collections.emptySet; /** * 'Sidekick container' with the sole purpose of recording the VNC screen output from another container. @@ -46,10 +50,11 @@ protected void containerIsStarting(InspectContainerResponse containerInfo) { // do nothing } + @NotNull @Override - protected Integer getLivenessCheckPort() { + protected Set getLivenessCheckPorts() { // no liveness check needed - return null; + return emptySet(); } @Override diff --git a/core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java b/core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java index babe0e8d592..b64535503c0 100644 --- a/core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java +++ b/core/src/main/java/org/testcontainers/containers/wait/HostPortWaitStrategy.java @@ -1,19 +1,18 @@ package org.testcontainers.containers.wait; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang.SystemUtils; import org.rnorth.ducttape.TimeoutException; import org.rnorth.ducttape.unreliables.Unreliables; -import org.testcontainers.DockerClientFactory; import org.testcontainers.containers.ContainerLaunchException; import org.testcontainers.containers.GenericContainer; -import org.testcontainers.dockerclient.DockerMachineClientProviderStrategy; -import org.testcontainers.dockerclient.WindowsClientProviderStrategy; +import org.testcontainers.containers.wait.internal.ExternalPortListeningCheck; +import org.testcontainers.containers.wait.internal.InternalCommandPortListeningCheck; -import java.net.Socket; import java.util.List; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; /** * Waits until a socket connection can be established on a port exposed or mapped by the container. @@ -23,87 +22,40 @@ @Slf4j public class HostPortWaitStrategy extends GenericContainer.AbstractWaitStrategy { - private static final String SUCCESS_MARKER = "TESTCONTAINERS_SUCCESS"; @Override protected void waitUntilReady() { - final Integer port = getLivenessCheckPort(); - if (null == port) { - log.debug("Liveness check port of {} is empty. Not waiting.", container.getContainerName()); + final Set externalLivenessCheckPorts = getLivenessCheckPorts(); + if (externalLivenessCheckPorts.isEmpty()) { + log.debug("Liveness check ports of {} is empty. Not waiting.", container.getContainerName()); return; } - Callable checkStrategy; + @SuppressWarnings("unchecked") + List exposedPorts = container.getExposedPorts(); - if (shouldCheckWithCommand()) { - List exposedPorts = container.getExposedPorts(); + final Set internalPorts = getInternalPorts(externalLivenessCheckPorts, exposedPorts); - Integer exposedPort = exposedPorts.stream() - .filter(it -> port.equals(container.getMappedPort(it))) - .findFirst() - .orElse(null); + Callable internalCheck = new InternalCommandPortListeningCheck(container, internalPorts); - if (null == exposedPort) { - log.warn("Liveness check port of {} is set to {}, but it's not listed in exposed ports.", - container.getContainerName(), port); - return; - } - - String[][] commands = { - { "/bin/sh", "-c", "nc -vz -w 1 localhost " + exposedPort + " && echo " + SUCCESS_MARKER }, - { "/bin/bash", "-c", " { - for (String[] command : commands) { - try { - if (container.execInContainer(command).getStdout().contains(SUCCESS_MARKER)) { - return true; - } - } catch (InterruptedException e) { - throw new RuntimeException(e); - } catch (Exception e) { - continue; - } - } - - return false; - }; - } else { - checkStrategy = () -> { - new Socket(container.getContainerIpAddress(), port).close(); - return true; - }; - } + Callable externalCheck = new ExternalPortListeningCheck(container, externalLivenessCheckPorts); try { Unreliables.retryUntilTrue((int) startupTimeout.getSeconds(), TimeUnit.SECONDS, () -> { - return getRateLimiter().getWhenReady(() -> { - try { - return checkStrategy.call(); - } catch (Exception e) { - throw new RuntimeException(e); - } - }); + return getRateLimiter().getWhenReady(() -> internalCheck.call() && externalCheck.call()); }); } catch (TimeoutException e) { throw new ContainerLaunchException("Timed out waiting for container port to open (" + - container.getContainerIpAddress() + ":" + port + " should be listening)"); + container.getContainerIpAddress() + + " ports: " + + externalLivenessCheckPorts + + " should be listening)"); } } - private boolean shouldCheckWithCommand() { - // Special case for Docker for Mac, see #160 - if (! DockerClientFactory.instance().isUsing(DockerMachineClientProviderStrategy.class) && - SystemUtils.IS_OS_MAC_OSX) { - return true; - } - - // Special case for Docker for Windows, see #160 - if (DockerClientFactory.instance().isUsing(WindowsClientProviderStrategy.class)) { - return true; - } - - return false; + private Set getInternalPorts(Set externalLivenessCheckPorts, List exposedPorts) { + return exposedPorts.stream() + .filter(it -> externalLivenessCheckPorts.contains(container.getMappedPort(it))) + .collect(Collectors.toSet()); } } diff --git a/core/src/main/java/org/testcontainers/containers/wait/WaitAllStrategy.java b/core/src/main/java/org/testcontainers/containers/wait/WaitAllStrategy.java new file mode 100644 index 00000000000..b4bf27ef98b --- /dev/null +++ b/core/src/main/java/org/testcontainers/containers/wait/WaitAllStrategy.java @@ -0,0 +1,38 @@ +package org.testcontainers.containers.wait; + +import org.rnorth.ducttape.timeouts.Timeouts; +import org.testcontainers.containers.GenericContainer; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +/** + * Wait strategy that waits for a number of other strategies to pass in series. + */ +public class WaitAllStrategy implements WaitStrategy { + + private final List strategies = new ArrayList<>(); + private Duration timeout = Duration.ofSeconds(30); + + @Override + public void waitUntilReady(GenericContainer container) { + Timeouts.doWithTimeout((int) timeout.toMillis(), TimeUnit.MILLISECONDS, () -> { + for (WaitStrategy strategy : strategies) { + strategy.waitUntilReady(container); + } + }); + } + + public WaitAllStrategy withStrategy(WaitStrategy strategy) { + this.strategies.add(strategy); + return this; + } + + @Override + public WaitStrategy withStartupTimeout(Duration startupTimeout) { + this.timeout = startupTimeout; + return this; + } +} diff --git a/core/src/main/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheck.java b/core/src/main/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheck.java new file mode 100644 index 00000000000..65f6182b2bc --- /dev/null +++ b/core/src/main/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheck.java @@ -0,0 +1,32 @@ +package org.testcontainers.containers.wait.internal; + +import lombok.RequiredArgsConstructor; +import org.testcontainers.containers.Container; + +import java.io.IOException; +import java.net.Socket; +import java.util.Set; +import java.util.concurrent.Callable; + +/** + * Mechanism for testing that a socket is listening when run from the test host. + */ +@RequiredArgsConstructor +public class ExternalPortListeningCheck implements Callable { + private final Container container; + private final Set externalLivenessCheckPorts; + + @Override + public Boolean call() { + String address = container.getContainerIpAddress(); + + for (Integer externalPort : externalLivenessCheckPorts) { + try { + new Socket(address, externalPort).close(); + } catch (IOException e) { + throw new IllegalStateException("Socket not listening yet: " + externalPort); + } + } + return true; + } +} diff --git a/core/src/main/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheck.java b/core/src/main/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheck.java new file mode 100644 index 00000000000..a6f51d9a2de --- /dev/null +++ b/core/src/main/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheck.java @@ -0,0 +1,49 @@ +package org.testcontainers.containers.wait.internal; + +import lombok.RequiredArgsConstructor; +import org.testcontainers.containers.Container; + +import java.util.Set; + +import static java.lang.String.format; + +/** + * Mechanism for testing that a socket is listening when run from the container being checked. + */ +@RequiredArgsConstructor +public class InternalCommandPortListeningCheck implements java.util.concurrent.Callable { + + private static final String SUCCESS_MARKER = "TESTCONTAINERS_SUCCESS"; + + private final Container container; + private final Set internalPorts; + + @Override + public Boolean call() { + for (Integer internalPort : internalPorts) { + tryPort(internalPort); + } + + return true; + } + + private void tryPort(Integer internalPort) { + String[][] commands = { + {"/bin/sh", "-c", format("cat /proc/net/tcp | awk '{print $2}' | grep :%x && echo %s", internalPort, SUCCESS_MARKER)}, + {"/bin/sh", "-c", format("nc -vz -w 1 localhost %d && echo %s", internalPort, SUCCESS_MARKER)}, + {"/bin/bash", "-c", format(" { + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.MILLISECONDS); + return null; + }).when(strategy1).waitUntilReady(eq(container)); + + assertThrows("The outer strategy timeout applies", TimeoutException.class, () -> { + underTest.waitUntilReady(container); + }); + } +} \ No newline at end of file diff --git a/core/src/test/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheckTest.java b/core/src/test/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheckTest.java new file mode 100644 index 00000000000..231942bf568 --- /dev/null +++ b/core/src/test/java/org/testcontainers/containers/wait/internal/ExternalPortListeningCheckTest.java @@ -0,0 +1,71 @@ +package org.testcontainers.containers.wait.internal; + +import com.google.common.collect.ImmutableSet; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.rnorth.visibleassertions.VisibleAssertions; +import org.testcontainers.containers.Container; + +import java.net.ServerSocket; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.rnorth.visibleassertions.VisibleAssertions.assertThrows; + +public class ExternalPortListeningCheckTest { + + private ServerSocket listeningSocket1; + private ServerSocket listeningSocket2; + private ServerSocket nonListeningSocket; + private Container mockContainer; + + @Before + public void setUp() throws Exception { + listeningSocket1 = new ServerSocket(0); + listeningSocket2 = new ServerSocket(0); + + nonListeningSocket = new ServerSocket(0); + nonListeningSocket.close(); + + mockContainer = mock(Container.class); + when(mockContainer.getContainerIpAddress()).thenReturn("127.0.0.1"); + } + + @Test + public void singleListening() { + + final ExternalPortListeningCheck check = new ExternalPortListeningCheck(mockContainer, ImmutableSet.of(listeningSocket1.getLocalPort())); + + final Boolean result = check.call(); + + VisibleAssertions.assertTrue("ExternalPortListeningCheck identifies a single listening port", result); + } + + @Test + public void multipleListening() { + + final ExternalPortListeningCheck check = new ExternalPortListeningCheck(mockContainer, ImmutableSet.of(listeningSocket1.getLocalPort(), listeningSocket2.getLocalPort())); + + final Boolean result = check.call(); + + VisibleAssertions.assertTrue("ExternalPortListeningCheck identifies multiple listening port", result); + } + + @Test + public void oneNotListening() { + + final ExternalPortListeningCheck check = new ExternalPortListeningCheck(mockContainer, ImmutableSet.of(listeningSocket1.getLocalPort(), nonListeningSocket.getLocalPort())); + + assertThrows("ExternalPortListeningCheck detects a non-listening port among many", + IllegalStateException.class, + (Runnable) check::call); + + } + + @After + public void tearDown() throws Exception { + listeningSocket1.close(); + listeningSocket2.close(); + } +} \ No newline at end of file diff --git a/core/src/test/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheckTest.java b/core/src/test/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheckTest.java new file mode 100644 index 00000000000..75684256997 --- /dev/null +++ b/core/src/test/java/org/testcontainers/containers/wait/internal/InternalCommandPortListeningCheckTest.java @@ -0,0 +1,33 @@ +package org.testcontainers.containers.wait.internal; + +import com.google.common.collect.ImmutableSet; +import org.junit.Rule; +import org.junit.Test; +import org.testcontainers.containers.GenericContainer; + +import static org.rnorth.visibleassertions.VisibleAssertions.assertThrows; +import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue; + +public class InternalCommandPortListeningCheckTest { + + @Rule + public GenericContainer nginx = new GenericContainer<>("nginx:1.9.4"); + + @Test + public void singleListening() { + final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(nginx, ImmutableSet.of(80)); + + final Boolean result = check.call(); + + assertTrue("InternalCommandPortListeningCheck identifies a single listening port", result); + } + + @Test + public void nonListening() { + final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(nginx, ImmutableSet.of(80, 1234)); + + assertThrows("InternalCommandPortListeningCheck detects a non-listening port among many", + IllegalStateException.class, + (Runnable) check::call); + } +} \ No newline at end of file diff --git a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java index fb44c90a6a2..5f7fb72241a 100644 --- a/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java +++ b/modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java @@ -149,9 +149,6 @@ protected void optionallyMapResourceParameterAsVolume(@NotNull String paramName, } } - @Override - protected abstract Integer getLivenessCheckPort(); - public void setParameters(Map parameters) { this.parameters = parameters; } diff --git a/modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java b/modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java index 9401dce238d..75a94824b82 100644 --- a/modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java +++ b/modules/mysql/src/main/java/org/testcontainers/containers/MySQLContainer.java @@ -1,5 +1,10 @@ package org.testcontainers.containers; +import org.jetbrains.annotations.NotNull; + +import java.util.HashSet; +import java.util.Set; + /** * @author richardnorth */ @@ -21,9 +26,10 @@ public MySQLContainer(String dockerImageName) { super(dockerImageName); } + @NotNull @Override - protected Integer getLivenessCheckPort() { - return getMappedPort(MYSQL_PORT); + protected Set getLivenessCheckPorts() { + return new HashSet<>(getMappedPort(MYSQL_PORT)); } @Override diff --git a/modules/nginx/src/main/java/org/testcontainers/containers/NginxContainer.java b/modules/nginx/src/main/java/org/testcontainers/containers/NginxContainer.java index 2b0257ca409..1e2ad9e47fc 100644 --- a/modules/nginx/src/main/java/org/testcontainers/containers/NginxContainer.java +++ b/modules/nginx/src/main/java/org/testcontainers/containers/NginxContainer.java @@ -1,9 +1,12 @@ package org.testcontainers.containers; +import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.traits.LinkableContainer; import java.net.MalformedURLException; import java.net.URL; +import java.util.HashSet; +import java.util.Set; /** * @author richardnorth @@ -16,9 +19,10 @@ public NginxContainer() { super("nginx:1.9.4"); } + @NotNull @Override - protected Integer getLivenessCheckPort() { - return getMappedPort(80); + protected Set getLivenessCheckPorts() { + return new HashSet<>(getMappedPort(80)); } @Override diff --git a/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java b/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java index 9ba7d23b7aa..325aad44332 100644 --- a/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java +++ b/modules/postgresql/src/main/java/org/testcontainers/containers/PostgreSQLContainer.java @@ -1,8 +1,11 @@ package org.testcontainers.containers; +import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.wait.LogMessageWaitStrategy; import java.time.Duration; +import java.util.HashSet; +import java.util.Set; import static java.time.temporal.ChronoUnit.SECONDS; @@ -29,9 +32,10 @@ public PostgreSQLContainer(final String dockerImageName) { .withStartupTimeout(Duration.of(60, SECONDS)); } + @NotNull @Override - protected Integer getLivenessCheckPort() { - return getMappedPort(POSTGRESQL_PORT); + protected Set getLivenessCheckPorts() { + return new HashSet<>(getMappedPort(POSTGRESQL_PORT)); } @Override diff --git a/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java b/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java index 81c84786f6d..d3f66dd372d 100644 --- a/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java +++ b/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java @@ -1,6 +1,8 @@ package org.testcontainers.containers; import com.github.dockerjava.api.command.InspectContainerResponse; +import com.google.common.collect.ImmutableSet; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.runner.Description; import org.openqa.selenium.remote.BrowserType; @@ -12,7 +14,10 @@ import org.slf4j.LoggerFactory; import org.testcontainers.containers.traits.LinkableContainer; import org.testcontainers.containers.traits.VncService; +import org.testcontainers.containers.wait.HostPortWaitStrategy; import org.testcontainers.containers.wait.LogMessageWaitStrategy; +import org.testcontainers.containers.wait.WaitAllStrategy; +import org.testcontainers.containers.wait.WaitStrategy; import java.io.File; import java.net.MalformedURLException; @@ -20,6 +25,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collection; +import java.util.Set; import java.util.concurrent.TimeUnit; import static java.time.temporal.ChronoUnit.SECONDS; @@ -55,9 +61,15 @@ public class BrowserWebDriverContainer getLivenessCheckPorts() { + return ImmutableSet.of(getMappedPort(SELENIUM_PORT), getMappedPort(VNC_PORT)); } @Override @@ -154,9 +167,7 @@ protected void containerIsStarted(InspectContainerResponse containerInfo) { if (recordingMode != VncRecordingMode.SKIP) { LOGGER.debug("Starting VNC recording"); - // Use multiple startup attempts due to race condition between Selenium being available and VNC being available - VncRecordingSidekickContainer recordingSidekickContainer = new VncRecordingSidekickContainer<>(this) - .withStartupAttempts(3); + VncRecordingSidekickContainer recordingSidekickContainer = new VncRecordingSidekickContainer<>(this); recordingSidekickContainer.start(); currentVncRecordings.add(recordingSidekickContainer); diff --git a/modules/selenium/src/test/java/org/testcontainers/junit/FirefoxWebDriverContainerTest.java b/modules/selenium/src/test/java/org/testcontainers/junit/FirefoxWebDriverContainerTest.java index 7bd7e3b9ad1..6b8f915498a 100644 --- a/modules/selenium/src/test/java/org/testcontainers/junit/FirefoxWebDriverContainerTest.java +++ b/modules/selenium/src/test/java/org/testcontainers/junit/FirefoxWebDriverContainerTest.java @@ -5,8 +5,6 @@ import org.openqa.selenium.remote.DesiredCapabilities; import org.testcontainers.containers.BrowserWebDriverContainer; -import java.io.IOException; - /** * */ @@ -17,12 +15,12 @@ public class FirefoxWebDriverContainerTest extends BaseWebDriverContainerTest { .withDesiredCapabilities(DesiredCapabilities.firefox()); @Test - public void simpleTest() throws IOException { + public void simpleTest() { doSimpleWebdriverTest(firefox); } @Test - public void simpleExploreTest() throws IOException { + public void simpleExploreTest() { doSimpleExplore(firefox); } }