fix(deployment): own overlap retirement with a durable marker
A superseded container left by an overlap replacement is now marked retirement-pending, with the predecessor's stop grace, before the switch. The periodic recovery pass retires it once its successor is verified and routed, so a newer operation cannot lose the cleanup. Pending publication withdrawals are resolved before the switch so they cannot unroute the new generation. Interrupted-operation reconciliation no longer rebuilds traffic from unverified persisted binds. Services with a UDP interface keep the sequential replacement. Remove retires pending superseded containers before ending the incarnation.
- commit
- 733740bd0bc2776a9c3b5073f816b11b0d61e679
- author
- brice · · 2026-10-16 19:47 UTC
- parent
- 7ce0d9c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +668 -330
M internal/domain/app_state.go +9 -10
| @@ -192,14 +192,6 @@ type AppOperationStep struct { | |||
| 192 | 192 | Image string `json:"image,omitempty"` | |
| 193 | 193 | // Service names the app service this step applies to. | |
| 194 | 194 | Service string `json:"service,omitempty"` | |
| 195 | - | // Overlap marks a replacement that started the new generation while the | |
| 196 | - | // superseded container (Before) kept serving. It is journaled before any | |
| 197 | - | // effect, so reconciliation knows Before may still run beside After. | |
| 198 | - | Overlap bool `json:"overlap,omitempty"` | |
| 199 | - | // BeforeRetired is set once the superseded container of an Overlap step | |
| 200 | - | // was confirmed gone (or no longer needs retiring). While it is unset, | |
| 201 | - | // reconciliation retires Before once After is the published container. | |
| 202 | - | BeforeRetired bool `json:"before_retired,omitempty"` | |
| 203 | 195 | // Diagnostics is bounded, redacted failure output kept separate from | |
| 204 | 196 | // the stable Error. Retrieval requires the logs read scope. | |
| 205 | 197 | Diagnostics []string `json:"diagnostics,omitempty"` | |
| @@ -313,6 +305,10 @@ type AppRecoveryInhibition struct { | |||
| 313 | 305 | Reason string `json:"reason"` | |
| 314 | 306 | Operation string `json:"operation,omitempty"` | |
| 315 | 307 | CreatedAt time.Time `json:"created_at"` | |
| 308 | + | // StopGrace is the effective stop grace of the generation to retire | |
| 309 | + | // (retirement-pending only). Zero, as in records written before it | |
| 310 | + | // existed, means the app default. | |
| 311 | + | StopGrace time.Duration `json:"stop_grace,omitempty"` | |
| 316 | 312 | } | |
| 317 | 313 | ||
| 318 | 314 | // AppRecoveryInhibition reasons. | |
| @@ -320,8 +316,11 @@ const ( | |||
| 320 | 316 | // AppInhibitReplacementPending marks the generation being replaced | |
| 321 | 317 | // while a volume-owning candidate may already be writing. | |
| 322 | 318 | AppInhibitReplacementPending = "replacement-pending" | |
| 323 | - | // AppInhibitRetirementPending marks a published predecessor that must | |
| 324 | - | // remain stopped and be retired by a later reconciliation pass. | |
| 319 | + | // AppInhibitRetirementPending marks a superseded generation that an | |
| 320 | + | // overlap replacement left running beside its published successor. The | |
| 321 | + | // periodic reconciliation retires it, with StopGrace, once the | |
| 322 | + | // successor is verified and routed; it is never retired, and the | |
| 323 | + | // marker is dropped, while ACTIVE names it. | |
| 325 | 324 | AppInhibitRetirementPending = "retirement-pending" | |
| 326 | 325 | ) | |
| 327 | 326 | ||
| @@ -192,14 +192,6 @@ type AppOperationStep struct { | |||||
| 192 | Image string `json:"image,omitempty"` | 192 | Image string `json:"image,omitempty"` | ||
| 193 | // Service names the app service this step applies to. | 193 | // Service names the app service this step applies to. | ||
| 194 | Service string `json:"service,omitempty"` | 194 | Service string `json:"service,omitempty"` | ||
| 195 | - | // Overlap marks a replacement that started the new generation while the | |||
| 196 | - | // superseded container (Before) kept serving. It is journaled before any | |||
| 197 | - | // effect, so reconciliation knows Before may still run beside After. | |||
| 198 | - | Overlap bool `json:"overlap,omitempty"` | |||
| 199 | - | // BeforeRetired is set once the superseded container of an Overlap step | |||
| 200 | - | // was confirmed gone (or no longer needs retiring). While it is unset, | |||
| 201 | - | // reconciliation retires Before once After is the published container. | |||
| 202 | - | BeforeRetired bool `json:"before_retired,omitempty"` | |||
| 203 | // Diagnostics is bounded, redacted failure output kept separate from | 195 | // Diagnostics is bounded, redacted failure output kept separate from | ||
| 204 | // the stable Error. Retrieval requires the logs read scope. | 196 | // the stable Error. Retrieval requires the logs read scope. | ||
| 205 | Diagnostics []string `json:"diagnostics,omitempty"` | 197 | Diagnostics []string `json:"diagnostics,omitempty"` | ||
| @@ -313,6 +305,10 @@ type AppRecoveryInhibition struct { | |||||
| 313 | Reason string `json:"reason"` | 305 | Reason string `json:"reason"` | ||
| 314 | Operation string `json:"operation,omitempty"` | 306 | Operation string `json:"operation,omitempty"` | ||
| 315 | CreatedAt time.Time `json:"created_at"` | 307 | CreatedAt time.Time `json:"created_at"` | ||
| 308 | + | // StopGrace is the effective stop grace of the generation to retire | |||
| 309 | + | // (retirement-pending only). Zero, as in records written before it | |||
| 310 | + | // existed, means the app default. | |||
| 311 | + | StopGrace time.Duration `json:"stop_grace,omitempty"` | |||
| 316 | } | 312 | } | ||
| 317 | 313 | ||||
| 318 | // AppRecoveryInhibition reasons. | 314 | // AppRecoveryInhibition reasons. | ||
| @@ -320,8 +316,11 @@ const ( | |||||
| 320 | // AppInhibitReplacementPending marks the generation being replaced | 316 | // AppInhibitReplacementPending marks the generation being replaced | ||
| 321 | // while a volume-owning candidate may already be writing. | 317 | // while a volume-owning candidate may already be writing. | ||
| 322 | AppInhibitReplacementPending = "replacement-pending" | 318 | AppInhibitReplacementPending = "replacement-pending" | ||
| 323 | - | // AppInhibitRetirementPending marks a published predecessor that must | 319 | + | // AppInhibitRetirementPending marks a superseded generation that an |
| 324 | - | // remain stopped and be retired by a later reconciliation pass. | 320 | + | // overlap replacement left running beside its published successor. The |
| 321 | + | // periodic reconciliation retires it, with StopGrace, once the | |||
| 322 | + | // successor is verified and routed; it is never retired, and the | |||
| 323 | + | // marker is dropped, while ACTIVE names it. | |||
| 325 | AppInhibitRetirementPending = "retirement-pending" | 324 | AppInhibitRetirementPending = "retirement-pending" | ||
| 326 | ) | 325 | ) | ||
| 327 | 326 | ||||
M internal/usecase/deployment/deploy_mock_test.go +1 -0
| @@ -287,6 +287,7 @@ func TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration(t *testi | |||
| 287 | 287 | return nil | |
| 288 | 288 | }).Once() | |
| 289 | 289 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 290 | + | expectRetirementMarker(state, "c-old") | |
| 290 | 291 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 291 | 292 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).RunAndReturn(func(context.Context, string, time.Duration) error { | |
| 292 | 293 | step++ | |
| @@ -287,6 +287,7 @@ func TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration(t *testi | |||||
| 287 | return nil | 287 | return nil | ||
| 288 | }).Once() | 288 | }).Once() | ||
| 289 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | 289 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | ||
| 290 | + | expectRetirementMarker(state, "c-old") | |||
| 290 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 291 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | ||
| 291 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).RunAndReturn(func(context.Context, string, time.Duration) error { | 292 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).RunAndReturn(func(context.Context, string, time.Duration) error { | ||
| 292 | step++ | 293 | step++ | ||
M internal/usecase/deployment/execute.go +83 -46
| @@ -20,7 +20,7 @@ const failLogTailLines = 50 | |||
| 20 | 20 | const failLogTailTimeout = 10 * time.Second | |
| 21 | 21 | ||
| 22 | 22 | // Deploy executes a preflighted revision: fail-fast across services in | |
| 23 | - | // sorted name order. A service without volumes, binds, or devices is | |
| 23 | + | // sorted name order. A service without volumes, binds, devices, or UDP interfaces is | |
| 24 | 24 | // replaced with a short overlap: its replacement starts and becomes ready | |
| 25 | 25 | // while the superseded container keeps serving, traffic switches to it, and | |
| 26 | 26 | // only then is the superseded container stopped, so the service is not | |
| @@ -311,21 +311,22 @@ func (s *Service) claimRevision(ctx context.Context, claim DeployClaim, op domai | |||
| 311 | 311 | // is never ahead of what is routed. | |
| 312 | 312 | // | |
| 313 | 313 | // An overlap-eligible service (see overlapEligible) is replaced without | |
| 314 | - | // withdrawing the superseded container: it keeps serving, and is retired | |
| 315 | - | // only after the new generation is published and routed. The step records | |
| 316 | - | // Overlap before any effect, so an interrupted deploy leaves enough trace | |
| 317 | - | // for reconciliation to retire the superseded container. | |
| 314 | + | // withdrawing the superseded container: it keeps serving and is retired only | |
| 315 | + | // after the new generation is published and routed. Before the switch the | |
| 316 | + | // superseded container gets a durable retirement-pending marker carrying its | |
| 317 | + | // stop grace. That marker, not the operation journal, owns the cleanup: it | |
| 318 | + | // survives newer operations, and the periodic reconciliation retires the | |
| 319 | + | // container once its successor is verified and routed. | |
| 318 | 320 | // | |
| 319 | 321 | // If the publication or the traffic apply of an overlap step fails, the | |
| 320 | 322 | // superseded container is deliberately left running: the proxy may still | |
| 321 | - | // route to it, and ACTIVE may already name the new container, so stopping | |
| 322 | - | // it could create an outage the failed deploy cannot repair. The step is | |
| 323 | - | // journaled as failed with the new container as After, the superseded one as | |
| 324 | - | // Before and BeforeRetired unset, and a cleanup warning names it. The next | |
| 325 | - | // reconciliation of the app finds the published new container, re-applies | |
| 326 | - | // traffic and only then retires the superseded one; if ACTIVE was never | |
| 327 | - | // updated the unpublished candidate is removed instead and the superseded | |
| 328 | - | // container simply keeps serving. | |
| 323 | + | // route to it, and ACTIVE may already name the new container, so stopping it | |
| 324 | + | // could create an outage the failed deploy cannot repair. The step is | |
| 325 | + | // journaled as failed with the new container as After and a cleanup warning | |
| 326 | + | // names the superseded one. If ACTIVE names the new container, the periodic | |
| 327 | + | // reconciliation verifies and republishes it and then retires the superseded | |
| 328 | + | // one through its marker; if ACTIVE never changed, it still names the | |
| 329 | + | // superseded container, which keeps serving, and the marker is dropped. | |
| 329 | 330 | func (s *Service) runServiceStep( | |
| 330 | 331 | ctx context.Context, | |
| 331 | 332 | app, revision string, | |
| @@ -368,8 +369,7 @@ func (s *Service) runServiceStep( | |||
| 368 | 369 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | |
| 369 | 370 | } else if before != "" && overlapEligible(p.spec) { | |
| 370 | 371 | overlap = true | |
| 371 | - | step.Overlap = true | |
| 372 | - | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, s.journalCandidate(op, index+1)) | |
| 372 | + | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | |
| 373 | 373 | } else { | |
| 374 | 374 | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | |
| 375 | 375 | } | |
| @@ -408,7 +408,7 @@ func (s *Service) runServiceStep( | |||
| 408 | 408 | return fail(err) | |
| 409 | 409 | } | |
| 410 | 410 | if overlap { | |
| 411 | - | svcResult, step.BeforeRetired = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) | |
| 411 | + | svcResult = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) | |
| 412 | 412 | } | |
| 413 | 413 | result.Services[p.name] = svcResult | |
| 414 | 414 | step.State = domain.AppStepSucceeded | |
| @@ -442,19 +442,20 @@ func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p p | |||
| 442 | 442 | // retireSuperseded stops the container an overlap replacement superseded, | |
| 443 | 443 | // once the new generation is published and routed. It uses the stop grace of | |
| 444 | 444 | // the generation being stopped: the runtime sends SIGTERM, then kills after | |
| 445 | - | // the grace. Backend claims are released by retireContainer. A retirement | |
| 446 | - | // that fails is not a deploy failure — the new generation already serves — | |
| 447 | - | // but a cleanup warning on the service, and the step stays unretired so | |
| 448 | - | // reconciliation retries it. It reports whether the container is gone. | |
| 449 | - | func (s *Service) retireSuperseded(ctx context.Context, app, service, before string, grace time.Duration, result ServiceResult) (ServiceResult, bool) { | |
| 450 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace}, before) | |
| 445 | + | // the grace. Backend claims are released and the retirement-pending marker is | |
| 446 | + | // cleared by retireContainer once the container is confirmed gone. A | |
| 447 | + | // retirement that fails is not a deploy failure — the new generation already | |
| 448 | + | // serves — but a cleanup warning on the service; the marker stays, so the | |
| 449 | + | // periodic reconciliation retries it. | |
| 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 | 452 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | |
| 452 | 453 | if !retired.Gone && len(retired.Warnings) == 0 { | |
| 453 | 454 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | |
| 454 | 455 | Service: service, Leftover: before, Detail: "superseded container not confirmed gone", | |
| 455 | 456 | }) | |
| 456 | 457 | } | |
| 457 | - | return result, retired.Gone | |
| 458 | + | return result | |
| 458 | 459 | } | |
| 459 | 460 | ||
| 460 | 461 | // unchangedService reports whether the service already runs exactly what p | |
| @@ -697,18 +698,46 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 697 | 698 | } | |
| 698 | 699 | ||
| 699 | 700 | // deployOverlap creates the replacement of an overlap-eligible service while | |
| 700 | - | // the superseded container keeps running and stays routed: no withdrawal, no | |
| 701 | - | // inhibition, no retirement. Container names carry the operation, so the two | |
| 702 | - | // generations never collide, and their loopback binds are ephemeral. It | |
| 703 | - | // returns the ready candidate with its binds; on any failure the candidate is | |
| 704 | - | // removed and the superseded container is left exactly as it was. Retiring | |
| 705 | - | // the superseded container is the caller's job, once the candidate is | |
| 701 | + | // the superseded container keeps running and stays routed: no inhibition and | |
| 702 | + | // no retirement. Container names carry the operation, so the two generations | |
| 703 | + | // never collide, and their loopback binds are ephemeral. | |
| 704 | + | // | |
| 705 | + | // The only withdrawal it can perform is the retry of one already pending for | |
| 706 | + | // this service: an earlier withdrawal that could not be proven leaves the | |
| 707 | + | // service marked, and a marked service would be withdrawn again by the | |
| 708 | + | // traffic refresh that follows the switch, erasing the binds the switch just | |
| 709 | + | // published. The retry runs before the candidate exists, so a failure aborts | |
| 710 | + | // the deploy untouched. | |
| 711 | + | // | |
| 712 | + | // It returns the ready candidate with its binds, after durably recording the | |
| 713 | + | // retirement-pending marker of the superseded container; on any failure the | |
| 714 | + | // candidate is removed and the superseded container is left exactly as it was. | |
| 715 | + | // Retiring the superseded container is the caller's job, once the candidate is | |
| 706 | 716 | // published and routed (retireSuperseded). | |
| 707 | - | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | |
| 717 | + | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, beforeGrace time.Duration, journal candidateJournal) ServiceResult { | |
| 708 | 718 | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | |
| 709 | 719 | return failed | |
| 710 | 720 | } | |
| 711 | - | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | |
| 721 | + | if s.publication.inhibited(app, p.name) { | |
| 722 | + | if err := s.withdrawForRecovery(ctx, app, p.name); err != nil { | |
| 723 | + | return s.failResult(revision, before, "", err) | |
| 724 | + | } | |
| 725 | + | } | |
| 726 | + | result := s.startCandidate(ctx, app, revision, p, opID, before, journal) | |
| 727 | + | if result.Result != "deployed" { | |
| 728 | + | return result | |
| 729 | + | } | |
| 730 | + | // The marker is durable before anything is published: whatever happens | |
| 731 | + | // next, the superseded container is retired by the periodic | |
| 732 | + | // reconciliation instead of being forgotten, and recovery never revives it. | |
| 733 | + | if err := s.inhibitRetirement(ctx, app, p.name, before, opID, beforeGrace); err != nil { | |
| 734 | + | result.Result = "failed" | |
| 735 | + | result.Error = err.Error() | |
| 736 | + | result.CleanupWarnings = append(result.CleanupWarnings, s.retireCandidate(ctx, app, p.name, result.After)...) | |
| 737 | + | return result | |
| 738 | + | } | |
| 739 | + | result.SupersededRunning = true | |
| 740 | + | return result | |
| 712 | 741 | } | |
| 713 | 742 | ||
| 714 | 743 | // revalidateService re-resolves the binds and devices of a service against | |
| @@ -773,19 +802,23 @@ func (s *Service) startCandidate(ctx context.Context, app, revision string, p pi | |||
| 773 | 802 | // singleWriterRequired reports services that must never have two generations | |
| 774 | 803 | // running concurrently. Persistent volumes and any bind (especially a | |
| 775 | 804 | // writable one) may be written by both, so they are treated identically. | |
| 776 | - | // Device grants are exclusive host resources and are treated the same way by | |
| 777 | - | // overlapEligible. | |
| 805 | + | // Devices and UDP interfaces are also excluded from the overlap (see | |
| 806 | + | // overlapEligible). | |
| 778 | 807 | func singleWriterRequired(spec domain.AppService) bool { | |
| 779 | 808 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | |
| 780 | 809 | } | |
| 781 | 810 | ||
| 782 | 811 | // overlapEligible reports whether a service may be replaced with a short | |
| 783 | - | // overlap of the old and the new generation: it has no volume, bind, or | |
| 784 | - | // device, so nothing stateful or exclusive is shared between the two. Every | |
| 785 | - | // other service keeps the sequential replacement, so two generations of a | |
| 786 | - | // single-writer service never run together. | |
| 812 | + | // 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 | |
| 814 | + | // resources two generations must not share. UDP sessions are pinned by the | |
| 815 | + | // relay to the backend they started on and outlive the switch for the | |
| 816 | + | // configured drain timeout, so they would keep reaching a backend that is | |
| 817 | + | // about to be stopped and whose loopback port may be recycled. TCP is fine: | |
| 818 | + | // established relays do not redial. Every other service keeps the sequential | |
| 819 | + | // replacement, so two generations of such a service never run together. | |
| 787 | 820 | func overlapEligible(spec domain.AppService) bool { | |
| 788 | - | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 | |
| 821 | + | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 && len(spec.UDP) == 0 | |
| 789 | 822 | } | |
| 790 | 823 | ||
| 791 | 824 | // refreshTraffic rebuilds the proxy host index and applies the full | |
| @@ -859,15 +892,19 @@ func (s *Service) publishService(ctx context.Context, app, revision string, p pi | |||
| 859 | 892 | if err := s.recordOwnership(ctx, app, p); err != nil { | |
| 860 | 893 | return err | |
| 861 | 894 | } | |
| 862 | - | // The superseded generation is now safely replaced: clearing its | |
| 863 | - | // inhibition is what makes this generation authoritative. A failed | |
| 864 | - | // publication leaves the marker in place. | |
| 865 | - | if result.Before != "" && result.Before != result.After { | |
| 866 | - | if err := s.clearRecoveryInhibition(ctx, app, p.name, result.Before); err != nil { | |
| 867 | - | return err | |
| 868 | - | } | |
| 895 | + | return s.clearSupersededMarker(ctx, app, p.name, result) | |
| 896 | + | } | |
| 897 | + | ||
| 898 | + | // clearSupersededMarker clears the recovery inhibition of the generation a | |
| 899 | + | // sequential replacement superseded: that clearing is what makes the new | |
| 900 | + | // generation authoritative, and a failed publication leaves the marker in | |
| 901 | + | // place. An overlap replacement is the exception: its superseded container | |
| 902 | + | // still runs, and its retirement-pending marker is what gets it retired. | |
| 903 | + | func (s *Service) clearSupersededMarker(ctx context.Context, app, service string, result ServiceResult) error { | |
| 904 | + | if result.Before == "" || result.Before == result.After || result.SupersededRunning { | |
| 905 | + | return nil | |
| 869 | 906 | } | |
| 870 | - | return nil | |
| 907 | + | return s.clearRecoveryInhibition(ctx, app, service, result.Before) | |
| 871 | 908 | } | |
| 872 | 909 | ||
| 873 | 910 | // recordOwnership stamps volume/secret/network ownership after publication. | |
| @@ -20,7 +20,7 @@ const failLogTailLines = 50 | |||||
| 20 | const failLogTailTimeout = 10 * time.Second | 20 | const failLogTailTimeout = 10 * time.Second | ||
| 21 | 21 | ||||
| 22 | // Deploy executes a preflighted revision: fail-fast across services in | 22 | // Deploy executes a preflighted revision: fail-fast across services in | ||
| 23 | - | // sorted name order. A service without volumes, binds, or devices is | 23 | + | // sorted name order. A service without volumes, binds, devices, or UDP interfaces is |
| 24 | // replaced with a short overlap: its replacement starts and becomes ready | 24 | // replaced with a short overlap: its replacement starts and becomes ready | ||
| 25 | // while the superseded container keeps serving, traffic switches to it, and | 25 | // while the superseded container keeps serving, traffic switches to it, and | ||
| 26 | // only then is the superseded container stopped, so the service is not | 26 | // only then is the superseded container stopped, so the service is not | ||
| @@ -311,21 +311,22 @@ func (s *Service) claimRevision(ctx context.Context, claim DeployClaim, op domai | |||||
| 311 | // is never ahead of what is routed. | 311 | // is never ahead of what is routed. | ||
| 312 | // | 312 | // | ||
| 313 | // An overlap-eligible service (see overlapEligible) is replaced without | 313 | // An overlap-eligible service (see overlapEligible) is replaced without | ||
| 314 | - | // withdrawing the superseded container: it keeps serving, and is retired | 314 | + | // withdrawing the superseded container: it keeps serving and is retired only |
| 315 | - | // only after the new generation is published and routed. The step records | 315 | + | // after the new generation is published and routed. Before the switch the |
| 316 | - | // Overlap before any effect, so an interrupted deploy leaves enough trace | 316 | + | // superseded container gets a durable retirement-pending marker carrying its |
| 317 | - | // for reconciliation to retire the superseded container. | 317 | + | // stop grace. That marker, not the operation journal, owns the cleanup: it |
| 318 | + | // survives newer operations, and the periodic reconciliation retires the | |||
| 319 | + | // container once its successor is verified and routed. | |||
| 318 | // | 320 | // | ||
| 319 | // If the publication or the traffic apply of an overlap step fails, the | 321 | // If the publication or the traffic apply of an overlap step fails, the | ||
| 320 | // superseded container is deliberately left running: the proxy may still | 322 | // superseded container is deliberately left running: the proxy may still | ||
| 321 | - | // route to it, and ACTIVE may already name the new container, so stopping | 323 | + | // route to it, and ACTIVE may already name the new container, so stopping it |
| 322 | - | // it could create an outage the failed deploy cannot repair. The step is | 324 | + | // could create an outage the failed deploy cannot repair. The step is |
| 323 | - | // journaled as failed with the new container as After, the superseded one as | 325 | + | // journaled as failed with the new container as After and a cleanup warning |
| 324 | - | // Before and BeforeRetired unset, and a cleanup warning names it. The next | 326 | + | // names the superseded one. If ACTIVE names the new container, the periodic |
| 325 | - | // reconciliation of the app finds the published new container, re-applies | 327 | + | // reconciliation verifies and republishes it and then retires the superseded |
| 326 | - | // traffic and only then retires the superseded one; if ACTIVE was never | 328 | + | // one through its marker; if ACTIVE never changed, it still names the |
| 327 | - | // updated the unpublished candidate is removed instead and the superseded | 329 | + | // superseded container, which keeps serving, and the marker is dropped. |
| 328 | - | // container simply keeps serving. | |||
| 329 | func (s *Service) runServiceStep( | 330 | func (s *Service) runServiceStep( | ||
| 330 | ctx context.Context, | 331 | ctx context.Context, | ||
| 331 | app, revision string, | 332 | app, revision string, | ||
| @@ -368,8 +369,7 @@ func (s *Service) runServiceStep( | |||||
| 368 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | 369 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | ||
| 369 | } else if before != "" && overlapEligible(p.spec) { | 370 | } else if before != "" && overlapEligible(p.spec) { | ||
| 370 | overlap = true | 371 | overlap = true | ||
| 371 | - | step.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, s.journalCandidate(op, index+1)) | |||
| 373 | } else { | 373 | } else { | ||
| 374 | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | 374 | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | ||
| 375 | } | 375 | } | ||
| @@ -408,7 +408,7 @@ func (s *Service) runServiceStep( | |||||
| 408 | return fail(err) | 408 | return fail(err) | ||
| 409 | } | 409 | } | ||
| 410 | if overlap { | 410 | if overlap { | ||
| 411 | - | svcResult, step.BeforeRetired = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) | 411 | + | svcResult = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) |
| 412 | } | 412 | } | ||
| 413 | result.Services[p.name] = svcResult | 413 | result.Services[p.name] = svcResult | ||
| 414 | step.State = domain.AppStepSucceeded | 414 | step.State = domain.AppStepSucceeded | ||
| @@ -442,19 +442,20 @@ func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p p | |||||
| 442 | // retireSuperseded stops the container an overlap replacement superseded, | 442 | // retireSuperseded stops the container an overlap replacement superseded, | ||
| 443 | // once the new generation is published and routed. It uses the stop grace of | 443 | // once the new generation is published and routed. It uses the stop grace of | ||
| 444 | // the generation being stopped: the runtime sends SIGTERM, then kills after | 444 | // the generation being stopped: the runtime sends SIGTERM, then kills after | ||
| 445 | - | // the grace. Backend claims are released by retireContainer. A retirement | 445 | + | // the grace. Backend claims are released and the retirement-pending marker is |
| 446 | - | // that fails is not a deploy failure — the new generation already serves — | 446 | + | // cleared by retireContainer once the container is confirmed gone. A |
| 447 | - | // but a cleanup warning on the service, and the step stays unretired so | 447 | + | // retirement that fails is not a deploy failure — the new generation already |
| 448 | - | // reconciliation retries it. It reports whether the container is gone. | 448 | + | // serves — but a cleanup warning on the service; the marker stays, so the |
| 449 | - | func (s *Service) retireSuperseded(ctx context.Context, app, service, before string, grace time.Duration, result ServiceResult) (ServiceResult, bool) { | 449 | + | // periodic reconciliation retries it. |
| 450 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: service, Grace: grace}, before) | 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 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | 452 | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | ||
| 452 | if !retired.Gone && len(retired.Warnings) == 0 { | 453 | if !retired.Gone && len(retired.Warnings) == 0 { | ||
| 453 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | 454 | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | ||
| 454 | Service: service, Leftover: before, Detail: "superseded container not confirmed gone", | 455 | Service: service, Leftover: before, Detail: "superseded container not confirmed gone", | ||
| 455 | }) | 456 | }) | ||
| 456 | } | 457 | } | ||
| 457 | - | return result, retired.Gone | 458 | + | return result |
| 458 | } | 459 | } | ||
| 459 | 460 | ||||
| 460 | // unchangedService reports whether the service already runs exactly what p | 461 | // unchangedService reports whether the service already runs exactly what p | ||
| @@ -697,18 +698,46 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 697 | } | 698 | } | ||
| 698 | 699 | ||||
| 699 | // deployOverlap creates the replacement of an overlap-eligible service while | 700 | // deployOverlap creates the replacement of an overlap-eligible service while | ||
| 700 | - | // the superseded container keeps running and stays routed: no withdrawal, no | 701 | + | // the superseded container keeps running and stays routed: no inhibition and |
| 701 | - | // inhibition, no retirement. Container names carry the operation, so the two | 702 | + | // no retirement. Container names carry the operation, so the two generations |
| 702 | - | // generations never collide, and their loopback binds are ephemeral. It | 703 | + | // never collide, and their loopback binds are ephemeral. |
| 703 | - | // returns the ready candidate with its binds; on any failure the candidate is | 704 | + | // |
| 704 | - | // removed and the superseded container is left exactly as it was. Retiring | 705 | + | // The only withdrawal it can perform is the retry of one already pending for |
| 705 | - | // the superseded container is the caller's job, once the candidate is | 706 | + | // this service: an earlier withdrawal that could not be proven leaves the |
| 707 | + | // service marked, and a marked service would be withdrawn again by the | |||
| 708 | + | // traffic refresh that follows the switch, erasing the binds the switch just | |||
| 709 | + | // published. The retry runs before the candidate exists, so a failure aborts | |||
| 710 | + | // the deploy untouched. | |||
| 711 | + | // | |||
| 712 | + | // It returns the ready candidate with its binds, after durably recording the | |||
| 713 | + | // retirement-pending marker of the superseded container; on any failure the | |||
| 714 | + | // candidate is removed and the superseded container is left exactly as it was. | |||
| 715 | + | // Retiring the superseded container is the caller's job, once the candidate is | |||
| 706 | // published and routed (retireSuperseded). | 716 | // published and routed (retireSuperseded). | ||
| 707 | - | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | 717 | + | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, beforeGrace time.Duration, journal candidateJournal) ServiceResult { |
| 708 | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | 718 | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | ||
| 709 | return failed | 719 | return failed | ||
| 710 | } | 720 | } | ||
| 711 | - | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | 721 | + | if s.publication.inhibited(app, p.name) { |
| 722 | + | if err := s.withdrawForRecovery(ctx, app, p.name); err != nil { | |||
| 723 | + | return s.failResult(revision, before, "", err) | |||
| 724 | + | } | |||
| 725 | + | } | |||
| 726 | + | result := s.startCandidate(ctx, app, revision, p, opID, before, journal) | |||
| 727 | + | if result.Result != "deployed" { | |||
| 728 | + | return result | |||
| 729 | + | } | |||
| 730 | + | // The marker is durable before anything is published: whatever happens | |||
| 731 | + | // next, the superseded container is retired by the periodic | |||
| 732 | + | // reconciliation instead of being forgotten, and recovery never revives it. | |||
| 733 | + | if err := s.inhibitRetirement(ctx, app, p.name, before, opID, beforeGrace); err != nil { | |||
| 734 | + | result.Result = "failed" | |||
| 735 | + | result.Error = err.Error() | |||
| 736 | + | result.CleanupWarnings = append(result.CleanupWarnings, s.retireCandidate(ctx, app, p.name, result.After)...) | |||
| 737 | + | return result | |||
| 738 | + | } | |||
| 739 | + | result.SupersededRunning = true | |||
| 740 | + | return result | |||
| 712 | } | 741 | } | ||
| 713 | 742 | ||||
| 714 | // revalidateService re-resolves the binds and devices of a service against | 743 | // revalidateService re-resolves the binds and devices of a service against | ||
| @@ -773,19 +802,23 @@ func (s *Service) startCandidate(ctx context.Context, app, revision string, p pi | |||||
| 773 | // singleWriterRequired reports services that must never have two generations | 802 | // singleWriterRequired reports services that must never have two generations | ||
| 774 | // running concurrently. Persistent volumes and any bind (especially a | 803 | // running concurrently. Persistent volumes and any bind (especially a | ||
| 775 | // writable one) may be written by both, so they are treated identically. | 804 | // writable one) may be written by both, so they are treated identically. | ||
| 776 | - | // Device grants are exclusive host resources and are treated the same way by | 805 | + | // Devices and UDP interfaces are also excluded from the overlap (see |
| 777 | - | // overlapEligible. | 806 | + | // overlapEligible). |
| 778 | func singleWriterRequired(spec domain.AppService) bool { | 807 | func singleWriterRequired(spec domain.AppService) bool { | ||
| 779 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | 808 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | ||
| 780 | } | 809 | } | ||
| 781 | 810 | ||||
| 782 | // overlapEligible reports whether a service may be replaced with a short | 811 | // overlapEligible reports whether a service may be replaced with a short | ||
| 783 | - | // overlap of the old and the new generation: it has no volume, bind, or | 812 | + | // overlap of the old and the new generation: it has no volume, bind, device, |
| 784 | - | // device, so nothing stateful or exclusive is shared between the two. Every | 813 | + | // or UDP interface. Volumes, binds, and devices are stateful or exclusive |
| 785 | - | // other service keeps the sequential replacement, so two generations of a | 814 | + | // resources two generations must not share. UDP sessions are pinned by the |
| 786 | - | // single-writer service never run together. | 815 | + | // relay to the backend they started on and outlive the switch for the |
| 816 | + | // configured drain timeout, so they would keep reaching a backend that is | |||
| 817 | + | // about to be stopped and whose loopback port may be recycled. TCP is fine: | |||
| 818 | + | // established relays do not redial. Every other service keeps the sequential | |||
| 819 | + | // replacement, so two generations of such a service never run together. | |||
| 787 | func overlapEligible(spec domain.AppService) bool { | 820 | func overlapEligible(spec domain.AppService) bool { | ||
| 788 | - | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 | 821 | + | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 && len(spec.UDP) == 0 |
| 789 | } | 822 | } | ||
| 790 | 823 | ||||
| 791 | // refreshTraffic rebuilds the proxy host index and applies the full | 824 | // refreshTraffic rebuilds the proxy host index and applies the full | ||
| @@ -859,15 +892,19 @@ func (s *Service) publishService(ctx context.Context, app, revision string, p pi | |||||
| 859 | if err := s.recordOwnership(ctx, app, p); err != nil { | 892 | if err := s.recordOwnership(ctx, app, p); err != nil { | ||
| 860 | return err | 893 | return err | ||
| 861 | } | 894 | } | ||
| 862 | - | // The superseded generation is now safely replaced: clearing its | 895 | + | return s.clearSupersededMarker(ctx, app, p.name, result) |
| 863 | - | // inhibition is what makes this generation authoritative. A failed | 896 | + | } |
| 864 | - | // publication leaves the marker in place. | 897 | + | |
| 865 | - | if result.Before != "" && result.Before != result.After { | 898 | + | // clearSupersededMarker clears the recovery inhibition of the generation a |
| 866 | - | if err := s.clearRecoveryInhibition(ctx, app, p.name, result.Before); err != nil { | 899 | + | // sequential replacement superseded: that clearing is what makes the new |
| 867 | - | return err | 900 | + | // generation authoritative, and a failed publication leaves the marker in |
| 868 | - | } | 901 | + | // place. An overlap replacement is the exception: its superseded container |
| 902 | + | // still runs, and its retirement-pending marker is what gets it retired. | |||
| 903 | + | func (s *Service) clearSupersededMarker(ctx context.Context, app, service string, result ServiceResult) error { | |||
| 904 | + | if result.Before == "" || result.Before == result.After || result.SupersededRunning { | |||
| 905 | + | return nil | |||
| 869 | } | 906 | } | ||
| 870 | - | return nil | 907 | + | return s.clearRecoveryInhibition(ctx, app, service, result.Before) |
| 871 | } | 908 | } | ||
| 872 | 909 | ||||
| 873 | // recordOwnership stamps volume/secret/network ownership after publication. | 910 | // recordOwnership stamps volume/secret/network ownership after publication. | ||
M internal/usecase/deployment/interrupted.go +67 -153
| @@ -3,8 +3,6 @@ package deployment | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | - | "reflect" | |
| 7 | - | "slices" | |
| 8 | 6 | ||
| 9 | 7 | "github.com/bnema/zerowrap" | |
| 10 | 8 | ||
| @@ -90,27 +88,18 @@ func (s *Service) reconcileInterruptedDeploy(ctx context.Context, app string) er | |||
| 90 | 88 | ||
| 91 | 89 | // reconcileNeedsContainer reports whether one step still needs container | |
| 92 | 90 | // work: a step that recorded a candidate. A terminal operation only retries | |
| 93 | - | // the step that recorded a failed candidate, or an overlap step whose | |
| 94 | - | // superseded container was not confirmed retired; a pending step exists only | |
| 95 | - | // in an interrupted operation. | |
| 91 | + | // the step that recorded a failed candidate; a pending step exists only in an | |
| 92 | + | // interrupted operation. | |
| 96 | 93 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | |
| 97 | 94 | if step.After == "" { | |
| 98 | 95 | return false | |
| 99 | 96 | } | |
| 100 | 97 | if terminal { | |
| 101 | - | return step.State == domain.AppStepFailed || supersededPending(step) | |
| 98 | + | return step.State == domain.AppStepFailed | |
| 102 | 99 | } | |
| 103 | 100 | return true | |
| 104 | 101 | } | |
| 105 | 102 | ||
| 106 | - | // supersededPending reports whether an overlap step may still have its | |
| 107 | - | // superseded container running beside the candidate: the replacement started | |
| 108 | - | // the new generation while the old one kept serving, and the old one was not | |
| 109 | - | // yet confirmed gone. | |
| 110 | - | func supersededPending(step domain.AppOperationStep) bool { | |
| 111 | - | return step.Overlap && !step.BeforeRetired && step.Before != "" | |
| 112 | - | } | |
| 113 | - | ||
| 114 | 103 | // convergeInterruptedSteps retries the recorded candidate of every step that | |
| 115 | 104 | // needs container work and accumulates the bounded leftovers in the journal. | |
| 116 | 105 | // It reports whether the journal changed and fails closed on the first | |
| @@ -122,19 +111,18 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 122 | 111 | if !reconcileNeedsContainer(*step, terminal) { | |
| 123 | 112 | continue | |
| 124 | 113 | } | |
| 125 | - | recorded := *step | |
| 126 | 114 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | |
| 127 | - | if !reflect.DeepEqual(recorded, *step) { | |
| 128 | - | // Converging the candidate changes the recorded step (it clears | |
| 129 | - | // After, or marks the superseded container retired), so the | |
| 115 | + | if step.After == "" { | |
| 116 | + | // Converging the candidate clears its recorded After, so the | |
| 130 | 117 | // journal changed even when nothing was left over to report: the | |
| 131 | 118 | // converged step must be persisted, or every later pass reconciles | |
| 132 | 119 | // it again. | |
| 133 | 120 | changed = true | |
| 134 | 121 | } | |
| 135 | - | if appendNewWarnings(op, warnings) { | |
| 122 | + | if len(warnings) > 0 { | |
| 136 | 123 | // A candidate that will not go away is operator-visible instead of | |
| 137 | 124 | // staying untracked. | |
| 125 | + | op.Warnings = append(op.Warnings, journalWarnings(warnings)...) | |
| 138 | 126 | changed = true | |
| 139 | 127 | } | |
| 140 | 128 | if convErr != nil { | |
| @@ -156,18 +144,10 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 156 | 144 | // rebuild the generation recorded in ACTIVE without two generations running | |
| 157 | 145 | // at once. An interrupted operation's step is marked failed. | |
| 158 | 146 | // | |
| 159 | - | // An overlap step differs in one way: its superseded container (Before) may | |
| 160 | - | // still run beside the candidate, because it is only retired after the | |
| 161 | - | // candidate was published and routed. When the candidate is the published | |
| 162 | - | // container, the superseded one is retired here (see convergeSuperseded); | |
| 163 | - | // when it was never published, the candidate is removed and the superseded | |
| 164 | - | // container, which still serves, is left alone. | |
| 165 | - | // | |
| 166 | 147 | // A candidate that cannot be removed is returned as an error, not a | |
| 167 | 148 | // recoverable warning: the orphan may still run, so every later mutation must | |
| 168 | 149 | // fail closed instead of proceeding. The returned warnings are the leftovers | |
| 169 | - | // of that removal. A superseded container that cannot be retired is only a | |
| 170 | - | // warning (see convergeSuperseded). | |
| 150 | + | // of that removal. | |
| 171 | 151 | // | |
| 172 | 152 | // Success clears the step's recorded After and keeps the removed (or kept) | |
| 173 | 153 | // container id in Detail, so the audit trail survives while the converged step | |
| @@ -175,147 +155,81 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 175 | 155 | // in place, keeping a failed convergence eligible for retry. | |
| 176 | 156 | // | |
| 177 | 157 | // Once the candidate is converged, the inhibition the replacement wrote for | |
| 178 | - | // the superseded container is cleared. A step carrying both Before and After | |
| 179 | - | // of a sequential replacement records that the superseded container was | |
| 180 | - | // confirmed gone before the candidate was created (deployService retires it | |
| 181 | - | // first), so the marker protects nothing and would otherwise refuse the | |
| 182 | - | // boot/start/restart rebuild from ACTIVE. An overlap step wrote no | |
| 183 | - | // inhibition. | |
| 158 | + | // the superseded container is cleared. A sequential replacement confirmed the | |
| 159 | + | // superseded container gone before the candidate was created (deployService | |
| 160 | + | // retires it first), so its marker protects nothing and would otherwise | |
| 161 | + | // refuse the boot/start/restart rebuild from ACTIVE. An overlap replacement | |
| 162 | + | // leaves its superseded container running: when the candidate is published, | |
| 163 | + | // its retirement-pending marker stays, and the periodic reconciliation | |
| 164 | + | // retires that container once the candidate is verified and routed. When the | |
| 165 | + | // candidate was never published the superseded container still serves, so | |
| 166 | + | // the marker is dropped and the container is kept. | |
| 167 | + | // | |
| 168 | + | // Reconciliation never touches traffic: the persisted binds of a published | |
| 169 | + | // candidate are unverified here, and the verified start/recovery path | |
| 170 | + | // publishes them. | |
| 184 | 171 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | |
| 185 | 172 | candidate := step.After | |
| 186 | - | var warnings []CleanupWarning | |
| 187 | - | // retry keeps the step eligible for a later pass: the candidate is kept | |
| 188 | - | // and its superseded container is not gone yet. | |
| 189 | - | retry := false | |
| 190 | - | switch { | |
| 191 | - | case publishedContainer(active, candidate): | |
| 192 | - | warnings = s.convergePublished(ctx, app, active, step, terminal, log) | |
| 193 | - | retry = supersededPending(*step) | |
| 194 | - | case terminal && step.State == domain.AppStepSucceeded: | |
| 195 | - | // A succeeded overlap step whose candidate is no longer published | |
| 196 | - | // (ACTIVE moved on): only its superseded container is left to settle. | |
| 197 | - | step.Detail = "converged: container " + candidate + " is no longer published" | |
| 198 | - | warnings = s.convergeSuperseded(ctx, app, active, step, false, log) | |
| 199 | - | retry = supersededPending(*step) | |
| 200 | - | default: | |
| 201 | - | var err error | |
| 202 | - | if warnings, err = s.removeUnpublished(ctx, app, step, terminal, log); err != nil { | |
| 203 | - | return warnings, err | |
| 173 | + | if publishedContainer(active, candidate) { | |
| 174 | + | // The published generation: keep it. A step that already succeeded is | |
| 175 | + | // left as recorded; a routing apply that never ran is republished by | |
| 176 | + | // the periodic recovery pass. | |
| 177 | + | if !terminal && step.State != domain.AppStepSucceeded { | |
| 178 | + | step.State = domain.AppStepFailed | |
| 179 | + | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" | |
| 180 | + | } | |
| 181 | + | step.Detail = "converged: published container " + candidate + " kept" | |
| 182 | + | } else { | |
| 183 | + | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) | |
| 184 | + | if !retired.Gone { | |
| 185 | + | if !terminal { | |
| 186 | + | step.State = domain.AppStepFailed | |
| 187 | + | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) | |
| 188 | + | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") | |
| 189 | + | } | |
| 190 | + | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) | |
| 191 | + | } | |
| 192 | + | if !terminal { | |
| 193 | + | step.State = domain.AppStepFailed | |
| 194 | + | step.Error = "interrupted before publication; the unpublished candidate was removed" | |
| 204 | 195 | } | |
| 196 | + | step.Detail = "converged: removed unpublished candidate " + candidate | |
| 205 | 197 | } | |
| 206 | 198 | // The candidate is gone (or published): the replacement-pending | |
| 207 | 199 | // inhibition of the superseded container is stale. Dropping it lets | |
| 208 | 200 | // boot/start/restart rebuild that generation from ACTIVE; the single | |
| 209 | 201 | // writer is preserved because the superseded container was proven gone | |
| 210 | - | // before the candidate existed. An overlap step wrote no inhibition. | |
| 211 | - | if !step.Overlap && step.Service != "" && step.Before != "" && step.Before != candidate { | |
| 212 | - | if err := s.clearRecoveryInhibition(ctx, app, step.Service, step.Before); err != nil { | |
| 202 | + | // before the candidate existed. | |
| 203 | + | if step.Service != "" && step.Before != "" && step.Before != candidate { | |
| 204 | + | if err := s.clearSupersededInhibition(ctx, app, step.Service, step.Before, publishedContainer(active, candidate)); err != nil { | |
| 213 | 205 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | |
| 214 | - | return append(warnings, CleanupWarning{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}), nil | |
| 206 | + | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}}, nil | |
| 215 | 207 | } | |
| 216 | 208 | } | |
| 217 | - | if !retry && (!terminal || step.State != domain.AppStepSucceeded) { | |
| 218 | - | // Only reconciliation removes a recorded candidate: clearing After | |
| 219 | - | // here is what marks the step converged. The After of a succeeded | |
| 220 | - | // step is the container that serves, and stays reported. | |
| 221 | - | step.After = "" | |
| 222 | - | } | |
| 223 | - | return warnings, nil | |
| 224 | - | } | |
| 225 | - | ||
| 226 | - | // appendNewWarnings adds the leftovers that the journal does not already | |
| 227 | - | // carry, so a leftover retried on every pass is reported once. It reports | |
| 228 | - | // whether the journal changed. | |
| 229 | - | func appendNewWarnings(op *domain.AppOperation, warnings []CleanupWarning) bool { | |
| 230 | - | changed := false | |
| 231 | - | for _, warning := range journalWarnings(warnings) { | |
| 232 | - | if slices.Contains(op.Warnings, warning) { | |
| 233 | - | continue | |
| 234 | - | } | |
| 235 | - | op.Warnings = append(op.Warnings, warning) | |
| 236 | - | changed = true | |
| 237 | - | } | |
| 238 | - | return changed | |
| 209 | + | // Only reconciliation removes a recorded candidate: clearing After here is | |
| 210 | + | // what marks the step converged. | |
| 211 | + | step.After = "" | |
| 212 | + | return nil, nil | |
| 239 | 213 | } | |
| 240 | 214 | ||
| 241 | - | // convergePublished keeps the published candidate of a step. A step that | |
| 242 | - | // already succeeded is left as recorded; a routing apply that never ran is | |
| 243 | - | // republished. An overlap step also retires its superseded container. | |
| 244 | - | func (s *Service) convergePublished(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) []CleanupWarning { | |
| 245 | - | if !terminal && step.State != domain.AppStepSucceeded { | |
| 246 | - | step.State = domain.AppStepFailed | |
| 247 | - | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" | |
| 248 | - | if step.Overlap { | |
| 249 | - | step.Error = "interrupted before the switch completed; the published container is kept and routing is republished" | |
| 250 | - | } | |
| 251 | - | } | |
| 252 | - | step.Detail = "converged: published container " + step.After + " kept" | |
| 253 | - | return s.convergeSuperseded(ctx, app, active, step, true, log) | |
| 254 | - | } | |
| 255 | - | ||
| 256 | - | // convergeSuperseded retires the superseded container of an overlap step. For | |
| 257 | - | // a published candidate (republish) routing is re-applied first: the | |
| 258 | - | // superseded container is what a stale graph could still reach, so it is | |
| 259 | - | // stopped only once the graph is known to point at the published container. | |
| 260 | - | // It stops the container gracefully with the app default grace: the grace of | |
| 261 | - | // the generation being stopped is no longer recorded once ACTIVE names its | |
| 262 | - | // replacement. | |
| 263 | - | // | |
| 264 | - | // A superseded container that cannot be retired right now is a leftover | |
| 265 | - | // warning, not an error: the published container already serves, so a | |
| 266 | - | // redundant old generation must not block every later mutation of the app. | |
| 267 | - | // The step keeps its After and stays unretired, so a later pass retries. A | |
| 268 | - | // step without a pending superseded container, or whose superseded container | |
| 269 | - | // ACTIVE names again (a rollback to it), is a no-op: that container is the | |
| 270 | - | // serving generation and is never retired here. | |
| 271 | - | func (s *Service) convergeSuperseded(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, republish bool, log zerowrap.Logger) []CleanupWarning { | |
| 272 | - | if !supersededPending(*step) { | |
| 273 | - | return nil | |
| 274 | - | } | |
| 275 | - | if publishedContainer(active, step.Before) { | |
| 276 | - | step.BeforeRetired = true | |
| 277 | - | return nil | |
| 278 | - | } | |
| 279 | - | kept := func(detail string, err error) []CleanupWarning { | |
| 280 | - | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: superseded container not retired") | |
| 281 | - | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: detail}} | |
| 282 | - | } | |
| 283 | - | if republish { | |
| 284 | - | if err := s.refreshTraffic(ctx, app); err != nil { | |
| 285 | - | return kept("superseded container kept: routing could not be republished", err) | |
| 286 | - | } | |
| 287 | - | } | |
| 288 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service}, step.Before) | |
| 289 | - | if !retired.Gone { | |
| 290 | - | if len(retired.Warnings) == 0 { | |
| 291 | - | return kept("superseded container not confirmed gone", nil) | |
| 215 | + | // clearSupersededInhibition drops the inhibition the interrupted replacement | |
| 216 | + | // recorded for the superseded container. A retirement-pending marker of a | |
| 217 | + | // published candidate is kept: the superseded container of an overlap | |
| 218 | + | // replacement may still be running, and that marker is what makes the | |
| 219 | + | // periodic reconciliation retire it. Every other marker is stale. | |
| 220 | + | func (s *Service) clearSupersededInhibition(ctx context.Context, app, service, before string, published bool) error { | |
| 221 | + | if published { | |
| 222 | + | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | |
| 223 | + | if err != nil { | |
| 224 | + | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |
| 292 | 225 | } | |
| 293 | - | return retired.Warnings | |
| 294 | - | } | |
| 295 | - | step.BeforeRetired = true | |
| 296 | - | step.Detail += "; retired superseded container " + step.Before | |
| 297 | - | return retired.Warnings | |
| 298 | - | } | |
| 299 | - | ||
| 300 | - | // removeUnpublished force-removes a candidate that was never published. An | |
| 301 | - | // overlap step keeps its superseded container, which still serves. | |
| 302 | - | func (s *Service) removeUnpublished(ctx context.Context, app string, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | |
| 303 | - | candidate := step.After | |
| 304 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) | |
| 305 | - | if !retired.Gone { | |
| 306 | - | if !terminal { | |
| 307 | - | step.State = domain.AppStepFailed | |
| 308 | - | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) | |
| 309 | - | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") | |
| 226 | + | for _, inhibition := range inhibitions { | |
| 227 | + | if inhibition.Service == service && inhibition.ContainerID == before && inhibition.Reason == domain.AppInhibitRetirementPending { | |
| 228 | + | return nil | |
| 229 | + | } | |
| 310 | 230 | } | |
| 311 | - | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) | |
| 312 | - | } | |
| 313 | - | if !terminal { | |
| 314 | - | step.State = domain.AppStepFailed | |
| 315 | - | step.Error = "interrupted before publication; the unpublished candidate was removed" | |
| 316 | 231 | } | |
| 317 | - | step.Detail = "converged: removed unpublished candidate " + candidate | |
| 318 | - | return retired.Warnings, nil | |
| 232 | + | return s.clearRecoveryInhibition(ctx, app, service, before) | |
| 319 | 233 | } | |
| 320 | 234 | ||
| 321 | 235 | // interruptedOutcome classifies an interrupted operation: a failure, unless | |
| @@ -3,8 +3,6 @@ package deployment | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | - | "reflect" | |||
| 7 | - | "slices" | |||
| 8 | 6 | ||||
| 9 | "github.com/bnema/zerowrap" | 7 | "github.com/bnema/zerowrap" | ||
| 10 | 8 | ||||
| @@ -90,27 +88,18 @@ func (s *Service) reconcileInterruptedDeploy(ctx context.Context, app string) er | |||||
| 90 | 88 | ||||
| 91 | // reconcileNeedsContainer reports whether one step still needs container | 89 | // reconcileNeedsContainer reports whether one step still needs container | ||
| 92 | // work: a step that recorded a candidate. A terminal operation only retries | 90 | // work: a step that recorded a candidate. A terminal operation only retries | ||
| 93 | - | // the step that recorded a failed candidate, or an overlap step whose | 91 | + | // the step that recorded a failed candidate; a pending step exists only in an |
| 94 | - | // superseded container was not confirmed retired; a pending step exists only | 92 | + | // interrupted operation. |
| 95 | - | // in an interrupted operation. | |||
| 96 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | 93 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | ||
| 97 | if step.After == "" { | 94 | if step.After == "" { | ||
| 98 | return false | 95 | return false | ||
| 99 | } | 96 | } | ||
| 100 | if terminal { | 97 | if terminal { | ||
| 101 | - | return step.State == domain.AppStepFailed || supersededPending(step) | 98 | + | return step.State == domain.AppStepFailed |
| 102 | } | 99 | } | ||
| 103 | return true | 100 | return true | ||
| 104 | } | 101 | } | ||
| 105 | 102 | ||||
| 106 | - | // supersededPending reports whether an overlap step may still have its | |||
| 107 | - | // superseded container running beside the candidate: the replacement started | |||
| 108 | - | // the new generation while the old one kept serving, and the old one was not | |||
| 109 | - | // yet confirmed gone. | |||
| 110 | - | func supersededPending(step domain.AppOperationStep) bool { | |||
| 111 | - | return step.Overlap && !step.BeforeRetired && step.Before != "" | |||
| 112 | - | } | |||
| 113 | - | ||||
| 114 | // convergeInterruptedSteps retries the recorded candidate of every step that | 103 | // convergeInterruptedSteps retries the recorded candidate of every step that | ||
| 115 | // needs container work and accumulates the bounded leftovers in the journal. | 104 | // needs container work and accumulates the bounded leftovers in the journal. | ||
| 116 | // It reports whether the journal changed and fails closed on the first | 105 | // It reports whether the journal changed and fails closed on the first | ||
| @@ -122,19 +111,18 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 122 | if !reconcileNeedsContainer(*step, terminal) { | 111 | if !reconcileNeedsContainer(*step, terminal) { | ||
| 123 | continue | 112 | continue | ||
| 124 | } | 113 | } | ||
| 125 | - | recorded := *step | |||
| 126 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | 114 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | ||
| 127 | - | if !reflect.DeepEqual(recorded, *step) { | 115 | + | if step.After == "" { |
| 128 | - | // Converging the candidate changes the recorded step (it clears | 116 | + | // Converging the candidate clears its recorded After, so the |
| 129 | - | // After, or marks the superseded container retired), so the | |||
| 130 | // journal changed even when nothing was left over to report: the | 117 | // journal changed even when nothing was left over to report: the | ||
| 131 | // converged step must be persisted, or every later pass reconciles | 118 | // converged step must be persisted, or every later pass reconciles | ||
| 132 | // it again. | 119 | // it again. | ||
| 133 | changed = true | 120 | changed = true | ||
| 134 | } | 121 | } | ||
| 135 | - | if appendNewWarnings(op, warnings) { | 122 | + | if len(warnings) > 0 { |
| 136 | // A candidate that will not go away is operator-visible instead of | 123 | // A candidate that will not go away is operator-visible instead of | ||
| 137 | // staying untracked. | 124 | // staying untracked. | ||
| 125 | + | op.Warnings = append(op.Warnings, journalWarnings(warnings)...) | |||
| 138 | changed = true | 126 | changed = true | ||
| 139 | } | 127 | } | ||
| 140 | if convErr != nil { | 128 | if convErr != nil { | ||
| @@ -156,18 +144,10 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 156 | // rebuild the generation recorded in ACTIVE without two generations running | 144 | // rebuild the generation recorded in ACTIVE without two generations running | ||
| 157 | // at once. An interrupted operation's step is marked failed. | 145 | // at once. An interrupted operation's step is marked failed. | ||
| 158 | // | 146 | // | ||
| 159 | - | // An overlap step differs in one way: its superseded container (Before) may | |||
| 160 | - | // still run beside the candidate, because it is only retired after the | |||
| 161 | - | // candidate was published and routed. When the candidate is the published | |||
| 162 | - | // container, the superseded one is retired here (see convergeSuperseded); | |||
| 163 | - | // when it was never published, the candidate is removed and the superseded | |||
| 164 | - | // container, which still serves, is left alone. | |||
| 165 | - | // | |||
| 166 | // A candidate that cannot be removed is returned as an error, not a | 147 | // A candidate that cannot be removed is returned as an error, not a | ||
| 167 | // recoverable warning: the orphan may still run, so every later mutation must | 148 | // recoverable warning: the orphan may still run, so every later mutation must | ||
| 168 | // fail closed instead of proceeding. The returned warnings are the leftovers | 149 | // fail closed instead of proceeding. The returned warnings are the leftovers | ||
| 169 | - | // of that removal. A superseded container that cannot be retired is only a | 150 | + | // of that removal. |
| 170 | - | // warning (see convergeSuperseded). | |||
| 171 | // | 151 | // | ||
| 172 | // Success clears the step's recorded After and keeps the removed (or kept) | 152 | // Success clears the step's recorded After and keeps the removed (or kept) | ||
| 173 | // container id in Detail, so the audit trail survives while the converged step | 153 | // container id in Detail, so the audit trail survives while the converged step | ||
| @@ -175,147 +155,81 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 175 | // in place, keeping a failed convergence eligible for retry. | 155 | // in place, keeping a failed convergence eligible for retry. | ||
| 176 | // | 156 | // | ||
| 177 | // Once the candidate is converged, the inhibition the replacement wrote for | 157 | // Once the candidate is converged, the inhibition the replacement wrote for | ||
| 178 | - | // the superseded container is cleared. A step carrying both Before and After | 158 | + | // the superseded container is cleared. A sequential replacement confirmed the |
| 179 | - | // of a sequential replacement records that the superseded container was | 159 | + | // superseded container gone before the candidate was created (deployService |
| 180 | - | // confirmed gone before the candidate was created (deployService retires it | 160 | + | // retires it first), so its marker protects nothing and would otherwise |
| 181 | - | // first), so the marker protects nothing and would otherwise refuse the | 161 | + | // refuse the boot/start/restart rebuild from ACTIVE. An overlap replacement |
| 182 | - | // boot/start/restart rebuild from ACTIVE. An overlap step wrote no | 162 | + | // leaves its superseded container running: when the candidate is published, |
| 183 | - | // inhibition. | 163 | + | // its retirement-pending marker stays, and the periodic reconciliation |
| 164 | + | // retires that container once the candidate is verified and routed. When the | |||
| 165 | + | // candidate was never published the superseded container still serves, so | |||
| 166 | + | // the marker is dropped and the container is kept. | |||
| 167 | + | // | |||
| 168 | + | // Reconciliation never touches traffic: the persisted binds of a published | |||
| 169 | + | // candidate are unverified here, and the verified start/recovery path | |||
| 170 | + | // publishes them. | |||
| 184 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | 171 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | ||
| 185 | candidate := step.After | 172 | candidate := step.After | ||
| 186 | - | var warnings []CleanupWarning | 173 | + | if publishedContainer(active, candidate) { |
| 187 | - | // retry keeps the step eligible for a later pass: the candidate is kept | 174 | + | // The published generation: keep it. A step that already succeeded is |
| 188 | - | // and its superseded container is not gone yet. | 175 | + | // left as recorded; a routing apply that never ran is republished by |
| 189 | - | retry := false | 176 | + | // the periodic recovery pass. |
| 190 | - | switch { | 177 | + | if !terminal && step.State != domain.AppStepSucceeded { |
| 191 | - | case publishedContainer(active, candidate): | 178 | + | step.State = domain.AppStepFailed |
| 192 | - | warnings = s.convergePublished(ctx, app, active, step, terminal, log) | 179 | + | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" |
| 193 | - | retry = supersededPending(*step) | 180 | + | } |
| 194 | - | case terminal && step.State == domain.AppStepSucceeded: | 181 | + | step.Detail = "converged: published container " + candidate + " kept" |
| 195 | - | // A succeeded overlap step whose candidate is no longer published | 182 | + | } else { |
| 196 | - | // (ACTIVE moved on): only its superseded container is left to settle. | 183 | + | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) |
| 197 | - | step.Detail = "converged: container " + candidate + " is no longer published" | 184 | + | if !retired.Gone { |
| 198 | - | warnings = s.convergeSuperseded(ctx, app, active, step, false, log) | 185 | + | if !terminal { |
| 199 | - | retry = supersededPending(*step) | 186 | + | step.State = domain.AppStepFailed |
| 200 | - | default: | 187 | + | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) |
| 201 | - | var err error | 188 | + | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") |
| 202 | - | if warnings, err = s.removeUnpublished(ctx, app, step, terminal, log); err != nil { | 189 | + | } |
| 203 | - | return warnings, err | 190 | + | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) |
| 191 | + | } | |||
| 192 | + | if !terminal { | |||
| 193 | + | step.State = domain.AppStepFailed | |||
| 194 | + | step.Error = "interrupted before publication; the unpublished candidate was removed" | |||
| 204 | } | 195 | } | ||
| 196 | + | step.Detail = "converged: removed unpublished candidate " + candidate | |||
| 205 | } | 197 | } | ||
| 206 | // The candidate is gone (or published): the replacement-pending | 198 | // The candidate is gone (or published): the replacement-pending | ||
| 207 | // inhibition of the superseded container is stale. Dropping it lets | 199 | // inhibition of the superseded container is stale. Dropping it lets | ||
| 208 | // boot/start/restart rebuild that generation from ACTIVE; the single | 200 | // boot/start/restart rebuild that generation from ACTIVE; the single | ||
| 209 | // writer is preserved because the superseded container was proven gone | 201 | // writer is preserved because the superseded container was proven gone | ||
| 210 | - | // before the candidate existed. An overlap step wrote no inhibition. | 202 | + | // before the candidate existed. |
| 211 | - | if !step.Overlap && step.Service != "" && step.Before != "" && step.Before != candidate { | 203 | + | if step.Service != "" && step.Before != "" && step.Before != candidate { |
| 212 | - | if err := s.clearRecoveryInhibition(ctx, app, step.Service, step.Before); err != nil { | 204 | + | if err := s.clearSupersededInhibition(ctx, app, step.Service, step.Before, publishedContainer(active, candidate)); err != nil { |
| 213 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | 205 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | ||
| 214 | - | return append(warnings, CleanupWarning{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}), nil | 206 | + | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}}, nil |
| 215 | } | 207 | } | ||
| 216 | } | 208 | } | ||
| 217 | - | if !retry && (!terminal || step.State != domain.AppStepSucceeded) { | 209 | + | // Only reconciliation removes a recorded candidate: clearing After here is |
| 218 | - | // Only reconciliation removes a recorded candidate: clearing After | 210 | + | // what marks the step converged. |
| 219 | - | // here is what marks the step converged. The After of a succeeded | 211 | + | step.After = "" |
| 220 | - | // step is the container that serves, and stays reported. | 212 | + | return nil, nil |
| 221 | - | step.After = "" | |||
| 222 | - | } | |||
| 223 | - | return warnings, nil | |||
| 224 | - | } | |||
| 225 | - | ||||
| 226 | - | // appendNewWarnings adds the leftovers that the journal does not already | |||
| 227 | - | // carry, so a leftover retried on every pass is reported once. It reports | |||
| 228 | - | // whether the journal changed. | |||
| 229 | - | func appendNewWarnings(op *domain.AppOperation, warnings []CleanupWarning) bool { | |||
| 230 | - | changed := false | |||
| 231 | - | for _, warning := range journalWarnings(warnings) { | |||
| 232 | - | if slices.Contains(op.Warnings, warning) { | |||
| 233 | - | continue | |||
| 234 | - | } | |||
| 235 | - | op.Warnings = append(op.Warnings, warning) | |||
| 236 | - | changed = true | |||
| 237 | - | } | |||
| 238 | - | return changed | |||
| 239 | } | 213 | } | ||
| 240 | 214 | ||||
| 241 | - | // convergePublished keeps the published candidate of a step. A step that | 215 | + | // clearSupersededInhibition drops the inhibition the interrupted replacement |
| 242 | - | // already succeeded is left as recorded; a routing apply that never ran is | 216 | + | // recorded for the superseded container. A retirement-pending marker of a |
| 243 | - | // republished. An overlap step also retires its superseded container. | 217 | + | // published candidate is kept: the superseded container of an overlap |
| 244 | - | func (s *Service) convergePublished(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) []CleanupWarning { | 218 | + | // replacement may still be running, and that marker is what makes the |
| 245 | - | if !terminal && step.State != domain.AppStepSucceeded { | 219 | + | // periodic reconciliation retire it. Every other marker is stale. |
| 246 | - | step.State = domain.AppStepFailed | 220 | + | func (s *Service) clearSupersededInhibition(ctx context.Context, app, service, before string, published bool) error { |
| 247 | - | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" | 221 | + | if published { |
| 248 | - | if step.Overlap { | 222 | + | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) |
| 249 | - | step.Error = "interrupted before the switch completed; the published container is kept and routing is republished" | 223 | + | if err != nil { |
| 250 | - | } | 224 | + | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) |
| 251 | - | } | |||
| 252 | - | step.Detail = "converged: published container " + step.After + " kept" | |||
| 253 | - | return s.convergeSuperseded(ctx, app, active, step, true, log) | |||
| 254 | - | } | |||
| 255 | - | ||||
| 256 | - | // convergeSuperseded retires the superseded container of an overlap step. For | |||
| 257 | - | // a published candidate (republish) routing is re-applied first: the | |||
| 258 | - | // superseded container is what a stale graph could still reach, so it is | |||
| 259 | - | // stopped only once the graph is known to point at the published container. | |||
| 260 | - | // It stops the container gracefully with the app default grace: the grace of | |||
| 261 | - | // the generation being stopped is no longer recorded once ACTIVE names its | |||
| 262 | - | // replacement. | |||
| 263 | - | // | |||
| 264 | - | // A superseded container that cannot be retired right now is a leftover | |||
| 265 | - | // warning, not an error: the published container already serves, so a | |||
| 266 | - | // redundant old generation must not block every later mutation of the app. | |||
| 267 | - | // The step keeps its After and stays unretired, so a later pass retries. A | |||
| 268 | - | // step without a pending superseded container, or whose superseded container | |||
| 269 | - | // ACTIVE names again (a rollback to it), is a no-op: that container is the | |||
| 270 | - | // serving generation and is never retired here. | |||
| 271 | - | func (s *Service) convergeSuperseded(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, republish bool, log zerowrap.Logger) []CleanupWarning { | |||
| 272 | - | if !supersededPending(*step) { | |||
| 273 | - | return nil | |||
| 274 | - | } | |||
| 275 | - | if publishedContainer(active, step.Before) { | |||
| 276 | - | step.BeforeRetired = true | |||
| 277 | - | return nil | |||
| 278 | - | } | |||
| 279 | - | kept := func(detail string, err error) []CleanupWarning { | |||
| 280 | - | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: superseded container not retired") | |||
| 281 | - | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: detail}} | |||
| 282 | - | } | |||
| 283 | - | if republish { | |||
| 284 | - | if err := s.refreshTraffic(ctx, app); err != nil { | |||
| 285 | - | return kept("superseded container kept: routing could not be republished", err) | |||
| 286 | - | } | |||
| 287 | - | } | |||
| 288 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service}, step.Before) | |||
| 289 | - | if !retired.Gone { | |||
| 290 | - | if len(retired.Warnings) == 0 { | |||
| 291 | - | return kept("superseded container not confirmed gone", nil) | |||
| 292 | } | 225 | } | ||
| 293 | - | return retired.Warnings | 226 | + | for _, inhibition := range inhibitions { |
| 294 | - | } | 227 | + | if inhibition.Service == service && inhibition.ContainerID == before && inhibition.Reason == domain.AppInhibitRetirementPending { |
| 295 | - | step.BeforeRetired = true | 228 | + | return nil |
| 296 | - | step.Detail += "; retired superseded container " + step.Before | 229 | + | } |
| 297 | - | return retired.Warnings | |||
| 298 | - | } | |||
| 299 | - | ||||
| 300 | - | // removeUnpublished force-removes a candidate that was never published. An | |||
| 301 | - | // overlap step keeps its superseded container, which still serves. | |||
| 302 | - | func (s *Service) removeUnpublished(ctx context.Context, app string, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | |||
| 303 | - | candidate := step.After | |||
| 304 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) | |||
| 305 | - | if !retired.Gone { | |||
| 306 | - | if !terminal { | |||
| 307 | - | step.State = domain.AppStepFailed | |||
| 308 | - | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) | |||
| 309 | - | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") | |||
| 310 | } | 230 | } | ||
| 311 | - | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) | |||
| 312 | - | } | |||
| 313 | - | if !terminal { | |||
| 314 | - | step.State = domain.AppStepFailed | |||
| 315 | - | step.Error = "interrupted before publication; the unpublished candidate was removed" | |||
| 316 | } | 231 | } | ||
| 317 | - | step.Detail = "converged: removed unpublished candidate " + candidate | 232 | + | return s.clearRecoveryInhibition(ctx, app, service, before) |
| 318 | - | return retired.Warnings, nil | |||
| 319 | } | 233 | } | ||
| 320 | 234 | ||||
| 321 | // interruptedOutcome classifies an interrupted operation: a failure, unless | 235 | // interruptedOutcome classifies an interrupted operation: a failure, unless | ||
M internal/usecase/deployment/lifecycle.go +25 -0
| @@ -512,9 +512,34 @@ func (s *Service) removeServiceContainers(ctx context.Context, app, opID string, | |||
| 512 | 512 | step.State = domain.AppStepSucceeded | |
| 513 | 513 | steps = append(steps, step) | |
| 514 | 514 | } | |
| 515 | + | if err := s.retireAllPending(ctx, app); err != nil { | |
| 516 | + | return nil, err | |
| 517 | + | } | |
| 515 | 518 | return steps, nil | |
| 516 | 519 | } | |
| 517 | 520 | ||
| 521 | + | // retireAllPending retires every superseded container an overlap replacement | |
| 522 | + | // 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 | |
| 524 | + | // not confirmed gone here would be orphaned and untracked: a failure keeps the | |
| 525 | + | // markers and fails the caller. | |
| 526 | + | func (s *Service) retireAllPending(ctx context.Context, app string) error { | |
| 527 | + | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | |
| 528 | + | if err != nil { | |
| 529 | + | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |
| 530 | + | } | |
| 531 | + | _, pending := splitRecoveryInhibitions(app, inhibitions) | |
| 532 | + | for _, marker := range pending { | |
| 533 | + | retired := s.retireContainer(ctx, app, retireOptions{ | |
| 534 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |
| 535 | + | }, 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)) | |
| 538 | + | } | |
| 539 | + | } | |
| 540 | + | return nil | |
| 541 | + | } | |
| 542 | + | ||
| 518 | 543 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | |
| 519 | 544 | // every non-stopped app with ACTIVE services is verified through Start, | |
| 520 | 545 | // including all-running apps: ephemeral loopback binds may shift across | |
| @@ -512,9 +512,34 @@ func (s *Service) removeServiceContainers(ctx context.Context, app, opID string, | |||||
| 512 | step.State = domain.AppStepSucceeded | 512 | step.State = domain.AppStepSucceeded | ||
| 513 | steps = append(steps, step) | 513 | steps = append(steps, step) | ||
| 514 | } | 514 | } | ||
| 515 | + | if err := s.retireAllPending(ctx, app); err != nil { | |||
| 516 | + | return nil, err | |||
| 517 | + | } | |||
| 515 | return steps, nil | 518 | return steps, nil | ||
| 516 | } | 519 | } | ||
| 517 | 520 | ||||
| 521 | + | // retireAllPending retires every superseded container an overlap replacement | |||
| 522 | + | // 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 | |||
| 524 | + | // not confirmed gone here would be orphaned and untracked: a failure keeps the | |||
| 525 | + | // markers and fails the caller. | |||
| 526 | + | func (s *Service) retireAllPending(ctx context.Context, app string) error { | |||
| 527 | + | inhibitions, err := s.deps.State.LoadRecoveryInhibitions(ctx, app) | |||
| 528 | + | if err != nil { | |||
| 529 | + | return fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |||
| 530 | + | } | |||
| 531 | + | _, pending := splitRecoveryInhibitions(app, inhibitions) | |||
| 532 | + | for _, marker := range pending { | |||
| 533 | + | retired := s.retireContainer(ctx, app, retireOptions{ | |||
| 534 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |||
| 535 | + | }, 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)) | |||
| 538 | + | } | |||
| 539 | + | } | |||
| 540 | + | return nil | |||
| 541 | + | } | |||
| 542 | + | ||||
| 518 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | 543 | // ReconcileBoot reconciles apps intended to run after a daemon boot: | ||
| 519 | // every non-stopped app with ACTIVE services is verified through Start, | 544 | // every non-stopped app with ACTIVE services is verified through Start, | ||
| 520 | // including all-running apps: ephemeral loopback binds may shift across | 545 | // including all-running apps: ephemeral loopback binds may shift across | ||
M internal/usecase/deployment/monitor.go +100 -10
| @@ -288,6 +288,14 @@ 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 | + | ||
| 291 | 299 | // forget drops one generation's observation. | |
| 292 | 300 | func (t *executionTracker) forget(key recoveryKey) { | |
| 293 | 301 | t.mu.Lock() | |
| @@ -358,7 +366,9 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 358 | 366 | if err != nil { | |
| 359 | 367 | return fmt.Errorf("deployment: reconcile %q: load recovery inhibitions: %w", app, err) | |
| 360 | 368 | } | |
| 361 | - | inhibited, failures := s.processRecoveryInhibitions(ctx, app, inhibitions) | |
| 369 | + | inhibited, pending := splitRecoveryInhibitions(app, inhibitions) | |
| 370 | + | var failures []error | |
| 371 | + | failed := map[string]struct{}{} | |
| 362 | 372 | for _, name := range sortedServiceNames(active) { | |
| 363 | 373 | eff := active.Services[name] | |
| 364 | 374 | if intent.Stopped { | |
| @@ -369,6 +379,7 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 369 | 379 | } | |
| 370 | 380 | if err := s.reconcileRunningService(ctx, app, name, eff, inhibited); err != nil { | |
| 371 | 381 | failures = append(failures, err) | |
| 382 | + | failed[name] = struct{}{} | |
| 372 | 383 | } | |
| 373 | 384 | } | |
| 374 | 385 | if intent.Stopped { | |
| @@ -381,24 +392,77 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||
| 381 | 392 | } | |
| 382 | 393 | cancel() | |
| 383 | 394 | } | |
| 395 | + | // Superseded containers are retired last: only after their successors | |
| 396 | + | // were verified and routed in this pass. | |
| 397 | + | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, failed)...) | |
| 384 | 398 | s.pruneRecoveryHistory(app, active) | |
| 385 | 399 | return errors.Join(failures...) | |
| 386 | 400 | } | |
| 387 | 401 | ||
| 388 | - | func (s *Service) processRecoveryInhibitions(ctx context.Context, app string, inhibitions []domain.AppRecoveryInhibition) (map[recoveryKey]struct{}, []error) { | |
| 402 | + | // splitRecoveryInhibitions separates the generations recovery must never | |
| 403 | + | // start from the superseded containers waiting to be retired. | |
| 404 | + | func splitRecoveryInhibitions(app string, inhibitions []domain.AppRecoveryInhibition) (map[recoveryKey]struct{}, []domain.AppRecoveryInhibition) { | |
| 389 | 405 | inhibited := map[recoveryKey]struct{}{} | |
| 390 | - | var failures []error | |
| 406 | + | var pending []domain.AppRecoveryInhibition | |
| 391 | 407 | for _, inhibition := range inhibitions { | |
| 392 | - | if inhibition.Reason != domain.AppInhibitRetirementPending { | |
| 393 | - | inhibited[recoveryKey{app: app, service: inhibition.Service, container: inhibition.ContainerID}] = struct{}{} | |
| 408 | + | if inhibition.Reason == domain.AppInhibitRetirementPending { | |
| 409 | + | pending = append(pending, inhibition) | |
| 410 | + | continue | |
| 411 | + | } | |
| 412 | + | inhibited[recoveryKey{app: app, service: inhibition.Service, container: inhibition.ContainerID}] = struct{}{} | |
| 413 | + | } | |
| 414 | + | return inhibited, pending | |
| 415 | + | } | |
| 416 | + | ||
| 417 | + | // retirePendingContainers retires the superseded containers an overlap | |
| 418 | + | // replacement left running, with the stop grace recorded at the marker | |
| 419 | + | // (the app default for a record that has none). A container is retired only | |
| 420 | + | // when it can no longer be the serving one: | |
| 421 | + | // - ACTIVE names it (the replacement never took over): it is the serving | |
| 422 | + | // 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. | |
| 426 | + | // | |
| 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 { | |
| 429 | + | var failures []error | |
| 430 | + | for _, marker := range pending { | |
| 431 | + | if publishedContainer(active, marker.ContainerID) { | |
| 432 | + | if err := s.clearRecoveryInhibition(ctx, app, marker.Service, marker.ContainerID); err != nil { | |
| 433 | + | failures = append(failures, err) | |
| 434 | + | } | |
| 435 | + | continue | |
| 436 | + | } | |
| 437 | + | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, failed) { | |
| 394 | 438 | continue | |
| 395 | 439 | } | |
| 396 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: inhibition.Service}, inhibition.ContainerID) | |
| 440 | + | retired := s.retireContainer(ctx, app, retireOptions{ | |
| 441 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |
| 442 | + | }, marker.ContainerID) | |
| 397 | 443 | if !retired.Gone { | |
| 398 | - | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s", app, inhibition.ContainerID)) | |
| 444 | + | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s: %s", app, marker.ContainerID, cleanupDetail(retired))) | |
| 399 | 445 | } | |
| 400 | 446 | } | |
| 401 | - | return inhibited, failures | |
| 447 | + | return failures | |
| 448 | + | } | |
| 449 | + | ||
| 450 | + | // 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 | + | successor, ok := active.Services[service] | |
| 458 | + | if !ok || successor.Container == "" { | |
| 459 | + | return true | |
| 460 | + | } | |
| 461 | + | if !s.executions.verified(recoveryKey{app: app, service: service, container: successor.Container}) { | |
| 462 | + | return false | |
| 463 | + | } | |
| 464 | + | container, err := s.deps.Runtime.InspectContainer(ctx, successor.Container) | |
| 465 | + | return err == nil && container.Status == string(domain.ContainerStatusRunning) | |
| 402 | 466 | } | |
| 403 | 467 | ||
| 404 | 468 | // convergeStoppedService enforces durable stopped intent: it never | |
| @@ -762,7 +826,10 @@ func (s *Service) withdrawForRecovery(ctx context.Context, app, service string) | |||
| 762 | 826 | } | |
| 763 | 827 | ||
| 764 | 828 | // recoveryInhibited reports whether one exact generation is durably | |
| 765 | - | // inhibited for recovery. | |
| 829 | + | // inhibited for recovery. A retirement-pending marker does not count: it | |
| 830 | + | // names a superseded container that is being retired, not a generation | |
| 831 | + | // recovery must keep stopped, and when ACTIVE still names that container | |
| 832 | + | // (the replacement never took over) it must stay startable. | |
| 766 | 833 | func (s *Service) recoveryInhibited(ctx context.Context, app, service, containerID string) (bool, error) { | |
| 767 | 834 | if containerID == "" { | |
| 768 | 835 | return false, nil | |
| @@ -772,7 +839,8 @@ func (s *Service) recoveryInhibited(ctx context.Context, app, service, container | |||
| 772 | 839 | return false, fmt.Errorf("deployment: load recovery inhibitions: %w", err) | |
| 773 | 840 | } | |
| 774 | 841 | for _, inhibition := range inhibitions { | |
| 775 | - | if inhibition.Service == service && inhibition.ContainerID == containerID { | |
| 842 | + | if inhibition.Service == service && inhibition.ContainerID == containerID && | |
| 843 | + | inhibition.Reason != domain.AppInhibitRetirementPending { | |
| 776 | 844 | return true, nil | |
| 777 | 845 | } | |
| 778 | 846 | } | |
| @@ -799,6 +867,28 @@ func (s *Service) inhibitRecovery(ctx context.Context, app, service, containerID | |||
| 799 | 867 | return nil | |
| 800 | 868 | } | |
| 801 | 869 | ||
| 870 | + | // inhibitRetirement durably marks a superseded container that an overlap | |
| 871 | + | // replacement left running beside its successor. The periodic reconciliation | |
| 872 | + | // retires it with grace, once the successor is verified and routed. | |
| 873 | + | func (s *Service) inhibitRetirement(ctx context.Context, app, service, containerID, operation string, grace time.Duration) error { | |
| 874 | + | if containerID == "" { | |
| 875 | + | return nil | |
| 876 | + | } | |
| 877 | + | inhibition := domain.AppRecoveryInhibition{ | |
| 878 | + | App: app, | |
| 879 | + | Service: service, | |
| 880 | + | ContainerID: containerID, | |
| 881 | + | Reason: domain.AppInhibitRetirementPending, | |
| 882 | + | Operation: operation, | |
| 883 | + | CreatedAt: time.Now().UTC(), | |
| 884 | + | StopGrace: grace, | |
| 885 | + | } | |
| 886 | + | if err := s.deps.State.SaveRecoveryInhibition(ctx, inhibition); err != nil { | |
| 887 | + | return fmt.Errorf("deployment: record retirement of %s/%s: %w", app, containerID, err) | |
| 888 | + | } | |
| 889 | + | return nil | |
| 890 | + | } | |
| 891 | + | ||
| 802 | 892 | // clearRecoveryInhibition drops the inhibition of a safely superseded | |
| 803 | 893 | // generation. | |
| 804 | 894 | func (s *Service) clearRecoveryInhibition(ctx context.Context, app, service, containerID string) error { | |
| @@ -288,6 +288,14 @@ 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 | + | ||||
| 291 | // forget drops one generation's observation. | 299 | // forget drops one generation's observation. | ||
| 292 | func (t *executionTracker) forget(key recoveryKey) { | 300 | func (t *executionTracker) forget(key recoveryKey) { | ||
| 293 | t.mu.Lock() | 301 | t.mu.Lock() | ||
| @@ -358,7 +366,9 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 358 | if err != nil { | 366 | if err != nil { | ||
| 359 | return fmt.Errorf("deployment: reconcile %q: load recovery inhibitions: %w", app, err) | 367 | return fmt.Errorf("deployment: reconcile %q: load recovery inhibitions: %w", app, err) | ||
| 360 | } | 368 | } | ||
| 361 | - | inhibited, failures := s.processRecoveryInhibitions(ctx, app, inhibitions) | 369 | + | inhibited, pending := splitRecoveryInhibitions(app, inhibitions) |
| 370 | + | var failures []error | |||
| 371 | + | failed := map[string]struct{}{} | |||
| 362 | for _, name := range sortedServiceNames(active) { | 372 | for _, name := range sortedServiceNames(active) { | ||
| 363 | eff := active.Services[name] | 373 | eff := active.Services[name] | ||
| 364 | if intent.Stopped { | 374 | if intent.Stopped { | ||
| @@ -369,6 +379,7 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 369 | } | 379 | } | ||
| 370 | if err := s.reconcileRunningService(ctx, app, name, eff, inhibited); err != nil { | 380 | if err := s.reconcileRunningService(ctx, app, name, eff, inhibited); err != nil { | ||
| 371 | failures = append(failures, err) | 381 | failures = append(failures, err) | ||
| 382 | + | failed[name] = struct{}{} | |||
| 372 | } | 383 | } | ||
| 373 | } | 384 | } | ||
| 374 | if intent.Stopped { | 385 | if intent.Stopped { | ||
| @@ -381,24 +392,77 @@ func (s *Service) reconcileAppRunning(ctx context.Context, app string) error { | |||||
| 381 | } | 392 | } | ||
| 382 | cancel() | 393 | cancel() | ||
| 383 | } | 394 | } | ||
| 395 | + | // Superseded containers are retired last: only after their successors | |||
| 396 | + | // were verified and routed in this pass. | |||
| 397 | + | failures = append(failures, s.retirePendingContainers(ctx, app, intent.Stopped, active, pending, failed)...) | |||
| 384 | s.pruneRecoveryHistory(app, active) | 398 | s.pruneRecoveryHistory(app, active) | ||
| 385 | return errors.Join(failures...) | 399 | return errors.Join(failures...) | ||
| 386 | } | 400 | } | ||
| 387 | 401 | ||||
| 388 | - | func (s *Service) processRecoveryInhibitions(ctx context.Context, app string, inhibitions []domain.AppRecoveryInhibition) (map[recoveryKey]struct{}, []error) { | 402 | + | // splitRecoveryInhibitions separates the generations recovery must never |
| 403 | + | // start from the superseded containers waiting to be retired. | |||
| 404 | + | func splitRecoveryInhibitions(app string, inhibitions []domain.AppRecoveryInhibition) (map[recoveryKey]struct{}, []domain.AppRecoveryInhibition) { | |||
| 389 | inhibited := map[recoveryKey]struct{}{} | 405 | inhibited := map[recoveryKey]struct{}{} | ||
| 390 | - | var failures []error | 406 | + | var pending []domain.AppRecoveryInhibition |
| 391 | for _, inhibition := range inhibitions { | 407 | for _, inhibition := range inhibitions { | ||
| 392 | - | if inhibition.Reason != domain.AppInhibitRetirementPending { | 408 | + | if inhibition.Reason == domain.AppInhibitRetirementPending { |
| 393 | - | inhibited[recoveryKey{app: app, service: inhibition.Service, container: inhibition.ContainerID}] = struct{}{} | 409 | + | pending = append(pending, inhibition) |
| 410 | + | continue | |||
| 411 | + | } | |||
| 412 | + | inhibited[recoveryKey{app: app, service: inhibition.Service, container: inhibition.ContainerID}] = struct{}{} | |||
| 413 | + | } | |||
| 414 | + | return inhibited, pending | |||
| 415 | + | } | |||
| 416 | + | ||||
| 417 | + | // retirePendingContainers retires the superseded containers an overlap | |||
| 418 | + | // replacement left running, with the stop grace recorded at the marker | |||
| 419 | + | // (the app default for a record that has none). A container is retired only | |||
| 420 | + | // when it can no longer be the serving one: | |||
| 421 | + | // - ACTIVE names it (the replacement never took over): it is the serving | |||
| 422 | + | // 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. | |||
| 426 | + | // | |||
| 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 { | |||
| 429 | + | var failures []error | |||
| 430 | + | for _, marker := range pending { | |||
| 431 | + | if publishedContainer(active, marker.ContainerID) { | |||
| 432 | + | if err := s.clearRecoveryInhibition(ctx, app, marker.Service, marker.ContainerID); err != nil { | |||
| 433 | + | failures = append(failures, err) | |||
| 434 | + | } | |||
| 435 | + | continue | |||
| 436 | + | } | |||
| 437 | + | if !stopped && !s.successorVerified(ctx, app, marker.Service, active, failed) { | |||
| 394 | continue | 438 | continue | ||
| 395 | } | 439 | } | ||
| 396 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: inhibition.Service}, inhibition.ContainerID) | 440 | + | retired := s.retireContainer(ctx, app, retireOptions{ |
| 441 | + | Service: marker.Service, Grace: marker.StopGrace, ClearInhibition: true, | |||
| 442 | + | }, marker.ContainerID) | |||
| 397 | if !retired.Gone { | 443 | if !retired.Gone { | ||
| 398 | - | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s", app, inhibition.ContainerID)) | 444 | + | failures = append(failures, fmt.Errorf("deployment: retirement pending for %s/%s: %s", app, marker.ContainerID, cleanupDetail(retired))) |
| 399 | } | 445 | } | ||
| 400 | } | 446 | } | ||
| 401 | - | return inhibited, failures | 447 | + | return failures |
| 448 | + | } | |||
| 449 | + | ||||
| 450 | + | // 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 | + | successor, ok := active.Services[service] | |||
| 458 | + | if !ok || successor.Container == "" { | |||
| 459 | + | return true | |||
| 460 | + | } | |||
| 461 | + | if !s.executions.verified(recoveryKey{app: app, service: service, container: successor.Container}) { | |||
| 462 | + | return false | |||
| 463 | + | } | |||
| 464 | + | container, err := s.deps.Runtime.InspectContainer(ctx, successor.Container) | |||
| 465 | + | return err == nil && container.Status == string(domain.ContainerStatusRunning) | |||
| 402 | } | 466 | } | ||
| 403 | 467 | ||||
| 404 | // convergeStoppedService enforces durable stopped intent: it never | 468 | // convergeStoppedService enforces durable stopped intent: it never | ||
| @@ -762,7 +826,10 @@ func (s *Service) withdrawForRecovery(ctx context.Context, app, service string) | |||||
| 762 | } | 826 | } | ||
| 763 | 827 | ||||
| 764 | // recoveryInhibited reports whether one exact generation is durably | 828 | // recoveryInhibited reports whether one exact generation is durably | ||
| 765 | - | // inhibited for recovery. | 829 | + | // inhibited for recovery. A retirement-pending marker does not count: it |
| 830 | + | // names a superseded container that is being retired, not a generation | |||
| 831 | + | // recovery must keep stopped, and when ACTIVE still names that container | |||
| 832 | + | // (the replacement never took over) it must stay startable. | |||
| 766 | func (s *Service) recoveryInhibited(ctx context.Context, app, service, containerID string) (bool, error) { | 833 | func (s *Service) recoveryInhibited(ctx context.Context, app, service, containerID string) (bool, error) { | ||
| 767 | if containerID == "" { | 834 | if containerID == "" { | ||
| 768 | return false, nil | 835 | return false, nil | ||
| @@ -772,7 +839,8 @@ func (s *Service) recoveryInhibited(ctx context.Context, app, service, container | |||||
| 772 | return false, fmt.Errorf("deployment: load recovery inhibitions: %w", err) | 839 | return false, fmt.Errorf("deployment: load recovery inhibitions: %w", err) | ||
| 773 | } | 840 | } | ||
| 774 | for _, inhibition := range inhibitions { | 841 | for _, inhibition := range inhibitions { | ||
| 775 | - | if inhibition.Service == service && inhibition.ContainerID == containerID { | 842 | + | if inhibition.Service == service && inhibition.ContainerID == containerID && |
| 843 | + | inhibition.Reason != domain.AppInhibitRetirementPending { | |||
| 776 | return true, nil | 844 | return true, nil | ||
| 777 | } | 845 | } | ||
| 778 | } | 846 | } | ||
| @@ -799,6 +867,28 @@ func (s *Service) inhibitRecovery(ctx context.Context, app, service, containerID | |||||
| 799 | return nil | 867 | return nil | ||
| 800 | } | 868 | } | ||
| 801 | 869 | ||||
| 870 | + | // inhibitRetirement durably marks a superseded container that an overlap | |||
| 871 | + | // replacement left running beside its successor. The periodic reconciliation | |||
| 872 | + | // retires it with grace, once the successor is verified and routed. | |||
| 873 | + | func (s *Service) inhibitRetirement(ctx context.Context, app, service, containerID, operation string, grace time.Duration) error { | |||
| 874 | + | if containerID == "" { | |||
| 875 | + | return nil | |||
| 876 | + | } | |||
| 877 | + | inhibition := domain.AppRecoveryInhibition{ | |||
| 878 | + | App: app, | |||
| 879 | + | Service: service, | |||
| 880 | + | ContainerID: containerID, | |||
| 881 | + | Reason: domain.AppInhibitRetirementPending, | |||
| 882 | + | Operation: operation, | |||
| 883 | + | CreatedAt: time.Now().UTC(), | |||
| 884 | + | StopGrace: grace, | |||
| 885 | + | } | |||
| 886 | + | if err := s.deps.State.SaveRecoveryInhibition(ctx, inhibition); err != nil { | |||
| 887 | + | return fmt.Errorf("deployment: record retirement of %s/%s: %w", app, containerID, err) | |||
| 888 | + | } | |||
| 889 | + | return nil | |||
| 890 | + | } | |||
| 891 | + | ||||
| 802 | // clearRecoveryInhibition drops the inhibition of a safely superseded | 892 | // clearRecoveryInhibition drops the inhibition of a safely superseded | ||
| 803 | // generation. | 893 | // generation. | ||
| 804 | func (s *Service) clearRecoveryInhibition(ctx context.Context, app, service, containerID string) error { | 894 | func (s *Service) clearRecoveryInhibition(ctx context.Context, app, service, containerID string) error { | ||
M internal/usecase/deployment/overlap_replacement_test.go +358 -106
| @@ -2,8 +2,10 @@ package deployment_test | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "maps" | |
| 5 | 6 | "os" | |
| 6 | 7 | "path/filepath" | |
| 8 | + | "sync" | |
| 7 | 9 | "testing" | |
| 8 | 10 | "time" | |
| 9 | 11 | ||
| @@ -12,6 +14,7 @@ import ( | |||
| 12 | 14 | "github.com/stretchr/testify/mock" | |
| 13 | 15 | "github.com/stretchr/testify/require" | |
| 14 | 16 | ||
| 17 | + | "github.com/bnema/gordon/internal/adapters/out/appstate" | |
| 15 | 18 | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | |
| 16 | 19 | "github.com/bnema/gordon/internal/domain" | |
| 17 | 20 | "github.com/bnema/gordon/internal/usecase/deployment" | |
| @@ -37,6 +40,9 @@ func TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder(t *testing.T) { | |||
| 37 | 40 | s.Binds = []domain.AppBind{{Name: "config", Path: "/etc/app.conf", ReadOnly: true}} | |
| 38 | 41 | }, true, false}, | |
| 39 | 42 | {"device", func(s *domain.AppService) { s.Devices = []string{"test_gpu"} }, false, true}, | |
| 43 | + | {"udp", func(s *domain.AppService) { | |
| 44 | + | s.UDP = []domain.AppUDPInterface{{Entrypoint: "game", Port: 28015, Publish: "0.0.0.0:28015"}} | |
| 45 | + | }, false, false}, | |
| 40 | 46 | } | |
| 41 | 47 | for _, tc := range cases { | |
| 42 | 48 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -110,6 +116,7 @@ func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |||
| 110 | 116 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 111 | 117 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 112 | 118 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 119 | + | expectRetirementMarker(state, "c-old") | |
| 113 | 120 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(assert.AnError).Once() | |
| 114 | 121 | ||
| 115 | 122 | var events []string | |
| @@ -133,153 +140,398 @@ func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |||
| 133 | 140 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |
| 134 | 141 | } | |
| 135 | 142 | ||
| 136 | - | // seedSwitchedApp seeds an app whose ACTIVE record already names the new | |
| 137 | - | // container c-new, while the superseded c-old was left running by an overlap | |
| 138 | - | // replacement that stopped before retiring it. The caller supplies the journal | |
| 139 | - | // step recording that replacement. | |
| 140 | - | func seedSwitchedApp(t *testing.T, ctx context.Context, store interface { | |
| 141 | - | SaveIntent(context.Context, domain.AppStopIntent) error | |
| 142 | - | SaveActive(context.Context, domain.AppActive) error | |
| 143 | - | SaveOperation(context.Context, domain.AppOperation) error | |
| 144 | - | }, terminal bool, step domain.AppOperationStep) { | |
| 143 | + | // storeTraffic is a traffic publisher that behaves like the real one against | |
| 144 | + | // a real app-state store: a withdrawal drops the service's recorded binds from | |
| 145 | + | // ACTIVE (and can then report a failed graph apply), and a rebuild routes | |
| 146 | + | // exactly the binds ACTIVE records at that moment. Tests assert on what is | |
| 147 | + | // routed, not on call order. | |
| 148 | + | type storeTraffic struct { | |
| 149 | + | mu sync.Mutex | |
| 150 | + | store *appstate.Store | |
| 151 | + | failWithdraw int | |
| 152 | + | failRebuild int | |
| 153 | + | routed []map[string]map[int]int | |
| 154 | + | } | |
| 155 | + | ||
| 156 | + | func (t *storeTraffic) WithdrawService(ctx context.Context, app, service string) error { | |
| 157 | + | t.mu.Lock() | |
| 158 | + | defer t.mu.Unlock() | |
| 159 | + | active, ok, err := t.store.LoadActive(ctx, app) | |
| 160 | + | if err != nil || !ok { | |
| 161 | + | return err | |
| 162 | + | } | |
| 163 | + | if eff, found := active.Services[service]; found { | |
| 164 | + | eff.BackendBinds, eff.UDPBackendBinds = nil, nil | |
| 165 | + | active.Services[service] = eff | |
| 166 | + | if err := t.store.SaveActive(ctx, active); err != nil { | |
| 167 | + | return err | |
| 168 | + | } | |
| 169 | + | } | |
| 170 | + | if t.failWithdraw > 0 { | |
| 171 | + | t.failWithdraw-- | |
| 172 | + | return assert.AnError | |
| 173 | + | } | |
| 174 | + | return nil | |
| 175 | + | } | |
| 176 | + | ||
| 177 | + | func (t *storeTraffic) WithdrawServiceState(ctx context.Context, app, service string) error { | |
| 178 | + | return t.WithdrawService(ctx, app, service) | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | func (t *storeTraffic) RebuildTraffic(ctx context.Context) error { | |
| 182 | + | t.mu.Lock() | |
| 183 | + | defer t.mu.Unlock() | |
| 184 | + | if t.failRebuild > 0 { | |
| 185 | + | t.failRebuild-- | |
| 186 | + | return assert.AnError | |
| 187 | + | } | |
| 188 | + | active, _, err := t.store.LoadActive(ctx, "blog") | |
| 189 | + | if err != nil { | |
| 190 | + | return err | |
| 191 | + | } | |
| 192 | + | routes := map[string]map[int]int{} | |
| 193 | + | for name, eff := range active.Services { | |
| 194 | + | routes[name] = maps.Clone(eff.BackendBinds) | |
| 195 | + | } | |
| 196 | + | t.routed = append(t.routed, routes) | |
| 197 | + | return nil | |
| 198 | + | } | |
| 199 | + | ||
| 200 | + | // lastRoute returns the binds the most recent graph routed for a service. | |
| 201 | + | func (t *storeTraffic) lastRoute(service string) map[int]int { | |
| 202 | + | t.mu.Lock() | |
| 203 | + | defer t.mu.Unlock() | |
| 204 | + | if len(t.routed) == 0 { | |
| 205 | + | return nil | |
| 206 | + | } | |
| 207 | + | return t.routed[len(t.routed)-1][service] | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | // allRoutes returns the binds of every graph routed for a service. | |
| 211 | + | func (t *storeTraffic) allRoutes(service string) []map[int]int { | |
| 212 | + | t.mu.Lock() | |
| 213 | + | defer t.mu.Unlock() | |
| 214 | + | var routes []map[int]int | |
| 215 | + | for _, routed := range t.routed { | |
| 216 | + | routes = append(routes, routed[service]) | |
| 217 | + | } | |
| 218 | + | return routes | |
| 219 | + | } | |
| 220 | + | ||
| 221 | + | func (t *storeTraffic) rebuildCount() int { | |
| 222 | + | t.mu.Lock() | |
| 223 | + | defer t.mu.Unlock() | |
| 224 | + | return len(t.routed) | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | const overlapImage = "registry.example.com/blog/web:1.4.2" | |
| 228 | + | ||
| 229 | + | // overlapWebSpec is a stateless public HTTP service. | |
| 230 | + | func overlapWebSpec(grace time.Duration) domain.AppService { | |
| 231 | + | return domain.AppService{ | |
| 232 | + | Name: "web", Image: overlapImage, Secrets: map[string]string{}, StopGrace: grace, | |
| 233 | + | HTTP: []domain.AppHTTPInterface{{Host: "blog.example.com", Port: 8080, TLS: domain.AppTLSAuto}}, | |
| 234 | + | Readiness: domain.AppReadiness{ | |
| 235 | + | Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second, | |
| 236 | + | }, | |
| 237 | + | } | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | func overlapOldBinds() map[int]int { return map[int]int{8080: 17000} } | |
| 241 | + | ||
| 242 | + | // seedServingApp seeds an app whose container c-old serves on 17000, with a | |
| 243 | + | // materialized revision to deploy over it. | |
| 244 | + | func seedServingApp(t *testing.T, ctx context.Context, store *appstate.Store, spec domain.AppService) { | |
| 145 | 245 | t.Helper() | |
| 146 | - | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |
| 246 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |
| 247 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |
| 147 | 248 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 148 | - | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: headlessSpec(), BackendBinds: map[int]int{8080: 18081}}, | |
| 249 | + | "web": {Container: "c-old", EffectiveRevision: "rev-0", Spec: spec, BackendBinds: overlapOldBinds()}, | |
| 149 | 250 | }})) | |
| 150 | - | op := domain.AppOperation{ | |
| 151 | - | Op: "op-overlap", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), | |
| 152 | - | Steps: []domain.AppOperationStep{{ID: "preflight", State: domain.AppStepSucceeded}, step}, | |
| 153 | - | } | |
| 154 | - | if terminal { | |
| 155 | - | op.Outcome = domain.AppOutcomeSuccess | |
| 156 | - | } | |
| 157 | - | require.NoError(t, store.SaveOperation(ctx, op)) | |
| 251 | + | seedDesired(t, ctx, store, testRevision("blog", spec)) | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | // overlapDeployDeps wires the preflight and candidate creation of one overlap | |
| 255 | + | // deploy that creates c-new on 18081 and expects readiness to pass. | |
| 256 | + | func overlapDeployMocks(runtime *outmocks.MockContainerRuntime, images *outmocks.MockImageResolver) { | |
| 257 | + | images.EXPECT().ResolveDigest(mock.Anything, overlapImage).Return(restartTestDigest, nil).Once() | |
| 258 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, overlapImage).Return(nil, nil).Once() | |
| 259 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 260 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |
| 261 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 262 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 263 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 264 | + | } | |
| 265 | + | ||
| 266 | + | func overlapService(store *appstate.Store, runtime *outmocks.MockContainerRuntime, images *outmocks.MockImageResolver, secrets *outmocks.MockSecretProvider, traffic *storeTraffic) *deployment.Service { | |
| 267 | + | return deployment.NewService(deployment.Deps{ | |
| 268 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, Traffic: traffic, | |
| 269 | + | ImagePolicy: domain.ImageSourcePolicy{AllowedRegistries: []string{"registry.example.com"}}, | |
| 270 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 271 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 272 | + | func(context.Context, string) error { return nil }, | |
| 273 | + | )) | |
| 158 | 274 | } | |
| 159 | 275 | ||
| 160 | - | // TestBootRecovery_RetiresSupersededContainerAfterInterruptedSwitch proves the | |
| 161 | - | // crash window after the switch: ACTIVE names the new container, the old one | |
| 162 | - | // is still running and unpublished, so reconciliation republishes routing and | |
| 163 | - | // then stops it gracefully with the default grace. | |
| 164 | - | func TestBootRecovery_RetiresSupersededContainerAfterInterruptedSwitch(t *testing.T) { | |
| 276 | + | // expectVerifiedRunning wires one recovery verification of a running | |
| 277 | + | // container that publishes the given loopback port. | |
| 278 | + | func expectVerifiedRunning(runtime *outmocks.MockContainerRuntime, id string, port int) { | |
| 279 | + | started := time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC) | |
| 280 | + | runtime.EXPECT().InspectContainer(mock.Anything, id). | |
| 281 | + | Return(&domain.Container{ID: id, Status: "running", StartedAt: started}, nil) | |
| 282 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, id).Return("", false, nil) | |
| 283 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, id, mock.Anything).Return( | |
| 284 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: port, Protocol: domain.NetworkProtocolTCP}}, nil) | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | // TestDeploy_OverlapSwitchIsNotErasedByAPendingWithdrawal proves that a | |
| 288 | + | // publication withdrawal left pending by an earlier failed recovery does not | |
| 289 | + | // unroute the new generation: the deploy resolves it before publishing, so | |
| 290 | + | // the graph the switch applies routes the candidate's binds. The traffic fake | |
| 291 | + | // mutates ACTIVE like the real publisher, so a stray withdrawal after the | |
| 292 | + | // switch would leave the service unrouted. | |
| 293 | + | func TestDeploy_OverlapSwitchIsNotErasedByAPendingWithdrawal(t *testing.T) { | |
| 165 | 294 | ctx := context.Background() | |
| 166 | 295 | store := newTestStore(t) | |
| 296 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 167 | 297 | images := outmocks.NewMockImageResolver(t) | |
| 168 | 298 | secrets := outmocks.NewMockSecretProvider(t) | |
| 169 | - | seedSwitchedApp(t, ctx, store, false, domain.AppOperationStep{ | |
| 170 | - | ID: "service.web.replace", State: domain.AppStepPending, Service: "web", | |
| 171 | - | Before: "c-old", After: "c-new", Overlap: true, | |
| 172 | - | }) | |
| 299 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) | |
| 300 | + | traffic := &storeTraffic{store: store, failWithdraw: 1} | |
| 301 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 302 | + | ||
| 303 | + | // A recovery pass finds c-old down and cannot prove its withdrawal: the | |
| 304 | + | // service is marked as having a pending withdrawal. | |
| 305 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |
| 306 | + | Return(&domain.Container{ID: "c-old", Status: "exited"}, nil).Once() | |
| 307 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |
| 308 | + | ||
| 309 | + | overlapDeployMocks(runtime, images) | |
| 310 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |
| 311 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 312 | + | ||
| 313 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 314 | + | require.NoError(t, err) | |
| 315 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 316 | + | ||
| 317 | + | assert.Equal(t, map[int]int{8080: 18081}, traffic.lastRoute("web"), "the service routes the new generation") | |
| 318 | + | active, _, err := store.LoadActive(ctx, "blog") | |
| 319 | + | require.NoError(t, err) | |
| 320 | + | assert.Equal(t, "c-new", active.Services["web"].Container) | |
| 321 | + | assert.Equal(t, map[int]int{8080: 18081}, active.Services["web"].BackendBinds, | |
| 322 | + | "ACTIVE keeps only the new generation's binds") | |
| 323 | + | } | |
| 173 | 324 | ||
| 174 | - | var order []string | |
| 325 | + | // TestDeploy_OverlapRetirementSurvivesANewerOperation proves the cleanup of a | |
| 326 | + | // superseded container is owned by the durable retirement marker, not by the | |
| 327 | + | // operation journal: after the first retirement fails and a newer operation | |
| 328 | + | // becomes the latest journal, the periodic reconciliation still retires it, | |
| 329 | + | // with the predecessor's own stop grace, and only once its successor is | |
| 330 | + | // verified and routed. | |
| 331 | + | func TestDeploy_OverlapRetirementSurvivesANewerOperation(t *testing.T) { | |
| 332 | + | ctx := context.Background() | |
| 333 | + | store := newTestStore(t) | |
| 175 | 334 | runtime := outmocks.NewMockContainerRuntime(t) | |
| 176 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).RunAndReturn( | |
| 177 | - | func(context.Context, string, time.Duration) error { | |
| 178 | - | recordEvent(&order, "stop-old") | |
| 179 | - | return nil | |
| 180 | - | }).Once() | |
| 181 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).RunAndReturn( | |
| 182 | - | func(context.Context, string, bool) error { | |
| 183 | - | recordEvent(&order, "remove-old") | |
| 184 | - | return nil | |
| 185 | - | }).Once() | |
| 186 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "exited"}, nil) | |
| 335 | + | images := outmocks.NewMockImageResolver(t) | |
| 336 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 337 | + | seedServingApp(t, ctx, store, overlapWebSpec(45*time.Second)) | |
| 338 | + | traffic := &storeTraffic{store: store} | |
| 339 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 187 | 340 | ||
| 188 | - | svc := deployment.NewService(deployment.Deps{ | |
| 189 | - | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |
| 190 | - | Traffic: &orderedTraffic{events: &order}, | |
| 191 | - | }, zerowrap.Default()) | |
| 192 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |
| 341 | + | overlapDeployMocks(runtime, images) | |
| 342 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", 45*time.Second).Return(nil).Twice() | |
| 343 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 193 | 344 | ||
| 194 | - | assert.Equal(t, []string{"traffic", "stop-old", "remove-old"}, order, | |
| 195 | - | "routing is republished to the published container before the superseded one is stopped") | |
| 196 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-new", mock.Anything) | |
| 345 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 346 | + | require.NoError(t, err, "a retirement leftover does not fail a deploy whose new generation serves") | |
| 347 | + | require.Len(t, result.CleanupWarnings, 1) | |
| 348 | + | assert.Equal(t, "c-old", result.CleanupWarnings[0].Leftover) | |
| 197 | 349 | ||
| 198 | - | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |
| 350 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 199 | 351 | require.NoError(t, err) | |
| 200 | - | require.True(t, op.Terminal()) | |
| 201 | - | step, found := opStep(op, "service.web.replace") | |
| 202 | - | require.True(t, found) | |
| 203 | - | assert.True(t, step.BeforeRetired) | |
| 204 | - | assert.Empty(t, step.After, "a converged step is not retried") | |
| 205 | - | assert.Contains(t, step.Detail, "retired superseded container c-old") | |
| 352 | + | require.Len(t, markers, 1, "the failed retirement leaves its durable marker") | |
| 353 | + | assert.Equal(t, "c-old", markers[0].ContainerID) | |
| 354 | + | assert.Equal(t, domain.AppInhibitRetirementPending, markers[0].Reason) | |
| 355 | + | assert.Equal(t, 45*time.Second, markers[0].StopGrace, "the predecessor's grace travels with the marker") | |
| 356 | + | ||
| 357 | + | // A newer operation becomes the latest journal and masks the deploy's. | |
| 358 | + | require.NoError(t, store.SaveOperation(ctx, domain.AppOperation{ | |
| 359 | + | Op: "op-newer", Kind: "deploy", App: "blog", StartedAt: time.Now().Add(time.Minute).UTC(), | |
| 360 | + | Outcome: domain.AppOutcomeSuccess, | |
| 361 | + | })) | |
| 362 | + | ||
| 363 | + | expectVerifiedRunning(runtime, "c-new", 18082) | |
| 364 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 365 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 366 | + | ||
| 367 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |
| 368 | + | require.NoError(t, err) | |
| 369 | + | assert.Empty(t, markers, "the marker is cleared once the container is confirmed gone") | |
| 370 | + | assert.Equal(t, map[int]int{8080: 18082}, traffic.lastRoute("web"), "the verified successor is routed") | |
| 206 | 371 | active, _, err := store.LoadActive(ctx, "blog") | |
| 207 | 372 | require.NoError(t, err) | |
| 208 | 373 | assert.Equal(t, "c-new", active.Services["web"].Container) | |
| 209 | - | ||
| 210 | - | // A converged step is not retried on the next pass. | |
| 211 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |
| 212 | - | runtime.AssertNumberOfCalls(t, "StopContainer", 1) | |
| 213 | 374 | } | |
| 214 | 375 | ||
| 215 | - | // TestReconcile_RetriesSupersededContainerRetirementOfASucceededDeploy proves a | |
| 216 | - | // deploy that succeeded with a cleanup warning (the old container could not be | |
| 217 | - | // removed) is retried by the next reconciliation, and that a failure to retire | |
| 218 | - | // is a warning that does not block the app. | |
| 219 | - | func TestReconcile_RetriesSupersededContainerRetirementOfASucceededDeploy(t *testing.T) { | |
| 376 | + | // TestReconcileRunning_RetiresSupersededOnlyAfterSuccessorIsVerified proves a | |
| 377 | + | // superseded container is kept while its successor cannot be verified: the | |
| 378 | + | // working generation is never stopped in favour of one that is not serving. | |
| 379 | + | func TestReconcileRunning_RetiresSupersededOnlyAfterSuccessorIsVerified(t *testing.T) { | |
| 220 | 380 | ctx := context.Background() | |
| 221 | 381 | store := newTestStore(t) | |
| 382 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 222 | 383 | images := outmocks.NewMockImageResolver(t) | |
| 223 | 384 | secrets := outmocks.NewMockSecretProvider(t) | |
| 224 | - | seedSwitchedApp(t, ctx, store, true, domain.AppOperationStep{ | |
| 225 | - | ID: "service.web.replace", State: domain.AppStepSucceeded, Service: "web", | |
| 226 | - | Before: "c-old", After: "c-new", Overlap: true, | |
| 227 | - | }) | |
| 385 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) | |
| 386 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 387 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: overlapWebSpec(0), BackendBinds: map[int]int{8080: 18081}}, | |
| 388 | + | }})) | |
| 389 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |
| 390 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, | |
| 391 | + | })) | |
| 392 | + | traffic := &storeTraffic{store: store} | |
| 393 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 394 | + | ||
| 395 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(nil, assert.AnError).Once() | |
| 396 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |
| 397 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 398 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 399 | + | require.NoError(t, err) | |
| 400 | + | assert.Len(t, markers, 1, "the marker waits for a verified successor") | |
| 401 | + | ||
| 402 | + | // A record written before the grace was persisted falls back to the default. | |
| 403 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |
| 404 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |
| 405 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 406 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 407 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |
| 408 | + | require.NoError(t, err) | |
| 409 | + | assert.Empty(t, markers) | |
| 410 | + | } | |
| 228 | 411 | ||
| 412 | + | // TestReconcileRunning_NeverRetiresAPendingContainerThatActiveNamesAgain | |
| 413 | + | // proves a rollback to the superseded container is safe: when ACTIVE names the | |
| 414 | + | // container again, its stale retirement marker is dropped and it is kept. | |
| 415 | + | func TestReconcileRunning_NeverRetiresAPendingContainerThatActiveNamesAgain(t *testing.T) { | |
| 416 | + | ctx := context.Background() | |
| 417 | + | store := newTestStore(t) | |
| 229 | 418 | runtime := outmocks.NewMockContainerRuntime(t) | |
| 230 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "exited"}, nil) | |
| 231 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil) | |
| 232 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 233 | - | svc := deployment.NewService(deployment.Deps{ | |
| 234 | - | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |
| 235 | - | Traffic: &orderedTraffic{}, | |
| 236 | - | }, zerowrap.Default()) | |
| 419 | + | images := outmocks.NewMockImageResolver(t) | |
| 420 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 421 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) | |
| 422 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |
| 423 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, | |
| 424 | + | })) | |
| 425 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) | |
| 237 | 426 | ||
| 238 | - | require.NoError(t, svc.ReconcileBoot(ctx), "a leftover old container does not block reconciliation") | |
| 239 | - | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |
| 427 | + | expectVerifiedRunning(runtime, "c-old", 17000) | |
| 428 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 429 | + | ||
| 430 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 431 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 432 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 240 | 433 | require.NoError(t, err) | |
| 241 | - | step, _ := opStep(op, "service.web.replace") | |
| 242 | - | assert.False(t, step.BeforeRetired, "the retirement failed and stays pending") | |
| 243 | - | assert.Equal(t, domain.AppStepSucceeded, step.State, "the deploy outcome is untouched") | |
| 244 | - | assert.Equal(t, domain.AppOutcomeSuccess, op.Outcome) | |
| 245 | - | require.Len(t, op.Warnings, 1) | |
| 246 | - | assert.Equal(t, "c-old", op.Warnings[0].Leftover) | |
| 247 | - | ||
| 248 | - | // The next pass retries, and now succeeds. | |
| 249 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 250 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |
| 251 | - | op, err = store.LoadOperation(ctx, "blog", "op-overlap") | |
| 434 | + | assert.Empty(t, markers) | |
| 435 | + | active, _, err := store.LoadActive(ctx, "blog") | |
| 252 | 436 | require.NoError(t, err) | |
| 253 | - | step, _ = opStep(op, "service.web.replace") | |
| 254 | - | assert.True(t, step.BeforeRetired) | |
| 437 | + | assert.Equal(t, "c-old", active.Services["web"].Container) | |
| 255 | 438 | } | |
| 256 | 439 | ||
| 257 | - | // TestReconcile_NeverRetiresASupersededContainerThatActiveNamesAgain proves | |
| 258 | - | // reconciliation does not stop a container ACTIVE currently publishes, such | |
| 259 | - | // as when a rollback re-published the previous generation. | |
| 260 | - | func TestReconcile_NeverRetiresASupersededContainerThatActiveNamesAgain(t *testing.T) { | |
| 440 | + | // TestBootRecovery_DoesNotRepublishUnverifiedBindsOfAPublishedCandidate proves | |
| 441 | + | // interrupted-operation reconciliation never rebuilds traffic from the binds | |
| 442 | + | // ACTIVE persisted for a published candidate: a restarted candidate's | |
| 443 | + | // ephemeral port may belong to another workload. Only the verified start path | |
| 444 | + | // routes, and the superseded container is retired afterwards through its | |
| 445 | + | // marker. | |
| 446 | + | func TestBootRecovery_DoesNotRepublishUnverifiedBindsOfAPublishedCandidate(t *testing.T) { | |
| 261 | 447 | ctx := context.Background() | |
| 262 | 448 | store := newTestStore(t) | |
| 449 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 263 | 450 | images := outmocks.NewMockImageResolver(t) | |
| 264 | 451 | secrets := outmocks.NewMockSecretProvider(t) | |
| 265 | - | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |
| 452 | + | spec := overlapWebSpec(0) | |
| 453 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |
| 266 | 454 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 267 | - | "web": {Container: "c-old", EffectiveRevision: "rev-0", Spec: headlessSpec()}, | |
| 455 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: spec, BackendBinds: map[int]int{8080: 18081}}, | |
| 268 | 456 | }})) | |
| 269 | 457 | require.NoError(t, store.SaveOperation(ctx, domain.AppOperation{ | |
| 270 | - | Op: "op-overlap", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), Outcome: domain.AppOutcomeFailed, | |
| 271 | - | Steps: []domain.AppOperationStep{{ | |
| 272 | - | ID: "service.web.replace", State: domain.AppStepFailed, Service: "web", | |
| 273 | - | Before: "c-old", After: "c-new", Overlap: true, | |
| 274 | - | }}, | |
| 458 | + | Op: "op-switch", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), | |
| 459 | + | Steps: []domain.AppOperationStep{ | |
| 460 | + | {ID: "preflight", State: domain.AppStepSucceeded}, | |
| 461 | + | {ID: "service.web.replace", State: domain.AppStepPending, Service: "web", Before: "c-old", After: "c-new"}, | |
| 462 | + | }, | |
| 275 | 463 | })) | |
| 464 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |
| 465 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, | |
| 466 | + | })) | |
| 467 | + | traffic := &storeTraffic{store: store} | |
| 468 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 276 | 469 | ||
| 277 | - | runtime := outmocks.NewMockContainerRuntime(t) | |
| 278 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(nil).Once() | |
| 279 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-old").Return(&domain.Container{ID: "c-old", Status: "exited"}, nil) | |
| 280 | - | svc := deployment.NewService(deployment.Deps{State: store, Runtime: runtime, Images: images, Secrets: secrets}, zerowrap.Default()) | |
| 470 | + | // The candidate restarted and now publishes a different port. | |
| 471 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "c-new").Return(true, nil).Once() | |
| 472 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 473 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18090, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 281 | 474 | require.NoError(t, svc.ReconcileBoot(ctx)) | |
| 282 | 475 | ||
| 476 | + | require.Positive(t, traffic.rebuildCount()) | |
| 477 | + | for _, routed := range traffic.allRoutes("web") { | |
| 478 | + | assert.NotEqual(t, map[int]int{8080: 18081}, routed, "the unverified persisted port is never routed") | |
| 479 | + | } | |
| 480 | + | assert.Equal(t, map[int]int{8080: 18090}, traffic.lastRoute("web"), "the verified bind is routed") | |
| 481 | + | active, _, err := store.LoadActive(ctx, "blog") | |
| 482 | + | require.NoError(t, err) | |
| 483 | + | assert.Equal(t, map[int]int{8080: 18090}, active.Services["web"].BackendBinds) | |
| 484 | + | ||
| 485 | + | // Boot keeps the superseded container and its marker: the periodic pass | |
| 486 | + | // retires it once the successor is verified. | |
| 283 | 487 | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 284 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |
| 488 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 489 | + | require.NoError(t, err) | |
| 490 | + | require.Len(t, markers, 1) | |
| 491 | + | assert.Equal(t, "c-old", markers[0].ContainerID) | |
| 492 | + | } | |
| 493 | + | ||
| 494 | + | // TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass proves the | |
| 495 | + | // failed switch is recoverable and traceable: the graph apply is rejected after | |
| 496 | + | // ACTIVE named the new container, the superseded container keeps running with | |
| 497 | + | // its marker, the next deploy-side reconciliation keeps that marker, and the | |
| 498 | + | // periodic pass retires the old container once the successor is routed. | |
| 499 | + | func TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass(t *testing.T) { | |
| 500 | + | ctx := context.Background() | |
| 501 | + | store := newTestStore(t) | |
| 502 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 503 | + | images := outmocks.NewMockImageResolver(t) | |
| 504 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 505 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) | |
| 506 | + | traffic := &storeTraffic{store: store, failRebuild: 1} | |
| 507 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |
| 508 | + | ||
| 509 | + | overlapDeployMocks(runtime, images) | |
| 510 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 511 | + | require.Error(t, err, "a rejected graph is a failed deploy") | |
| 512 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 513 | + | ||
| 514 | + | active, _, err := store.LoadActive(ctx, "blog") | |
| 515 | + | require.NoError(t, err) | |
| 516 | + | assert.Equal(t, "c-new", active.Services["web"].Container, "ACTIVE names the new container") | |
| 517 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |
| 518 | + | require.NoError(t, err) | |
| 519 | + | require.Len(t, markers, 1) | |
| 520 | + | assert.Equal(t, "c-old", markers[0].ContainerID) | |
| 521 | + | ||
| 522 | + | // The next mutation reconciles the failed deploy: the published candidate | |
| 523 | + | // is kept, and so is the retirement marker. | |
| 524 | + | require.NoError(t, svc.AbandonDeploy(ctx, deployment.DeployClaim{App: "blog", Op: "op-none"})) | |
| 525 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |
| 526 | + | require.NoError(t, err) | |
| 527 | + | require.Len(t, markers, 1, "a published candidate's reconciliation keeps the retirement marker") | |
| 528 | + | ||
| 529 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |
| 530 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |
| 531 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 532 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |
| 533 | + | assert.Equal(t, map[int]int{8080: 18081}, traffic.lastRoute("web")) | |
| 534 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |
| 535 | + | require.NoError(t, err) | |
| 536 | + | assert.Empty(t, markers) | |
| 285 | 537 | } | |
| @@ -2,8 +2,10 @@ package deployment_test | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "maps" | |||
| 5 | "os" | 6 | "os" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| 8 | + | "sync" | |||
| 7 | "testing" | 9 | "testing" | ||
| 8 | "time" | 10 | "time" | ||
| 9 | 11 | ||||
| @@ -12,6 +14,7 @@ import ( | |||||
| 12 | "github.com/stretchr/testify/mock" | 14 | "github.com/stretchr/testify/mock" | ||
| 13 | "github.com/stretchr/testify/require" | 15 | "github.com/stretchr/testify/require" | ||
| 14 | 16 | ||||
| 17 | + | "github.com/bnema/gordon/internal/adapters/out/appstate" | |||
| 15 | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | 18 | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | ||
| 16 | "github.com/bnema/gordon/internal/domain" | 19 | "github.com/bnema/gordon/internal/domain" | ||
| 17 | "github.com/bnema/gordon/internal/usecase/deployment" | 20 | "github.com/bnema/gordon/internal/usecase/deployment" | ||
| @@ -37,6 +40,9 @@ func TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder(t *testing.T) { | |||||
| 37 | s.Binds = []domain.AppBind{{Name: "config", Path: "/etc/app.conf", ReadOnly: true}} | 40 | s.Binds = []domain.AppBind{{Name: "config", Path: "/etc/app.conf", ReadOnly: true}} | ||
| 38 | }, true, false}, | 41 | }, true, false}, | ||
| 39 | {"device", func(s *domain.AppService) { s.Devices = []string{"test_gpu"} }, false, true}, | 42 | {"device", func(s *domain.AppService) { s.Devices = []string{"test_gpu"} }, false, true}, | ||
| 43 | + | {"udp", func(s *domain.AppService) { | |||
| 44 | + | s.UDP = []domain.AppUDPInterface{{Entrypoint: "game", Port: 28015, Publish: "0.0.0.0:28015"}} | |||
| 45 | + | }, false, false}, | |||
| 40 | } | 46 | } | ||
| 41 | for _, tc := range cases { | 47 | for _, tc := range cases { | ||
| 42 | t.Run(tc.name, func(t *testing.T) { | 48 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -110,6 +116,7 @@ func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |||||
| 110 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | 116 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | ||
| 111 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 117 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 112 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | 118 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | ||
| 119 | + | expectRetirementMarker(state, "c-old") | |||
| 113 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(assert.AnError).Once() | 120 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(assert.AnError).Once() | ||
| 114 | 121 | ||||
| 115 | var events []string | 122 | var events []string | ||
| @@ -133,153 +140,398 @@ func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |||||
| 133 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | 140 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | ||
| 134 | } | 141 | } | ||
| 135 | 142 | ||||
| 136 | - | // seedSwitchedApp seeds an app whose ACTIVE record already names the new | 143 | + | // storeTraffic is a traffic publisher that behaves like the real one against |
| 137 | - | // container c-new, while the superseded c-old was left running by an overlap | 144 | + | // a real app-state store: a withdrawal drops the service's recorded binds from |
| 138 | - | // replacement that stopped before retiring it. The caller supplies the journal | 145 | + | // ACTIVE (and can then report a failed graph apply), and a rebuild routes |
| 139 | - | // step recording that replacement. | 146 | + | // exactly the binds ACTIVE records at that moment. Tests assert on what is |
| 140 | - | func seedSwitchedApp(t *testing.T, ctx context.Context, store interface { | 147 | + | // routed, not on call order. |
| 141 | - | SaveIntent(context.Context, domain.AppStopIntent) error | 148 | + | type storeTraffic struct { |
| 142 | - | SaveActive(context.Context, domain.AppActive) error | 149 | + | mu sync.Mutex |
| 143 | - | SaveOperation(context.Context, domain.AppOperation) error | 150 | + | store *appstate.Store |
| 144 | - | }, terminal bool, step domain.AppOperationStep) { | 151 | + | failWithdraw int |
| 152 | + | failRebuild int | |||
| 153 | + | routed []map[string]map[int]int | |||
| 154 | + | } | |||
| 155 | + | ||||
| 156 | + | func (t *storeTraffic) WithdrawService(ctx context.Context, app, service string) error { | |||
| 157 | + | t.mu.Lock() | |||
| 158 | + | defer t.mu.Unlock() | |||
| 159 | + | active, ok, err := t.store.LoadActive(ctx, app) | |||
| 160 | + | if err != nil || !ok { | |||
| 161 | + | return err | |||
| 162 | + | } | |||
| 163 | + | if eff, found := active.Services[service]; found { | |||
| 164 | + | eff.BackendBinds, eff.UDPBackendBinds = nil, nil | |||
| 165 | + | active.Services[service] = eff | |||
| 166 | + | if err := t.store.SaveActive(ctx, active); err != nil { | |||
| 167 | + | return err | |||
| 168 | + | } | |||
| 169 | + | } | |||
| 170 | + | if t.failWithdraw > 0 { | |||
| 171 | + | t.failWithdraw-- | |||
| 172 | + | return assert.AnError | |||
| 173 | + | } | |||
| 174 | + | return nil | |||
| 175 | + | } | |||
| 176 | + | ||||
| 177 | + | func (t *storeTraffic) WithdrawServiceState(ctx context.Context, app, service string) error { | |||
| 178 | + | return t.WithdrawService(ctx, app, service) | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | func (t *storeTraffic) RebuildTraffic(ctx context.Context) error { | |||
| 182 | + | t.mu.Lock() | |||
| 183 | + | defer t.mu.Unlock() | |||
| 184 | + | if t.failRebuild > 0 { | |||
| 185 | + | t.failRebuild-- | |||
| 186 | + | return assert.AnError | |||
| 187 | + | } | |||
| 188 | + | active, _, err := t.store.LoadActive(ctx, "blog") | |||
| 189 | + | if err != nil { | |||
| 190 | + | return err | |||
| 191 | + | } | |||
| 192 | + | routes := map[string]map[int]int{} | |||
| 193 | + | for name, eff := range active.Services { | |||
| 194 | + | routes[name] = maps.Clone(eff.BackendBinds) | |||
| 195 | + | } | |||
| 196 | + | t.routed = append(t.routed, routes) | |||
| 197 | + | return nil | |||
| 198 | + | } | |||
| 199 | + | ||||
| 200 | + | // lastRoute returns the binds the most recent graph routed for a service. | |||
| 201 | + | func (t *storeTraffic) lastRoute(service string) map[int]int { | |||
| 202 | + | t.mu.Lock() | |||
| 203 | + | defer t.mu.Unlock() | |||
| 204 | + | if len(t.routed) == 0 { | |||
| 205 | + | return nil | |||
| 206 | + | } | |||
| 207 | + | return t.routed[len(t.routed)-1][service] | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | // allRoutes returns the binds of every graph routed for a service. | |||
| 211 | + | func (t *storeTraffic) allRoutes(service string) []map[int]int { | |||
| 212 | + | t.mu.Lock() | |||
| 213 | + | defer t.mu.Unlock() | |||
| 214 | + | var routes []map[int]int | |||
| 215 | + | for _, routed := range t.routed { | |||
| 216 | + | routes = append(routes, routed[service]) | |||
| 217 | + | } | |||
| 218 | + | return routes | |||
| 219 | + | } | |||
| 220 | + | ||||
| 221 | + | func (t *storeTraffic) rebuildCount() int { | |||
| 222 | + | t.mu.Lock() | |||
| 223 | + | defer t.mu.Unlock() | |||
| 224 | + | return len(t.routed) | |||
| 225 | + | } | |||
| 226 | + | ||||
| 227 | + | const overlapImage = "registry.example.com/blog/web:1.4.2" | |||
| 228 | + | ||||
| 229 | + | // overlapWebSpec is a stateless public HTTP service. | |||
| 230 | + | func overlapWebSpec(grace time.Duration) domain.AppService { | |||
| 231 | + | return domain.AppService{ | |||
| 232 | + | Name: "web", Image: overlapImage, Secrets: map[string]string{}, StopGrace: grace, | |||
| 233 | + | HTTP: []domain.AppHTTPInterface{{Host: "blog.example.com", Port: 8080, TLS: domain.AppTLSAuto}}, | |||
| 234 | + | Readiness: domain.AppReadiness{ | |||
| 235 | + | Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second, | |||
| 236 | + | }, | |||
| 237 | + | } | |||
| 238 | + | } | |||
| 239 | + | ||||
| 240 | + | func overlapOldBinds() map[int]int { return map[int]int{8080: 17000} } | |||
| 241 | + | ||||
| 242 | + | // seedServingApp seeds an app whose container c-old serves on 17000, with a | |||
| 243 | + | // materialized revision to deploy over it. | |||
| 244 | + | func seedServingApp(t *testing.T, ctx context.Context, store *appstate.Store, spec domain.AppService) { | |||
| 145 | t.Helper() | 245 | t.Helper() | ||
| 146 | - | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | 246 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) |
| 247 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |||
| 147 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | 248 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | ||
| 148 | - | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: headlessSpec(), BackendBinds: map[int]int{8080: 18081}}, | 249 | + | "web": {Container: "c-old", EffectiveRevision: "rev-0", Spec: spec, BackendBinds: overlapOldBinds()}, |
| 149 | }})) | 250 | }})) | ||
| 150 | - | op := domain.AppOperation{ | 251 | + | seedDesired(t, ctx, store, testRevision("blog", spec)) |
| 151 | - | Op: "op-overlap", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), | 252 | + | } |
| 152 | - | Steps: []domain.AppOperationStep{{ID: "preflight", State: domain.AppStepSucceeded}, step}, | 253 | + | |
| 153 | - | } | 254 | + | // overlapDeployDeps wires the preflight and candidate creation of one overlap |
| 154 | - | if terminal { | 255 | + | // deploy that creates c-new on 18081 and expects readiness to pass. |
| 155 | - | op.Outcome = domain.AppOutcomeSuccess | 256 | + | func overlapDeployMocks(runtime *outmocks.MockContainerRuntime, images *outmocks.MockImageResolver) { |
| 156 | - | } | 257 | + | images.EXPECT().ResolveDigest(mock.Anything, overlapImage).Return(restartTestDigest, nil).Once() |
| 157 | - | require.NoError(t, store.SaveOperation(ctx, op)) | 258 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, overlapImage).Return(nil, nil).Once() |
| 259 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 260 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |||
| 261 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |||
| 262 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |||
| 263 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |||
| 264 | + | } | |||
| 265 | + | ||||
| 266 | + | func overlapService(store *appstate.Store, runtime *outmocks.MockContainerRuntime, images *outmocks.MockImageResolver, secrets *outmocks.MockSecretProvider, traffic *storeTraffic) *deployment.Service { | |||
| 267 | + | return deployment.NewService(deployment.Deps{ | |||
| 268 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, Traffic: traffic, | |||
| 269 | + | ImagePolicy: domain.ImageSourcePolicy{AllowedRegistries: []string{"registry.example.com"}}, | |||
| 270 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 271 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 272 | + | func(context.Context, string) error { return nil }, | |||
| 273 | + | )) | |||
| 158 | } | 274 | } | ||
| 159 | 275 | ||||
| 160 | - | // TestBootRecovery_RetiresSupersededContainerAfterInterruptedSwitch proves the | 276 | + | // expectVerifiedRunning wires one recovery verification of a running |
| 161 | - | // crash window after the switch: ACTIVE names the new container, the old one | 277 | + | // container that publishes the given loopback port. |
| 162 | - | // is still running and unpublished, so reconciliation republishes routing and | 278 | + | func expectVerifiedRunning(runtime *outmocks.MockContainerRuntime, id string, port int) { |
| 163 | - | // then stops it gracefully with the default grace. | 279 | + | started := time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC) |
| 164 | - | func TestBootRecovery_RetiresSupersededContainerAfterInterruptedSwitch(t *testing.T) { | 280 | + | runtime.EXPECT().InspectContainer(mock.Anything, id). |
| 281 | + | Return(&domain.Container{ID: id, Status: "running", StartedAt: started}, nil) | |||
| 282 | + | runtime.EXPECT().GetContainerHealthStatus(mock.Anything, id).Return("", false, nil) | |||
| 283 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, id, mock.Anything).Return( | |||
| 284 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: port, Protocol: domain.NetworkProtocolTCP}}, nil) | |||
| 285 | + | } | |||
| 286 | + | ||||
| 287 | + | // TestDeploy_OverlapSwitchIsNotErasedByAPendingWithdrawal proves that a | |||
| 288 | + | // publication withdrawal left pending by an earlier failed recovery does not | |||
| 289 | + | // unroute the new generation: the deploy resolves it before publishing, so | |||
| 290 | + | // the graph the switch applies routes the candidate's binds. The traffic fake | |||
| 291 | + | // mutates ACTIVE like the real publisher, so a stray withdrawal after the | |||
| 292 | + | // switch would leave the service unrouted. | |||
| 293 | + | func TestDeploy_OverlapSwitchIsNotErasedByAPendingWithdrawal(t *testing.T) { | |||
| 165 | ctx := context.Background() | 294 | ctx := context.Background() | ||
| 166 | store := newTestStore(t) | 295 | store := newTestStore(t) | ||
| 296 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 167 | images := outmocks.NewMockImageResolver(t) | 297 | images := outmocks.NewMockImageResolver(t) | ||
| 168 | secrets := outmocks.NewMockSecretProvider(t) | 298 | secrets := outmocks.NewMockSecretProvider(t) | ||
| 169 | - | seedSwitchedApp(t, ctx, store, false, domain.AppOperationStep{ | 299 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) |
| 170 | - | ID: "service.web.replace", State: domain.AppStepPending, Service: "web", | 300 | + | traffic := &storeTraffic{store: store, failWithdraw: 1} |
| 171 | - | Before: "c-old", After: "c-new", Overlap: true, | 301 | + | svc := overlapService(store, runtime, images, secrets, traffic) |
| 172 | - | }) | 302 | + | |
| 303 | + | // A recovery pass finds c-old down and cannot prove its withdrawal: the | |||
| 304 | + | // service is marked as having a pending withdrawal. | |||
| 305 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |||
| 306 | + | Return(&domain.Container{ID: "c-old", Status: "exited"}, nil).Once() | |||
| 307 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |||
| 308 | + | ||||
| 309 | + | overlapDeployMocks(runtime, images) | |||
| 310 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |||
| 311 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 312 | + | ||||
| 313 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 314 | + | require.NoError(t, err) | |||
| 315 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 316 | + | ||||
| 317 | + | assert.Equal(t, map[int]int{8080: 18081}, traffic.lastRoute("web"), "the service routes the new generation") | |||
| 318 | + | active, _, err := store.LoadActive(ctx, "blog") | |||
| 319 | + | require.NoError(t, err) | |||
| 320 | + | assert.Equal(t, "c-new", active.Services["web"].Container) | |||
| 321 | + | assert.Equal(t, map[int]int{8080: 18081}, active.Services["web"].BackendBinds, | |||
| 322 | + | "ACTIVE keeps only the new generation's binds") | |||
| 323 | + | } | |||
| 173 | 324 | ||||
| 174 | - | var order []string | 325 | + | // TestDeploy_OverlapRetirementSurvivesANewerOperation proves the cleanup of a |
| 326 | + | // superseded container is owned by the durable retirement marker, not by the | |||
| 327 | + | // operation journal: after the first retirement fails and a newer operation | |||
| 328 | + | // becomes the latest journal, the periodic reconciliation still retires it, | |||
| 329 | + | // with the predecessor's own stop grace, and only once its successor is | |||
| 330 | + | // verified and routed. | |||
| 331 | + | func TestDeploy_OverlapRetirementSurvivesANewerOperation(t *testing.T) { | |||
| 332 | + | ctx := context.Background() | |||
| 333 | + | store := newTestStore(t) | |||
| 175 | runtime := outmocks.NewMockContainerRuntime(t) | 334 | runtime := outmocks.NewMockContainerRuntime(t) | ||
| 176 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).RunAndReturn( | 335 | + | images := outmocks.NewMockImageResolver(t) |
| 177 | - | func(context.Context, string, time.Duration) error { | 336 | + | secrets := outmocks.NewMockSecretProvider(t) |
| 178 | - | recordEvent(&order, "stop-old") | 337 | + | seedServingApp(t, ctx, store, overlapWebSpec(45*time.Second)) |
| 179 | - | return nil | 338 | + | traffic := &storeTraffic{store: store} |
| 180 | - | }).Once() | 339 | + | svc := overlapService(store, runtime, images, secrets, traffic) |
| 181 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).RunAndReturn( | |||
| 182 | - | func(context.Context, string, bool) error { | |||
| 183 | - | recordEvent(&order, "remove-old") | |||
| 184 | - | return nil | |||
| 185 | - | }).Once() | |||
| 186 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "exited"}, nil) | |||
| 187 | 340 | ||||
| 188 | - | svc := deployment.NewService(deployment.Deps{ | 341 | + | overlapDeployMocks(runtime, images) |
| 189 | - | State: store, Runtime: runtime, Images: images, Secrets: secrets, | 342 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", 45*time.Second).Return(nil).Twice() |
| 190 | - | Traffic: &orderedTraffic{events: &order}, | 343 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() |
| 191 | - | }, zerowrap.Default()) | |||
| 192 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |||
| 193 | 344 | ||||
| 194 | - | assert.Equal(t, []string{"traffic", "stop-old", "remove-old"}, order, | 345 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) |
| 195 | - | "routing is republished to the published container before the superseded one is stopped") | 346 | + | require.NoError(t, err, "a retirement leftover does not fail a deploy whose new generation serves") |
| 196 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-new", mock.Anything) | 347 | + | require.Len(t, result.CleanupWarnings, 1) |
| 348 | + | assert.Equal(t, "c-old", result.CleanupWarnings[0].Leftover) | |||
| 197 | 349 | ||||
| 198 | - | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | 350 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") |
| 199 | require.NoError(t, err) | 351 | require.NoError(t, err) | ||
| 200 | - | require.True(t, op.Terminal()) | 352 | + | require.Len(t, markers, 1, "the failed retirement leaves its durable marker") |
| 201 | - | step, found := opStep(op, "service.web.replace") | 353 | + | assert.Equal(t, "c-old", markers[0].ContainerID) |
| 202 | - | require.True(t, found) | 354 | + | assert.Equal(t, domain.AppInhibitRetirementPending, markers[0].Reason) |
| 203 | - | assert.True(t, step.BeforeRetired) | 355 | + | assert.Equal(t, 45*time.Second, markers[0].StopGrace, "the predecessor's grace travels with the marker") |
| 204 | - | assert.Empty(t, step.After, "a converged step is not retried") | 356 | + | |
| 205 | - | assert.Contains(t, step.Detail, "retired superseded container c-old") | 357 | + | // A newer operation becomes the latest journal and masks the deploy's. |
| 358 | + | require.NoError(t, store.SaveOperation(ctx, domain.AppOperation{ | |||
| 359 | + | Op: "op-newer", Kind: "deploy", App: "blog", StartedAt: time.Now().Add(time.Minute).UTC(), | |||
| 360 | + | Outcome: domain.AppOutcomeSuccess, | |||
| 361 | + | })) | |||
| 362 | + | ||||
| 363 | + | expectVerifiedRunning(runtime, "c-new", 18082) | |||
| 364 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 365 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 366 | + | ||||
| 367 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 368 | + | require.NoError(t, err) | |||
| 369 | + | assert.Empty(t, markers, "the marker is cleared once the container is confirmed gone") | |||
| 370 | + | assert.Equal(t, map[int]int{8080: 18082}, traffic.lastRoute("web"), "the verified successor is routed") | |||
| 206 | active, _, err := store.LoadActive(ctx, "blog") | 371 | active, _, err := store.LoadActive(ctx, "blog") | ||
| 207 | require.NoError(t, err) | 372 | require.NoError(t, err) | ||
| 208 | assert.Equal(t, "c-new", active.Services["web"].Container) | 373 | assert.Equal(t, "c-new", active.Services["web"].Container) | ||
| 209 | - | ||||
| 210 | - | // A converged step is not retried on the next pass. | |||
| 211 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |||
| 212 | - | runtime.AssertNumberOfCalls(t, "StopContainer", 1) | |||
| 213 | } | 374 | } | ||
| 214 | 375 | ||||
| 215 | - | // TestReconcile_RetriesSupersededContainerRetirementOfASucceededDeploy proves a | 376 | + | // TestReconcileRunning_RetiresSupersededOnlyAfterSuccessorIsVerified proves a |
| 216 | - | // deploy that succeeded with a cleanup warning (the old container could not be | 377 | + | // superseded container is kept while its successor cannot be verified: the |
| 217 | - | // removed) is retried by the next reconciliation, and that a failure to retire | 378 | + | // working generation is never stopped in favour of one that is not serving. |
| 218 | - | // is a warning that does not block the app. | 379 | + | func TestReconcileRunning_RetiresSupersededOnlyAfterSuccessorIsVerified(t *testing.T) { |
| 219 | - | func TestReconcile_RetriesSupersededContainerRetirementOfASucceededDeploy(t *testing.T) { | |||
| 220 | ctx := context.Background() | 380 | ctx := context.Background() | ||
| 221 | store := newTestStore(t) | 381 | store := newTestStore(t) | ||
| 382 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 222 | images := outmocks.NewMockImageResolver(t) | 383 | images := outmocks.NewMockImageResolver(t) | ||
| 223 | secrets := outmocks.NewMockSecretProvider(t) | 384 | secrets := outmocks.NewMockSecretProvider(t) | ||
| 224 | - | seedSwitchedApp(t, ctx, store, true, domain.AppOperationStep{ | 385 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) |
| 225 | - | ID: "service.web.replace", State: domain.AppStepSucceeded, Service: "web", | 386 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ |
| 226 | - | Before: "c-old", After: "c-new", Overlap: true, | 387 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: overlapWebSpec(0), BackendBinds: map[int]int{8080: 18081}}, |
| 227 | - | }) | 388 | + | }})) |
| 389 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |||
| 390 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, | |||
| 391 | + | })) | |||
| 392 | + | traffic := &storeTraffic{store: store} | |||
| 393 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |||
| 394 | + | ||||
| 395 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(nil, assert.AnError).Once() | |||
| 396 | + | require.Error(t, svc.ReconcileRunning(ctx)) | |||
| 397 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |||
| 398 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 399 | + | require.NoError(t, err) | |||
| 400 | + | assert.Len(t, markers, 1, "the marker waits for a verified successor") | |||
| 401 | + | ||||
| 402 | + | // A record written before the grace was persisted falls back to the default. | |||
| 403 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |||
| 404 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |||
| 405 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 406 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 407 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 408 | + | require.NoError(t, err) | |||
| 409 | + | assert.Empty(t, markers) | |||
| 410 | + | } | |||
| 228 | 411 | ||||
| 412 | + | // TestReconcileRunning_NeverRetiresAPendingContainerThatActiveNamesAgain | |||
| 413 | + | // proves a rollback to the superseded container is safe: when ACTIVE names the | |||
| 414 | + | // container again, its stale retirement marker is dropped and it is kept. | |||
| 415 | + | func TestReconcileRunning_NeverRetiresAPendingContainerThatActiveNamesAgain(t *testing.T) { | |||
| 416 | + | ctx := context.Background() | |||
| 417 | + | store := newTestStore(t) | |||
| 229 | runtime := outmocks.NewMockContainerRuntime(t) | 418 | runtime := outmocks.NewMockContainerRuntime(t) | ||
| 230 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-new").Return(&domain.Container{ID: "c-new", Status: "exited"}, nil) | 419 | + | images := outmocks.NewMockImageResolver(t) |
| 231 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil) | 420 | + | secrets := outmocks.NewMockSecretProvider(t) |
| 232 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | 421 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) |
| 233 | - | svc := deployment.NewService(deployment.Deps{ | 422 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ |
| 234 | - | State: store, Runtime: runtime, Images: images, Secrets: secrets, | 423 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, |
| 235 | - | Traffic: &orderedTraffic{}, | 424 | + | })) |
| 236 | - | }, zerowrap.Default()) | 425 | + | svc := overlapService(store, runtime, images, secrets, &storeTraffic{store: store}) |
| 237 | 426 | ||||
| 238 | - | require.NoError(t, svc.ReconcileBoot(ctx), "a leftover old container does not block reconciliation") | 427 | + | expectVerifiedRunning(runtime, "c-old", 17000) |
| 239 | - | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | 428 | + | require.NoError(t, svc.ReconcileRunning(ctx)) |
| 429 | + | ||||
| 430 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 431 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 432 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 240 | require.NoError(t, err) | 433 | require.NoError(t, err) | ||
| 241 | - | step, _ := opStep(op, "service.web.replace") | 434 | + | assert.Empty(t, markers) |
| 242 | - | assert.False(t, step.BeforeRetired, "the retirement failed and stays pending") | 435 | + | active, _, err := store.LoadActive(ctx, "blog") |
| 243 | - | assert.Equal(t, domain.AppStepSucceeded, step.State, "the deploy outcome is untouched") | |||
| 244 | - | assert.Equal(t, domain.AppOutcomeSuccess, op.Outcome) | |||
| 245 | - | require.Len(t, op.Warnings, 1) | |||
| 246 | - | assert.Equal(t, "c-old", op.Warnings[0].Leftover) | |||
| 247 | - | ||||
| 248 | - | // The next pass retries, and now succeeds. | |||
| 249 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 250 | - | require.NoError(t, svc.ReconcileBoot(ctx)) | |||
| 251 | - | op, err = store.LoadOperation(ctx, "blog", "op-overlap") | |||
| 252 | require.NoError(t, err) | 436 | require.NoError(t, err) | ||
| 253 | - | step, _ = opStep(op, "service.web.replace") | 437 | + | assert.Equal(t, "c-old", active.Services["web"].Container) |
| 254 | - | assert.True(t, step.BeforeRetired) | |||
| 255 | } | 438 | } | ||
| 256 | 439 | ||||
| 257 | - | // TestReconcile_NeverRetiresASupersededContainerThatActiveNamesAgain proves | 440 | + | // TestBootRecovery_DoesNotRepublishUnverifiedBindsOfAPublishedCandidate proves |
| 258 | - | // reconciliation does not stop a container ACTIVE currently publishes, such | 441 | + | // interrupted-operation reconciliation never rebuilds traffic from the binds |
| 259 | - | // as when a rollback re-published the previous generation. | 442 | + | // ACTIVE persisted for a published candidate: a restarted candidate's |
| 260 | - | func TestReconcile_NeverRetiresASupersededContainerThatActiveNamesAgain(t *testing.T) { | 443 | + | // ephemeral port may belong to another workload. Only the verified start path |
| 444 | + | // routes, and the superseded container is retired afterwards through its | |||
| 445 | + | // marker. | |||
| 446 | + | func TestBootRecovery_DoesNotRepublishUnverifiedBindsOfAPublishedCandidate(t *testing.T) { | |||
| 261 | ctx := context.Background() | 447 | ctx := context.Background() | ||
| 262 | store := newTestStore(t) | 448 | store := newTestStore(t) | ||
| 449 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 263 | images := outmocks.NewMockImageResolver(t) | 450 | images := outmocks.NewMockImageResolver(t) | ||
| 264 | secrets := outmocks.NewMockSecretProvider(t) | 451 | secrets := outmocks.NewMockSecretProvider(t) | ||
| 265 | - | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | 452 | + | spec := overlapWebSpec(0) |
| 453 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |||
| 266 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | 454 | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | ||
| 267 | - | "web": {Container: "c-old", EffectiveRevision: "rev-0", Spec: headlessSpec()}, | 455 | + | "web": {Container: "c-new", EffectiveRevision: "rev-1", Spec: spec, BackendBinds: map[int]int{8080: 18081}}, |
| 268 | }})) | 456 | }})) | ||
| 269 | require.NoError(t, store.SaveOperation(ctx, domain.AppOperation{ | 457 | require.NoError(t, store.SaveOperation(ctx, domain.AppOperation{ | ||
| 270 | - | Op: "op-overlap", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), Outcome: domain.AppOutcomeFailed, | 458 | + | Op: "op-switch", Kind: "deploy", App: "blog", StartedAt: time.Now().UTC(), |
| 271 | - | Steps: []domain.AppOperationStep{{ | 459 | + | Steps: []domain.AppOperationStep{ |
| 272 | - | ID: "service.web.replace", State: domain.AppStepFailed, Service: "web", | 460 | + | {ID: "preflight", State: domain.AppStepSucceeded}, |
| 273 | - | Before: "c-old", After: "c-new", Overlap: true, | 461 | + | {ID: "service.web.replace", State: domain.AppStepPending, Service: "web", Before: "c-old", After: "c-new"}, |
| 274 | - | }}, | 462 | + | }, |
| 275 | })) | 463 | })) | ||
| 464 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |||
| 465 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitRetirementPending, | |||
| 466 | + | })) | |||
| 467 | + | traffic := &storeTraffic{store: store} | |||
| 468 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |||
| 276 | 469 | ||||
| 277 | - | runtime := outmocks.NewMockContainerRuntime(t) | 470 | + | // The candidate restarted and now publishes a different port. |
| 278 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(nil).Once() | 471 | + | runtime.EXPECT().IsContainerRunning(mock.Anything, "c-new").Return(true, nil).Once() |
| 279 | - | runtime.EXPECT().InspectContainer(mock.Anything, "c-old").Return(&domain.Container{ID: "c-old", Status: "exited"}, nil) | 472 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( |
| 280 | - | svc := deployment.NewService(deployment.Deps{State: store, Runtime: runtime, Images: images, Secrets: secrets}, zerowrap.Default()) | 473 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18090, Protocol: domain.NetworkProtocolTCP}}, nil).Once() |
| 281 | require.NoError(t, svc.ReconcileBoot(ctx)) | 474 | require.NoError(t, svc.ReconcileBoot(ctx)) | ||
| 282 | 475 | ||||
| 476 | + | require.Positive(t, traffic.rebuildCount()) | |||
| 477 | + | for _, routed := range traffic.allRoutes("web") { | |||
| 478 | + | assert.NotEqual(t, map[int]int{8080: 18081}, routed, "the unverified persisted port is never routed") | |||
| 479 | + | } | |||
| 480 | + | assert.Equal(t, map[int]int{8080: 18090}, traffic.lastRoute("web"), "the verified bind is routed") | |||
| 481 | + | active, _, err := store.LoadActive(ctx, "blog") | |||
| 482 | + | require.NoError(t, err) | |||
| 483 | + | assert.Equal(t, map[int]int{8080: 18090}, active.Services["web"].BackendBinds) | |||
| 484 | + | ||||
| 485 | + | // Boot keeps the superseded container and its marker: the periodic pass | |||
| 486 | + | // retires it once the successor is verified. | |||
| 283 | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | 487 | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | ||
| 284 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | 488 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") |
| 489 | + | require.NoError(t, err) | |||
| 490 | + | require.Len(t, markers, 1) | |||
| 491 | + | assert.Equal(t, "c-old", markers[0].ContainerID) | |||
| 492 | + | } | |||
| 493 | + | ||||
| 494 | + | // TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass proves the | |||
| 495 | + | // failed switch is recoverable and traceable: the graph apply is rejected after | |||
| 496 | + | // ACTIVE named the new container, the superseded container keeps running with | |||
| 497 | + | // its marker, the next deploy-side reconciliation keeps that marker, and the | |||
| 498 | + | // periodic pass retires the old container once the successor is routed. | |||
| 499 | + | func TestDeploy_OverlapTrafficFailureIsRecoveredByThePeriodicPass(t *testing.T) { | |||
| 500 | + | ctx := context.Background() | |||
| 501 | + | store := newTestStore(t) | |||
| 502 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 503 | + | images := outmocks.NewMockImageResolver(t) | |||
| 504 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 505 | + | seedServingApp(t, ctx, store, overlapWebSpec(0)) | |||
| 506 | + | traffic := &storeTraffic{store: store, failRebuild: 1} | |||
| 507 | + | svc := overlapService(store, runtime, images, secrets, traffic) | |||
| 508 | + | ||||
| 509 | + | overlapDeployMocks(runtime, images) | |||
| 510 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 511 | + | require.Error(t, err, "a rejected graph is a failed deploy") | |||
| 512 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 513 | + | ||||
| 514 | + | active, _, err := store.LoadActive(ctx, "blog") | |||
| 515 | + | require.NoError(t, err) | |||
| 516 | + | assert.Equal(t, "c-new", active.Services["web"].Container, "ACTIVE names the new container") | |||
| 517 | + | markers, err := store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 518 | + | require.NoError(t, err) | |||
| 519 | + | require.Len(t, markers, 1) | |||
| 520 | + | assert.Equal(t, "c-old", markers[0].ContainerID) | |||
| 521 | + | ||||
| 522 | + | // The next mutation reconciles the failed deploy: the published candidate | |||
| 523 | + | // is kept, and so is the retirement marker. | |||
| 524 | + | require.NoError(t, svc.AbandonDeploy(ctx, deployment.DeployClaim{App: "blog", Op: "op-none"})) | |||
| 525 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 526 | + | require.NoError(t, err) | |||
| 527 | + | require.Len(t, markers, 1, "a published candidate's reconciliation keeps the retirement marker") | |||
| 528 | + | ||||
| 529 | + | expectVerifiedRunning(runtime, "c-new", 18081) | |||
| 530 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", domain.AppDefaultStopGrace).Return(nil).Once() | |||
| 531 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 532 | + | require.NoError(t, svc.ReconcileRunning(ctx)) | |||
| 533 | + | assert.Equal(t, map[int]int{8080: 18081}, traffic.lastRoute("web")) | |||
| 534 | + | markers, err = store.LoadRecoveryInhibitions(ctx, "blog") | |||
| 535 | + | require.NoError(t, err) | |||
| 536 | + | assert.Empty(t, markers) | |||
| 285 | } | 537 | } | ||
M internal/usecase/deployment/recovery_integration_test.go +2 -0
| @@ -211,6 +211,7 @@ func TestRemove_InhibitsBeforeRuntimeWithdrawal(t *testing.T) { | |||
| 211 | 211 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | |
| 212 | 212 | // Ownership is read while it is still live; the name-only record owns | |
| 213 | 213 | // no network, so nothing is reclaimed. | |
| 214 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 214 | 215 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | |
| 215 | 216 | // The incarnation is retired atomically after the workload is gone. | |
| 216 | 217 | state.EXPECT().RetireApp(mock.Anything, "blog").RunAndReturn(func(context.Context, string) error { | |
| @@ -417,6 +418,7 @@ func TestRemovalOfMissingContainerIsIdempotent(t *testing.T) { | |||
| 417 | 418 | // the disappeared container are released. | |
| 418 | 419 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | |
| 419 | 420 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | |
| 421 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 420 | 422 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | |
| 421 | 423 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | |
| 422 | 424 | ||
| @@ -211,6 +211,7 @@ func TestRemove_InhibitsBeforeRuntimeWithdrawal(t *testing.T) { | |||||
| 211 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | 211 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | ||
| 212 | // Ownership is read while it is still live; the name-only record owns | 212 | // Ownership is read while it is still live; the name-only record owns | ||
| 213 | // no network, so nothing is reclaimed. | 213 | // no network, so nothing is reclaimed. | ||
| 214 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 214 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | 215 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | ||
| 215 | // The incarnation is retired atomically after the workload is gone. | 216 | // The incarnation is retired atomically after the workload is gone. | ||
| 216 | state.EXPECT().RetireApp(mock.Anything, "blog").RunAndReturn(func(context.Context, string) error { | 217 | state.EXPECT().RetireApp(mock.Anything, "blog").RunAndReturn(func(context.Context, string) error { | ||
| @@ -417,6 +418,7 @@ func TestRemovalOfMissingContainerIsIdempotent(t *testing.T) { | |||||
| 417 | // the disappeared container are released. | 418 | // the disappeared container are released. | ||
| 418 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | 419 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | ||
| 419 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | 420 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | ||
| 421 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 420 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | 422 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | ||
| 421 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | 423 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | ||
| 422 | 424 | ||||
M internal/usecase/deployment/retirement_test.go +1 -0
| @@ -77,6 +77,7 @@ func TestLifecycleVerbsPassTheEffectiveStopGrace(t *testing.T) { | |||
| 77 | 77 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | |
| 78 | 78 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | |
| 79 | 79 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | |
| 80 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |
| 80 | 81 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | |
| 81 | 82 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | |
| 82 | 83 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil) | |
| @@ -77,6 +77,7 @@ func TestLifecycleVerbsPassTheEffectiveStopGrace(t *testing.T) { | |||||
| 77 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | 77 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-1", false).Return(nil).Once() | ||
| 78 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | 78 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-1").Return(nil).Once() | ||
| 79 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | 79 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-1").Return(nil).Once() | ||
| 80 | + | state.EXPECT().LoadRecoveryInhibitions(mock.Anything, "blog").Return(nil, nil).Once() | |||
| 80 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | 81 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog"}, nil).Once() | ||
| 81 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | 82 | state.EXPECT().RetireApp(mock.Anything, "blog").Return(nil).Once() | ||
| 82 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil) | 83 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil) | ||
M internal/usecase/deployment/sequential_replacement_test.go +13 -2
| @@ -117,6 +117,14 @@ func expectSequentialCreate( | |||
| 117 | 117 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | |
| 118 | 118 | } | |
| 119 | 119 | ||
| 120 | + | // expectRetirementMarker wires the durable retirement-pending marker an | |
| 121 | + | // overlap replacement records for the superseded container before the switch. | |
| 122 | + | func expectRetirementMarker(state *outmocks.MockAppState, containerID string) { | |
| 123 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.MatchedBy(func(i domain.AppRecoveryInhibition) bool { | |
| 124 | + | return i.ContainerID == containerID && i.Reason == domain.AppInhibitRetirementPending | |
| 125 | + | })).Return(nil).Once() | |
| 126 | + | } | |
| 127 | + | ||
| 120 | 128 | // expectRetireOld wires the confirmed retirement of the superseded container. | |
| 121 | 129 | func expectRetireOld( | |
| 122 | 130 | state *outmocks.MockAppState, | |
| @@ -164,6 +172,8 @@ func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing | |||
| 164 | 172 | return nil | |
| 165 | 173 | }).Once() | |
| 166 | 174 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 175 | + | expectRetirementMarker(state, "c-old") | |
| 176 | + | // Confirmed retirement drops the marker. | |
| 167 | 177 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 168 | 178 | expectRetireOld(state, runtime, &order, "c-old") | |
| 169 | 179 | ||
| @@ -189,7 +199,6 @@ func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing | |||
| 189 | 199 | []string{"create-new", "start-new", "ready", "publish-active", "traffic", "stop-old", "remove-old"}, | |
| 190 | 200 | order, "the old container serves until the new one is published and routed; it is never withdrawn") | |
| 191 | 201 | assert.Empty(t, result.CleanupWarnings) | |
| 192 | - | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |
| 193 | 202 | } | |
| 194 | 203 | ||
| 195 | 204 | // TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace proves the | |
| @@ -212,6 +221,7 @@ func TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace(t *testing.T) { | |||
| 212 | 221 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 213 | 222 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |
| 214 | 223 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 224 | + | expectRetirementMarker(state, "c-old") | |
| 215 | 225 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 216 | 226 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", 45*time.Second).Return(nil).Once() | |
| 217 | 227 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| @@ -244,7 +254,7 @@ func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |||
| 244 | 254 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 245 | 255 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |
| 246 | 256 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 247 | - | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 257 | + | expectRetirementMarker(state, "c-old") | |
| 248 | 258 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 249 | 259 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | |
| 250 | 260 | ||
| @@ -263,6 +273,7 @@ func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |||
| 263 | 273 | assert.Equal(t, "web", result.CleanupWarnings[0].Service) | |
| 264 | 274 | assert.Contains(t, result.CleanupWarnings[0].Detail, "remove") | |
| 265 | 275 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |
| 276 | + | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | |
| 266 | 277 | } | |
| 267 | 278 | ||
| 268 | 279 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | |
| @@ -117,6 +117,14 @@ func expectSequentialCreate( | |||||
| 117 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | 117 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 118 | } | 118 | } | ||
| 119 | 119 | ||||
| 120 | + | // expectRetirementMarker wires the durable retirement-pending marker an | |||
| 121 | + | // overlap replacement records for the superseded container before the switch. | |||
| 122 | + | func expectRetirementMarker(state *outmocks.MockAppState, containerID string) { | |||
| 123 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.MatchedBy(func(i domain.AppRecoveryInhibition) bool { | |||
| 124 | + | return i.ContainerID == containerID && i.Reason == domain.AppInhibitRetirementPending | |||
| 125 | + | })).Return(nil).Once() | |||
| 126 | + | } | |||
| 127 | + | ||||
| 120 | // expectRetireOld wires the confirmed retirement of the superseded container. | 128 | // expectRetireOld wires the confirmed retirement of the superseded container. | ||
| 121 | func expectRetireOld( | 129 | func expectRetireOld( | ||
| 122 | state *outmocks.MockAppState, | 130 | state *outmocks.MockAppState, | ||
| @@ -164,6 +172,8 @@ func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing | |||||
| 164 | return nil | 172 | return nil | ||
| 165 | }).Once() | 173 | }).Once() | ||
| 166 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | 174 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | ||
| 175 | + | expectRetirementMarker(state, "c-old") | |||
| 176 | + | // Confirmed retirement drops the marker. | |||
| 167 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 177 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | ||
| 168 | expectRetireOld(state, runtime, &order, "c-old") | 178 | expectRetireOld(state, runtime, &order, "c-old") | ||
| 169 | 179 | ||||
| @@ -189,7 +199,6 @@ func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing | |||||
| 189 | []string{"create-new", "start-new", "ready", "publish-active", "traffic", "stop-old", "remove-old"}, | 199 | []string{"create-new", "start-new", "ready", "publish-active", "traffic", "stop-old", "remove-old"}, | ||
| 190 | order, "the old container serves until the new one is published and routed; it is never withdrawn") | 200 | order, "the old container serves until the new one is published and routed; it is never withdrawn") | ||
| 191 | assert.Empty(t, result.CleanupWarnings) | 201 | assert.Empty(t, result.CleanupWarnings) | ||
| 192 | - | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |||
| 193 | } | 202 | } | ||
| 194 | 203 | ||||
| 195 | // TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace proves the | 204 | // TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace proves the | ||
| @@ -212,6 +221,7 @@ func TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace(t *testing.T) { | |||||
| 212 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | 221 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | ||
| 213 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | 222 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 214 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | 223 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | ||
| 224 | + | expectRetirementMarker(state, "c-old") | |||
| 215 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 225 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | ||
| 216 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", 45*time.Second).Return(nil).Once() | 226 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", 45*time.Second).Return(nil).Once() | ||
| 217 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | 227 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | ||
| @@ -244,7 +254,7 @@ func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |||||
| 244 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | 254 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | ||
| 245 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | 255 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 246 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | 256 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | ||
| 247 | - | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 257 | + | expectRetirementMarker(state, "c-old") |
| 248 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | 258 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | ||
| 249 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | 259 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(assert.AnError).Once() | ||
| 250 | 260 | ||||
| @@ -263,6 +273,7 @@ func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |||||
| 263 | assert.Equal(t, "web", result.CleanupWarnings[0].Service) | 273 | assert.Equal(t, "web", result.CleanupWarnings[0].Service) | ||
| 264 | assert.Contains(t, result.CleanupWarnings[0].Detail, "remove") | 274 | assert.Contains(t, result.CleanupWarnings[0].Detail, "remove") | ||
| 265 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | 275 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | ||
| 276 | + | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | |||
| 266 | } | 277 | } | ||
| 267 | 278 | ||||
| 268 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | 279 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | ||
M internal/usecase/deployment/service.go +4 -0
| @@ -385,6 +385,10 @@ type ServiceResult struct { | |||
| 385 | 385 | After string | |
| 386 | 386 | RestartUnsafe bool | |
| 387 | 387 | Error string | |
| 388 | + | // SupersededRunning is set by an overlap replacement: the container in | |
| 389 | + | // Before still runs beside After and carries a retirement-pending marker | |
| 390 | + | // until it is retired. | |
| 391 | + | SupersededRunning bool | |
| 388 | 392 | // BackendBinds carries the created container's TCP loopback publishes | |
| 389 | 393 | // (container port -> 127.0.0.1 host port) for the active record. | |
| 390 | 394 | BackendBinds map[int]int | |
| @@ -385,6 +385,10 @@ type ServiceResult struct { | |||||
| 385 | After string | 385 | After string | ||
| 386 | RestartUnsafe bool | 386 | RestartUnsafe bool | ||
| 387 | Error string | 387 | Error string | ||
| 388 | + | // SupersededRunning is set by an overlap replacement: the container in | |||
| 389 | + | // Before still runs beside After and carries a retirement-pending marker | |||
| 390 | + | // until it is retired. | |||
| 391 | + | SupersededRunning bool | |||
| 388 | // BackendBinds carries the created container's TCP loopback publishes | 392 | // BackendBinds carries the created container's TCP loopback publishes | ||
| 389 | // (container port -> 127.0.0.1 host port) for the active record. | 393 | // (container port -> 127.0.0.1 host port) for the active record. | ||
| 390 | BackendBinds map[int]int | 394 | BackendBinds map[int]int | ||
M internal/usecase/deployment/traffic_failure_test.go +5 -3
| @@ -57,7 +57,11 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||
| 57 | 57 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 58 | 58 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 59 | 59 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | |
| 60 | - | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 60 | + | // The durable retirement marker is the cleanup owner of the superseded | |
| 61 | + | // container; it is recorded before the switch and never cleared here. | |
| 62 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.MatchedBy(func(i domain.AppRecoveryInhibition) bool { | |
| 63 | + | return i.ContainerID == "c-old" && i.Reason == domain.AppInhibitRetirementPending | |
| 64 | + | })).Return(nil).Once() | |
| 61 | 65 | ||
| 62 | 66 | var saved []domain.AppOperation | |
| 63 | 67 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).RunAndReturn( | |
| @@ -94,8 +98,6 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||
| 94 | 98 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |
| 95 | 99 | assert.Equal(t, "c-old", step.Before) | |
| 96 | 100 | assert.Equal(t, "c-new", step.After) | |
| 97 | - | assert.True(t, step.Overlap) | |
| 98 | - | assert.False(t, step.BeforeRetired, "the superseded container was not retired") | |
| 99 | 101 | require.Len(t, result.Services["web"].CleanupWarnings, 1) | |
| 100 | 102 | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | |
| 101 | 103 | require.Len(t, last.Warnings, 1) | |
| @@ -57,7 +57,11 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||||
| 57 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | 57 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | ||
| 58 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | 58 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | ||
| 59 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | 59 | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 60 | - | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 60 | + | // The durable retirement marker is the cleanup owner of the superseded |
| 61 | + | // container; it is recorded before the switch and never cleared here. | |||
| 62 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.MatchedBy(func(i domain.AppRecoveryInhibition) bool { | |||
| 63 | + | return i.ContainerID == "c-old" && i.Reason == domain.AppInhibitRetirementPending | |||
| 64 | + | })).Return(nil).Once() | |||
| 61 | 65 | ||||
| 62 | var saved []domain.AppOperation | 66 | var saved []domain.AppOperation | ||
| 63 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).RunAndReturn( | 67 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).RunAndReturn( | ||
| @@ -94,8 +98,6 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||||
| 94 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | 98 | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | ||
| 95 | assert.Equal(t, "c-old", step.Before) | 99 | assert.Equal(t, "c-old", step.Before) | ||
| 96 | assert.Equal(t, "c-new", step.After) | 100 | assert.Equal(t, "c-new", step.After) | ||
| 97 | - | assert.True(t, step.Overlap) | |||
| 98 | - | assert.False(t, step.BeforeRetired, "the superseded container was not retired") | |||
| 99 | require.Len(t, result.Services["web"].CleanupWarnings, 1) | 101 | require.Len(t, result.Services["web"].CleanupWarnings, 1) | ||
| 100 | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | 102 | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | ||
| 101 | require.Len(t, last.Warnings, 1) | 103 | require.Len(t, last.Warnings, 1) | ||