Ignore stopped containers in resource leak detector #21280
Conversation
Stopped containers can be left behind when using `withStartupAttempts()`. They should only use up disk space, not other resources, and will still be cleaned up by testcontainers on exit.
|
@wendigo would you like to take a look too? |
| @@ -81,6 +81,8 @@ public void testPlanExecutionFinished(TestPlan testPlan) | |||
|
|
|||
| List<Container> containers = dockerClient.listContainersCmd() | |||
| .withLabelFilter(Map.of(DockerClientFactory.TESTCONTAINERS_SESSION_ID_LABEL, DockerClientFactory.SESSION_ID)) | |||
| // ignore status "exited" - for example, failed containers after using `withStartupAttempts()` | |||
| .withStatusFilter(List.of("created", "restarting", "running", "paused")) | |||
There was a problem hiding this comment.
why isn't "starting" on the list?
There was a problem hiding this comment.
I based this on the Javadoc for withStatusFilter: https://github.com/docker-java/docker-java/blob/main/docker-java-api/src/main/java/com/github/dockerjava/api/command/ListContainersCmd.java#L78
This matches the CLI docs: https://docs.docker.com/reference/cli/docker/container/ls/#filter
Do you have a better reference?
There was a problem hiding this comment.
The set of status can change in the future, right?
There was a problem hiding this comment.
Yes, although I'm not sure how likely is that. This list hasn't changed at least since 8 years: docker-java/docker-java@acfe65d
There was a problem hiding this comment.
Java API javadoc may or may not be up to date. It's not the source of truth for docker container statuses.
There was a problem hiding this comment.
Sure, do you have a better reference?
There was a problem hiding this comment.
No, but also i didn't write this code :)
reportLeakedContainers adapted from trinodb/trino#20297 trinodb/trino#21280 Co-authored-by: Piotr Findeisen <[email protected]> Co-authored-by: Jan Waś <[email protected]>
reportLeakedContainers adapted from trinodb/trino#20297 trinodb/trino#21280 Co-authored-by: Piotr Findeisen <[email protected]> Co-authored-by: Jan Waś <[email protected]>
reportLeakedContainers adapted from trinodb/trino#20297 trinodb/trino#21280 Co-authored-by: Piotr Findeisen <[email protected]> Co-authored-by: Jan Waś <[email protected]>
Description
Stopped containers can be left behind when using
withStartupAttempts(). They should only use up disk space, not other resources, and will still be cleaned up by testcontainers on exit.Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: