Skip to content

[troubleshooting] Add ryuk troubleshooting section to README#3298

Open
jsmid1 wants to merge 1 commit into
conforma:mainfrom
jsmid1:troubleshooting
Open

[troubleshooting] Add ryuk troubleshooting section to README#3298
jsmid1 wants to merge 1 commit into
conforma:mainfrom
jsmid1:troubleshooting

Conversation

@jsmid1

@jsmid1 jsmid1 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add troubleshooting entry for ryuk reaper container failures in acceptance tests
  • Documents how to diagnose the issue by running ryuk manually and how to fix it via ~/.testcontainers.properties

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

A new troubleshooting subsection is added to the README for ryuk reaper container startup failures, including a diagnostic command and the ryuk.container.privileged=true configuration setting.

Changes

Ryuk Reaper Troubleshooting Documentation

Layer / File(s) Summary
Troubleshooting subsection
README.md
Adds subsection 2.4 for ryuk reaper container startup failures, including a foreground diagnostic command and ~/.testcontainers.properties guidance for ryuk.container.privileged=true.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly reflects the main change: adding a Ryuk troubleshooting section to the README.
Description check ✅ Passed The description matches the README update and accurately describes the new Ryuk troubleshooting guidance.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@st3penta st3penta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

@simonbaird simonbaird left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm.

@codecov

codecov Bot commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
acceptance 53.44% <ø> (ø)
generative 16.79% <ø> (ø)
integration 27.66% <ø> (ø)
unit 69.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@st3penta

Copy link
Copy Markdown
Contributor

/ok-to-test

Document how to diagnose and fix acceptance test failures caused by the ryuk reaper container being unable to access the Docker socket.

Signed-off-by: Jan Smid <jsmid@redhat.com>
@jsmid1 jsmid1 force-pushed the troubleshooting branch from f069923 to d5c3619 Compare June 24, 2026 07:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 115-121: The README guidance for setting
ryuk.container.privileged=true needs an explicit scope-and-safety warning
because it affects all Testcontainers usage for that user, not just this repo.
Update the troubleshooting text around the existing Docker daemon permission
note to clearly label it as a temporary local workaround, mention the security
impact of enabling privileged Ryuk globally, and add rollback guidance to remove
or reset the ~/.testcontainers.properties entry after diagnosing the issue.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 7094679b-289b-4554-845a-5fd17f3487a1

📥 Commits

Reviewing files that changed from the base of the PR and between f069923 and d5c3619.

📒 Files selected for processing (1)
  • README.md

Comment thread README.md
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:57 AM UTC · Completed 8:05 AM UTC
Commit: 47d3320 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Looks good to me.

Findings

Low

  • [edge-case] README.md:112 — The troubleshooting command hardcodes testcontainers/ryuk:0.11.0, but the actual ryuk version used at runtime is determined by the testcontainers-go library (currently v0.34.0 per acceptance/go.mod). If the library is updated, this diagnostic command will reference a different image than what tests actually use, potentially giving misleading results. Consider noting that users should check docker ps output during a test run to find the exact ryuk image tag in use.

Comment thread README.md
@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 24, 2026
@jsmid1

jsmid1 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants