Allow for graceful container stop

[iNikem]

Currently I see only 1 method to stop a running testcontainer: org.testcontainers.containers.GenericContainer#stop. But to my great surprise and dismay that method ends up calling dockerClient.killContainerCmd. So there is no way for graceful shutdown of running container which would allow for the application running inside it to perform any expected cleanup.

Please, provide a graceful stop.

[kiview]

Yes, you are right in this regard.
Can you provide your use case, so we can have a bit more context why this is a desired feature?

[iNikem]

Well, I think it is not very common one... I use testcontainers as a very convenient library to start/stop docker containers from my Java application. It is kinda "orchestrator", which starts and stops several containers in a sequence to achieve some result. So on one of the steps I have to start a container, wait for some externally observable effect and then stop this container. But I need to ensure that all shutdown hooks of that application (Spring's @PreDestroy annotations) complete successfully. Currently it is not possible, container is killed too soon.

[kiview]

Okay I see.
We will definitely consider this when working on Testcontainers 2.0, since this will probably also include a general purpose object oriented container API.

We can probably also add this to the current version as a manually callable method on GenericContainer.

[iNikem]

Yes, that was what I thinking about. IMO ideally we should introduce new method GenericContainer.kill, which will result in killContainerCmd and change current stop method to do stopContainerCmd. But this is backward incompatible change. So may be it is easier to make new method GenericContainer.stopGracefully, which clients could call if needed.

[kiview]

Yes, while I'd also like to align our API more closely to the domain language used by Docker (start/stop/pause/kill), I don't think the breaking change is worth it at the moment.

So adding a more explicit method makes sense IMO.

[iNikem]

Hei, guys. What do you think about marking this for 2.0 milestone? :)

[iNikem]

I have taken a look how easy it would be for me to propose a PR. I was unable to find a suitable place for my new proposed methods. Currently container.stop delegates directly to ResourceReaper. It would be strange if the latter was able to just stop containers, without removing them. So it seems to me that my original wish is not so easy implemented.

On the other hand, in our particular case we have found a satisfying workaround (shutting applications down via Spring Boot's actuator), so this issue does not bother us any more at all.

As a result, I suggest just closing this issue as not relevant.

[jesusha123]

This proposal is still useful for test cases that involve microservice disconnections/reconnections due to docker containers stopping.

[vsuharnikov]

Also it is strange, that stop stops and removes a container from the point of a Docker user (sooner or later you have to deal with the Docker).

[marcphilipp]

I think a good first step would be to keep everything as-is but use a stop command instead of a kill one. The default timeout of 10s should be fine in most cases. If necessary, it could be made configurable or an overloaded stop(Duration) method could be added. WDYT?

[bsideup]

There is containerIsStopping that one can use to execute stopContainerCmd. It is also possible to stop the container externally:

container.getDockerClient().stopContainerCmd(container.getContainerId()).exec();

The default timeout of 10s should be fine in most cases.
Most of containers don't need a graceful shutdown, and it would only slow down the tests.

Also note that stopping and then starting a container won't work properly (one of the use cases for not killing the container from stop), as Docker will assign new random ports and other state adjustments. For those who want to test what happens when the process is restarted, it is recommended to restart the process inside the container and not the container itself.

Given this, I suggest we close this issue. @rnorth @kiview WDYT?