testcontainers / testcontainers-java

Testcontainers is a Java library that supports JUnit tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.org
MIT License
7.96k stars 1.64k forks source link

[Bug]: Shutdown hook in RyukResourceReaper prevents a graceful shutdown with spring framework #8558

Open jobayle opened 4 months ago

jobayle commented 4 months ago

Module

Core

Testcontainers version

1.19.7

Using the latest Testcontainers version?

Yes

Host OS

Linux, Windows

Host Arch

x86_64

Docker version

Client:
 Cloud integration: v1.0.35+desktop.13
 Version:           26.0.0
 API version:       1.45
 Go version:        go1.21.8
 Git commit:        2ae903e
 Built:             Wed Mar 20 15:18:56 2024
 OS/Arch:           windows/amd64
 Context:           default

Server: Docker Desktop 4.29.0 (145265)
 Engine:
  Version:          26.0.0
  API version:      1.45 (minimum version 1.24)
  Go version:       go1.21.8
  Git commit:       8b79278
  Built:            Wed Mar 20 15:18:01 2024
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.28
  GitCommit:        ae07eda36dd25f8a1b98dfbf587313b99c0190bb
 runc:
  Version:          1.1.12
  GitCommit:        v1.1.12-0-g51d5e94
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

What happened?

Dears,

With the introduction of this feature: #7717

The ryuk instance is terminated by a shutdown hook, therefore all containers are closed very early.

This prevents other shutdown hooks (eg: those set by the spring framework) to perform actions needed for a graceful shutdown that still require containers to be up. Due to a retry mechanism in Spring's shutdown hook, it greatly increases the overall time to run tests!

Could you please revert this change or introduce a mean to disable this shutdown hook, thanks!

Relevant log output

No response

Additional Information

No response

s-jepsen commented 4 months ago

Will this be fixed any time soon?

EAlf91 commented 4 months ago

Also relevant for #7871 there was a solution proposed in the issue as well

jobayle commented 3 months ago

Hi @EAlf91, I cannot find the solution in the linked issue, could you please add a link to the specific comment? thanks!

EAlf91 commented 3 months ago

@jobayle It's mentioned here: https://github.com/testcontainers/testcontainers-java/issues/7871#issuecomment-2041169911 but it would require a change I guess. The author made the hook configurable via spring properties: https://github.com/alex-arana/testcontainers-java/commit/a0fcbdf001ee17b5649882052ec62ad7092d1a74

I didn't try it myself and didn't see any way to configure this without using this fork. I'd highly appreciate to see a revert of #7717 or maybe using this way to configure it via spring properties

jobayle commented 3 months ago

Thanks @EAlf91, but imho, this is not a viable solution if we have to maintain a patched version of testcontainers.

s-jepsen commented 2 months ago

Any progress on this issue?