Skip to content

fix: remove all containers on stop to prevent name conflicts#4859

Open
7ttp wants to merge 1 commit intosupabase:developfrom
7ttp:fix/stop
Open

fix: remove all containers on stop to prevent name conflicts#4859
7ttp wants to merge 1 commit intosupabase:developfrom
7ttp:fix/stop

Conversation

@7ttp
Copy link
Contributor

@7ttp 7ttp commented Feb 15, 2026

Problem

supabase stop only stopped "running" containers, leaving containers in other states (restarting, created, exited) orphaned.
This caused name conflicts on restart:

Error: The container name "" is already in use

Solution

Remove state check to process all containers, ensuring clean removal regardless of state.

Related

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed Docker container removal to stop all containers during cleanup operations, not just those in a running state. This ensures more complete removal processes.

@7ttp 7ttp requested a review from a team as a code owner February 15, 2026 17:06
@coderabbitai
Copy link

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

The change modifies the Docker container stopping logic in DockerRemoveAll to stop all containers regardless of their state, rather than only stopping those in a "running" state. This ensures containers in other states are also properly cleaned up during removal.

Changes

Cohort / File(s) Summary
Docker Container Cleanup
internal/utils/docker.go
Removed the state check when iterating over containers to stop, causing all container IDs to be added to the stop list unconditionally. This allows containers in non-running states to be targeted for stopping during removal operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing all containers on stop to prevent name conflicts, which matches the core fix in the changeset.
Linked Issues check ✅ Passed The changes directly address the issue by removing the state check to process all containers, ensuring clean removal regardless of state and preventing Docker name conflicts on restart.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the stop/start workflow issue by modifying container removal logic in docker.go, with no extraneous changes detected.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into develop

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/utils/docker.go (1)

106-114: ⚠️ Potential issue | 🟠 Major

Use errdefs.IsNotModified() to ignore benign "already stopped" errors.

Now that every container ID is stopped, Docker returns HTTP 304 Not Modified for containers that are already stopped or in created/exited states, which will abort DockerRemoveAll and skip pruning. Treat those errors as non-fatal so cleanup continues.

🔧 Proposed fix
  result := WaitAll(ids, func(id string) error {
-	if err := Docker.ContainerStop(ctx, id, container.StopOptions{}); err != nil {
-		return errors.Errorf("failed to stop container: %w", err)
-	}
+	if err := Docker.ContainerStop(ctx, id, container.StopOptions{}); err != nil {
+		// Ignore benign errors for already-stopped containers.
+		if errdefs.IsNotModified(err) || errdefs.IsNotFound(err) {
+			return nil
+		}
+		return errors.Errorf("failed to stop container: %w", err)
+	}
 	return nil
  })

Copy link

@IdellHeaney IdellHeaney left a comment

Choose a reason for hiding this comment

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

me parece bien

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

supabase stop + start fails with "supabase_vector_supabase is already in use"

2 participants

Comments