fix: recover missing ImageID via inspect for redundant deploy skip
When tracked container has no ImageID in memory (stale state after restart/sync), Deploy now inspects the running container to recover its ImageID before the redundancy check. This prevents unnecessary full redeploys when the same image is already running.
- commit
- b03b115c8386317139af251acdefd5945e8fef14
- author
- brice · · 2026-03-02 15:46 UTC
- parent
- d16e8bc
- signature
- unsigned
2 files · +83 -3
M internal/usecase/container/service.go +29 -3
| @@ -287,9 +287,15 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||
| 287 | 287 | // the exact same image (by Docker image ID), return it immediately. | |
| 288 | 288 | // This prevents the double-deploy caused by the event-based deploy | |
| 289 | 289 | // (triggered by image.pushed) racing with the explicit CLI deploy call. | |
| 290 | - | if hasExisting && existing.ImageID != "" { | |
| 291 | - | if skip, container := s.skipRedundantDeploy(ctx, existing, resources.actualImageRef); skip { | |
| 292 | - | return container, nil | |
| 290 | + | if hasExisting { | |
| 291 | + | existingForSkip := existing | |
| 292 | + | if existingForSkip.ImageID == "" && existingForSkip.Image != "" && normalizeImageRef(existingForSkip.Image) == normalizeImageRef(route.Image) { | |
| 293 | + | existingForSkip = s.containerForRedundantCheck(ctx, existingForSkip) | |
| 294 | + | } | |
| 295 | + | if existingForSkip.ImageID != "" { | |
| 296 | + | if skip, container := s.skipRedundantDeploy(ctx, existingForSkip, resources.actualImageRef); skip { | |
| 297 | + | return container, nil | |
| 298 | + | } | |
| 293 | 299 | } | |
| 294 | 300 | } | |
| 295 | 301 | ||
| @@ -347,6 +353,26 @@ func (s *Service) skipRedundantDeploy(ctx context.Context, existing *domain.Cont | |||
| 347 | 353 | return true, existing | |
| 348 | 354 | } | |
| 349 | 355 | ||
| 356 | + | func (s *Service) containerForRedundantCheck(ctx context.Context, existing *domain.Container) *domain.Container { | |
| 357 | + | if existing == nil || existing.ImageID != "" || existing.ID == "" { | |
| 358 | + | return existing | |
| 359 | + | } | |
| 360 | + | ||
| 361 | + | log := zerowrap.FromCtx(ctx) | |
| 362 | + | inspected, err := s.runtime.InspectContainer(ctx, existing.ID) | |
| 363 | + | if err != nil { | |
| 364 | + | log.Debug().Err(err).Str("container_id", existing.ID).Msg("cannot inspect existing container for redundancy check") | |
| 365 | + | return existing | |
| 366 | + | } | |
| 367 | + | if inspected == nil || inspected.ImageID == "" { | |
| 368 | + | return existing | |
| 369 | + | } | |
| 370 | + | ||
| 371 | + | existingCopy := *existing | |
| 372 | + | existingCopy.ImageID = inspected.ImageID | |
| 373 | + | return &existingCopy | |
| 374 | + | } | |
| 375 | + | ||
| 350 | 376 | type deployResources struct { | |
| 351 | 377 | networkName string | |
| 352 | 378 | actualImageRef string | |
| @@ -287,9 +287,15 @@ func (s *Service) Deploy(ctx context.Context, route domain.Route) (*domain.Conta | |||||
| 287 | // the exact same image (by Docker image ID), return it immediately. | 287 | // the exact same image (by Docker image ID), return it immediately. | ||
| 288 | // This prevents the double-deploy caused by the event-based deploy | 288 | // This prevents the double-deploy caused by the event-based deploy | ||
| 289 | // (triggered by image.pushed) racing with the explicit CLI deploy call. | 289 | // (triggered by image.pushed) racing with the explicit CLI deploy call. | ||
| 290 | - | if hasExisting && existing.ImageID != "" { | 290 | + | if hasExisting { |
| 291 | - | if skip, container := s.skipRedundantDeploy(ctx, existing, resources.actualImageRef); skip { | 291 | + | existingForSkip := existing |
| 292 | - | return container, nil | 292 | + | if existingForSkip.ImageID == "" && existingForSkip.Image != "" && normalizeImageRef(existingForSkip.Image) == normalizeImageRef(route.Image) { |
| 293 | + | existingForSkip = s.containerForRedundantCheck(ctx, existingForSkip) | |||
| 294 | + | } | |||
| 295 | + | if existingForSkip.ImageID != "" { | |||
| 296 | + | if skip, container := s.skipRedundantDeploy(ctx, existingForSkip, resources.actualImageRef); skip { | |||
| 297 | + | return container, nil | |||
| 298 | + | } | |||
| 293 | } | 299 | } | ||
| 294 | } | 300 | } | ||
| 295 | 301 | ||||
| @@ -347,6 +353,26 @@ func (s *Service) skipRedundantDeploy(ctx context.Context, existing *domain.Cont | |||||
| 347 | return true, existing | 353 | return true, existing | ||
| 348 | } | 354 | } | ||
| 349 | 355 | ||||
| 356 | + | func (s *Service) containerForRedundantCheck(ctx context.Context, existing *domain.Container) *domain.Container { | |||
| 357 | + | if existing == nil || existing.ImageID != "" || existing.ID == "" { | |||
| 358 | + | return existing | |||
| 359 | + | } | |||
| 360 | + | ||||
| 361 | + | log := zerowrap.FromCtx(ctx) | |||
| 362 | + | inspected, err := s.runtime.InspectContainer(ctx, existing.ID) | |||
| 363 | + | if err != nil { | |||
| 364 | + | log.Debug().Err(err).Str("container_id", existing.ID).Msg("cannot inspect existing container for redundancy check") | |||
| 365 | + | return existing | |||
| 366 | + | } | |||
| 367 | + | if inspected == nil || inspected.ImageID == "" { | |||
| 368 | + | return existing | |||
| 369 | + | } | |||
| 370 | + | ||||
| 371 | + | existingCopy := *existing | |||
| 372 | + | existingCopy.ImageID = inspected.ImageID | |||
| 373 | + | return &existingCopy | |||
| 374 | + | } | |||
| 375 | + | ||||
| 350 | type deployResources struct { | 376 | type deployResources struct { | ||
| 351 | networkName string | 377 | networkName string | ||
| 352 | actualImageRef string | 378 | actualImageRef string | ||
M internal/usecase/container/service_test.go +54 -0
| @@ -2273,6 +2273,60 @@ func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |||
| 2273 | 2273 | assert.Equal(t, "existing-container", tracked.ID) | |
| 2274 | 2274 | } | |
| 2275 | 2275 | ||
| 2276 | + | func TestService_Deploy_SkipsRedundantDeploy_WhenTrackedImageIDMissing(t *testing.T) { | |
| 2277 | + | runtime := mocks.NewMockContainerRuntime(t) | |
| 2278 | + | envLoader := mocks.NewMockEnvLoader(t) | |
| 2279 | + | eventBus := mocks.NewMockEventPublisher(t) | |
| 2280 | + | ||
| 2281 | + | config := Config{ | |
| 2282 | + | ReadinessDelay: time.Millisecond, | |
| 2283 | + | DrainDelay: time.Millisecond, | |
| 2284 | + | } | |
| 2285 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |
| 2286 | + | ctx := testContext() | |
| 2287 | + | ||
| 2288 | + | // Pre-populate with existing container missing ImageID (common after stale in-memory state). | |
| 2289 | + | existingContainer := &domain.Container{ | |
| 2290 | + | ID: "existing-container", | |
| 2291 | + | Name: "gordon-test.example.com", | |
| 2292 | + | Image: "myapp:latest", | |
| 2293 | + | Status: "running", | |
| 2294 | + | } | |
| 2295 | + | svc.containers["test.example.com"] = existingContainer | |
| 2296 | + | ||
| 2297 | + | route := domain.Route{ | |
| 2298 | + | Domain: "test.example.com", | |
| 2299 | + | Image: "myapp:latest", | |
| 2300 | + | } | |
| 2301 | + | ||
| 2302 | + | // prepareDeployResources | |
| 2303 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |
| 2304 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |
| 2305 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |
| 2306 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |
| 2307 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |
| 2308 | + | ||
| 2309 | + | // Existing container image ID is recovered from runtime inspection, | |
| 2310 | + | // then redundancy check can skip the deploy. | |
| 2311 | + | runtime.EXPECT().InspectContainer(mock.Anything, "existing-container").Return(&domain.Container{ | |
| 2312 | + | ID: "existing-container", | |
| 2313 | + | ImageID: "sha256:abc123", | |
| 2314 | + | Status: "running", | |
| 2315 | + | }, nil) | |
| 2316 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |
| 2317 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(true, nil) | |
| 2318 | + | ||
| 2319 | + | result, err := svc.Deploy(ctx, route) | |
| 2320 | + | ||
| 2321 | + | assert.NoError(t, err) | |
| 2322 | + | assert.NotNil(t, result) | |
| 2323 | + | assert.Equal(t, "existing-container", result.ID) | |
| 2324 | + | ||
| 2325 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |
| 2326 | + | assert.True(t, exists) | |
| 2327 | + | assert.Equal(t, "existing-container", tracked.ID) | |
| 2328 | + | } | |
| 2329 | + | ||
| 2276 | 2330 | func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | |
| 2277 | 2331 | runtime := mocks.NewMockContainerRuntime(t) | |
| 2278 | 2332 | envLoader := mocks.NewMockEnvLoader(t) | |
| @@ -2273,6 +2273,60 @@ func TestService_Deploy_SkipsRedundantDeploy(t *testing.T) { | |||||
| 2273 | assert.Equal(t, "existing-container", tracked.ID) | 2273 | assert.Equal(t, "existing-container", tracked.ID) | ||
| 2274 | } | 2274 | } | ||
| 2275 | 2275 | ||||
| 2276 | + | func TestService_Deploy_SkipsRedundantDeploy_WhenTrackedImageIDMissing(t *testing.T) { | |||
| 2277 | + | runtime := mocks.NewMockContainerRuntime(t) | |||
| 2278 | + | envLoader := mocks.NewMockEnvLoader(t) | |||
| 2279 | + | eventBus := mocks.NewMockEventPublisher(t) | |||
| 2280 | + | ||||
| 2281 | + | config := Config{ | |||
| 2282 | + | ReadinessDelay: time.Millisecond, | |||
| 2283 | + | DrainDelay: time.Millisecond, | |||
| 2284 | + | } | |||
| 2285 | + | svc := NewService(runtime, envLoader, eventBus, nil, config) | |||
| 2286 | + | ctx := testContext() | |||
| 2287 | + | ||||
| 2288 | + | // Pre-populate with existing container missing ImageID (common after stale in-memory state). | |||
| 2289 | + | existingContainer := &domain.Container{ | |||
| 2290 | + | ID: "existing-container", | |||
| 2291 | + | Name: "gordon-test.example.com", | |||
| 2292 | + | Image: "myapp:latest", | |||
| 2293 | + | Status: "running", | |||
| 2294 | + | } | |||
| 2295 | + | svc.containers["test.example.com"] = existingContainer | |||
| 2296 | + | ||||
| 2297 | + | route := domain.Route{ | |||
| 2298 | + | Domain: "test.example.com", | |||
| 2299 | + | Image: "myapp:latest", | |||
| 2300 | + | } | |||
| 2301 | + | ||||
| 2302 | + | // prepareDeployResources | |||
| 2303 | + | runtime.EXPECT().ListContainers(mock.Anything, true).Return([]*domain.Container{}, nil) | |||
| 2304 | + | runtime.EXPECT().ListImages(mock.Anything).Return([]string{"myapp:latest"}, nil) | |||
| 2305 | + | runtime.EXPECT().GetImageExposedPorts(mock.Anything, "myapp:latest").Return([]int{8080}, nil) | |||
| 2306 | + | envLoader.EXPECT().LoadEnv(mock.Anything, "test.example.com").Return([]string{}, nil) | |||
| 2307 | + | runtime.EXPECT().InspectImageEnv(mock.Anything, "myapp:latest").Return([]string{}, nil) | |||
| 2308 | + | ||||
| 2309 | + | // Existing container image ID is recovered from runtime inspection, | |||
| 2310 | + | // then redundancy check can skip the deploy. | |||
| 2311 | + | runtime.EXPECT().InspectContainer(mock.Anything, "existing-container").Return(&domain.Container{ | |||
| 2312 | + | ID: "existing-container", | |||
| 2313 | + | ImageID: "sha256:abc123", | |||
| 2314 | + | Status: "running", | |||
| 2315 | + | }, nil) | |||
| 2316 | + | runtime.EXPECT().GetImageID(mock.Anything, "myapp:latest").Return("sha256:abc123", nil) | |||
| 2317 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "existing-container").Return(true, nil) | |||
| 2318 | + | ||||
| 2319 | + | result, err := svc.Deploy(ctx, route) | |||
| 2320 | + | ||||
| 2321 | + | assert.NoError(t, err) | |||
| 2322 | + | assert.NotNil(t, result) | |||
| 2323 | + | assert.Equal(t, "existing-container", result.ID) | |||
| 2324 | + | ||||
| 2325 | + | tracked, exists := svc.Get(ctx, "test.example.com") | |||
| 2326 | + | assert.True(t, exists) | |||
| 2327 | + | assert.Equal(t, "existing-container", tracked.ID) | |||
| 2328 | + | } | |||
| 2329 | + | ||||
| 2276 | func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | 2330 | func TestService_Deploy_DoesNotSkipWhenImageIDDiffers(t *testing.T) { | ||
| 2277 | runtime := mocks.NewMockContainerRuntime(t) | 2331 | runtime := mocks.NewMockContainerRuntime(t) | ||
| 2278 | envLoader := mocks.NewMockEnvLoader(t) | 2332 | envLoader := mocks.NewMockEnvLoader(t) | ||