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

Automatically add LAMBDA_DOCKER_FLAGS with testcontainer labels #8595

Merged
merged 2 commits into from
May 6, 2024

Conversation

dfangl
Copy link
Contributor

@dfangl dfangl commented May 6, 2024

Motivation

We have been seeing issues (like localstack/localstack#8616) where the Lambda containers spawned by LocalStack were not removed after the testcontainer session finished.
This can happen if the LocalStack container is killed before it can finish cleaning up all spawned resources.

Using LAMBDA_DOCKER_FLAGS we can, for example, add labels to all spawned lambda containers. Using this mechanism, we can add the same labels testcontainers sets on spawned containers on the lambda containers created by LocalStack, and let testcontainers handle the termination after the test session finishes.

Changes

  • Add internal testcontainer marker labels to LAMBDA_DOCKER_FLAGS
  • Add test asserting the labels on the spawned lambda container

@dfangl dfangl requested a review from a team as a code owner May 6, 2024 13:53
@eddumelendez eddumelendez added this to the next milestone May 6, 2024
@eddumelendez eddumelendez merged commit 2195610 into testcontainers:main May 6, 2024
99 checks passed
@eddumelendez
Copy link
Member

Thanks for a great contribution, @dfangl !

@dfangl dfangl deleted the localstack/shutdown-labels branch May 6, 2024 17:30
if (existingLambdaDockerFlags != null) {
lambdaDockerFlags = existingLambdaDockerFlags + " " + lambdaDockerFlags;
}
withEnv("LAMBDA_DOCKER_FLAGS", lambdaDockerFlags);
Copy link
Member

Choose a reason for hiding this comment

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

Thinking out loud: LocalStack could detect running in a container and copy its own flags (via inspect since it has access to Docker socket) to Lambda containers it creates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be an option as well, I agree. However, this was not a feature ever requested by our customers, often it is sufficient to manually specify it via the various docker flags.

My personal issue with it is: Would the user expect labels to automatically propagate in all cases, and I am not too sure about that. Of course we could make this configurable as well. I will put some more thought onto that and discuss it internally.

Another thing that comes to mind is the propagation of labels of the image itself, and of e.g. docker compose, which have to be considered and maybe filtered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants