-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[Bug]: Container Shutdown behavior changed between 1.19.1 and 1.19.2 ressources not cleaned up properly #7871
Comments
This is not related to changes in Testcontainers 1.19.2. I guess you are using
Between step 4 and 5, the container and service it will become unavailable and the client is still running and sometimes trying to connect to the service that is not running anymore. You can use |
Hey @eddumelendez thanks for your reply. Initially I thought about something similar but the problem is, that this never happened before the upgrade of testcontainers from 1.19.1 to 1.19.2. I tested both versions and only this one seems to fail. It's also the only dependency that has been updated around the time it started coming up in our projects. To me this seems highly related to this version upgrade and I also saw changes in the ryuk setup in 1.19.2. |
I've been always experiencing this with sqs, activemq, artemis, r2dbc clients sometimes so far. The change about ryuk is that it will not be running 10 additional seconds after test are done but kill just right after the JVM shutdowm. |
You can see others reporting the same in previous versions. |
Thanks @eddumelendez your hints and the previous issue you mentioned was really helpful! 👍 |
@eddumelendez Is it possible to add an ability to optionally disable the registering of this shudown hook? If I understand you correctly, At the moment after upgrading to Spring Boot 3.2.2 (and TestContainers 1.19.3) this behavior just makes any logic, that executes during Spring Application shutdown phase, and requires a container to exist, to fail. One example I described here: #7454 (comment) Or what else can be done except marking each method with |
I can confirm that the issue occurs when I bump the version fom 1.19.1 to 1.19.2 where a shutdown hook has been added. @eddumelendez - Could the shutdown hook registration be configurable so devs could decide if they want to use new or legacy approach which works Spring context? I agree with @xak2000, test containers are widely used in Spring applications tests so I think that integration should also be tested before releasing a new version of test containers. Spring major version has been not bumped up so it means that testcontainers should work with all 6.x.x versions. DirtyContext is not an option for apps with a lot of tests. I want to use shared context between tests to speed them up. |
Can this be reopened? The change from #7717 actually broke shutdown procedure in a lot of Spring Boot applications and there's no way to fix it without reverting back to 1.19.1. In this thread it was incorrectly stated that Ryuk will shut down immediately after JVM shutdown, but the truth is that Ryuk now receives SIGTERM the moment JVM receives SIGTERM which introduces race condition between Spring's shutdown hook and Ryuk. |
@mjagus can you share a project that reproduces the issue? I'm constantly running several examples and those works well. Also, spring boot keep updated with latest versions without issues. The well-known behavior is described here but that's related to logging and no test failures. |
This is an issue that I am facing in projects that I am working on as well. As soon as I update to 1.19.2 getting errors like:
|
@eddumelendez - Do you need a project which reproduces the issue even you know that the test containers stops before spring context is closed? I can provide one if you need it. Saying that this is "just logging issue" is difficult for me to understand. I do not accept any exception in tests unless it is expected exception. Mongo connectivity exceptions are not expected. I will stay with version which does not clutter my logs. I think test containers is great and I'm keeping fingers crossed to find satisfying solution. |
I'll add my 5 cents to the last comment. The problem with this "just logging issue" is that it is not always just logging issue, but sometimes it is also actual behavior issue. As I alredy described here there are legitimate cases when Spring Framework (or one of its libraries) tries to execute an SQL query when Spring Context is closing. One example is It's just my experience. Who knows what else libraries this behavior could affect and in what way. I can't believe that this behavior is considered as OK now. What I believe is that it is probably not that easy to synchronize the shutdown of TestContainers with the shutdown of Spring Application Context. But this doesn't mean this problem should be ignored. Let's try to find a solution together! And while it is not found yet, at least return the old behavior of 10s delay before shutdown TestContainers. Probably with a config property to override this behavior, if these 10s more time of living containers is really a problem for someone. You could argue that an application should be written in a way that non-functional external services should not prevent it from proper shutdown (12-factors and all this stuff) and I would agree, but this should not be "normal" situation that reproduces on every test run. A separate test should test this scenario, not all the tests that just test normal app behaviour scenarios. |
@eddumelendez Do you still need a reproduction project or more arguments why this new behavior (instant shutdown) is problematic? And, if you don't think that the new behavior is problematic, then can you please provide counter-arguments why it is not problematic in your opinion? I and several other people described (with arguments) that this behavior leads to inability for Spring Context to properly shutdown. "Properly shutdown" is not directly related to logging. The application has rights to do anything during shutdown. Do you disagree? And, if this "anything" happens to be something that is related to a container, that was already stopped, then this "anything" could not be done anymore. Right? WIll it lead to a failed test? Probably, not (because all test methods are already finished at this point). But it could easily lead to thousand of other problems. E.g. very long shutdown (or even infinite) because the app tries to do something important on shutdown and it will try to reconnect multiple times to the "failed" service until the action is succeeded. The situation when a service that the app depends on is stopped before the app is stopped is not normal. Yes, this situation should be handled, as it could happen in the producation too, but it is still not normal. It should not happen on every execution of the tests suite. I'm scratching my head trying to understand why we are even discussing this... What is more important? The correctly executed shutdown sequence or freeing some memory 10s ealier? 🤷 I understand that not all apps do something important, that depends on a container during shutdown, but if even some applications do this, then the delay should be configurable at least. |
An experiment that implements the idea of configurable shutdown hook: #7454 (reply in thread) The direct link to the commit that implements it: alex-arana@a0fcbdf |
I've went down this rabbit hole today and I found this issue. What @xak2000 explain in #7871 (comment) and #7871 (comment) is spot on. I saw that there is PR #8732 that is making this shutdown hook configurable. Is that PR an option for the test containers team to accept it and then the Spring Boot team would be able to set that property for their integration and / or we as users will be able to set this property to avoid such issues. |
Any progress on this issue? |
Hello, guys, is there any progress or news regarding this topic? |
Module
Core
Testcontainers version
1.19.2
Using the latest Testcontainers version?
Yes
Host OS
Linux
Host Arch
x86
Docker version
What happened?
We're facing problems running our integration tests using Testcontainers since 1.19.2 and even though our tests run fine, we're facing a lot of errors and flooded log outputs.
out test setup looks like this:
when completing our tests we face a lot of errors due to our sqs listeners not being able to poll messages anymore.
The complete log output can be found in the relevant log output section
Relevant log output
Additional Information
No response
The text was updated successfully, but these errors were encountered: