detached 8547d4d

fix: async old container cleanup during zero-downtime deploy

During zero-downtime deploys, the old container stop was blocking the
deploy response for up to 30s (now 20s) while the new container was
already serving traffic. This made every deploy feel slow regardless
of the app.

- Run old container cleanup (stop + remove + rename) in a background
  goroutine so the deploy response returns immediately
- Reduce container stop timeout from 30s to 20s
- Add WaitForCleanup() for test synchronization
- Wait for in-flight cleanups during Shutdown()
commit
8547d4d3fc7448224d64834eee8d7cbfd192f124
author
brice · · 2026-03-19 03:56 UTC
parent
9cd24cc
signature
unsigned
3 files · +38 -4
Split
UnifiedSplit
M internal/adapters/out/docker/runtime.go +2 -2
@@ -217,7 +217,7 @@ func (r *Runtime) StopContainer(ctx context.Context, containerID string) error {
217217 })
218218 log := zerowrap.FromCtx(ctx)
219219
220- timeout := 30 // 30 seconds timeout
220+ timeout := 20 // 20 seconds before SIGKILL
221221 err := r.client.ContainerStop(ctx, containerID, container.StopOptions{Timeout: &timeout})
222222 if err != nil {
223223 return log.WrapErr(err, "failed to stop container")
@@ -237,7 +237,7 @@ func (r *Runtime) RestartContainer(ctx context.Context, containerID string) erro
237237 })
238238 log := zerowrap.FromCtx(ctx)
239239
240- timeout := 30 // 30 seconds timeout
240+ timeout := 20 // 20 seconds before SIGKILL
241241 err := r.client.ContainerRestart(ctx, containerID, container.StopOptions{Timeout: &timeout})
242242 if err != nil {
243243 return log.WrapErr(err, "failed to restart container")
@@ -217,7 +217,7 @@ func (r *Runtime) StopContainer(ctx context.Context, containerID string) error {
217 })217 })
218 log := zerowrap.FromCtx(ctx)218 log := zerowrap.FromCtx(ctx)
219 219
220- timeout := 30 // 30 seconds timeout220+ timeout := 20 // 20 seconds before SIGKILL
221 err := r.client.ContainerStop(ctx, containerID, container.StopOptions{Timeout: &timeout})221 err := r.client.ContainerStop(ctx, containerID, container.StopOptions{Timeout: &timeout})
222 if err != nil {222 if err != nil {
223 return log.WrapErr(err, "failed to stop container")223 return log.WrapErr(err, "failed to stop container")
@@ -237,7 +237,7 @@ func (r *Runtime) RestartContainer(ctx context.Context, containerID string) erro
237 })237 })
238 log := zerowrap.FromCtx(ctx)238 log := zerowrap.FromCtx(ctx)
239 239
240- timeout := 30 // 30 seconds timeout240+ timeout := 20 // 20 seconds before SIGKILL
241 err := r.client.ContainerRestart(ctx, containerID, container.StopOptions{Timeout: &timeout})241 err := r.client.ContainerRestart(ctx, containerID, container.StopOptions{Timeout: &timeout})
242 if err != nil {242 if err != nil {
243 return log.WrapErr(err, "failed to restart container")243 return log.WrapErr(err, "failed to restart container")
M internal/usecase/container/service.go +20 -2
@@ -82,7 +82,8 @@ type Service struct {
8282 attachments map[string][]string // ownerDomain → []containerIDs
8383 managedCount int64 // tracks UpDownCounter value for delta computation
8484 mu sync.RWMutex
85- deployMu sync.Map // per-domain deploy locks (domain → *domainDeployLock)
85+ deployMu sync.Map // per-domain deploy locks (domain → *domainDeployLock)
86+ cleanupWg sync.WaitGroup // tracks background old-container cleanup goroutines
8687 monitor *Monitor
8788 }
8889
@@ -344,7 +345,15 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta
344345 }
345346 }
346347
347- s.finalizePreviousContainer(ctx, route.Domain, existing, hasExisting, invalidated, newContainer.ID)
348+ // Finalize old container in the background — the new container is already
349+ // serving traffic, so there's no reason to block the deploy response while
350+ // waiting for the old container to stop (which can take 20s if the app
351+ // doesn't handle SIGTERM).
352+ s.cleanupWg.Add(1)
353+ go func() {
354+ defer s.cleanupWg.Done()
355+ s.finalizePreviousContainer(context.WithoutCancel(ctx), route.Domain, existing, hasExisting, invalidated, newContainer.ID)
356+ }()
348357
349358 // Start container log collection (non-blocking, errors don't fail deployment)
350359 s.startLogCollection(ctx, newContainer.ID, route.Domain)
@@ -1314,6 +1323,9 @@ func (s *Service) Shutdown(ctx context.Context) error {
13141323 log := zerowrap.FromCtx(ctx)
13151324 log.Info().Msg("shutting down container manager...")
13161325
1326+ // Wait for any in-flight background cleanup goroutines to finish
1327+ s.cleanupWg.Wait()
1328+
13171329 // Containers are left running across Gordon restarts.
13181330 // SyncContainers + AutoStart will pick them back up on next boot.
13191331
@@ -1328,6 +1340,12 @@ func (s *Service) Shutdown(ctx context.Context) error {
13281340 return nil
13291341 }
13301342
1343+// WaitForCleanup blocks until all background cleanup goroutines complete.
1344+// Intended for use in tests to avoid mock assertion races.
1345+func (s *Service) WaitForCleanup() {
1346+ s.cleanupWg.Wait()
1347+}
1348+
13311349 // StartMonitor begins background monitoring of tracked containers.
13321350 // The monitor restarts crashed containers and detects crash loops.
13331351 // Safe to call multiple times; subsequent calls are no-ops.
@@ -82,7 +82,8 @@ type Service struct {
82 attachments map[string][]string // ownerDomain → []containerIDs82 attachments map[string][]string // ownerDomain → []containerIDs
83 managedCount int64 // tracks UpDownCounter value for delta computation83 managedCount int64 // tracks UpDownCounter value for delta computation
84 mu sync.RWMutex84 mu sync.RWMutex
85- deployMu sync.Map // per-domain deploy locks (domain → *domainDeployLock)85+ deployMu sync.Map // per-domain deploy locks (domain → *domainDeployLock)
86+ cleanupWg sync.WaitGroup // tracks background old-container cleanup goroutines
86 monitor *Monitor87 monitor *Monitor
87 }88 }
88 89
@@ -344,7 +345,15 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta
344 }345 }
345 }346 }
346 347
347- s.finalizePreviousContainer(ctx, route.Domain, existing, hasExisting, invalidated, newContainer.ID)348+ // Finalize old container in the background — the new container is already
349+ // serving traffic, so there's no reason to block the deploy response while
350+ // waiting for the old container to stop (which can take 20s if the app
351+ // doesn't handle SIGTERM).
352+ s.cleanupWg.Add(1)
353+ go func() {
354+ defer s.cleanupWg.Done()
355+ s.finalizePreviousContainer(context.WithoutCancel(ctx), route.Domain, existing, hasExisting, invalidated, newContainer.ID)
356+ }()
348 357
349 // Start container log collection (non-blocking, errors don't fail deployment)358 // Start container log collection (non-blocking, errors don't fail deployment)
350 s.startLogCollection(ctx, newContainer.ID, route.Domain)359 s.startLogCollection(ctx, newContainer.ID, route.Domain)
@@ -1314,6 +1323,9 @@ func (s *Service) Shutdown(ctx context.Context) error {
1314 log := zerowrap.FromCtx(ctx)1323 log := zerowrap.FromCtx(ctx)
1315 log.Info().Msg("shutting down container manager...")1324 log.Info().Msg("shutting down container manager...")
1316 1325
1326+ // Wait for any in-flight background cleanup goroutines to finish
1327+ s.cleanupWg.Wait()
1328+
1317 // Containers are left running across Gordon restarts.1329 // Containers are left running across Gordon restarts.
1318 // SyncContainers + AutoStart will pick them back up on next boot.1330 // SyncContainers + AutoStart will pick them back up on next boot.
1319 1331
@@ -1328,6 +1340,12 @@ func (s *Service) Shutdown(ctx context.Context) error {
1328 return nil1340 return nil
1329 }1341 }
1330 1342
1343+// WaitForCleanup blocks until all background cleanup goroutines complete.
1344+// Intended for use in tests to avoid mock assertion races.
1345+func (s *Service) WaitForCleanup() {
1346+ s.cleanupWg.Wait()
1347+}
1348+
1331 // StartMonitor begins background monitoring of tracked containers.1349 // StartMonitor begins background monitoring of tracked containers.
1332 // The monitor restarts crashed containers and detects crash loops.1350 // The monitor restarts crashed containers and detects crash loops.
1333 // Safe to call multiple times; subsequent calls are no-ops.1351 // Safe to call multiple times; subsequent calls are no-ops.
M internal/usecase/container/service_test.go +16 -0
@@ -609,6 +609,7 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) {
609609 result, err := svc.Deploy(ctx, route)
610610
611611 assert.NoError(t, err)
612+ svc.WaitForCleanup()
612613 assert.Equal(t, "new-container", result.ID)
613614 }
614615
@@ -706,6 +707,7 @@ func TestService_Deploy_ResolvesExistingFromRuntime_WhenMemoryStale(t *testing.T
706707 result, err := svc.Deploy(ctx, route)
707708
708709 assert.NoError(t, err)
710+ svc.WaitForCleanup()
709711 assert.Equal(t, "new-container", result.ID)
710712
711713 // Verify new container is now tracked
@@ -772,6 +774,7 @@ func TestService_Deploy_SkipRedundantDeploy_GetImageIDError(t *testing.T) {
772774 result, err := svc.Deploy(ctx, route)
773775
774776 assert.NoError(t, err)
777+ svc.WaitForCleanup()
775778 assert.Equal(t, "new-container", result.ID)
776779 }
777780
@@ -2082,6 +2085,7 @@ func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) {
20822085 result, err := svc.Deploy(ctx, route)
20832086
20842087 assert.NoError(t, err)
2088+ svc.WaitForCleanup()
20852089 assert.Equal(t, "new-container", result.ID)
20862090
20872091 // Verify new container is now tracked
@@ -2239,6 +2243,7 @@ func TestService_Deploy_OrphanCleanupRemovesStaleNewContainer(t *testing.T) {
22392243
22402244 result, err := svc.Deploy(ctx, route)
22412245 assert.NoError(t, err)
2246+ svc.WaitForCleanup()
22422247 assert.NotNil(t, result)
22432248 assert.Equal(t, "new-container", result.ID)
22442249 }
@@ -2303,6 +2308,7 @@ func TestService_Deploy_TrackedTempContainerUsesAlternateTempName(t *testing.T)
23032308
23042309 result, err := svc.Deploy(ctx, route)
23052310 assert.NoError(t, err)
2311+ svc.WaitForCleanup()
23062312 assert.NotNil(t, result)
23072313 assert.Equal(t, "new-container", result.ID)
23082314 }
@@ -2525,6 +2531,7 @@ func TestService_Deploy_ConcurrentSameDomain(t *testing.T) {
25252531 }(i)
25262532 }
25272533 wg.Wait()
2534+ svc.WaitForCleanup()
25282535
25292536 // Both should succeed (second deploy waits for the first to finish)
25302537 assert.NoError(t, errs[0], "first deploy should succeed")
@@ -2637,6 +2644,7 @@ func TestService_Deploy_CacheInvalidationBeforeOldContainerStop(t *testing.T) {
26372644 result, err := svc.Deploy(ctx, route)
26382645
26392646 assert.NoError(t, err)
2647+ svc.WaitForCleanup()
26402648 assert.Equal(t, "new-container", result.ID)
26412649
26422650 // Verify ordering: cache invalidation MUST happen before old container stop
@@ -2693,6 +2701,7 @@ func TestService_Deploy_NilCacheInvalidator(t *testing.T) {
26932701 result, err := svc.Deploy(ctx, route)
26942702
26952703 assert.NoError(t, err)
2704+ svc.WaitForCleanup()
26962705 assert.Equal(t, "new-container", result.ID)
26972706 }
26982707
@@ -2870,6 +2879,7 @@ func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) {
28702879 result, err := svc.Deploy(ctx, route)
28712880
28722881 assert.NoError(t, err)
2882+ svc.WaitForCleanup()
28732883 assert.Equal(t, "new-container", result.ID)
28742884 }
28752885
@@ -2975,6 +2985,7 @@ func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) {
29752985 result, err := svc.Deploy(ctx, route)
29762986
29772987 assert.NoError(t, err)
2988+ svc.WaitForCleanup()
29782989 assert.Equal(t, "new-container", result.ID)
29792990 }
29802991
@@ -3032,6 +3043,7 @@ func TestService_Deploy_DoesNotSkip_WhenEnvHashDiffers(t *testing.T) {
30323043 result, err := svc.Deploy(ctx, route)
30333044
30343045 assert.NoError(t, err)
3046+ svc.WaitForCleanup()
30353047 assert.Equal(t, "new-container", result.ID)
30363048 }
30373049
@@ -3087,6 +3099,7 @@ func TestService_Deploy_DoesNotSkip_WhenEnvHashMissing(t *testing.T) {
30873099 result, err := svc.Deploy(ctx, route)
30883100
30893101 assert.NoError(t, err)
3102+ svc.WaitForCleanup()
30903103 assert.Equal(t, "new-container", result.ID)
30913104 }
30923105
@@ -3243,6 +3256,7 @@ func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
32433256 result, err := svc.Deploy(ctx, route)
32443257
32453258 assert.NoError(t, err)
3259+ svc.WaitForCleanup()
32463260 assert.Equal(t, "new-container", result.ID)
32473261
32483262 // Verify new container is tracked
@@ -3421,6 +3435,7 @@ func TestService_Deploy_StabilizationSuccess(t *testing.T) {
34213435 result, err := svc.Deploy(ctx, route)
34223436
34233437 assert.NoError(t, err)
3438+ svc.WaitForCleanup()
34243439 assert.NotNil(t, result)
34253440 assert.Equal(t, "new-container", result.ID, "should return new container on successful stabilization")
34263441
@@ -3515,6 +3530,7 @@ func TestService_Deploy_ZeroDowntime_OldNeverStoppedBeforeNewReady(t *testing.T)
35153530 result, err := svc.Deploy(ctx, route)
35163531
35173532 assert.NoError(t, err)
3533+ svc.WaitForCleanup()
35183534 assert.NotNil(t, result)
35193535 assert.Equal(t, "new-container", result.ID)
35203536
@@ -609,6 +609,7 @@ func TestService_Deploy_ReplacesExistingContainer(t *testing.T) {
609 result, err := svc.Deploy(ctx, route)609 result, err := svc.Deploy(ctx, route)
610 610
611 assert.NoError(t, err)611 assert.NoError(t, err)
612+ svc.WaitForCleanup()
612 assert.Equal(t, "new-container", result.ID)613 assert.Equal(t, "new-container", result.ID)
613 }614 }
614 615
@@ -706,6 +707,7 @@ func TestService_Deploy_ResolvesExistingFromRuntime_WhenMemoryStale(t *testing.T
706 result, err := svc.Deploy(ctx, route)707 result, err := svc.Deploy(ctx, route)
707 708
708 assert.NoError(t, err)709 assert.NoError(t, err)
710+ svc.WaitForCleanup()
709 assert.Equal(t, "new-container", result.ID)711 assert.Equal(t, "new-container", result.ID)
710 712
711 // Verify new container is now tracked713 // Verify new container is now tracked
@@ -772,6 +774,7 @@ func TestService_Deploy_SkipRedundantDeploy_GetImageIDError(t *testing.T) {
772 result, err := svc.Deploy(ctx, route)774 result, err := svc.Deploy(ctx, route)
773 775
774 assert.NoError(t, err)776 assert.NoError(t, err)
777+ svc.WaitForCleanup()
775 assert.Equal(t, "new-container", result.ID)778 assert.Equal(t, "new-container", result.ID)
776 }779 }
777 780
@@ -2082,6 +2085,7 @@ func TestService_Deploy_OrphanCleanupSkipsTrackedContainer(t *testing.T) {
2082 result, err := svc.Deploy(ctx, route)2085 result, err := svc.Deploy(ctx, route)
2083 2086
2084 assert.NoError(t, err)2087 assert.NoError(t, err)
2088+ svc.WaitForCleanup()
2085 assert.Equal(t, "new-container", result.ID)2089 assert.Equal(t, "new-container", result.ID)
2086 2090
2087 // Verify new container is now tracked2091 // Verify new container is now tracked
@@ -2239,6 +2243,7 @@ func TestService_Deploy_OrphanCleanupRemovesStaleNewContainer(t *testing.T) {
2239 2243
2240 result, err := svc.Deploy(ctx, route)2244 result, err := svc.Deploy(ctx, route)
2241 assert.NoError(t, err)2245 assert.NoError(t, err)
2246+ svc.WaitForCleanup()
2242 assert.NotNil(t, result)2247 assert.NotNil(t, result)
2243 assert.Equal(t, "new-container", result.ID)2248 assert.Equal(t, "new-container", result.ID)
2244 }2249 }
@@ -2303,6 +2308,7 @@ func TestService_Deploy_TrackedTempContainerUsesAlternateTempName(t *testing.T)
2303 2308
2304 result, err := svc.Deploy(ctx, route)2309 result, err := svc.Deploy(ctx, route)
2305 assert.NoError(t, err)2310 assert.NoError(t, err)
2311+ svc.WaitForCleanup()
2306 assert.NotNil(t, result)2312 assert.NotNil(t, result)
2307 assert.Equal(t, "new-container", result.ID)2313 assert.Equal(t, "new-container", result.ID)
2308 }2314 }
@@ -2525,6 +2531,7 @@ func TestService_Deploy_ConcurrentSameDomain(t *testing.T) {
2525 }(i)2531 }(i)
2526 }2532 }
2527 wg.Wait()2533 wg.Wait()
2534+ svc.WaitForCleanup()
2528 2535
2529 // Both should succeed (second deploy waits for the first to finish)2536 // Both should succeed (second deploy waits for the first to finish)
2530 assert.NoError(t, errs[0], "first deploy should succeed")2537 assert.NoError(t, errs[0], "first deploy should succeed")
@@ -2637,6 +2644,7 @@ func TestService_Deploy_CacheInvalidationBeforeOldContainerStop(t *testing.T) {
2637 result, err := svc.Deploy(ctx, route)2644 result, err := svc.Deploy(ctx, route)
2638 2645
2639 assert.NoError(t, err)2646 assert.NoError(t, err)
2647+ svc.WaitForCleanup()
2640 assert.Equal(t, "new-container", result.ID)2648 assert.Equal(t, "new-container", result.ID)
2641 2649
2642 // Verify ordering: cache invalidation MUST happen before old container stop2650 // Verify ordering: cache invalidation MUST happen before old container stop
@@ -2693,6 +2701,7 @@ func TestService_Deploy_NilCacheInvalidator(t *testing.T) {
2693 result, err := svc.Deploy(ctx, route)2701 result, err := svc.Deploy(ctx, route)
2694 2702
2695 assert.NoError(t, err)2703 assert.NoError(t, err)
2704+ svc.WaitForCleanup()
2696 assert.Equal(t, "new-container", result.ID)2705 assert.Equal(t, "new-container", result.ID)
2697 }2706 }
2698 2707
@@ -2870,6 +2879,7 @@ func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) {
2870 result, err := svc.Deploy(ctx, route)2879 result, err := svc.Deploy(ctx, route)
2871 2880
2872 assert.NoError(t, err)2881 assert.NoError(t, err)
2882+ svc.WaitForCleanup()
2873 assert.Equal(t, "new-container", result.ID)2883 assert.Equal(t, "new-container", result.ID)
2874 }2884 }
2875 2885
@@ -2975,6 +2985,7 @@ func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) {
2975 result, err := svc.Deploy(ctx, route)2985 result, err := svc.Deploy(ctx, route)
2976 2986
2977 assert.NoError(t, err)2987 assert.NoError(t, err)
2988+ svc.WaitForCleanup()
2978 assert.Equal(t, "new-container", result.ID)2989 assert.Equal(t, "new-container", result.ID)
2979 }2990 }
2980 2991
@@ -3032,6 +3043,7 @@ func TestService_Deploy_DoesNotSkip_WhenEnvHashDiffers(t *testing.T) {
3032 result, err := svc.Deploy(ctx, route)3043 result, err := svc.Deploy(ctx, route)
3033 3044
3034 assert.NoError(t, err)3045 assert.NoError(t, err)
3046+ svc.WaitForCleanup()
3035 assert.Equal(t, "new-container", result.ID)3047 assert.Equal(t, "new-container", result.ID)
3036 }3048 }
3037 3049
@@ -3087,6 +3099,7 @@ func TestService_Deploy_DoesNotSkip_WhenEnvHashMissing(t *testing.T) {
3087 result, err := svc.Deploy(ctx, route)3099 result, err := svc.Deploy(ctx, route)
3088 3100
3089 assert.NoError(t, err)3101 assert.NoError(t, err)
3102+ svc.WaitForCleanup()
3090 assert.Equal(t, "new-container", result.ID)3103 assert.Equal(t, "new-container", result.ID)
3091 }3104 }
3092 3105
@@ -3243,6 +3256,7 @@ func TestService_Deploy_OrphanCleanup_NeverKillsRunningCanonical(t *testing.T) {
3243 result, err := svc.Deploy(ctx, route)3256 result, err := svc.Deploy(ctx, route)
3244 3257
3245 assert.NoError(t, err)3258 assert.NoError(t, err)
3259+ svc.WaitForCleanup()
3246 assert.Equal(t, "new-container", result.ID)3260 assert.Equal(t, "new-container", result.ID)
3247 3261
3248 // Verify new container is tracked3262 // Verify new container is tracked
@@ -3421,6 +3435,7 @@ func TestService_Deploy_StabilizationSuccess(t *testing.T) {
3421 result, err := svc.Deploy(ctx, route)3435 result, err := svc.Deploy(ctx, route)
3422 3436
3423 assert.NoError(t, err)3437 assert.NoError(t, err)
3438+ svc.WaitForCleanup()
3424 assert.NotNil(t, result)3439 assert.NotNil(t, result)
3425 assert.Equal(t, "new-container", result.ID, "should return new container on successful stabilization")3440 assert.Equal(t, "new-container", result.ID, "should return new container on successful stabilization")
3426 3441
@@ -3515,6 +3530,7 @@ func TestService_Deploy_ZeroDowntime_OldNeverStoppedBeforeNewReady(t *testing.T)
3515 result, err := svc.Deploy(ctx, route)3530 result, err := svc.Deploy(ctx, route)
3516 3531
3517 assert.NoError(t, err)3532 assert.NoError(t, err)
3533+ svc.WaitForCleanup()
3518 assert.NotNil(t, result)3534 assert.NotNil(t, result)
3519 assert.Equal(t, "new-container", result.ID)3535 assert.Equal(t, "new-container", result.ID)
3520 3536

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close