testcontainers / moby-ryuk

Schedule Moby/Docker containers cleanup after specific delay.
https://www.testcontainers.com
MIT License
182 stars 33 forks source link

fix: timeout and shutdown #121

Closed stevenh closed 5 months ago

stevenh commented 6 months ago

Fix waitForPruneCondition timeout handling which was not resetting the connection timeout when new connections came in resulting in the reaper shutting down incorrectly.

Don't log EOF errors.

Add buffer to connection channels so we don't block the accepting goroutine.

Fixes https://github.com/testcontainers/testcontainers-go/issues/2348

stevenh commented 5 months ago

@mdelapenya any chance you can help get this one across the line, as its causing random failures in testcontainers-go?

stevenh commented 5 months ago

LGTM, thanks and sorry for the delay in the review. I've been focused in other tasks regarding testcontainers-go.

Thank you for your time!

Thanks for that, most appreciated!

To get this one fully put to bed, what's the release process, as we'll need to a PR against testcontainers-go to bump it to the new version to get this fix?

mdelapenya commented 5 months ago

Exactly, I'm planning to create a release of Ryuk soon this week, the 0.7.1

stevenh commented 4 months ago

Just checking in on the release @mdelapenya to make sure it doesn't slip though 😃

stevenh commented 2 months ago

Hey @mdelapenya just checking in on this, as we're seeing constant test failures on very basic setups due to this, so would really appreciate a new release here and then one for testcontainers-go with the updated image.

If there's anything I can do assist let me know.

emetsger commented 2 months ago

We are hitting this bug as well, and I was planning on fixing it this weekend, but happy to see @stevenh beat me to it.

For posterity: