fix(deployment): verify the successor per pass before retiring a predecessor
Overlap eligibility now also requires the predecessor's effective spec to own no volume, bind, device, or UDP interface. The periodic pass retires a pending predecessor only when the same pass verified and routed the successor and a final inspection shows the same execution. A retirement marker is cleared only once the container is gone and its backend claims are released, and leftovers are logged and returned. Stop retires pending predecessors like Remove, and Remove attempts every marker.
- commit
- 99672d015e745402941ff7154d91a335f0f2d01d
- author
- brice · · 2026-10-16 19:56 UTC
- parent
- 7d22851
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +386 -49
M internal/usecase/deployment/execute.go +12 -2
| @@ -367,7 +367,7 @@ func (s *Service) runServiceStep( | |||
| 367 | 367 | RestartUnsafe: singleWriterRequired(p.spec), | |
| 368 | 368 | } | |
| 369 | 369 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | |
| 370 | - | } else if before != "" && overlapEligible(p.spec) { | |
| 370 | + | } else if before != "" && canOverlap(p.spec, active.Services[p.name].Spec) { | |
| 371 | 371 | overlap = true | |
| 372 | 372 | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | |
| 373 | 373 | } else { | |
| @@ -448,7 +448,7 @@ func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p p | |||
| 448 | 448 | // serves — but a cleanup warning on the service; the marker stays, so the | |
| 449 | 449 | // periodic reconciliation retries it. | |
| 450 | 450 | func (s *Service) retireSuperseded(ctx context.Context, app, service, before string, grace time.Duration, result ServiceResult) ServiceResult { | |
| 451 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace, ClearInhibition: true}, before) | |
| 451 | + | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace, ClearInhibition: true, RetryOnLeak: true}, before) | |
| 452 | 452 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | |
| 453 | 453 | if !retired.Gone && len(retired.Warnings) == 0 { | |
| 454 | 454 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | |
| @@ -808,6 +808,16 @@ func singleWriterRequired(spec domain.AppService) bool { | |||
| 808 | 808 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | |
| 809 | 809 | } | |
| 810 | 810 | ||
| 811 | + | // canOverlap reports whether a replacement may run beside its predecessor: both | |
| 812 | + | // the new spec and the predecessor's effective spec (as recorded in ACTIVE) | |
| 813 | + | // must be overlap-eligible. A predecessor that owned a volume, bind, device, | |
| 814 | + | // or UDP interface is a single writer or has pinned UDP sessions whatever the | |
| 815 | + | // replacement declares, so removing the resource from the spec does not make | |
| 816 | + | // the transition safe. | |
| 817 | + | func canOverlap(next, previous domain.AppService) bool { | |
| 818 | + | return overlapEligible(next) && overlapEligible(previous) | |
| 819 | + | } | |
| 820 | + | ||
| 811 | 821 | // overlapEligible reports whether a service may be replaced with a short | |
| 812 | 822 | // overlap of the old and the new generation: it has no volume, bind, device, | |
| 813 | 823 | // or UDP interface. Volumes, binds, and devices are stateful or exclusive | |
| @@ -367,7 +367,7 @@ func (s *Service) runServiceStep( | |||||
| 367 | RestartUnsafe: singleWriterRequired(p.spec), | 367 | RestartUnsafe: singleWriterRequired(p.spec), | ||
| 368 | } | 368 | } | ||
| 369 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | 369 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | ||
| 370 | - | } else if before != "" && overlapEligible(p.spec) { | 370 | + | } else if before != "" && canOverlap(p.spec, active.Services[p.name].Spec) { |
| 371 | overlap = true | 371 | overlap = true | ||
| 372 | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | 372 | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | ||
| 373 | } else { | 373 | } else { | ||
| @@ -448,7 +448,7 @@ func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p p | |||||
| 448 | // serves — but a cleanup warning on the service; the marker stays, so the | 448 | // serves — but a cleanup warning on the service; the marker stays, so the | ||
| 449 | // periodic reconciliation retries it. | 449 | // periodic reconciliation retries it. | ||
| 450 | func (s *Service) retireSuperseded(ctx context.Context, app, service, before string, grace time.Duration, result ServiceResult) ServiceResult { | 450 | func (s *Service) retireSuperseded(ctx context.Context, app, service, before string, grace time.Duration, result ServiceResult) ServiceResult { | ||
| 451 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace, ClearInhibition: true}, before) | 451 | + | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace, ClearInhibition: true, RetryOnLeak: true}, before) |
| 452 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | 452 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | ||
| 453 | if !retired.Gone && len(retired.Warnings) == 0 { | 453 | if !retired.Gone && len(retired.Warnings) == 0 { | ||
| 454 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | 454 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | ||
| @@ -808,6 +808,16 @@ func singleWriterRequired(spec domain.AppService) bool { | |||||
| 808 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | 808 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | ||
| 809 | } | 809 | } | ||
| 810 | 810 | ||||
| 811 | + | // canOverlap reports whether a replacement may run beside its predecessor: both | |||
| 812 | + | // the new spec and the predecessor's effective spec (as recorded in ACTIVE) | |||
| 813 | + | // must be overlap-eligible. A predecessor that owned a volume, bind, device, | |||
| 814 | + | // or UDP interface is a single writer or has pinned UDP sessions whatever the | |||
| 815 | + | // replacement declares, so removing the resource from the spec does not make | |||
| 816 | + | // the transition safe. | |||
| 817 | + | func canOverlap(next, previous domain.AppService) bool { | |||
| 818 | + | return overlapEligible(next) && overlapEligible(previous) | |||
| 819 | + | } | |||
| 820 | + | ||||
| 811 | // overlapEligible reports whether a service may be replaced with a short | 821 | // overlapEligible reports whether a service may be replaced with a short | ||
| 812 | // overlap of the old and the new generation: it has no volume, bind, device, | 822 | // overlap of the old and the new generation: it has no volume, bind, device, | ||
| 813 | // or UDP interface. Volumes, binds, and devices are stateful or exclusive | 823 | // or UDP interface. Volumes, binds, and devices are stateful or exclusive | ||
M internal/usecase/deployment/lifecycle.go +48 -6
| @@ -86,6 +86,10 @@ func (s *Service) stopLocked(ctx context.Context, app, opID string) (*LifecycleR | |||
| 86 | 86 | } | |
| 87 | 87 | result.Services[name] = ServiceResult{Result: "deployed", Before: container, After: "", CleanupWarnings: warnings} | |
| 88 | 88 | } | |
| 89 | + | // A superseded predecessor an overlap replacement left running is not in | |
| 90 | + | // ACTIVE, so the loop above never saw it: a stopped app must not keep it | |
| 91 | + | // running. One that cannot be retired keeps its marker and fails the stop. | |
| 92 | + | failures = append(failures, s.retirePendingForStop(ctx, app, result)...) | |
| 89 | 93 | op.Outcome = ComputeOutcome(result.Services) | |
| 90 | 94 | op.Warnings = journalWarnings(collectCleanupWarnings(result.Services)) | |
| 91 | 95 | if err := s.deps.State.SaveOperation(ctx, op); err != nil { | |
| @@ -522,22 +526,60 @@ func (s *Service) removeServiceContainers(ctx context.Context, app, opID string, | |||
| 522 | 526 | // left running beside its successor. Ending the incarnation deletes the | |
| 523 | 527 | // retirement markers with the rest of the app state, so a container that is | |
| 524 | 528 | // not confirmed gone here would be orphaned and untracked: a failure keeps the | |
| 525 | - | // markers and fails the caller. | |
| 529 | + | // markers and fails the caller. Every marker is attempted, and a marker whose | |
| 530 | + | // claims could not be released is a failure too, so it is retried instead of | |
| 531 | + | // leaking its claims. | |
| 526 | 532 | func (s *Service) retireAllPending(ctx context.Context, app string) error { | |
| 533 | + | pending, err := s.pendingRetirements(ctx, app) | |
| 534 | + | if err != nil { | |
| 535 | + | return err | |
| 536 | + | } | |
| 537 | + | var failures []error | |
| 538 | + | for _, marker := range pending { | |
| 539 | + | retired := s.retireContainer(ctx, app, retireOptions{ | |
| 540 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, | |
| 541 | + | }, marker.ContainerID) | |
| 542 | + | if !retired.Settled { | |
| 543 | + | failures = append(failures, fmt.Errorf("deployment: remove %q/%q: superseded container %s not retired: %s", app, marker.Service, marker.ContainerID, cleanupDetail(retired))) | |
| 544 | + | } | |
| 545 | + | } | |
| 546 | + | return errors.Join(failures...) | |
| 547 | + | } | |
| 548 | + | ||
| 549 | + | // pendingRetirements loads the retirement-pending markers of one app. | |
| 550 | + | func (s *Service) pendingRetirements(ctx context.Context, app string) ([]domain.AppRecoveryInhibition, error) { | |
| 527 | 551 | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | |
| 528 | 552 | if err != nil { | |
| 529 | - | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |
| 553 | + | return nil, fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |
| 530 | 554 | } | |
| 531 | 555 | _, pending := splitRecoveryInhibitions(app, inhibitions) | |
| 556 | + | return pending, nil | |
| 557 | + | } | |
| 558 | + | ||
| 559 | + | // retirePendingForStop retires the pending superseded containers of a stopped | |
| 560 | + | // app, recording a leftover or a failure on the service they belong to. It | |
| 561 | + | // returns the services whose predecessor is not confirmed retired. | |
| 562 | + | func (s *Service) retirePendingForStop(ctx context.Context, app string, result *LifecycleResult) []string { | |
| 563 | + | pending, err := s.pendingRetirements(ctx, app) | |
| 564 | + | if err != nil { | |
| 565 | + | result.Warnings = append(result.Warnings, CleanupWarning{Detail: err.Error()}) | |
| 566 | + | return []string{"pending retirements"} | |
| 567 | + | } | |
| 568 | + | var failed []string | |
| 532 | 569 | for _, marker := range pending { | |
| 533 | 570 | retired := s.retireContainer(ctx, app, retireOptions{ | |
| 534 | - | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |
| 571 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, | |
| 535 | 572 | }, marker.ContainerID) | |
| 536 | - | if !retired.Gone { | |
| 537 | - | return fmt.Errorf("deployment: remove %q/%q: superseded container %s not confirmed gone: %s", app, marker.Service, marker.ContainerID, cleanupDetail(retired)) | |
| 573 | + | svc := result.Services[marker.Service] | |
| 574 | + | svc.CleanupWarnings = append(svc.CleanupWarnings, retired.Warnings...) | |
| 575 | + | if !retired.Settled { | |
| 576 | + | svc.Result = "failed" | |
| 577 | + | svc.Error = "superseded container " + marker.ContainerID + " not retired: " + cleanupDetail(retired) | |
| 578 | + | failed = append(failed, marker.Service) | |
| 538 | 579 | } | |
| 580 | + | result.Services[marker.Service] = svc | |
| 539 | 581 | } | |
| 540 | - | return nil | |
| 582 | + | return failed | |
| 541 | 583 | } | |
| 542 | 584 | ||
| 543 | 585 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | |
| @@ -86,6 +86,10 @@ func (s *Service) stopLocked(ctx context.Context, app, opID string) (*LifecycleR | |||||
| 86 | } | 86 | } | ||
| 87 | result.Services[name] = ServiceResult{Result: "deployed", Before: container, After: "", CleanupWarnings: warnings} | 87 | result.Services[name] = ServiceResult{Result: "deployed", Before: container, After: "", CleanupWarnings: warnings} | ||
| 88 | } | 88 | } | ||
| 89 | + | // A superseded predecessor an overlap replacement left running is not in | |||
| 90 | + | // ACTIVE, so the loop above never saw it: a stopped app must not keep it | |||
| 91 | + | // running. One that cannot be retired keeps its marker and fails the stop. | |||
| 92 | + | failures = append(failures, s.retirePendingForStop(ctx, app, result)...) | |||
| 89 | op.Outcome = ComputeOutcome(result.Services) | 93 | op.Outcome = ComputeOutcome(result.Services) | ||
| 90 | op.Warnings = journalWarnings(collectCleanupWarnings(result.Services)) | 94 | op.Warnings = journalWarnings(collectCleanupWarnings(result.Services)) | ||
| 91 | if err := s.deps.State.SaveOperation(ctx, op); err != nil { | 95 | if err := s.deps.State.SaveOperation(ctx, op); err != nil { | ||
| @@ -522,22 +526,60 @@ func (s *Service) removeServiceContainers(ctx context.Context, app, opID string, | |||||
| 522 | // left running beside its successor. Ending the incarnation deletes the | 526 | // left running beside its successor. Ending the incarnation deletes the | ||
| 523 | // retirement markers with the rest of the app state, so a container that is | 527 | // retirement markers with the rest of the app state, so a container that is | ||
| 524 | // not confirmed gone here would be orphaned and untracked: a failure keeps the | 528 | // not confirmed gone here would be orphaned and untracked: a failure keeps the | ||
| 525 | - | // markers and fails the caller. | 529 | + | // markers and fails the caller. Every marker is attempted, and a marker whose |
| 530 | + | // claims could not be released is a failure too, so it is retried instead of | |||
| 531 | + | // leaking its claims. | |||
| 526 | func (s *Service) retireAllPending(ctx context.Context, app string) error { | 532 | func (s *Service) retireAllPending(ctx context.Context, app string) error { | ||
| 533 | + | pending, err := s.pendingRetirements(ctx, app) | |||
| 534 | + | if err != nil { | |||
| 535 | + | return err | |||
| 536 | + | } | |||
| 537 | + | var failures []error | |||
| 538 | + | for _, marker := range pending { | |||
| 539 | + | retired := s.retireContainer(ctx, app, retireOptions{ | |||
| 540 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, | |||
| 541 | + | }, marker.ContainerID) | |||
| 542 | + | if !retired.Settled { | |||
| 543 | + | failures = append(failures, fmt.Errorf("deployment: remove %q/%q: superseded container %s not retired: %s", app, marker.Service, marker.ContainerID, cleanupDetail(retired))) | |||
| 544 | + | } | |||
| 545 | + | } | |||
| 546 | + | return errors.Join(failures...) | |||
| 547 | + | } | |||
| 548 | + | ||||
| 549 | + | // pendingRetirements loads the retirement-pending markers of one app. | |||
| 550 | + | func (s *Service) pendingRetirements(ctx context.Context, app string) ([]domain.AppRecoveryInhibition, error) { | |||
| 527 | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | 551 | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | ||
| 528 | if err != nil { | 552 | if err != nil { | ||
| 529 | - | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) | 553 | + | return nil, fmt.Errorf("deployment: load recovery inhibitions: %w", err) |
| 530 | } | 554 | } | ||
| 531 | _, pending := splitRecoveryInhibitions(app, inhibitions) | 555 | _, pending := splitRecoveryInhibitions(app, inhibitions) | ||
| 556 | + | return pending, nil | |||
| 557 | + | } | |||
| 558 | + | ||||
| 559 | + | // retirePendingForStop retires the pending superseded containers of a stopped | |||
| 560 | + | // app, recording a leftover or a failure on the service they belong to. It | |||
| 561 | + | // returns the services whose predecessor is not confirmed retired. | |||
| 562 | + | func (s *Service) retirePendingForStop(ctx context.Context, app string, result *LifecycleResult) []string { | |||
| 563 | + | pending, err := s.pendingRetirements(ctx, app) | |||
| 564 | + | if err != nil { | |||
| 565 | + | result.Warnings = append(result.Warnings, CleanupWarning{Detail: err.Error()}) | |||
| 566 | + | return []string{"pending retirements"} | |||
| 567 | + | } | |||
| 568 | + | var failed []string | |||
| 532 | for _, marker := range pending { | 569 | for _, marker := range pending { | ||
| 533 | retired := s.retireContainer(ctx, app, retireOptions{ | 570 | retired := s.retireContainer(ctx, app, retireOptions{ | ||
| 534 | - | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | 571 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, |
| 535 | }, marker.ContainerID) | 572 | }, marker.ContainerID) | ||
| 536 | - | if !retired.Gone { | 573 | + | svc := result.Services[marker.Service] |
| 537 | - | return fmt.Errorf("deployment: remove %q/%q: superseded container %s not confirmed gone: %s", app, marker.Service, marker.ContainerID, cleanupDetail(retired)) | 574 | + | svc.CleanupWarnings = append(svc.CleanupWarnings, retired.Warnings...) |
| 575 | + | if !retired.Settled { | |||
| 576 | + | svc.Result = "failed" | |||
| 577 | + | svc.Error = "superseded container " + marker.ContainerID + " not retired: " + cleanupDetail(retired) | |||
| 578 | + | failed = append(failed, marker.Service) | |||
| 538 | } | 579 | } | ||
| 580 | + | result.Services[marker.Service] = svc | |||
| 539 | } | 581 | } | ||
| 540 | - | return nil | 582 | + | return failed |
| 541 | } | 583 | } | ||
| 542 | 584 | ||||
| 543 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | 585 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | ||
M internal/usecase/deployment/monitor.go +46 -39
| @@ -288,14 +288,6 @@ func (t *executionTracker) record(key recoveryKey, startedAt time.Time) { | |||
| 288 | 288 | t.seen[key] = startedAt | |
| 289 | 289 | } | |
| 290 | 290 | ||
| 291 | - | // verified reports whether a verified execution of the generation is recorded. | |
| 292 | - | func (t *executionTracker) verified(key recoveryKey) bool { | |
| 293 | - | t.mu.Lock() | |
| 294 | - | defer t.mu.Unlock() | |
| 295 | - | _, ok := t.seen[key] | |
| 296 | - | return ok | |
| 297 | - | } | |
| 298 | - | ||
| 299 | 291 | // forget drops one generation's observation. | |
| 300 | 292 | func (t *executionTracker) forget(key recoveryKey) { | |
| 301 | 293 | t.mu.Lock() | |
| @@ -368,7 +360,9 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 368 | 360 | } | |
| 369 | 361 | inhibited, pending := splitRecoveryInhibitions(app, inhibitions) | |
| 370 | 362 | var failures []error | |
| 371 | - | failed := map[string]struct{}{} | |
| 363 | + | // verified records, per service, the execution this pass verified and | |
| 364 | + | // routed. A successor that is not in it is not known to serve. | |
| 365 | + | verified := map[string]time.Time{} | |
| 372 | 366 | for _, name := range sortedServiceNames(active) { | |
| 373 | 367 | eff := active.Services[name] | |
| 374 | 368 | if intent.Stopped { | |
| @@ -377,9 +371,12 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 377 | 371 | } | |
| 378 | 372 | continue | |
| 379 | 373 | } | |
| 380 | - | if err := s.reconcileRunningService(ctx, app, name, eff, inhibited); err != nil { | |
| 374 | + | startedAt, ok, err := s.reconcileRunningService(ctx, app, name, eff, inhibited) | |
| 375 | + | if err != nil { | |
| 381 | 376 | failures = append(failures, err) | |
| 382 | - | failed[name] = struct{}{} | |
| 377 | + | } | |
| 378 | + | if ok { | |
| 379 | + | verified[name] = startedAt | |
| 383 | 380 | } | |
| 384 | 381 | } | |
| 385 | 382 | if intent.Stopped { | |
| @@ -394,7 +391,7 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 394 | 391 | } | |
| 395 | 392 | // Superseded containers are retired last: only after their successors | |
| 396 | 393 | // were verified and routed in this pass. | |
| 397 | - | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, failed)...) | |
| 394 | + | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, verified)...) | |
| 398 | 395 | s.pruneRecoveryHistory(app, active) | |
| 399 | 396 | return errors.Join(failures...) | |
| 400 | 397 | } | |
| @@ -420,12 +417,15 @@ func splitRecoveryInhibitions(app string, inhibitions []domain.AppRecoveryInhibi | |||
| 420 | 417 | // when it can no longer be the serving one: | |
| 421 | 418 | // - ACTIVE names it (the replacement never took over): it is the serving | |
| 422 | 419 | // generation, so the marker is dropped and the container is kept; | |
| 423 | - | // - its successor was not verified and routed in this pass, or its | |
| 424 | - | // reconciliation failed: the superseded container is retried later, so a | |
| 425 | - | // successor that is not serving never costs the working generation. | |
| 420 | + | // - its successor was not verified and routed by THIS pass, or restarted | |
| 421 | + | // since: the superseded container is retried later, so a successor that | |
| 422 | + | // is not serving never costs the working generation. | |
| 426 | 423 | // | |
| 427 | - | // A stopped app has no successor to wait for: nothing may run. | |
| 428 | - | func (s *Service) retirePendingContainers(ctx context.Context, app string, stopped bool, active domain.AppActive, pending []domain.AppRecoveryInhibition, failed map[string]struct{}) []error { | |
| 424 | + | // A stopped app has no successor to wait for: nothing may run. Cleanup | |
| 425 | + | // warnings of a retirement (claims not released, secret files) are logged and | |
| 426 | + | // returned, and the marker stays until the container is gone with its claims | |
| 427 | + | // released, so the next pass retries. | |
| 428 | + | func (s *Service) retirePendingContainers(ctx context.Context, app string, stopped bool, active domain.AppActive, pending []domain.AppRecoveryInhibition, verified map[string]time.Time) []error { | |
| 429 | 429 | var failures []error | |
| 430 | 430 | for _, marker := range pending { | |
| 431 | 431 | if publishedContainer(active, marker.ContainerID) { | |
| @@ -434,35 +434,41 @@ func (s *Service) retirePendingContainers(ctx context.Context, app string, stopp | |||
| 434 | 434 | } | |
| 435 | 435 | continue | |
| 436 | 436 | } | |
| 437 | - | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, failed) { | |
| 437 | + | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, verified) { | |
| 438 | 438 | continue | |
| 439 | 439 | } | |
| 440 | 440 | retired := s.retireContainer(ctx, app, retireOptions{ | |
| 441 | - | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |
| 441 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, | |
| 442 | 442 | }, marker.ContainerID) | |
| 443 | - | if !retired.Gone { | |
| 443 | + | if !retired.Settled { | |
| 444 | 444 | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s: %s", app, marker.ContainerID, cleanupDetail(retired))) | |
| 445 | + | continue | |
| 446 | + | } | |
| 447 | + | for _, warning := range retired.Warnings { | |
| 448 | + | s.log.Warn().Str("app", app).Str("service", warning.Service).Str("container", warning.Leftover). | |
| 449 | + | Str("detail", warning.Detail).Msg("deployment: superseded container retired with leftovers") | |
| 450 | + | failures = append(failures, fmt.Errorf("deployment: retired superseded container %s/%s with leftovers: %s", app, warning.Leftover, warning.Detail)) | |
| 445 | 451 | } | |
| 446 | 452 | } | |
| 447 | 453 | return failures | |
| 448 | 454 | } | |
| 449 | 455 | ||
| 450 | 456 | // successorVerified reports whether the container ACTIVE names for the | |
| 451 | - | // service was verified and routed by this daemon, did not fail this pass, and | |
| 452 | - | // still runs. | |
| 453 | - | func (s *Service) successorVerified(ctx context.Context, app, service string, active domain.AppActive, failed map[string]struct{}) bool { | |
| 454 | - | if _, bad := failed[service]; bad { | |
| 455 | - | return false | |
| 456 | - | } | |
| 457 | + | // service was verified and routed by this very pass, and is still the same | |
| 458 | + | // running execution: a final inspection that shows a different start time | |
| 459 | + | // means the successor restarted after it was verified, so it is unverified | |
| 460 | + | // again. A service ACTIVE no longer carries has no successor to wait for. | |
| 461 | + | func (s *Service) successorVerified(ctx context.Context, app, service string, active domain.AppActive, verified map[string]time.Time) bool { | |
| 457 | 462 | successor, ok := active.Services[service] | |
| 458 | 463 | if !ok || successor.Container == "" { | |
| 459 | 464 | return true | |
| 460 | 465 | } | |
| 461 | - | if !s.executions.verified(recoveryKey{app: app, service: service, container: successor.Container}) { | |
| 466 | + | startedAt, ok := verified[service] | |
| 467 | + | if !ok || s.deps.Runtime == nil { | |
| 462 | 468 | return false | |
| 463 | 469 | } | |
| 464 | 470 | container, err := s.deps.Runtime.InspectContainer(ctx, successor.Container) | |
| 465 | - | return err == nil && container.Status == string(domain.ContainerStatusRunning) | |
| 471 | + | return err == nil && container.Status == string(domain.ContainerStatusRunning) && container.StartedAt.Equal(startedAt) | |
| 466 | 472 | } | |
| 467 | 473 | ||
| 468 | 474 | // convergeStoppedService enforces durable stopped intent: it never | |
| @@ -497,23 +503,23 @@ func (s *Service) convergeStoppedService(ctx context.Context, app, name string, | |||
| 497 | 503 | ||
| 498 | 504 | // reconcileRunningService recovers one service to the running intent | |
| 499 | 505 | // carried by ACTIVE. It only ever acts on the exact ACTIVE container ID. | |
| 500 | - | func (s *Service) reconcileRunningService(ctx context.Context, app, name string, eff domain.AppEffectiveService, inhibited map[recoveryKey]struct{}) error { | |
| 506 | + | func (s *Service) reconcileRunningService(ctx context.Context, app, name string, eff domain.AppEffectiveService, inhibited map[recoveryKey]struct{}) (time.Time, bool, error) { | |
| 501 | 507 | if eff.Container == "" { | |
| 502 | 508 | // Absent ACTIVE: never act, never reconstruct. | |
| 503 | - | return nil | |
| 509 | + | return time.Time{}, false, nil | |
| 504 | 510 | } | |
| 505 | 511 | if _, blocked := inhibited[recoveryKey{app: app, service: name, container: eff.Container}]; blocked { | |
| 506 | - | return s.refuseInhibitedRecovery(ctx, app, name, eff.Container) | |
| 512 | + | return time.Time{}, false, s.refuseInhibitedRecovery(ctx, app, name, eff.Container) | |
| 507 | 513 | } | |
| 508 | 514 | if s.deps.Runtime == nil { | |
| 509 | - | return nil | |
| 515 | + | return time.Time{}, false, nil | |
| 510 | 516 | } | |
| 511 | 517 | serviceCtx, cancel := context.WithTimeout(ctx, reconcileServiceDeadline) | |
| 512 | 518 | defer cancel() | |
| 513 | 519 | key := recoveryKey{app: app, service: name, container: eff.Container} | |
| 514 | 520 | container, fresh, err := s.inspectForRecovery(serviceCtx, app, name, eff, key) | |
| 515 | 521 | if err != nil { | |
| 516 | - | return err | |
| 522 | + | return time.Time{}, false, err | |
| 517 | 523 | } | |
| 518 | 524 | return s.convergeRecoveredService(serviceCtx, app, name, eff, container, key, fresh) | |
| 519 | 525 | } | |
| @@ -558,30 +564,31 @@ func (s *Service) inspectForRecovery(ctx context.Context, app, name string, eff | |||
| 558 | 564 | // convergeRecoveredService drives the exact ACTIVE container to a | |
| 559 | 565 | // verified, published state, or leaves it withdrawn with a reported | |
| 560 | 566 | // reason. | |
| 561 | - | func (s *Service) convergeRecoveredService(ctx context.Context, app, name string, eff domain.AppEffectiveService, container *domain.Container, key recoveryKey, fresh bool) error { | |
| 567 | + | func (s *Service) convergeRecoveredService(ctx context.Context, app, name string, eff domain.AppEffectiveService, container *domain.Container, key recoveryKey, fresh bool) (time.Time, bool, error) { | |
| 562 | 568 | started, verify, err := s.driveRecoveredContainer(ctx, app, name, eff, container, key) | |
| 563 | 569 | if err != nil { | |
| 564 | - | return errors.Join(s.withdrawForRecovery(ctx, app, name), err) | |
| 570 | + | return time.Time{}, false, errors.Join(s.withdrawForRecovery(ctx, app, name), err) | |
| 565 | 571 | } | |
| 566 | 572 | if !verify { | |
| 567 | 573 | // Transitional or backoff-delayed: observe now, converge later. | |
| 568 | 574 | s.backoff.invalidateStable(key) | |
| 569 | - | return nil | |
| 575 | + | return time.Time{}, false, nil | |
| 570 | 576 | } | |
| 571 | 577 | restarted, stable, err := s.recoverHealth(ctx, app, name, eff, key) | |
| 572 | 578 | if err != nil { | |
| 573 | 579 | s.backoff.invalidateStable(key) | |
| 574 | - | return errors.Join(s.withdrawForRecovery(ctx, app, name), err) | |
| 580 | + | return time.Time{}, false, errors.Join(s.withdrawForRecovery(ctx, app, name), err) | |
| 575 | 581 | } | |
| 576 | 582 | if err := s.publishRecoveredService(ctx, app, name, eff, container, key, started, restarted, fresh); err != nil { | |
| 577 | - | return err | |
| 583 | + | return time.Time{}, false, err | |
| 578 | 584 | } | |
| 579 | 585 | if stable { | |
| 580 | 586 | s.backoff.observeStable(key) | |
| 581 | 587 | } else { | |
| 582 | 588 | s.backoff.invalidateStable(key) | |
| 583 | 589 | } | |
| 584 | - | return nil | |
| 590 | + | // Verified and published: report the execution this pass vouches for. | |
| 591 | + | return container.StartedAt, true, nil | |
| 585 | 592 | } | |
| 586 | 593 | ||
| 587 | 594 | // refuseInhibitedRecovery reports a generation whose recovery is durably | |
| @@ -288,14 +288,6 @@ func (t *executionTracker) record(key recoveryKey, startedAt time.Time) { | |||||
| 288 | t.seen[key] = startedAt | 288 | t.seen[key] = startedAt | ||
| 289 | } | 289 | } | ||
| 290 | 290 | ||||
| 291 | - | // verified reports whether a verified execution of the generation is recorded. | |||
| 292 | - | func (t *executionTracker) verified(key recoveryKey) bool { | |||
| 293 | - | t.mu.Lock() | |||
| 294 | - | defer t.mu.Unlock() | |||
| 295 | - | _, ok := t.seen[key] | |||
| 296 | - | return ok | |||
| 297 | - | } | |||
| 298 | - | ||||
| 299 | // forget drops one generation's observation. | 291 | // forget drops one generation's observation. | ||
| 300 | func (t *executionTracker) forget(key recoveryKey) { | 292 | func (t *executionTracker) forget(key recoveryKey) { | ||
| 301 | t.mu.Lock() | 293 | t.mu.Lock() | ||
| @@ -368,7 +360,9 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 368 | } | 360 | } | ||
| 369 | inhibited, pending := splitRecoveryInhibitions(app, inhibitions) | 361 | inhibited, pending := splitRecoveryInhibitions(app, inhibitions) | ||
| 370 | var failures []error | 362 | var failures []error | ||
| 371 | - | failed := map[string]struct{}{} | 363 | + | // verified records, per service, the execution this pass verified and |
| 364 | + | // routed. A successor that is not in it is not known to serve. | |||
| 365 | + | verified := map[string]time.Time{} | |||
| 372 | for _, name := range sortedServiceNames(active) { | 366 | for _, name := range sortedServiceNames(active) { | ||
| 373 | eff := active.Services[name] | 367 | eff := active.Services[name] | ||
| 374 | if intent.Stopped { | 368 | if intent.Stopped { | ||
| @@ -377,9 +371,12 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 377 | } | 371 | } | ||
| 378 | continue | 372 | continue | ||
| 379 | } | 373 | } | ||
| 380 | - | if err := s.reconcileRunningService(ctx, app, name, eff, inhibited); err != nil { | 374 | + | startedAt, ok, err := s.reconcileRunningService(ctx, app, name, eff, inhibited) |
| 375 | + | if err != nil { | |||
| 381 | failures = append(failures, err) | 376 | failures = append(failures, err) | ||
| 382 | - | failed[name] = struct{}{} | 377 | + | } |
| 378 | + | if ok { | |||
| 379 | + | verified[name] = startedAt | |||
| 383 | } | 380 | } | ||
| 384 | } | 381 | } | ||
| 385 | if intent.Stopped { | 382 | if intent.Stopped { | ||
| @@ -394,7 +391,7 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 394 | } | 391 | } | ||
| 395 | // Superseded containers are retired last: only after their successors | 392 | // Superseded containers are retired last: only after their successors | ||
| 396 | // were verified and routed in this pass. | 393 | // were verified and routed in this pass. | ||
| 397 | - | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, failed)...) | 394 | + | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, verified)...) |
| 398 | s.pruneRecoveryHistory(app, active) | 395 | s.pruneRecoveryHistory(app, active) | ||
| 399 | return errors.Join(failures...) | 396 | return errors.Join(failures...) | ||
| 400 | } | 397 | } | ||
| @@ -420,12 +417,15 @@ func splitRecoveryInhibitions(app string, inhibitions []domain.AppRecoveryInhibi | |||||
| 420 | // when it can no longer be the serving one: | 417 | // when it can no longer be the serving one: | ||
| 421 | // - ACTIVE names it (the replacement never took over): it is the serving | 418 | // - ACTIVE names it (the replacement never took over): it is the serving | ||
| 422 | // generation, so the marker is dropped and the container is kept; | 419 | // generation, so the marker is dropped and the container is kept; | ||
| 423 | - | // - its successor was not verified and routed in this pass, or its | 420 | + | // - its successor was not verified and routed by THIS pass, or restarted |
| 424 | - | // reconciliation failed: the superseded container is retried later, so a | 421 | + | // since: the superseded container is retried later, so a successor that |
| 425 | - | // successor that is not serving never costs the working generation. | 422 | + | // is not serving never costs the working generation. |
| 426 | // | 423 | // | ||
| 427 | - | // A stopped app has no successor to wait for: nothing may run. | 424 | + | // A stopped app has no successor to wait for: nothing may run. Cleanup |
| 428 | - | func (s *Service) retirePendingContainers(ctx context.Context, app string, stopped bool, active domain.AppActive, pending []domain.AppRecoveryInhibition, failed map[string]struct{}) []error { | 425 | + | // warnings of a retirement (claims not released, secret files) are logged and |
| 426 | + | // returned, and the marker stays until the container is gone with its claims | |||
| 427 | + | // released, so the next pass retries. | |||
| 428 | + | func (s *Service) retirePendingContainers(ctx context.Context, app string, stopped bool, active domain.AppActive, pending []domain.AppRecoveryInhibition, verified map[string]time.Time) []error { | |||
| 429 | var failures []error | 429 | var failures []error | ||
| 430 | for _, marker := range pending { | 430 | for _, marker := range pending { | ||
| 431 | if publishedContainer(active, marker.ContainerID) { | 431 | if publishedContainer(active, marker.ContainerID) { | ||
| @@ -434,35 +434,41 @@ func (s *Service) retirePendingContainers(ctx context.Context, app string, stopp | |||||
| 434 | } | 434 | } | ||
| 435 | continue | 435 | continue | ||
| 436 | } | 436 | } | ||
| 437 | - | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, failed) { | 437 | + | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, verified) { |
| 438 | continue | 438 | continue | ||
| 439 | } | 439 | } | ||
| 440 | retired := s.retireContainer(ctx, app, retireOptions{ | 440 | retired := s.retireContainer(ctx, app, retireOptions{ | ||
| 441 | - | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | 441 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, RetryOnLeak: true, |
| 442 | }, marker.ContainerID) | 442 | }, marker.ContainerID) | ||
| 443 | - | if !retired.Gone { | 443 | + | if !retired.Settled { |
| 444 | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s: %s", app, marker.ContainerID, cleanupDetail(retired))) | 444 | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s: %s", app, marker.ContainerID, cleanupDetail(retired))) | ||
| 445 | + | continue | |||
| 446 | + | } | |||
| 447 | + | for _, warning := range retired.Warnings { | |||
| 448 | + | s.log.Warn().Str("app", app).Str("service", warning.Service).Str("container", warning.Leftover). | |||
| 449 | + | Str("detail", warning.Detail).Msg("deployment: superseded container retired with leftovers") | |||
| 450 | + | failures = append(failures, fmt.Errorf("deployment: retired superseded container %s/%s with leftovers: %s", app, warning.Leftover, warning.Detail)) | |||
| 445 | } | 451 | } | ||
| 446 | } | 452 | } | ||
| 447 | return failures | 453 | return failures | ||
| 448 | } | 454 | } | ||
| 449 | 455 | ||||
| 450 | // successorVerified reports whether the container ACTIVE names for the | 456 | // successorVerified reports whether the container ACTIVE names for the | ||
| 451 | - | // service was verified and routed by this daemon, did not fail this pass, and | 457 | + | // service was verified and routed by this very pass, and is still the same |
| 452 | - | // still runs. | 458 | + | // running execution: a final inspection that shows a different start time |
| 453 | - | func (s *Service) successorVerified(ctx context.Context, app, service string, active domain.AppActive, failed map[string]struct{}) bool { | 459 | + | // means the successor restarted after it was verified, so it is unverified |
| 454 | - | if _, bad := failed[service]; bad { | 460 | + | // again. A service ACTIVE no longer carries has no successor to wait for. |
| 455 | - | return false | 461 | + | func (s *Service) successorVerified(ctx context.Context, app, service string, active domain.AppActive, verified map[string]time.Time) bool { |
| 456 | - | } | |||
| 457 | successor, ok := active.Services[service] | 462 | successor, ok := active.Services[service] | ||
| 458 | if !ok || successor.Container == "" { | 463 | if !ok || successor.Container == "" { | ||
| 459 | return true | 464 | return true | ||
| 460 | } | 465 | } | ||
| 461 | - | if !s.executions.verified(recoveryKey{app: app, service: service, container: successor.Container}) { | 466 | + | startedAt, ok := verified[service] |
| 467 | + | if !ok || s.deps.Runtime == nil { | |||
| 462 | return false | 468 | return false | ||
| 463 | } | 469 | } | ||
| 464 | container, err := s.deps.Runtime.InspectContainer(ctx, successor.Container) | 470 | container, err := s.deps.Runtime.InspectContainer(ctx, successor.Container) | ||
| 465 | - | return err == nil && container.Status == string(domain.ContainerStatusRunning) | 471 | + | return err == nil && container.Status == string(domain.ContainerStatusRunning) && container.StartedAt.Equal(startedAt) |
| 466 | } | 472 | } | ||
| 467 | 473 | ||||
| 468 | // convergeStoppedService enforces durable stopped intent: it never | 474 | // convergeStoppedService enforces durable stopped intent: it never | ||
| @@ -497,23 +503,23 @@ func (s *Service) convergeStoppedService(ctx context.Context, app, name string, | |||||
| 497 | 503 | ||||
| 498 | // reconcileRunningService recovers one service to the running intent | 504 | // reconcileRunningService recovers one service to the running intent | ||
| 499 | // carried by ACTIVE. It only ever acts on the exact ACTIVE container ID. | 505 | // carried by ACTIVE. It only ever acts on the exact ACTIVE container ID. | ||
| 500 | - | func (s *Service) reconcileRunningService(ctx context.Context, app, name string, eff domain.AppEffectiveService, inhibited map[recoveryKey]struct{}) error { | 506 | + | func (s *Service) reconcileRunningService(ctx context.Context, app, name string, eff domain.AppEffectiveService, inhibited map[recoveryKey]struct{}) (time.Time, bool, error) { |
| 501 | if eff.Container == "" { | 507 | if eff.Container == "" { | ||
| 502 | // Absent ACTIVE: never act, never reconstruct. | 508 | // Absent ACTIVE: never act, never reconstruct. | ||
| 503 | - | return nil | 509 | + | return time.Time{}, false, nil |
| 504 | } | 510 | } | ||
| 505 | if _, blocked := inhibited[recoveryKey{app: app, service: name, container: eff.Container}]; blocked { | 511 | if _, blocked := inhibited[recoveryKey{app: app, service: name, container: eff.Container}]; blocked { | ||
| 506 | - | return s.refuseInhibitedRecovery(ctx, app, name, eff.Container) | 512 | + | return time.Time{}, false, s.refuseInhibitedRecovery(ctx, app, name, eff.Container) |
| 507 | } | 513 | } | ||
| 508 | if s.deps.Runtime == nil { | 514 | if s.deps.Runtime == nil { | ||
| 509 | - | return nil | 515 | + | return time.Time{}, false, nil |
| 510 | } | 516 | } | ||
| 511 | serviceCtx, cancel := context.WithTimeout(ctx, reconcileServiceDeadline) | 517 | serviceCtx, cancel := context.WithTimeout(ctx, reconcileServiceDeadline) | ||
| 512 | defer cancel() | 518 | defer cancel() | ||
| 513 | key := recoveryKey{app: app, service: name, container: eff.Container} | 519 | key := recoveryKey{app: app, service: name, container: eff.Container} | ||
| 514 | container, fresh, err := s.inspectForRecovery(serviceCtx, app, name, eff, key) | 520 | container, fresh, err := s.inspectForRecovery(serviceCtx, app, name, eff, key) | ||
| 515 | if err != nil { | 521 | if err != nil { | ||
| 516 | - | return err | 522 | + | return time.Time{}, false, err |
| 517 | } | 523 | } | ||
| 518 | return s.convergeRecoveredService(serviceCtx, app, name, eff, container, key, fresh) | 524 | return s.convergeRecoveredService(serviceCtx, app, name, eff, container, key, fresh) | ||
| 519 | } | 525 | } | ||
| @@ -558,30 +564,31 @@ func (s *Service) inspectForRecovery(ctx context.Context, app, name string, eff | |||||
| 558 | // convergeRecoveredService drives the exact ACTIVE container to a | 564 | // convergeRecoveredService drives the exact ACTIVE container to a | ||
| 559 | // verified, published state, or leaves it withdrawn with a reported | 565 | // verified, published state, or leaves it withdrawn with a reported | ||
| 560 | // reason. | 566 | // reason. | ||
| 561 | - | func (s *Service) convergeRecoveredService(ctx context.Context, app, name string, eff domain.AppEffectiveService, container *domain.Container, key recoveryKey, fresh bool) error { | 567 | + | func (s *Service) convergeRecoveredService(ctx context.Context, app, name string, eff domain.AppEffectiveService, container *domain.Container, key recoveryKey, fresh bool) (time.Time, bool, error) { |
| 562 | started, verify, err := s.driveRecoveredContainer(ctx, app, name, eff, container, key) | 568 | started, verify, err := s.driveRecoveredContainer(ctx, app, name, eff, container, key) | ||
| 563 | if err != nil { | 569 | if err != nil { | ||
| 564 | - | return errors.Join(s.withdrawForRecovery(ctx, app, name), err) | 570 | + | return time.Time{}, false, errors.Join(s.withdrawForRecovery(ctx, app, name), err) |
| 565 | } | 571 | } | ||
| 566 | if !verify { | 572 | if !verify { | ||
| 567 | // Transitional or backoff-delayed: observe now, converge later. | 573 | // Transitional or backoff-delayed: observe now, converge later. | ||
| 568 | s.backoff.invalidateStable(key) | 574 | s.backoff.invalidateStable(key) | ||
| 569 | - | return nil | 575 | + | return time.Time{}, false, nil |
| 570 | } | 576 | } | ||
| 571 | restarted, stable, err := s.recoverHealth(ctx, app, name, eff, key) | 577 | restarted, stable, err := s.recoverHealth(ctx, app, name, eff, key) | ||
| 572 | if err != nil { | 578 | if err != nil { | ||
| 573 | s.backoff.invalidateStable(key) | 579 | s.backoff.invalidateStable(key) | ||
| 574 | - | return errors.Join(s.withdrawForRecovery(ctx, app, name), err) | 580 | + | return time.Time{}, false, errors.Join(s.withdrawForRecovery(ctx, app, name), err) |
| 575 | } | 581 | } | ||
| 576 | if err := s.publishRecoveredService(ctx, app, name, eff, container, key, started, restarted, fresh); err != nil { | 582 | if err := s.publishRecoveredService(ctx, app, name, eff, container, key, started, restarted, fresh); err != nil { | ||
| 577 | - | return err | 583 | + | return time.Time{}, false, err |
| 578 | } | 584 | } | ||
| 579 | if stable { | 585 | if stable { | ||
| 580 | s.backoff.observeStable(key) | 586 | s.backoff.observeStable(key) | ||
| 581 | } else { | 587 | } else { | ||
| 582 | s.backoff.invalidateStable(key) | 588 | s.backoff.invalidateStable(key) | ||
| 583 | } | 589 | } | ||
| 584 | - | return nil | 590 | + | // Verified and published: report the execution this pass vouches for. |
| 591 | + | return container.StartedAt, true, nil | |||
| 585 | } | 592 | } | ||
| 586 | 593 | ||||
| 587 | // refuseInhibitedRecovery reports a generation whose recovery is durably | 594 | // refuseInhibitedRecovery reports a generation whose recovery is durably | ||
M internal/usecase/deployment/overlap_replacement_test.go +258 -0
| @@ -535,3 +535,261 @@ func TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass(t *testing.T) | |||
| 535 | 535 | require.NoError(t, err) | |
| 536 | 536 | assert.Empty(t, markers) | |
| 537 | 537 | } | |
| 538 | + | ||
| 539 | + | // TestDeploy_PredecessorWithOwnedResourcesNeverOverlaps proves eligibility | |
| 540 | + | // looks at the predecessor's effective spec too: a replacement that drops the | |
| 541 | + | // UDP interface or the volume of the generation it supersedes still replaces | |
| 542 | + | // it sequentially, because the old generation's UDP sessions or writes | |
| 543 | + | // outlive the switch. | |
| 544 | + | func TestDeploy_PredecessorWithOwnedResourcesNeverOverlaps(t *testing.T) { | |
| 545 | + | cases := map[string]func(*domain.AppService){ | |
| 546 | + | "udp": func(s *domain.AppService) { | |
| 547 | + | s.UDP = []domain.AppUDPInterface{{Entrypoint: "game", Port: 28015, Publish: "0.0.0.0:28015"}} | |
| 548 | + | }, | |
| 549 | + | "volume": func(s *domain.AppService) { s.Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} }, | |
| 550 | + | } | |
| 551 | + | for name, mutate := range cases { | |
| 552 | + | t.Run(name, func(t *testing.T) { | |
| 553 | + | ctx := context.Background() | |
| 554 | + | state, runtime, images, secrets := mockDeps(t) | |
| 555 | + | rev := mockRevision() // HTTP-only, no volume: eligible on its own | |
| 556 | + | previous := rev.Spec.Services[0] | |
| 557 | + | mutate(&previous) | |
| 558 | + | active := domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 559 | + | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: previous}, | |
| 560 | + | }} | |
| 561 | + | ||
| 562 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 563 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 564 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |
| 565 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 566 | + | var order []string | |
| 567 | + | expectRetireOld(state, runtime, &order, "c-old") | |
| 568 | + | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |
| 569 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |
| 570 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 571 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 572 | + | ||
| 573 | + | svc := deployment.NewService(deployment.Deps{ | |
| 574 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |
| 575 | + | Traffic: &orderedTraffic{events: &order}, | |
| 576 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 577 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 578 | + | func(context.Context, string) error { return nil }, | |
| 579 | + | )) | |
| 580 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 581 | + | require.NoError(t, err) | |
| 582 | + | assert.Equal(t, []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "traffic"}, order[:6], | |
| 583 | + | "the predecessor is gone before the replacement is created") | |
| 584 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |
| 585 | + | }) | |
| 586 | + | } | |
| 587 | + | } | |
| 588 | + | ||
| 589 | + | // seedPendingRetirement seeds ACTIVE naming c-new and one retirement-pending | |
| 590 | + | // marker per given superseded container. | |
| 591 | + | func seedPendingRetirement(t *testing.T, ctx context.Context, store *appstate.Store, superseded ...string) { | |
| 592 | + | t.Helper() | |
| 593 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |
| 594 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 595 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: overlapWebSpec(0), BackendBinds: map[int]int{8080: 18081}}, | |
| 596 | + | }})) | |
| 597 | + | for _, id := range superseded { | |
| 598 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |
| 599 | + | App: "blog", Service: "web", ContainerID: id, Reason: domain.AppInhibitRetirementPending, | |
| 600 | + | })) | |
| 601 | + | } | |
| 602 | + | } | |
| 603 | + | ||
| 604 | + | func markerIDs(t *testing.T, ctx context.Context, store *appstate.Store) []string { | |
| 605 | + | t.Helper() | |
| 606 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 607 | + | require.NoError(t, err) | |
| 608 | + | ids := make([]string, 0, len(markers)) | |
| 609 | + | for _, marker := range markers { | |
| 610 | + | ids = append(ids, marker.ContainerID) | |
| 611 | + | } | |
| 612 | + | return ids | |
| 613 | + | } | |
| 614 | + | ||
| 615 | + | // TestReconcileRunning_DoesNotRetireWhenSuccessorIsNotVerifiedByThisPass is the | |
| 616 | + | // regression for a successor that was verified by an earlier pass but is | |
| 617 | + | // restarting in this one: it is withdrawn, not verified, and a final | |
| 618 | + | // inspection that happens to see it running must not retire the predecessor. | |
| 619 | + | func TestReconcileRunning_DoesNotRetireWhenSuccessorIsNotVerifiedByThisPass(t *testing.T) { | |
| 620 | + | ctx := context.Background() | |
| 621 | + | store := newTestStore(t) | |
| 622 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 623 | + | images := outmocks.NewMockImageResolver(t) | |
| 624 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 625 | + | seedPendingRetirement(t, ctx, store, "c-old") | |
| 626 | + | traffic := &storeTraffic{store: store} | |
| 627 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 628 | + | ||
| 629 | + | // Earlier pass: the successor is verified and routed, retirement fails. | |
| 630 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |
| 631 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 632 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 633 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |
| 634 | + | ||
| 635 | + | // This pass: the successor is restarting, so it is withdrawn and not | |
| 636 | + | // verified. Any later inspection would see it running. | |
| 637 | + | runtime.ExpectedCalls, runtime.Calls = nil, nil | |
| 638 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new"). | |
| 639 | + | Return(&domain.Container{ID: "c-new", Status: "restarting"}, nil).Once() | |
| 640 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new"). | |
| 641 | + | Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: time.Now()}, nil).Maybe() | |
| 642 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 643 | + | ||
| 644 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 645 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the predecessor stays until a pass verifies the successor") | |
| 646 | + | } | |
| 647 | + | ||
| 648 | + | // TestReconcileRunning_DoesNotRetireWhenSuccessorRestartedAfterVerification | |
| 649 | + | // covers the other half: a successor verified at one start time and seen with | |
| 650 | + | // another by the final inspection is a different execution, so unverified. | |
| 651 | + | func TestReconcileRunning_DoesNotRetireWhenSuccessorRestartedAfterVerification(t *testing.T) { | |
| 652 | + | ctx := context.Background() | |
| 653 | + | store := newTestStore(t) | |
| 654 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 655 | + | images := outmocks.NewMockImageResolver(t) | |
| 656 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 657 | + | seedPendingRetirement(t, ctx, store, "c-old") | |
| 658 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |
| 659 | + | ||
| 660 | + | first := time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC) | |
| 661 | + | restarted := first.Add(time.Minute) | |
| 662 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: first}, nil).Twice() | |
| 663 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: restarted}, nil).Once() | |
| 664 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c-new").Return("", false, nil) | |
| 665 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 666 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil) | |
| 667 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 668 | + | ||
| 669 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 670 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store)) | |
| 671 | + | } | |
| 672 | + | ||
| 673 | + | // TestReconcileRunning_RetiresStackedPredecessors proves several superseded | |
| 674 | + | // containers of one service (deploys that each left one behind) are all | |
| 675 | + | // retired once the successor is verified. | |
| 676 | + | func TestReconcileRunning_RetiresStackedPredecessors(t *testing.T) { | |
| 677 | + | ctx := context.Background() | |
| 678 | + | store := newTestStore(t) | |
| 679 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 680 | + | images := outmocks.NewMockImageResolver(t) | |
| 681 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 682 | + | seedPendingRetirement(t, ctx, store, "c-old-1", "c-old-2") | |
| 683 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |
| 684 | + | ||
| 685 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |
| 686 | + | for _, id := range []string{"c-old-1", "c-old-2"} { | |
| 687 | + | runtime.EXPECT().StopContainer(mock.Anything, id, domain.AppDefaultStopGrace).Return(nil).Once() | |
| 688 | + | runtime.EXPECT().RemoveContainer(mock.Anything, id, false).Return(nil).Once() | |
| 689 | + | } | |
| 690 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 691 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |
| 692 | + | } | |
| 693 | + | ||
| 694 | + | // releaseFailingState wraps the real store and fails the first backend claim | |
| 695 | + | // releases, like a store write error would. | |
| 696 | + | type releaseFailingState struct { | |
| 697 | + | *appstate.Store | |
| 698 | + | failures int | |
| 699 | + | } | |
| 700 | + | ||
| 701 | + | func (s *releaseFailingState) ReleaseBackendBinds(ctx context.Context, app, containerID string) error { | |
| 702 | + | if s.failures > 0 { | |
| 703 | + | s.failures-- | |
| 704 | + | return assert.AnError | |
| 705 | + | } | |
| 706 | + | return s.Store.ReleaseBackendBinds(ctx, app, containerID) | |
| 707 | + | } | |
| 708 | + | ||
| 709 | + | // TestReconcileRunning_KeepsMarkerUntilBackendClaimsAreReleased proves a | |
| 710 | + | // retirement whose claim release failed does not drop the durable marker: the | |
| 711 | + | // leak is reported by the pass and retried (retiring a gone container is | |
| 712 | + | // idempotent) until the claims are released. | |
| 713 | + | func TestReconcileRunning_KeepsMarkerUntilBackendClaimsAreReleased(t *testing.T) { | |
| 714 | + | ctx := context.Background() | |
| 715 | + | store := newTestStore(t) | |
| 716 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 717 | + | images := outmocks.NewMockImageResolver(t) | |
| 718 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 719 | + | seedPendingRetirement(t, ctx, store, "c-old") | |
| 720 | + | state := &releaseFailingState{Store: store, failures: 1} | |
| 721 | + | traffic := &storeTraffic{store: store} | |
| 722 | + | svc := deployment.NewService(deployment.Deps{ | |
| 723 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: traffic, | |
| 724 | + | ImagePolicy: domain.ImageSourcePolicy{AllowedRegistries: []string{"registry.example.com"}}, | |
| 725 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 726 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 727 | + | func(context.Context, string) error { return nil }, | |
| 728 | + | )) | |
| 729 | + | ||
| 730 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |
| 731 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Twice() | |
| 732 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 733 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | |
| 734 | + | ||
| 735 | + | err := svc.ReconcileRunning(ctx) | |
| 736 | + | require.Error(t, err, "the leaked claims are reported, not discarded") | |
| 737 | + | assert.Contains(t, err.Error(), "release backend claims") | |
| 738 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the marker stays until the claims are released") | |
| 739 | + | ||
| 740 | + | require.NoError(t, svc.ReconcileRunning(ctx), "not-found counts as gone, so the retry settles it") | |
| 741 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |
| 742 | + | } | |
| 743 | + | ||
| 744 | + | // TestStop_RetiresPendingSupersededContainers proves a stopped app does not | |
| 745 | + | // keep a superseded predecessor running, and that one which cannot be retired | |
| 746 | + | // fails the stop and keeps its marker. | |
| 747 | + | func TestStop_RetiresPendingSupersededContainers(t *testing.T) { | |
| 748 | + | ctx := context.Background() | |
| 749 | + | store := newTestStore(t) | |
| 750 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 751 | + | images := outmocks.NewMockImageResolver(t) | |
| 752 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 753 | + | seedPendingRetirement(t, ctx, store, "c-old") | |
| 754 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |
| 755 | + | ||
| 756 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-new", mock.Anything).Return(nil).Twice() | |
| 757 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", false).Return(nil).Twice() | |
| 758 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Twice() | |
| 759 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 760 | + | ||
| 761 | + | result, err := svc.Stop(ctx, "blog", "") | |
| 762 | + | require.Error(t, err, "a predecessor that is still running fails the stop") | |
| 763 | + | assert.Equal(t, "failed", result.Services["web"].Result) | |
| 764 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "its marker stays for the next attempt") | |
| 765 | + | ||
| 766 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 767 | + | _, err = svc.Stop(ctx, "blog", "") | |
| 768 | + | require.NoError(t, err) | |
| 769 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |
| 770 | + | } | |
| 771 | + | ||
| 772 | + | // TestRemove_FailsAndKeepsStateWhenAPendingPredecessorCannotBeRetired proves | |
| 773 | + | // Remove does not end the incarnation (which deletes the markers) while a | |
| 774 | + | // superseded container is still running. | |
| 775 | + | func TestRemove_FailsAndKeepsStateWhenAPendingPredecessorCannotBeRetired(t *testing.T) { | |
| 776 | + | ctx := context.Background() | |
| 777 | + | store := newTestStore(t) | |
| 778 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 779 | + | images := outmocks.NewMockImageResolver(t) | |
| 780 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 781 | + | seedPendingRetirement(t, ctx, store, "c-old") | |
| 782 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |
| 783 | + | ||
| 784 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-new", mock.Anything).Return(nil).Once() | |
| 785 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", false).Return(nil).Once() | |
| 786 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 787 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 788 | + | ||
| 789 | + | _, err := svc.Remove(ctx, "blog", "") | |
| 790 | + | require.Error(t, err) | |
| 791 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the marker survives so the container stays tracked") | |
| 792 | + | _, ok, loadErr := store.LoadActive(ctx, "blog") | |
| 793 | + | require.NoError(t, loadErr) | |
| 794 | + | assert.True(t, ok, "the incarnation is not ended") | |
| 795 | + | } | |
| @@ -535,3 +535,261 @@ func TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass(t *testing.T) | |||||
| 535 | require.NoError(t, err) | 535 | require.NoError(t, err) | ||
| 536 | assert.Empty(t, markers) | 536 | assert.Empty(t, markers) | ||
| 537 | } | 537 | } | ||
| 538 | + | ||||
| 539 | + | // TestDeploy_PredecessorWithOwnedResourcesNeverOverlaps proves eligibility | |||
| 540 | + | // looks at the predecessor's effective spec too: a replacement that drops the | |||
| 541 | + | // UDP interface or the volume of the generation it supersedes still replaces | |||
| 542 | + | // it sequentially, because the old generation's UDP sessions or writes | |||
| 543 | + | // outlive the switch. | |||
| 544 | + | func TestDeploy_PredecessorWithOwnedResourcesNeverOverlaps(t *testing.T) { | |||
| 545 | + | cases := map[string]func(*domain.AppService){ | |||
| 546 | + | "udp": func(s *domain.AppService) { | |||
| 547 | + | s.UDP = []domain.AppUDPInterface{{Entrypoint: "game", Port: 28015, Publish: "0.0.0.0:28015"}} | |||
| 548 | + | }, | |||
| 549 | + | "volume": func(s *domain.AppService) { s.Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} }, | |||
| 550 | + | } | |||
| 551 | + | for name, mutate := range cases { | |||
| 552 | + | t.Run(name, func(t *testing.T) { | |||
| 553 | + | ctx := context.Background() | |||
| 554 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 555 | + | rev := mockRevision() // HTTP-only, no volume: eligible on its own | |||
| 556 | + | previous := rev.Spec.Services[0] | |||
| 557 | + | mutate(&previous) | |||
| 558 | + | active := domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |||
| 559 | + | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: previous}, | |||
| 560 | + | }} | |||
| 561 | + | ||||
| 562 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 563 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 564 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |||
| 565 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 566 | + | var order []string | |||
| 567 | + | expectRetireOld(state, runtime, &order, "c-old") | |||
| 568 | + | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |||
| 569 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 570 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |||
| 571 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |||
| 572 | + | ||||
| 573 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 574 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 575 | + | Traffic: &orderedTraffic{events: &order}, | |||
| 576 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 577 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 578 | + | func(context.Context, string) error { return nil }, | |||
| 579 | + | )) | |||
| 580 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 581 | + | require.NoError(t, err) | |||
| 582 | + | assert.Equal(t, []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "traffic"}, order[:6], | |||
| 583 | + | "the predecessor is gone before the replacement is created") | |||
| 584 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |||
| 585 | + | }) | |||
| 586 | + | } | |||
| 587 | + | } | |||
| 588 | + | ||||
| 589 | + | // seedPendingRetirement seeds ACTIVE naming c-new and one retirement-pending | |||
| 590 | + | // marker per given superseded container. | |||
| 591 | + | func seedPendingRetirement(t *testing.T, ctx context.Context, store *appstate.Store, superseded ...string) { | |||
| 592 | + | t.Helper() | |||
| 593 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |||
| 594 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |||
| 595 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: overlapWebSpec(0), BackendBinds: map[int]int{8080: 18081}}, | |||
| 596 | + | }})) | |||
| 597 | + | for _, id := range superseded { | |||
| 598 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |||
| 599 | + | App: "blog", Service: "web", ContainerID: id, Reason: domain.AppInhibitRetirementPending, | |||
| 600 | + | })) | |||
| 601 | + | } | |||
| 602 | + | } | |||
| 603 | + | ||||
| 604 | + | func markerIDs(t *testing.T, ctx context.Context, store *appstate.Store) []string { | |||
| 605 | + | t.Helper() | |||
| 606 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 607 | + | require.NoError(t, err) | |||
| 608 | + | ids := make([]string, 0, len(markers)) | |||
| 609 | + | for _, marker := range markers { | |||
| 610 | + | ids = append(ids, marker.ContainerID) | |||
| 611 | + | } | |||
| 612 | + | return ids | |||
| 613 | + | } | |||
| 614 | + | ||||
| 615 | + | // TestReconcileRunning_DoesNotRetireWhenSuccessorIsNotVerifiedByThisPass is the | |||
| 616 | + | // regression for a successor that was verified by an earlier pass but is | |||
| 617 | + | // restarting in this one: it is withdrawn, not verified, and a final | |||
| 618 | + | // inspection that happens to see it running must not retire the predecessor. | |||
| 619 | + | func TestReconcileRunning_DoesNotRetireWhenSuccessorIsNotVerifiedByThisPass(t *testing.T) { | |||
| 620 | + | ctx := context.Background() | |||
| 621 | + | store := newTestStore(t) | |||
| 622 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 623 | + | images := outmocks.NewMockImageResolver(t) | |||
| 624 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 625 | + | seedPendingRetirement(t, ctx, store, "c-old") | |||
| 626 | + | traffic := &storeTraffic{store: store} | |||
| 627 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |||
| 628 | + | ||||
| 629 | + | // Earlier pass: the successor is verified and routed, retirement fails. | |||
| 630 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |||
| 631 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 632 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |||
| 633 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |||
| 634 | + | ||||
| 635 | + | // This pass: the successor is restarting, so it is withdrawn and not | |||
| 636 | + | // verified. Any later inspection would see it running. | |||
| 637 | + | runtime.ExpectedCalls, runtime.Calls = nil, nil | |||
| 638 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new"). | |||
| 639 | + | Return(&domain.Container{ID: "c-new", Status: "restarting"}, nil).Once() | |||
| 640 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new"). | |||
| 641 | + | Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: time.Now()}, nil).Maybe() | |||
| 642 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 643 | + | ||||
| 644 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |||
| 645 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the predecessor stays until a pass verifies the successor") | |||
| 646 | + | } | |||
| 647 | + | ||||
| 648 | + | // TestReconcileRunning_DoesNotRetireWhenSuccessorRestartedAfterVerification | |||
| 649 | + | // covers the other half: a successor verified at one start time and seen with | |||
| 650 | + | // another by the final inspection is a different execution, so unverified. | |||
| 651 | + | func TestReconcileRunning_DoesNotRetireWhenSuccessorRestartedAfterVerification(t *testing.T) { | |||
| 652 | + | ctx := context.Background() | |||
| 653 | + | store := newTestStore(t) | |||
| 654 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 655 | + | images := outmocks.NewMockImageResolver(t) | |||
| 656 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 657 | + | seedPendingRetirement(t, ctx, store, "c-old") | |||
| 658 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |||
| 659 | + | ||||
| 660 | + | first := time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC) | |||
| 661 | + | restarted := first.Add(time.Minute) | |||
| 662 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: first}, nil).Twice() | |||
| 663 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "running", StartedAt: restarted}, nil).Once() | |||
| 664 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, "c-new").Return("", false, nil) | |||
| 665 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |||
| 666 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil) | |||
| 667 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 668 | + | ||||
| 669 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |||
| 670 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store)) | |||
| 671 | + | } | |||
| 672 | + | ||||
| 673 | + | // TestReconcileRunning_RetiresStackedPredecessors proves several superseded | |||
| 674 | + | // containers of one service (deploys that each left one behind) are all | |||
| 675 | + | // retired once the successor is verified. | |||
| 676 | + | func TestReconcileRunning_RetiresStackedPredecessors(t *testing.T) { | |||
| 677 | + | ctx := context.Background() | |||
| 678 | + | store := newTestStore(t) | |||
| 679 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 680 | + | images := outmocks.NewMockImageResolver(t) | |||
| 681 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 682 | + | seedPendingRetirement(t, ctx, store, "c-old-1", "c-old-2") | |||
| 683 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |||
| 684 | + | ||||
| 685 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |||
| 686 | + | for _, id := range []string{"c-old-1", "c-old-2"} { | |||
| 687 | + | runtime.EXPECT().StopContainer(mock.Anything, id, domain.AppDefaultStopGrace).Return(nil).Once() | |||
| 688 | + | runtime.EXPECT().RemoveContainer(mock.Anything, id, false).Return(nil).Once() | |||
| 689 | + | } | |||
| 690 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 691 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |||
| 692 | + | } | |||
| 693 | + | ||||
| 694 | + | // releaseFailingState wraps the real store and fails the first backend claim | |||
| 695 | + | // releases, like a store write error would. | |||
| 696 | + | type releaseFailingState struct { | |||
| 697 | + | *appstate.Store | |||
| 698 | + | failures int | |||
| 699 | + | } | |||
| 700 | + | ||||
| 701 | + | func (s *releaseFailingState) ReleaseBackendBinds(ctx context.Context, app, containerID string) error { | |||
| 702 | + | if s.failures > 0 { | |||
| 703 | + | s.failures-- | |||
| 704 | + | return assert.AnError | |||
| 705 | + | } | |||
| 706 | + | return s.Store.ReleaseBackendBinds(ctx, app, containerID) | |||
| 707 | + | } | |||
| 708 | + | ||||
| 709 | + | // TestReconcileRunning_KeepsMarkerUntilBackendClaimsAreReleased proves a | |||
| 710 | + | // retirement whose claim release failed does not drop the durable marker: the | |||
| 711 | + | // leak is reported by the pass and retried (retiring a gone container is | |||
| 712 | + | // idempotent) until the claims are released. | |||
| 713 | + | func TestReconcileRunning_KeepsMarkerUntilBackendClaimsAreReleased(t *testing.T) { | |||
| 714 | + | ctx := context.Background() | |||
| 715 | + | store := newTestStore(t) | |||
| 716 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 717 | + | images := outmocks.NewMockImageResolver(t) | |||
| 718 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 719 | + | seedPendingRetirement(t, ctx, store, "c-old") | |||
| 720 | + | state := &releaseFailingState{Store: store, failures: 1} | |||
| 721 | + | traffic := &storeTraffic{store: store} | |||
| 722 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 723 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: traffic, | |||
| 724 | + | ImagePolicy: domain.ImageSourcePolicy{AllowedRegistries: []string{"registry.example.com"}}, | |||
| 725 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 726 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 727 | + | func(context.Context, string) error { return nil }, | |||
| 728 | + | )) | |||
| 729 | + | ||||
| 730 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |||
| 731 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Twice() | |||
| 732 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 733 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | |||
| 734 | + | ||||
| 735 | + | err := svc.ReconcileRunning(ctx) | |||
| 736 | + | require.Error(t, err, "the leaked claims are reported, not discarded") | |||
| 737 | + | assert.Contains(t, err.Error(), "release backend claims") | |||
| 738 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the marker stays until the claims are released") | |||
| 739 | + | ||||
| 740 | + | require.NoError(t, svc.ReconcileRunning(ctx), "not-found counts as gone, so the retry settles it") | |||
| 741 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |||
| 742 | + | } | |||
| 743 | + | ||||
| 744 | + | // TestStop_RetiresPendingSupersededContainers proves a stopped app does not | |||
| 745 | + | // keep a superseded predecessor running, and that one which cannot be retired | |||
| 746 | + | // fails the stop and keeps its marker. | |||
| 747 | + | func TestStop_RetiresPendingSupersededContainers(t *testing.T) { | |||
| 748 | + | ctx := context.Background() | |||
| 749 | + | store := newTestStore(t) | |||
| 750 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 751 | + | images := outmocks.NewMockImageResolver(t) | |||
| 752 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 753 | + | seedPendingRetirement(t, ctx, store, "c-old") | |||
| 754 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |||
| 755 | + | ||||
| 756 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-new", mock.Anything).Return(nil).Twice() | |||
| 757 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", false).Return(nil).Twice() | |||
| 758 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Twice() | |||
| 759 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |||
| 760 | + | ||||
| 761 | + | result, err := svc.Stop(ctx, "blog", "") | |||
| 762 | + | require.Error(t, err, "a predecessor that is still running fails the stop") | |||
| 763 | + | assert.Equal(t, "failed", result.Services["web"].Result) | |||
| 764 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "its marker stays for the next attempt") | |||
| 765 | + | ||||
| 766 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 767 | + | _, err = svc.Stop(ctx, "blog", "") | |||
| 768 | + | require.NoError(t, err) | |||
| 769 | + | assert.Empty(t, markerIDs(t, ctx, store)) | |||
| 770 | + | } | |||
| 771 | + | ||||
| 772 | + | // TestRemove_FailsAndKeepsStateWhenAPendingPredecessorCannotBeRetired proves | |||
| 773 | + | // Remove does not end the incarnation (which deletes the markers) while a | |||
| 774 | + | // superseded container is still running. | |||
| 775 | + | func TestRemove_FailsAndKeepsStateWhenAPendingPredecessorCannotBeRetired(t *testing.T) { | |||
| 776 | + | ctx := context.Background() | |||
| 777 | + | store := newTestStore(t) | |||
| 778 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 779 | + | images := outmocks.NewMockImageResolver(t) | |||
| 780 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 781 | + | seedPendingRetirement(t, ctx, store, "c-old") | |||
| 782 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |||
| 783 | + | ||||
| 784 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-new", mock.Anything).Return(nil).Once() | |||
| 785 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", false).Return(nil).Once() | |||
| 786 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 787 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |||
| 788 | + | ||||
| 789 | + | _, err := svc.Remove(ctx, "blog", "") | |||
| 790 | + | require.Error(t, err) | |||
| 791 | + | assert.Equal(t, []string{"c-old"}, markerIDs(t, ctx, store), "the marker survives so the container stays tracked") | |||
| 792 | + | _, ok, loadErr := store.LoadActive(ctx, "blog") | |||
| 793 | + | require.NoError(t, loadErr) | |||
| 794 | + | assert.True(t, ok, "the incarnation is not ended") | |||
| 795 | + | } | |||
M internal/usecase/deployment/retirement.go +19 -2
| @@ -50,6 +50,14 @@ type retireOptions struct { | |||
| 50 | 50 | // published, because recovery must not recreate the replaced writer | |
| 51 | 51 | // from the still-active old revision. | |
| 52 | 52 | ClearInhibition bool | |
| 53 | + | // RetryOnLeak keeps the inhibition when the container is gone but its | |
| 54 | + | // backend claims could not be released, so a durable retirement marker is | |
| 55 | + | // retried (retiring a gone container is idempotent: not-found counts as | |
| 56 | + | // gone) instead of leaking the claims untracked. Callers that clear an | |
| 57 | + | // inhibition for a container that must simply never be revived (stop, | |
| 58 | + | // remove) leave it unset: a lingering marker there would refuse the | |
| 59 | + | // next start. | |
| 60 | + | RetryOnLeak bool | |
| 53 | 61 | } | |
| 54 | 62 | ||
| 55 | 63 | // containerRetirement is the outcome of one exact-container retirement. | |
| @@ -59,6 +67,10 @@ type containerRetirement struct { | |||
| 59 | 67 | // Gone is true only when the runtime confirmed the container no | |
| 60 | 68 | // longer exists. Not-found counts as confirmation. | |
| 61 | 69 | Gone bool | |
| 70 | + | // Settled is true when the container is gone AND its backend claims were | |
| 71 | + | // released. Only a settled retirement may drop a durable marker; a gone | |
| 72 | + | // container whose claims leaked is retried, which is idempotent. | |
| 73 | + | Settled bool | |
| 62 | 74 | // Warnings are bounded, log-free leftovers for operator action. | |
| 63 | 75 | Warnings []CleanupWarning | |
| 64 | 76 | } | |
| @@ -74,6 +86,7 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||
| 74 | 86 | result := containerRetirement{ContainerID: containerID} | |
| 75 | 87 | if containerID == "" { | |
| 76 | 88 | result.Gone = true | |
| 89 | + | result.Settled = true | |
| 77 | 90 | return result | |
| 78 | 91 | } | |
| 79 | 92 | if err := s.stopForRetirement(ctx, opts, containerID); err != nil { | |
| @@ -89,7 +102,9 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||
| 89 | 102 | return result | |
| 90 | 103 | } | |
| 91 | 104 | result.Gone = true | |
| 105 | + | released := true | |
| 92 | 106 | if err := s.deps.State.ReleaseBackendBinds(ctx, app, containerID); err != nil { | |
| 107 | + | released = false | |
| 93 | 108 | result.Warnings = append(result.Warnings, CleanupWarning{ | |
| 94 | 109 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | |
| 95 | 110 | }) | |
| @@ -105,9 +120,11 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||
| 105 | 120 | Service: opts.Service, Leftover: containerID, Detail: "secret files cleanup failed", | |
| 106 | 121 | }) | |
| 107 | 122 | } | |
| 108 | - | if opts.ClearInhibition { | |
| 123 | + | result.Settled = released | |
| 124 | + | if opts.ClearInhibition && (released || !opts.RetryOnLeak) { | |
| 109 | 125 | // Confirmed gone: the marker has nothing left to protect, and a | |
| 110 | - | // stale record would only confuse the next recreation. | |
| 126 | + | // stale record would only confuse the next recreation. With | |
| 127 | + | // RetryOnLeak it also waits for the claims to be released. | |
| 111 | 128 | if err := s.clearRecoveryInhibition(ctx, app, opts.Service, containerID); err != nil { | |
| 112 | 129 | result.Warnings = append(result.Warnings, CleanupWarning{ | |
| 113 | 130 | Service: opts.Service, Leftover: containerID, Detail: "clear recovery inhibition: " + err.Error(), | |
| @@ -50,6 +50,14 @@ type retireOptions struct { | |||||
| 50 | // published, because recovery must not recreate the replaced writer | 50 | // published, because recovery must not recreate the replaced writer | ||
| 51 | // from the still-active old revision. | 51 | // from the still-active old revision. | ||
| 52 | ClearInhibition bool | 52 | ClearInhibition bool | ||
| 53 | + | // RetryOnLeak keeps the inhibition when the container is gone but its | |||
| 54 | + | // backend claims could not be released, so a durable retirement marker is | |||
| 55 | + | // retried (retiring a gone container is idempotent: not-found counts as | |||
| 56 | + | // gone) instead of leaking the claims untracked. Callers that clear an | |||
| 57 | + | // inhibition for a container that must simply never be revived (stop, | |||
| 58 | + | // remove) leave it unset: a lingering marker there would refuse the | |||
| 59 | + | // next start. | |||
| 60 | + | RetryOnLeak bool | |||
| 53 | } | 61 | } | ||
| 54 | 62 | ||||
| 55 | // containerRetirement is the outcome of one exact-container retirement. | 63 | // containerRetirement is the outcome of one exact-container retirement. | ||
| @@ -59,6 +67,10 @@ type containerRetirement struct { | |||||
| 59 | // Gone is true only when the runtime confirmed the container no | 67 | // Gone is true only when the runtime confirmed the container no | ||
| 60 | // longer exists. Not-found counts as confirmation. | 68 | // longer exists. Not-found counts as confirmation. | ||
| 61 | Gone bool | 69 | Gone bool | ||
| 70 | + | // Settled is true when the container is gone AND its backend claims were | |||
| 71 | + | // released. Only a settled retirement may drop a durable marker; a gone | |||
| 72 | + | // container whose claims leaked is retried, which is idempotent. | |||
| 73 | + | Settled bool | |||
| 62 | // Warnings are bounded, log-free leftovers for operator action. | 74 | // Warnings are bounded, log-free leftovers for operator action. | ||
| 63 | Warnings []CleanupWarning | 75 | Warnings []CleanupWarning | ||
| 64 | } | 76 | } | ||
| @@ -74,6 +86,7 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||||
| 74 | result := containerRetirement{ContainerID: containerID} | 86 | result := containerRetirement{ContainerID: containerID} | ||
| 75 | if containerID == "" { | 87 | if containerID == "" { | ||
| 76 | result.Gone = true | 88 | result.Gone = true | ||
| 89 | + | result.Settled = true | |||
| 77 | return result | 90 | return result | ||
| 78 | } | 91 | } | ||
| 79 | if err := s.stopForRetirement(ctx, opts, containerID); err != nil { | 92 | if err := s.stopForRetirement(ctx, opts, containerID); err != nil { | ||
| @@ -89,7 +102,9 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||||
| 89 | return result | 102 | return result | ||
| 90 | } | 103 | } | ||
| 91 | result.Gone = true | 104 | result.Gone = true | ||
| 105 | + | released := true | |||
| 92 | if err := s.deps.State.ReleaseBackendBinds(ctx, app, containerID); err != nil { | 106 | if err := s.deps.State.ReleaseBackendBinds(ctx, app, containerID); err != nil { | ||
| 107 | + | released = false | |||
| 93 | result.Warnings = append(result.Warnings, CleanupWarning{ | 108 | result.Warnings = append(result.Warnings, CleanupWarning{ | ||
| 94 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | 109 | Service: opts.Service, Leftover: containerID, Detail: "release backend claims: " + err.Error(), | ||
| 95 | }) | 110 | }) | ||
| @@ -105,9 +120,11 @@ func (s *Service) retireContainer(ctx context.Context, app string, opts retireOp | |||||
| 105 | Service: opts.Service, Leftover: containerID, Detail: "secret files cleanup failed", | 120 | Service: opts.Service, Leftover: containerID, Detail: "secret files cleanup failed", | ||
| 106 | }) | 121 | }) | ||
| 107 | } | 122 | } | ||
| 108 | - | if opts.ClearInhibition { | 123 | + | result.Settled = released |
| 124 | + | if opts.ClearInhibition && (released || !opts.RetryOnLeak) { | |||
| 109 | // Confirmed gone: the marker has nothing left to protect, and a | 125 | // Confirmed gone: the marker has nothing left to protect, and a | ||
| 110 | - | // stale record would only confuse the next recreation. | 126 | + | // stale record would only confuse the next recreation. With |
| 127 | + | // RetryOnLeak it also waits for the claims to be released. | |||
| 111 | if err := s.clearRecoveryInhibition(ctx, app, opts.Service, containerID); err != nil { | 128 | if err := s.clearRecoveryInhibition(ctx, app, opts.Service, containerID); err != nil { | ||
| 112 | result.Warnings = append(result.Warnings, CleanupWarning{ | 129 | result.Warnings = append(result.Warnings, CleanupWarning{ | ||
| 113 | Service: opts.Service, Leftover: containerID, Detail: "clear recovery inhibition: " + err.Error(), | 130 | Service: opts.Service, Leftover: containerID, Detail: "clear recovery inhibition: " + err.Error(), | ||
M internal/usecase/deployment/retirement_test.go +2 -0
| @@ -39,6 +39,7 @@ func TestLifecycleVerbsPassTheEffectiveStopGrace(t *testing.T) { | |||
| 39 | 39 | state.EXPECT().Recover(mock.Anything).Return(nil) | |
| 40 | 40 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |
| 41 | 41 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | |
| 42 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 42 | 43 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", grace).Return(nil).Once() | |
| 43 | 44 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | |
| 44 | 45 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | |
| @@ -98,6 +99,7 @@ func TestStopWithoutDeclaredGraceUsesTheAppDefault(t *testing.T) { | |||
| 98 | 99 | state.EXPECT().Recover(mock.Anything).Return(nil) | |
| 99 | 100 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |
| 100 | 101 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | |
| 102 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 101 | 103 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", domain.AppDefaultStopGrace).Return(nil).Once() | |
| 102 | 104 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | |
| 103 | 105 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | |
| @@ -39,6 +39,7 @@ func TestLifecycleVerbsPassTheEffectiveStopGrace(t *testing.T) { | |||||
| 39 | state.EXPECT().Recover(mock.Anything).Return(nil) | 39 | state.EXPECT().Recover(mock.Anything).Return(nil) | ||
| 40 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | 40 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | ||
| 41 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | 41 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 42 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 42 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", grace).Return(nil).Once() | 43 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", grace).Return(nil).Once() | ||
| 43 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | 44 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | ||
| 44 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | 45 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | ||
| @@ -98,6 +99,7 @@ func TestStopWithoutDeclaredGraceUsesTheAppDefault(t *testing.T) { | |||||
| 98 | state.EXPECT().Recover(mock.Anything).Return(nil) | 99 | state.EXPECT().Recover(mock.Anything).Return(nil) | ||
| 99 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | 100 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | ||
| 100 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | 101 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 102 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 101 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", domain.AppDefaultStopGrace).Return(nil).Once() | 103 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", domain.AppDefaultStopGrace).Return(nil).Once() | ||
| 102 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | 104 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | ||
| 103 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | 105 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | ||
M internal/usecase/deployment/traffic_failure_test.go +1 -0
| @@ -117,6 +117,7 @@ func TestStop_TrafficFailureRecordsTerminalFailure(t *testing.T) { | |||
| 117 | 117 | state.EXPECT().Recover(mock.Anything).Return(nil) | |
| 118 | 118 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |
| 119 | 119 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | |
| 120 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 120 | 121 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", mock.Anything).Return(nil).Once() | |
| 121 | 122 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | |
| 122 | 123 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | |
| @@ -117,6 +117,7 @@ func TestStop_TrafficFailureRecordsTerminalFailure(t *testing.T) { | |||||
| 117 | state.EXPECT().Recover(mock.Anything).Return(nil) | 117 | state.EXPECT().Recover(mock.Anything).Return(nil) | ||
| 118 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | 118 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | ||
| 119 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | 119 | state.EXPECT().SaveIntent(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 120 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 120 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", mock.Anything).Return(nil).Once() | 121 | runtime.EXPECT().StopContainer(mock.Anything, "c-1", mock.Anything).Return(nil).Once() | ||
| 121 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | 122 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | ||
| 122 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | 123 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | ||