Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-37769] Failing to stop containers cleanly, so at least…
… stopping them quickly.
  • Loading branch information
jglick committed Sep 13, 2016
1 parent 7354b1c commit 0fed7a7
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Expand Up @@ -136,7 +136,7 @@ public String run(@Nonnull EnvVars launchEnv, @Nonnull String image, @CheckForNu
* @param containerId The container ID.
*/
public void stop(@Nonnull EnvVars launchEnv, @Nonnull String containerId) throws IOException, InterruptedException {
LaunchResult result = launch(launchEnv, false, "stop", containerId);
LaunchResult result = launch(launchEnv, false, "stop", "--time=1", containerId);
if (result.getStatus() != 0) {
throw new IOException(String.format("Failed to kill container '%s'.", containerId));
}
Expand Down
Expand Up @@ -62,7 +62,7 @@ public void test_run() throws IOException, InterruptedException {
EnvVars launchEnv = newLaunchEnv();
String containerId =
dockerClient.run(launchEnv, "learn/tutorial", null, null, Collections.<String, String>emptyMap(), Collections.<String>emptyList(), new EnvVars(),
dockerClient.whoAmI(), "true");
dockerClient.whoAmI(), "cat");
Assert.assertEquals(64, containerId.length());
ContainerRecord containerRecord = dockerClient.getContainerRecord(launchEnv, containerId);
Assert.assertEquals(dockerClient.inspect(launchEnv, "learn/tutorial", ".Id"), containerRecord.getImageId());
Expand Down

3 comments on commit 0fed7a7

@earlruby
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that the docker stop command sends a SIGTERM, waits 10 seconds, then sends a SIGKILL, and allows users to selectively overwrite the timeout value is so that the software running in the container can intercept the SIGTERM signal and do a proper cleanup before exiting. (Close file handles, delete half-finished builds, disconnect from databases, etc.)

By hard-coding a 1s delay you've effectively disabled cleanup routines, or cause them to break.

@jglick
Copy link
Member Author

@jglick jglick commented on 0fed7a7 Mar 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disconnect from databases, etc.

Image use cases other than chroot (i.e., collection of preinstalled tools) are not handled by withDockerContainer. For anything else, use explicit docker CLI commands.

@earlruby
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many CI docker image use cases that have cleanup routines. Most are going to intercept SIGTERM, run cleanup, and exit. None of them can use docker-workflow-plugin if cleanup takes longer than 1s because SIGKILL will clobber them while they clean up.

Please sign in to comment.