Skip to content

When container fails to start, 2/3 of containers are not cleaned up. #102

Closed

Description

@ostacey

Seems like there's an issue with GenericContainer when a container fails to start correctly. It looks like this is what happens:

  • start() runs tryStart()
  • tryStart() creates a container, stores the containerId, starts it, and waits for the port to become available.
  • the port does not open; tryStart() throws an exception
  • the "finally" block in but not before it schedules a shutdown hook to remove the container.
  • this happens again two more times; each time the containerId is overwritten
  • the first shutdown hook triggers. it cleans up based on the containerId currently stored in the class; that is, the third container that was created.
  • the second and third shutdown hooks also trigger, and also attempt to clean up the same ID.
  • the first and second containers stick around un-cleaned.

This happens both when the container fails immediately, and when the container keeps running but doesn't ever listen on the port. (In the latter case, the leftover containers are still running!)

import org.junit.Rule;
import org.junit.Test;
import org.testcontainers.containers.GenericContainer;

public class TestBadCleanup {
   @Rule
   public GenericContainer failingContainer =
         new ContainerWithExecSupport("alpine:3.3")
               .withCommand("/bin/sh","-c","false")
               .withExposedPorts(8080);   

   @Test
   public void testBadCleanup() throws Exception {
      // no op
      // this test will pass, but a "docker ps -a" afterward will see two leftover containers.
   }
}

Activity

  1. rnorth commented on Mar 29, 2016

    @rnorth
    Member

    Thanks - this is a silly one, so thank you for investigating. The simplest fix would probably be to add better cleanup code on startup failure, though I'm wary of repeating this code in multiple places.

    Another solution (which I'll mull over implementing soon) would be to refactor out the JVM shutdown hook and maintain a set of container IDs that potentially need to be cleaned up. I'll think about this...

  2. added a commit that references this issue on Mar 30, 2016
  3. rnorth commented on Mar 30, 2016

    @rnorth
    Member

    I've done some work on this so that container removal is decoupled from the container objects themselves, and instead operates on all container IDs. This has actually subsumed @bsideup's changes in #110 and gone a step further; it really should be quite a lot more strict about cleanup now.

    Please see #111 for this.

  4. added this to the 1.0.3 milestone on Mar 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions