-
-
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
Allow container reuse opt-in via API #5364
Comments
Hi @Sanne, Thanks for the extensive feedback! The reason for it being opt-in is because, if users "hardcode" it, they will end up having it on CIs too, and it leads to dangling/uncontrolled containers. And the So it isn't about the feature being experimental, but rather CI safety. I understand that devservices are unlikely to be used on CIs, but it is also not a "regular" usage (although an amazing one!) Would appreciate your ideas of how to address this safely 👍 |
Hi @bsideup ! sorry for the delay :) Ok interesting; in my case I was actually hoping for containers to be "leaked" (intentionally) on our CI run. If you look at the Quarkus build jobs, you'll see we have > 1,000 Maven modules - and that's just counting the primary repository. A dozen of which will need to run a PostgreSQL container, another dozen will need a DB2 container, Elasticsearch containers, etc.. in a variety of combinations and profiles. While the PostgreSQL is awesome at quick start, others such as DB2 take north of 10 minutes to boot; validating a PR takes us about 4 hours. Today we're starting containers "imperatively" via specific github action profiles to save time; this is error prone and often gets out of sync as rules need to match the build expectations; more importantly, it implies we're not testing with Testcontainers. I might have misunderstood some things about Ryuk - what is its purpose? I thought this was a registry to ~control reuse, and terminate lingering containers after some timeout? I actually meant to ask about that. In short I'd be totally fine to "leak" containers on our CI to allow better reuse, even across builds. Ideally perhaps with Ryuk terminating them in background if there's been no "start request" issued in hours - but that also doesn't feel necessary as most of our CI nodes are spawn brand new for each job. A pending, separate problem would be to do a little resource management; I doubt our test nodes can keep all those containers "alive" at the same time. |
Hey @Sanne, thanks for your detailed explanation of your use case 🙂 In the context of TC, Ryuk plays the role of our resource cleanup process. As a default, the lifecycle of resources created by TC is bound to the lifecycle of the JVM process. While it is possible that resources are consciously removed as part of individual test executions, Ryuk acts as a safeguard to ensure no lingering zombies containers (or other resources) are left hanging around after the tests. This is specifically useful on persistent CI runners, but also for local development. The current implementation of reusable mode will disable this mechanism and is intended for TDD-like workflows on local developer machines. It is not intended for CI usage and you already outlined some of the issues that would appear in such cases:
So I fear for the time being, tackling your CI problem challenges with the reusable mode in Testcontainers is not recommended. Of course, there is always potential to improve the mechanisms and make it more flexible and clever, but we currently don't intend to focus on it. |
Fair enough, thanks for all explanations! I'll close this now, hopefully a better proposal will emerge in time :) |
Hey guys, If I understood correctly, this feature is not unstable already and can be marked as official behaviour. So I'd remove unstable API in code and added extended javadoc for this, and a paragraph in the docs, that in this way we are trying to make the settings of the re-use of containers explicit to users from the outside. |
Hello all,
I've enabled support for container reuse in Quarkus dev-services in Quarkus:
It's working great, many thanks for the nice improvement; althought so far I've only enabled it for selected containers, specifically the ones running relational databases people use for testing.
I'm aware - and have documented - that people need to enable this opt-in by setting the relevant property in the
.testcontainers.properties
configuration file.This seems like a wise choice since the feature is currently experimental; yet people in our community have been asking to have this working by default on selected, cherry-picked containers, so it would be nice for us to have a way to opt-in for a particular container without people needing to edit the core configuration file.
I understand it's possibly not a good time to do so, but eventually as the feature is considered more mature I'd love to see a way for the API to give a stronger hint to the Testcontainer core that a particular container really should be reused.
Proposal
One approach could be for the
withReuse
method to accept a three-state enum rather than a boolean, like { OFF|ON|According_to_config }.Additional consequences
The logic to stop containers in Quarkus also needs to know if the container is allowed to be reused. So I suspect some other methods will need to be adapted to support this as wel, such as
isShouldBeReused
and possibly others.Many thanks for the consideration! Reported on suggestion from cc/ @kiview at Devoxx :)
The text was updated successfully, but these errors were encountered: