Skip to content

Commit

Permalink
Fix internal port check when other ports are opened as well on the ta…
Browse files Browse the repository at this point in the history
…rget container (testcontainers#2363)

* Remove redundant execCreateCmd call

This one is never used and probably causes some garbage on the docker side.

* Allow leading zeros in /proc/net/tcp* when doing internal port listen check

When at least one process is listening for a port that is significantly
higher then the port we're checking, this check will fail as the checked
port will contain leading zeros in the output of "cat /proc/net/tcp".

This commit fixes this by changing the grep to ":0*XXX".

* Surround grep parameter with '

Otherwise unnecessary shell expansion is tried

* Refactor/Rewrite InternalCommandPortListeningCheckTest to test all possible checks

This commit refactors InternalCommandPortListeningCheckTest to work with
3 different nginx based containers, each being able to only succeed one of
the 3 tests performed in InternalCommandPortListeningCheck. Each test is
now executed agains all 3 containers to ensure that all tests work as
expected.

* Add test for low+high ports

This test should fail without the leading zeros fix found a few commits
before this one.

* Check for bash existence instead of only mentioning it in a comment

* Rename nginx_on_8080.conf to nginx.conf

It's not only port 8080 anymore, so better not have it in the name.

* Make container a @rule again by moving initialization into contructor

* Wait a few seconds for InternalCommandPortListeningCheck to pass
  • Loading branch information
codablock authored and quincy committed May 28, 2020
1 parent 65202c3 commit 27299d8
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 17 deletions.
Expand Up @@ -66,10 +66,6 @@ public Container.ExecResult execInContainer(InspectContainerResponse containerIn

DockerClient dockerClient = DockerClientFactory.instance().client();

dockerClient
.execCreateCmd(containerId)
.withCmd(command);

log.debug("{}: Running \"exec\" command: {}", containerName, String.join(" ", command));
final ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(containerId)
.withAttachStdout(true).withAttachStderr(true).withCmd(command).exec();
Expand Down
Expand Up @@ -29,7 +29,7 @@ public Boolean call() {
for (int internalPort : internalPorts) {
command += " && ";
command += " (";
command += format("cat /proc/net/tcp* | awk '{print $2}' | grep -i :%x", internalPort);
command += format("cat /proc/net/tcp* | awk '{print $2}' | grep -i ':0*%x'", internalPort);
command += " || ";
command += format("nc -vz -w 1 localhost %d", internalPort);
command += " || ";
Expand Down
Expand Up @@ -3,23 +3,47 @@
import com.google.common.collect.ImmutableSet;
import org.junit.Rule;
import org.junit.Test;
import org.testcontainers.containers.BindMode;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.rnorth.ducttape.TimeoutException;
import org.rnorth.ducttape.unreliables.Unreliables;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.images.builder.ImageFromDockerfile;

import java.util.concurrent.TimeUnit;

import static java.util.Arrays.asList;
import static org.rnorth.visibleassertions.VisibleAssertions.assertFalse;
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;

@RunWith(Parameterized.class)
public class InternalCommandPortListeningCheckTest {

// Linking a custom configuration into the container so that nginx is listening on port 8080. This is necessary to prove
// that the command formatting uses the correct casing for hexadecimal numbers (i.e. 1F90 and not 1f90).
@Parameterized.Parameters(name = "{index} - {0}")
public static Iterable<Object[]> data() {
return asList(
new Object[][]{
{"internal-port-check-dockerfile/Dockerfile-tcp"},
{"internal-port-check-dockerfile/Dockerfile-nc"},
{"internal-port-check-dockerfile/Dockerfile-bash"},
});
}

@Rule
public GenericContainer nginx = new GenericContainer<>("nginx:1.9.4")
.withClasspathResourceMapping("nginx_on_8080.conf", "/etc/nginx/conf.d/default.conf", BindMode.READ_ONLY);
public GenericContainer container;

public InternalCommandPortListeningCheckTest(String dockerfile) {
container = new GenericContainer(new ImageFromDockerfile()
.withFileFromClasspath("Dockerfile", dockerfile)
.withFileFromClasspath("nginx.conf", "internal-port-check-dockerfile/nginx.conf")
);
}

@Test
public void singleListening() {
final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(nginx, ImmutableSet.of(8080));
final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(container, ImmutableSet.of(8080));

Unreliables.retryUntilTrue(5, TimeUnit.SECONDS, check);

final Boolean result = check.call();

Expand All @@ -28,10 +52,27 @@ public void singleListening() {

@Test
public void nonListening() {
final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(nginx, ImmutableSet.of(8080, 1234));
final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(container, ImmutableSet.of(8080, 1234));

try {
Unreliables.retryUntilTrue(5, TimeUnit.SECONDS, check);
} catch (TimeoutException e) {
// we expect it to timeout
}

final Boolean result = check.call();

assertFalse("InternalCommandPortListeningCheck detects a non-listening port among many", result);
}

@Test
public void lowAndHighPortListening() {
final InternalCommandPortListeningCheck check = new InternalCommandPortListeningCheck(container, ImmutableSet.of(100, 8080));

Unreliables.retryUntilTrue(5, TimeUnit.SECONDS, check);

final Boolean result = check.call();

assertTrue("InternalCommandPortListeningCheck identifies a low and a high port", result);
}
}
@@ -0,0 +1,11 @@
FROM nginx:1.17-alpine

RUN apk add bash

# Make sure the /proc/net/tcp* check fails in this container
RUN rm /usr/bin/awk

# Make sure the nc check fails in this container
RUN rm /usr/bin/nc

ADD nginx.conf /etc/nginx/conf.d/default.conf
@@ -0,0 +1,9 @@
FROM nginx:1.17-alpine

# If this fails, you ended up using a base image with bash installed. Consider removing /bin/bash in this case
RUN if bash -c true &> /dev/null; then exit 1; fi

# Make sure the /proc/net/tcp* check fails in this container
RUN rm /usr/bin/awk

ADD nginx.conf /etc/nginx/conf.d/default.conf
@@ -0,0 +1,9 @@
FROM nginx:1.17-alpine

# If this fails, you ended up using a base image with bash installed. Consider removing /bin/bash in this case
RUN if bash -c true &> /dev/null; then exit 1; fi

# Make sure the nc check fails in this container
RUN rm /usr/bin/nc

ADD nginx.conf /etc/nginx/conf.d/default.conf
@@ -0,0 +1,8 @@
# This configuration makes Nginx listen on port port 8080

server {
# Port 8080 is necessary to prove that the command formatting in the /proc/net/tcp* check uses the correct casing for hexadecimal numbers (i.e. 1F90 and not 1f90)
listen 8080;
# Port 100 is necessary to ensure that the /proc/net/tcp* check also succeeds with trailing zeros in the hexadecimal port
listen 100;
}
5 changes: 0 additions & 5 deletions core/src/test/resources/nginx_on_8080.conf

This file was deleted.

0 comments on commit 27299d8

Please sign in to comment.