feat: improve container conflict detection

[Desuuuu]

What does this PR do?

Modify the container conflict detection regex for improved Podman compatibility.

The shape of the conflict error comes from Podman's test suite.

Why is it important?

Container creation conflicts happen fairly often when creating the reaper container in parallel tests. Making the retry logic work with the Podman provider can prevent some of these errors from happening.

Related issues

• Closes [Enhancement]: Improve container conflict detection compatibility #3569

[netlify]

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	a60fd7e
🔍 Latest deploy log	https://app.netlify.com/projects/testcontainers-go/deploys/69ac49b96771dc0008e0e7cb
😎 Deploy Preview	https://deploy-preview-3574--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bdf868a9-26e4-4b57-b114-627cd1253e40

📥 Commits

Reviewing files that changed from the base of the PR and between c0e8ab1 and a60fd7e.

📒 Files selected for processing (2)

• docker.go
• reaper_test.go

---

Summary by CodeRabbit

• Bug Fixes

  • Broadened recognition of container name-conflict messages to handle additional wording and capitalization variations, improving reliable handling of creation races.

• Tests

  • Added tests covering error classification across container conflicts, syscall/OS errors, context timeouts, and other error scenarios, verifying which errors are treated as permanent versus transient.

Walkthrough

Broadened container-name conflict detection regex to match Docker and Podman messages; added table-driven tests for spawner.retryError covering multiple error types and verifying permanent vs. non-permanent classification.

Changes

Cohort / File(s)	Summary
Container Conflict Detection docker.go	Adjusted the container-name conflict regex from matching "Conflict. The container name ..." to a pattern matching "[Tt]he container name .* is already in use by .*", allowing detection of Podman-style errors.
Spawner Retry Error Testing reaper_test.go	Added TestSpawnerRetryError with multiple cases (nil path, Docker/Podman conflict messages, syscall/context/timeout-like errors, OS errors, arbitrary errors). Introduced timeoutErr test type and assertions verifying which errors are wrapped as backoff.PermanentError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I sniffed the names in logs and hay,
Found Podman words that once went astray,
Tweaked my pattern, tests came to play,
Timeouts and conflicts now sorted today,
I hop on guard — come what may.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The test additions in reaper_test.go extend coverage beyond the scope of the linked issue #3569, which focuses solely on regex pattern updates for conflict detection.	The new TestSpawnerRetryError test appears unrelated to container conflict detection regex changes; verify whether this test is necessary for the PR objectives or should be split into a separate change.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change - improving container conflict detection regex for better provider compatibility.
Description check	✅ Passed	The description is directly related to the changeset, explaining the regex modification and its motivation for Podman compatibility.
Linked Issues check	✅ Passed	The changes fully implement the objective from issue #3569 by updating the conflict detection regex to match both Docker and Podman error formats without requiring exact 'Conflict.' prefix.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

reaper_test.go (1)

544-613: Add coverage for the Timeout() bool retry path.

This table still misses the retryable branch for errors that only satisfy the timeout interface in reaperSpawner.retryError, so that path can regress unnoticed.

Suggested addition

 var errExpected = errors.New("expected")
+
+type timeoutErr struct{}
+
+func (timeoutErr) Error() string { return "timeout" }
+func (timeoutErr) Timeout() bool { return true }

 	tests := []struct {
 		name      string
 		err       error
 		permanent bool
 	}{
+		{
+			name:      "timeout interface error",
+			err:       fmt.Errorf("foo: %w", timeoutErr{}),
+			permanent: false,
+		},
 		{
 			name:      "random error",
 			err:       errors.New("some random error"),
 			permanent: true,
 		},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@reaper_test.go` around lines 544 - 613, Add a test case that exercises the
retry path for errors implementing the timeout interface (Timeout() bool) in
reaperSpawner.retryError: create a small error type that implements error and
Timeout() bool returning true (or wrap a net.Error-like timeout), add it to the
tests table in reaper_test.go alongside the other cases, assert that retryError
returns a non-nil error and that it is not a *backoff.PermanentError; place the
new case with name like "timeout interface error" and permanent: false so the
timeout-only branch is covered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@reaper_test.go`:
- Around line 544-613: Add a test case that exercises the retry path for errors
implementing the timeout interface (Timeout() bool) in reaperSpawner.retryError:
create a small error type that implements error and Timeout() bool returning
true (or wrap a net.Error-like timeout), add it to the tests table in
reaper_test.go alongside the other cases, assert that retryError returns a
non-nil error and that it is not a *backoff.PermanentError; place the new case
with name like "timeout interface error" and permanent: false so the
timeout-only branch is covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8354bc0d-025a-4a45-8cba-75900218670a

📥 Commits

Reviewing files that changed from the base of the PR and between bc5413d and bdb6d18.

📒 Files selected for processing (2)

• docker.go
• reaper_test.go

[coderabbitai]

🧹 Nitpick comments (1)

reaper_test.go (1)

554-614: Add a cleanup-safe case to this table.

retryError short-circuits on isCleanupSafe(err) before the name-conflict and timeout checks, but this matrix never exercises that branch. A regression in the cleanup-safe unwrapping logic would still pass this test, so I’d add at least one retryable case for a cleanup-safe error (for example a containerd not-found/already-in-progress error).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@reaper_test.go` around lines 554 - 614, The test matrix for retryError
doesn't include a case exercising the isCleanupSafe(err) branch, so add a test
entry in the tests slice that supplies an error which isCleanupSafe should
detect (e.g., a containerd "not found" or "already in progress" diagnostic
wrapped via fmt.Errorf) and assert permanent: false; update the table near the
other cases so retryError and isCleanupSafe are exercised together and prevent
regressions in the cleanup-safe unwrapping logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@reaper_test.go`:
- Around line 554-614: The test matrix for retryError doesn't include a case
exercising the isCleanupSafe(err) branch, so add a test entry in the tests slice
that supplies an error which isCleanupSafe should detect (e.g., a containerd
"not found" or "already in progress" diagnostic wrapped via fmt.Errorf) and
assert permanent: false; update the table near the other cases so retryError and
isCleanupSafe are exercised together and prevent regressions in the cleanup-safe
unwrapping logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe2266f8-98ef-4966-b6ba-052204227507

📥 Commits

Reviewing files that changed from the base of the PR and between bdb6d18 and c0e8ab1.

📒 Files selected for processing (2)

• docker.go
• reaper_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• docker.go

[mdelapenya]

LGTM, although we should look for another way to detect the conflicts, logs are fragile. cc/ @stevenh

[mdelapenya]

LGTM, thanks!