fix: skip internal port check for Ryuk - #3858
Conversation
The internal port check invokes /bin/sh, but the Ryuk images do not include a shell. WaitUntilReady ignores the resulting exit status 127, so the internal check has no effect.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. Summary by CodeRabbit
WalkthroughThe Ryuk reaper wait strategy skips the internal listening-port check and continues to verify external port readiness. ChangesRyuk reaper readiness
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This change skips Ryuk's ineffective internal shell check while preserving readiness behavior and improving compatibility with alternative Docker implementations; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What does this PR do?
This PR skips Ryuk's internal port check by calling
SkipInternalCheck().The internal port check invokes
/bin/sh, but the Ryuk images do not include a shell. As a result, it always exits with status 127, whichWaitUntilReadyignores, so the internal check has no effect.Why is it important?
I discovered this issue while using
testcontainers-gowithsocktainer. During my investigation, I found that Ryuk's internal check effectively relies on docker exec returning exit status 127 for a missing executable. As described in socktainer/socktainer#383,socktainerinstead returns a different exit status, causing the check to fail.Since the internal check is ineffective for the current Ryuk images anyway—the images do not include a shell, and
WaitUntilReadyignores exit status 127—skipping it does not change the readiness behavior while improving compatibility with alternative Docker implementations.Related issues
docker execreturns exit status 255 instead of 127 for missing executables socktainer/socktainer#383