fix(container): rebuild attachment map in SyncContainers to prevent orphaned containers after restart
- commit
- 7643cd4f04d679eb003d1d46c21858c94d1fea72
- author
- bnema · · 2026-02-24 21:54 UTC
- parent
- d35750b
- signature
- unsigned
2 files · +56 -3
M internal/usecase/container/service.go +13 -3
| @@ -903,16 +903,26 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||
| 903 | 903 | } | |
| 904 | 904 | ||
| 905 | 905 | managed := make(map[string]*domain.Container) | |
| 906 | + | attachments := make(map[string][]string) | |
| 906 | 907 | for _, c := range allContainers { | |
| 907 | - | if c.Labels != nil { | |
| 908 | - | if d, ok := c.Labels[domain.LabelDomain]; ok && c.Labels[domain.LabelManaged] == "true" { | |
| 909 | - | managed[d] = c | |
| 908 | + | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { | |
| 909 | + | continue | |
| 910 | + | } | |
| 911 | + | if c.Labels[domain.LabelAttachment] == "true" { | |
| 912 | + | owner := c.Labels[domain.LabelAttachedTo] | |
| 913 | + | if owner != "" { | |
| 914 | + | attachments[owner] = append(attachments[owner], c.ID) | |
| 910 | 915 | } | |
| 916 | + | continue | |
| 917 | + | } | |
| 918 | + | if d, ok := c.Labels[domain.LabelDomain]; ok { | |
| 919 | + | managed[d] = c | |
| 911 | 920 | } | |
| 912 | 921 | } | |
| 913 | 922 | ||
| 914 | 923 | s.mu.Lock() | |
| 915 | 924 | s.containers = managed | |
| 925 | + | s.attachments = attachments | |
| 916 | 926 | newCount := int64(len(managed)) | |
| 917 | 927 | delta := newCount - s.managedCount | |
| 918 | 928 | s.managedCount = newCount | |
| @@ -903,16 +903,26 @@ func (s *Service) SyncContainers(ctx context.Context) error { | |||||
| 903 | } | 903 | } | ||
| 904 | 904 | ||||
| 905 | managed := make(map[string]*domain.Container) | 905 | managed := make(map[string]*domain.Container) | ||
| 906 | + | attachments := make(map[string][]string) | |||
| 906 | for _, c := range allContainers { | 907 | for _, c := range allContainers { | ||
| 907 | - | if c.Labels != nil { | 908 | + | if c.Labels == nil || c.Labels[domain.LabelManaged] != "true" { |
| 908 | - | if d, ok := c.Labels[domain.LabelDomain]; ok && c.Labels[domain.LabelManaged] == "true" { | 909 | + | continue |
| 909 | - | managed[d] = c | 910 | + | } |
| 911 | + | if c.Labels[domain.LabelAttachment] == "true" { | |||
| 912 | + | owner := c.Labels[domain.LabelAttachedTo] | |||
| 913 | + | if owner != "" { | |||
| 914 | + | attachments[owner] = append(attachments[owner], c.ID) | |||
| 910 | } | 915 | } | ||
| 916 | + | continue | |||
| 917 | + | } | |||
| 918 | + | if d, ok := c.Labels[domain.LabelDomain]; ok { | |||
| 919 | + | managed[d] = c | |||
| 911 | } | 920 | } | ||
| 912 | } | 921 | } | ||
| 913 | 922 | ||||
| 914 | s.mu.Lock() | 923 | s.mu.Lock() | ||
| 915 | s.containers = managed | 924 | s.containers = managed | ||
| 925 | + | s.attachments = attachments | |||
| 916 | newCount := int64(len(managed)) | 926 | newCount := int64(len(managed)) | ||
| 917 | delta := newCount - s.managedCount | 927 | delta := newCount - s.managedCount | ||
| 918 | s.managedCount = newCount | 928 | s.managedCount = newCount | ||
M internal/usecase/container/service_test.go +43 -0
| @@ -2332,6 +2332,49 @@ func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | |||
| 2332 | 2332 | assert.Equal(t, "new-container", result.ID) | |
| 2333 | 2333 | } | |
| 2334 | 2334 | ||
| 2335 | + | func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |
| 2336 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 2337 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 2338 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 2339 | + | ||
| 2340 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |
| 2341 | + | ctx := testContext() | |
| 2342 | + | ||
| 2343 | + | // ListContainers returns one main container and one attachment container | |
| 2344 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |
| 2345 | + | { | |
| 2346 | + | ID: "main-container-1", | |
| 2347 | + | Labels: map[string]string{ | |
| 2348 | + | domain.LabelDomain: "app.example.com", | |
| 2349 | + | domain.LabelManaged: "true", | |
| 2350 | + | }, | |
| 2351 | + | }, | |
| 2352 | + | { | |
| 2353 | + | ID: "attachment-container-1", | |
| 2354 | + | Labels: map[string]string{ | |
| 2355 | + | domain.LabelManaged: "true", | |
| 2356 | + | domain.LabelAttachment: "true", | |
| 2357 | + | domain.LabelAttachedTo: "app.example.com", | |
| 2358 | + | }, | |
| 2359 | + | }, | |
| 2360 | + | }, nil).Once() | |
| 2361 | + | ||
| 2362 | + | require.NoError(t, svc.SyncContainers(ctx)) | |
| 2363 | + | ||
| 2364 | + | // Main container should be in s.containers | |
| 2365 | + | tracked, exists := svc.Get(ctx, "app.example.com") | |
| 2366 | + | require.True(t, exists, "main container should be tracked after SyncContainers") | |
| 2367 | + | assert.Equal(t, "main-container-1", tracked.ID) | |
| 2368 | + | ||
| 2369 | + | // Attachment container should be in s.attachments under the owner domain | |
| 2370 | + | svc.mu.RLock() | |
| 2371 | + | attachIDs := svc.attachments["app.example.com"] | |
| 2372 | + | svc.mu.RUnlock() | |
| 2373 | + | ||
| 2374 | + | require.Len(t, attachIDs, 1, "attachment map should contain one entry for app.example.com after SyncContainers") | |
| 2375 | + | assert.Equal(t, "attachment-container-1", attachIDs[0]) | |
| 2376 | + | } | |
| 2377 | + | ||
| 2335 | 2378 | func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) { | |
| 2336 | 2379 | runtime := mocks.NewMockContainerRuntime(t) | |
| 2337 | 2380 | envLoader := mocks.NewMockEnvLoader(t) | |
| @@ -2332,6 +2332,49 @@ func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | |||||
| 2332 | assert.Equal(t, "new-container", result.ID) | 2332 | assert.Equal(t, "new-container", result.ID) | ||
| 2333 | } | 2333 | } | ||
| 2334 | 2334 | ||||
| 2335 | + | func TestSyncContainersRebuildsAttachmentMap(t *testing.T) { | |||
| 2336 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 2337 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 2338 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 2339 | + | ||||
| 2340 | + | svc := NewService(runtime, envLoader, eventBus, nil, Config{}) | |||
| 2341 | + | ctx := testContext() | |||
| 2342 | + | ||||
| 2343 | + | // ListContainers returns one main container and one attachment container | |||
| 2344 | + | runtime.EXPECT().ListContainers(mock.Anything, false).Return([]*domain.Container{ | |||
| 2345 | + | { | |||
| 2346 | + | ID: "main-container-1", | |||
| 2347 | + | Labels: map[string]string{ | |||
| 2348 | + | domain.LabelDomain: "app.example.com", | |||
| 2349 | + | domain.LabelManaged: "true", | |||
| 2350 | + | }, | |||
| 2351 | + | }, | |||
| 2352 | + | { | |||
| 2353 | + | ID: "attachment-container-1", | |||
| 2354 | + | Labels: map[string]string{ | |||
| 2355 | + | domain.LabelManaged: "true", | |||
| 2356 | + | domain.LabelAttachment: "true", | |||
| 2357 | + | domain.LabelAttachedTo: "app.example.com", | |||
| 2358 | + | }, | |||
| 2359 | + | }, | |||
| 2360 | + | }, nil).Once() | |||
| 2361 | + | ||||
| 2362 | + | require.NoError(t, svc.SyncContainers(ctx)) | |||
| 2363 | + | ||||
| 2364 | + | // Main container should be in s.containers | |||
| 2365 | + | tracked, exists := svc.Get(ctx, "app.example.com") | |||
| 2366 | + | require.True(t, exists, "main container should be tracked after SyncContainers") | |||
| 2367 | + | assert.Equal(t, "main-container-1", tracked.ID) | |||
| 2368 | + | ||||
| 2369 | + | // Attachment container should be in s.attachments under the owner domain | |||
| 2370 | + | svc.mu.RLock() | |||
| 2371 | + | attachIDs := svc.attachments["app.example.com"] | |||
| 2372 | + | svc.mu.RUnlock() | |||
| 2373 | + | ||||
| 2374 | + | require.Len(t, attachIDs, 1, "attachment map should contain one entry for app.example.com after SyncContainers") | |||
| 2375 | + | assert.Equal(t, "attachment-container-1", attachIDs[0]) | |||
| 2376 | + | } | |||
| 2377 | + | ||||
| 2335 | func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) { | 2378 | func TestService_Deploy_SkipRedundantDeploy_ContainerNotRunning(t *testing.T) { | ||
| 2336 | runtime := mocks.NewMockContainerRuntime(t) | 2379 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 2337 | envLoader := mocks.NewMockEnvLoader(t) | 2380 | envLoader := mocks.NewMockEnvLoader(t) | ||