Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't rely on the '--rm' flag for stopping containers in integration tests #21119

Merged
merged 1 commit into from
Nov 1, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

import org.apache.commons.lang3.RandomStringUtils;

import io.quarkus.test.common.http.TestHTTPResourceManager;

public class DefaultDockerContainerLauncher implements DockerContainerArtifactLauncher {

private Process quarkusProcess;
private static final String DOCKER_BINARY = "docker";

private int httpPort;
private int httpsPort;
private long waitTimeSeconds;
Expand All @@ -35,6 +39,8 @@ public class DefaultDockerContainerLauncher implements DockerContainerArtifactLa

private boolean isSsl;

private String containerName;

@Override
public void init(DockerContainerArtifactLauncher.DockerInitContext initContext) {
this.httpPort = initContext.httpPort();
Expand All @@ -59,7 +65,7 @@ public void start() throws IOException {
System.out.println("Pulling container image '" + containerImage + "'");
try {
int pullResult = new ProcessBuilder().redirectError(DISCARD).redirectOutput(DISCARD)
.command(List.of("docker", "pull", containerImage).toArray(new String[0])).start().waitFor();
.command(DOCKER_BINARY, "pull", containerImage).start().waitFor();
if (pullResult > 0) {
throw new RuntimeException("Pulling container image '" + containerImage + "' completed unsuccessfully");
}
Expand All @@ -78,11 +84,14 @@ public void start() throws IOException {
}

List<String> args = new ArrayList<>();
args.add("docker"); // TODO: determine this dynamically?
args.add(DOCKER_BINARY); // TODO: determine this dynamically?
args.add("run");
if (!argLine.isEmpty()) {
args.addAll(argLine);
}
args.add("--name");
containerName = "quarkus-integration-test-" + RandomStringUtils.random(5, true, false);
args.add(containerName);
args.add("--rm");
geoand marked this conversation as resolved.
Show resolved Hide resolved
args.add("-p");
args.add(httpPort + ":" + httpPort);
Expand Down Expand Up @@ -120,7 +129,8 @@ public void start() throws IOException {
// the idea here is to obtain the logs of the application simply by redirecting all its output the a file
// this is done in contrast with the JarLauncher and NativeImageLauncher because in the case of the container
// the log itself is written inside the container
quarkusProcess = new ProcessBuilder(args).redirectError(logFile.toFile()).redirectOutput(logFile.toFile()).start();
Process quarkusProcess = new ProcessBuilder(args).redirectError(logFile.toFile()).redirectOutput(logFile.toFile())
.start();

if (startedFunction != null) {
IntegrationTestStartedNotifier.Result result = waitForStartedFunction(startedFunction, quarkusProcess,
Expand Down Expand Up @@ -163,7 +173,13 @@ private String convertPropertyToEnVar(String property) {

@Override
public void close() {
quarkusProcess.destroy();
try {
Process dockerStopProcess = new ProcessBuilder(DOCKER_BINARY, "stop", containerName).redirectError(DISCARD)
.redirectOutput(DISCARD).start();
dockerStopProcess.waitFor(10, TimeUnit.SECONDS);
} catch (IOException | InterruptedException e) {
System.out.println("Unable to stop container '" + containerName + "'");
}
}

}