feat(deployment): replace stateless services without downtime
Services without volumes, binds, or devices start the replacement beside the serving container, publish and route to it once ready, and only then stop the old one with its own stop grace. Other services keep the sequential replacement. Crash recovery retires a superseded container that outlived an interrupted switch and keeps the old one when the candidate was never published.
- commit
- 75991493e84bbb83d8dc3ea53c2225672d44cee6
- author
- brice · · 2026-10-16 19:20 UTC
- parent
- 4fc01fa
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +845 -165
M internal/domain/app_state.go +8 -0
| @@ -192,6 +192,14 @@ 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"` | |
| 195 | 203 | // Diagnostics is bounded, redacted failure output kept separate from | |
| 196 | 204 | // the stable Error. Retrieval requires the logs read scope. | |
| 197 | 205 | Diagnostics []string `json:"diagnostics,omitempty"` | |
| @@ -192,6 +192,14 @@ 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"` | |||
| 195 | // Diagnostics is bounded, redacted failure output kept separate from | 203 | // Diagnostics is bounded, redacted failure output kept separate from | ||
| 196 | // the stable Error. Retrieval requires the logs read scope. | 204 | // the stable Error. Retrieval requires the logs read scope. | ||
| 197 | Diagnostics []string `json:"diagnostics,omitempty"` | 205 | Diagnostics []string `json:"diagnostics,omitempty"` | ||
M internal/usecase/deployment/crash_recovery_test.go +33 -32
| @@ -109,8 +109,8 @@ func TestDeploy_CandidateIsJournaledBeforeItStarts(t *testing.T) { | |||
| 109 | 109 | ||
| 110 | 110 | // crashDuringReplacement runs one replacement deploy and interrupts it | |
| 111 | 111 | // between creating the candidate and publishing it. The candidate is left | |
| 112 | - | // running, ACTIVE still names the superseded generation, and the operation | |
| 113 | - | // journal stays in flight: the state a process crash or a failed cleanup | |
| 112 | + | // running, ACTIVE still names the superseded generation (which keeps running, | |
| 113 | + | // as an overlap replacement leaves it), and the operation journal stays in flight: the state a process crash or a failed cleanup | |
| 114 | 114 | // leaves behind. | |
| 115 | 115 | func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | |
| 116 | 116 | t.Helper() | |
| @@ -123,8 +123,7 @@ func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | |||
| 123 | 123 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |
| 124 | 124 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |
| 125 | 125 | expectNetworkProvision(runtime, "app-blog", 1) | |
| 126 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 127 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 126 | + | // The overlap replacement leaves the superseded container untouched. | |
| 128 | 127 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-orphan", Name: "web"}, nil).Once() | |
| 129 | 128 | runtime.EXPECT().StartContainer(mock.Anything, "c-orphan").Return(nil).Once() | |
| 130 | 129 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-orphan", mock.Anything).Return( | |
| @@ -165,12 +164,12 @@ func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | |||
| 165 | 164 | require.Error(t, <-done) | |
| 166 | 165 | } | |
| 167 | 166 | ||
| 168 | - | // TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished | |
| 169 | - | // proves the crash window between creating a replacement and publishing it is | |
| 170 | - | // recoverable: an interruption leaves the candidate journaled but unpublished, | |
| 171 | - | // and boot recovery removes that candidate before it rebuilds the generation | |
| 172 | - | // recorded in ACTIVE, so two generations of one service never run together. | |
| 173 | - | func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *testing.T) { | |
| 167 | + | // TestBootRecovery_RemovesUnpublishedCandidateAndKeepsServingGeneration | |
| 168 | + | // proves the crash window between creating an overlap replacement and | |
| 169 | + | // publishing it is recoverable: an interruption leaves the candidate | |
| 170 | + | // journaled but unpublished, and boot recovery removes that candidate and | |
| 171 | + | // keeps the generation recorded in ACTIVE, which was never stopped. | |
| 172 | + | func TestBootRecovery_RemovesUnpublishedCandidateAndKeepsServingGeneration(t *testing.T) { | |
| 174 | 173 | ctx := context.Background() | |
| 175 | 174 | store := newTestStore(t) | |
| 176 | 175 | images := outmocks.NewMockImageResolver(t) | |
| @@ -192,8 +191,8 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||
| 192 | 191 | require.True(t, ok) | |
| 193 | 192 | assert.Equal(t, "c-old", active.Services["web"].Container) | |
| 194 | 193 | ||
| 195 | - | // Boot recovery: the orphan is removed before the published generation is | |
| 196 | - | // rebuilt from its pinned revision. | |
| 194 | + | // Boot recovery: the orphan is removed; the published generation is | |
| 195 | + | // still running, so it is verified and kept, never rebuilt or stopped. | |
| 197 | 196 | var order []string | |
| 198 | 197 | runtime2 := outmocks.NewMockContainerRuntime(t) | |
| 199 | 198 | runtime2.EXPECT().RemoveContainer(mock.Anything, "c-orphan", true).RunAndReturn( | |
| @@ -201,19 +200,13 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||
| 201 | 200 | order = append(order, "remove-orphan") | |
| 202 | 201 | return nil | |
| 203 | 202 | }).Once() | |
| 204 | - | runtime2.EXPECT().IsContainerRunning(mock.Anything, "c-old").Return(false, nil).Once() | |
| 205 | - | runtime2.EXPECT().StartContainer(mock.Anything, "c-old").Return(domain.ErrContainerNotFound).Once() | |
| 206 | - | runtime2.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(domain.ErrContainerNotFound).Once() | |
| 207 | - | runtime2.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | |
| 208 | - | expectNetworkProvision(runtime2, "app-blog", 1) | |
| 209 | - | runtime2.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn( | |
| 210 | - | func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | |
| 211 | - | order = append(order, "rebuild-published-generation") | |
| 212 | - | return &domain.Container{ID: "c-rebuilt", Name: "web"}, nil | |
| 203 | + | runtime2.EXPECT().IsContainerRunning(mock.Anything, "c-old").RunAndReturn( | |
| 204 | + | func(context.Context, string) (bool, error) { | |
| 205 | + | order = append(order, "verify-published-generation") | |
| 206 | + | return true, nil | |
| 213 | 207 | }).Once() | |
| 214 | - | runtime2.EXPECT().StartContainer(mock.Anything, "c-rebuilt").Return(nil).Once() | |
| 215 | - | runtime2.EXPECT().GetContainerBackendBinds(mock.Anything, "c-rebuilt", mock.Anything).Return( | |
| 216 | - | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18082, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 208 | + | runtime2.EXPECT().GetContainerBackendBinds(mock.Anything, "c-old", mock.Anything).Return( | |
| 209 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 217 | 210 | ||
| 218 | 211 | rebooted := deployment.NewService(deployment.Deps{ | |
| 219 | 212 | State: store, Runtime: runtime2, Images: images, Secrets: secrets, | |
| @@ -225,8 +218,11 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||
| 225 | 218 | // context is gone with it. | |
| 226 | 219 | require.NoError(t, rebooted.ReconcileBoot(ctx)) | |
| 227 | 220 | ||
| 228 | - | assert.Equal(t, []string{"remove-orphan", "rebuild-published-generation"}, order, | |
| 229 | - | "the unpublished candidate is gone before the published generation is rebuilt") | |
| 221 | + | assert.Equal(t, []string{"remove-orphan", "verify-published-generation"}, order, | |
| 222 | + | "the unpublished candidate is gone and the serving generation is kept") | |
| 223 | + | runtime2.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 224 | + | runtime2.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |
| 225 | + | runtime2.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |
| 230 | 226 | runtime2.AssertNotCalled(t, "StartContainer", mock.Anything, "c-orphan") | |
| 231 | 227 | runtime2.AssertNotCalled(t, "RestartContainer", mock.Anything, "c-orphan", mock.Anything) | |
| 232 | 228 | ||
| @@ -241,12 +237,11 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||
| 241 | 237 | assert.Equal(t, domain.AppStepFailed, finalStep.State) | |
| 242 | 238 | assert.Contains(t, finalStep.Error, "unpublished candidate was removed") | |
| 243 | 239 | ||
| 244 | - | // ACTIVE now names the rebuilt generation: recovery converged the app | |
| 245 | - | // instead of leaving it pointing at a container that no longer exists. | |
| 240 | + | // ACTIVE still names the generation that kept serving. | |
| 246 | 241 | converged, ok, err := store.LoadActive(context.Background(), "blog") | |
| 247 | 242 | require.NoError(t, err) | |
| 248 | 243 | require.True(t, ok) | |
| 249 | - | assert.Equal(t, "c-rebuilt", converged.Services["web"].Container) | |
| 244 | + | assert.Equal(t, "c-old", converged.Services["web"].Container) | |
| 250 | 245 | assert.Equal(t, "rev-0", converged.Services["web"].EffectiveRevision) | |
| 251 | 246 | } | |
| 252 | 247 | ||
| @@ -272,14 +267,20 @@ func TestDeploy_ReconcilesInterruptedPredecessorBeforeCreatingAReplacement(t *te | |||
| 272 | 267 | }).Once() | |
| 273 | 268 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |
| 274 | 269 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |
| 275 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(domain.ErrContainerNotFound).Once() | |
| 276 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | |
| 277 | 270 | expectNetworkProvision(runtime, "app-blog", 1) | |
| 278 | 271 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn( | |
| 279 | 272 | func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | |
| 280 | 273 | order = append(order, "create-new-generation") | |
| 281 | 274 | return &domain.Container{ID: "c-next", Name: "web"}, nil | |
| 282 | 275 | }).Once() | |
| 276 | + | // The overlap replacement retires the superseded container only after | |
| 277 | + | // the new generation is published. | |
| 278 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).RunAndReturn( | |
| 279 | + | func(context.Context, string, time.Duration) error { | |
| 280 | + | order = append(order, "retire-superseded") | |
| 281 | + | return nil | |
| 282 | + | }).Once() | |
| 283 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 283 | 284 | runtime.EXPECT().StartContainer(mock.Anything, "c-next").Return(nil).Once() | |
| 284 | 285 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-next", mock.Anything).Return( | |
| 285 | 286 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18082, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| @@ -295,7 +296,7 @@ func TestDeploy_ReconcilesInterruptedPredecessorBeforeCreatingAReplacement(t *te | |||
| 295 | 296 | require.NoError(t, err) | |
| 296 | 297 | require.NotNil(t, result) | |
| 297 | 298 | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 298 | - | assert.Equal(t, []string{"remove-leftover-candidate", "create-new-generation"}, order) | |
| 299 | + | assert.Equal(t, []string{"remove-leftover-candidate", "create-new-generation", "retire-superseded"}, order) | |
| 299 | 300 | ||
| 300 | 301 | active, ok, err := store.LoadActive(ctx, "blog") | |
| 301 | 302 | require.NoError(t, err) | |
| @@ -109,8 +109,8 @@ func TestDeploy_CandidateIsJournaledBeforeItStarts(t *testing.T) { | |||||
| 109 | 109 | ||||
| 110 | // crashDuringReplacement runs one replacement deploy and interrupts it | 110 | // crashDuringReplacement runs one replacement deploy and interrupts it | ||
| 111 | // between creating the candidate and publishing it. The candidate is left | 111 | // between creating the candidate and publishing it. The candidate is left | ||
| 112 | - | // running, ACTIVE still names the superseded generation, and the operation | 112 | + | // running, ACTIVE still names the superseded generation (which keeps running, |
| 113 | - | // journal stays in flight: the state a process crash or a failed cleanup | 113 | + | // as an overlap replacement leaves it), and the operation journal stays in flight: the state a process crash or a failed cleanup |
| 114 | // leaves behind. | 114 | // leaves behind. | ||
| 115 | func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | 115 | func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | ||
| 116 | t.Helper() | 116 | t.Helper() | ||
| @@ -123,8 +123,7 @@ func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | |||||
| 123 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | 123 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | ||
| 124 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | 124 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | ||
| 125 | expectNetworkProvision(runtime, "app-blog", 1) | 125 | expectNetworkProvision(runtime, "app-blog", 1) | ||
| 126 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | 126 | + | // The overlap replacement leaves the superseded container untouched. |
| 127 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 128 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-orphan", Name: "web"}, nil).Once() | 127 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-orphan", Name: "web"}, nil).Once() | ||
| 129 | runtime.EXPECT().StartContainer(mock.Anything, "c-orphan").Return(nil).Once() | 128 | runtime.EXPECT().StartContainer(mock.Anything, "c-orphan").Return(nil).Once() | ||
| 130 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-orphan", mock.Anything).Return( | 129 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-orphan", mock.Anything).Return( | ||
| @@ -165,12 +164,12 @@ func crashDuringReplacement(t *testing.T, store *appstate.Store, opKey string) { | |||||
| 165 | require.Error(t, <-done) | 164 | require.Error(t, <-done) | ||
| 166 | } | 165 | } | ||
| 167 | 166 | ||||
| 168 | - | // TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished | 167 | + | // TestBootRecovery_RemovesUnpublishedCandidateAndKeepsServingGeneration |
| 169 | - | // proves the crash window between creating a replacement and publishing it is | 168 | + | // proves the crash window between creating an overlap replacement and |
| 170 | - | // recoverable: an interruption leaves the candidate journaled but unpublished, | 169 | + | // publishing it is recoverable: an interruption leaves the candidate |
| 171 | - | // and boot recovery removes that candidate before it rebuilds the generation | 170 | + | // journaled but unpublished, and boot recovery removes that candidate and |
| 172 | - | // recorded in ACTIVE, so two generations of one service never run together. | 171 | + | // keeps the generation recorded in ACTIVE, which was never stopped. |
| 173 | - | func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *testing.T) { | 172 | + | func TestBootRecovery_RemovesUnpublishedCandidateAndKeepsServingGeneration(t *testing.T) { |
| 174 | ctx := context.Background() | 173 | ctx := context.Background() | ||
| 175 | store := newTestStore(t) | 174 | store := newTestStore(t) | ||
| 176 | images := outmocks.NewMockImageResolver(t) | 175 | images := outmocks.NewMockImageResolver(t) | ||
| @@ -192,8 +191,8 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||||
| 192 | require.True(t, ok) | 191 | require.True(t, ok) | ||
| 193 | assert.Equal(t, "c-old", active.Services["web"].Container) | 192 | assert.Equal(t, "c-old", active.Services["web"].Container) | ||
| 194 | 193 | ||||
| 195 | - | // Boot recovery: the orphan is removed before the published generation is | 194 | + | // Boot recovery: the orphan is removed; the published generation is |
| 196 | - | // rebuilt from its pinned revision. | 195 | + | // still running, so it is verified and kept, never rebuilt or stopped. |
| 197 | var order []string | 196 | var order []string | ||
| 198 | runtime2 := outmocks.NewMockContainerRuntime(t) | 197 | runtime2 := outmocks.NewMockContainerRuntime(t) | ||
| 199 | runtime2.EXPECT().RemoveContainer(mock.Anything, "c-orphan", true).RunAndReturn( | 198 | runtime2.EXPECT().RemoveContainer(mock.Anything, "c-orphan", true).RunAndReturn( | ||
| @@ -201,19 +200,13 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||||
| 201 | order = append(order, "remove-orphan") | 200 | order = append(order, "remove-orphan") | ||
| 202 | return nil | 201 | return nil | ||
| 203 | }).Once() | 202 | }).Once() | ||
| 204 | - | runtime2.EXPECT().IsContainerRunning(mock.Anything, "c-old").Return(false, nil).Once() | 203 | + | runtime2.EXPECT().IsContainerRunning(mock.Anything, "c-old").RunAndReturn( |
| 205 | - | runtime2.EXPECT().StartContainer(mock.Anything, "c-old").Return(domain.ErrContainerNotFound).Once() | 204 | + | func(context.Context, string) (bool, error) { |
| 206 | - | runtime2.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(domain.ErrContainerNotFound).Once() | 205 | + | order = append(order, "verify-published-generation") |
| 207 | - | runtime2.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | 206 | + | return true, nil |
| 208 | - | expectNetworkProvision(runtime2, "app-blog", 1) | |||
| 209 | - | runtime2.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn( | |||
| 210 | - | func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | |||
| 211 | - | order = append(order, "rebuild-published-generation") | |||
| 212 | - | return &domain.Container{ID: "c-rebuilt", Name: "web"}, nil | |||
| 213 | }).Once() | 207 | }).Once() | ||
| 214 | - | runtime2.EXPECT().StartContainer(mock.Anything, "c-rebuilt").Return(nil).Once() | 208 | + | runtime2.EXPECT().GetContainerBackendBinds(mock.Anything, "c-old", mock.Anything).Return( |
| 215 | - | runtime2.EXPECT().GetContainerBackendBinds(mock.Anything, "c-rebuilt", mock.Anything).Return( | 209 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() |
| 216 | - | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18082, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |||
| 217 | 210 | ||||
| 218 | rebooted := deployment.NewService(deployment.Deps{ | 211 | rebooted := deployment.NewService(deployment.Deps{ | ||
| 219 | State: store, Runtime: runtime2, Images: images, Secrets: secrets, | 212 | State: store, Runtime: runtime2, Images: images, Secrets: secrets, | ||
| @@ -225,8 +218,11 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||||
| 225 | // context is gone with it. | 218 | // context is gone with it. | ||
| 226 | require.NoError(t, rebooted.ReconcileBoot(ctx)) | 219 | require.NoError(t, rebooted.ReconcileBoot(ctx)) | ||
| 227 | 220 | ||||
| 228 | - | assert.Equal(t, []string{"remove-orphan", "rebuild-published-generation"}, order, | 221 | + | assert.Equal(t, []string{"remove-orphan", "verify-published-generation"}, order, |
| 229 | - | "the unpublished candidate is gone before the published generation is rebuilt") | 222 | + | "the unpublished candidate is gone and the serving generation is kept") |
| 223 | + | runtime2.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |||
| 224 | + | runtime2.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |||
| 225 | + | runtime2.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |||
| 230 | runtime2.AssertNotCalled(t, "StartContainer", mock.Anything, "c-orphan") | 226 | runtime2.AssertNotCalled(t, "StartContainer", mock.Anything, "c-orphan") | ||
| 231 | runtime2.AssertNotCalled(t, "RestartContainer", mock.Anything, "c-orphan", mock.Anything) | 227 | runtime2.AssertNotCalled(t, "RestartContainer", mock.Anything, "c-orphan", mock.Anything) | ||
| 232 | 228 | ||||
| @@ -241,12 +237,11 @@ func TestBootRecovery_RemovesUnpublishedCandidateBeforeRebuildingPublished(t *te | |||||
| 241 | assert.Equal(t, domain.AppStepFailed, finalStep.State) | 237 | assert.Equal(t, domain.AppStepFailed, finalStep.State) | ||
| 242 | assert.Contains(t, finalStep.Error, "unpublished candidate was removed") | 238 | assert.Contains(t, finalStep.Error, "unpublished candidate was removed") | ||
| 243 | 239 | ||||
| 244 | - | // ACTIVE now names the rebuilt generation: recovery converged the app | 240 | + | // ACTIVE still names the generation that kept serving. |
| 245 | - | // instead of leaving it pointing at a container that no longer exists. | |||
| 246 | converged, ok, err := store.LoadActive(context.Background(), "blog") | 241 | converged, ok, err := store.LoadActive(context.Background(), "blog") | ||
| 247 | require.NoError(t, err) | 242 | require.NoError(t, err) | ||
| 248 | require.True(t, ok) | 243 | require.True(t, ok) | ||
| 249 | - | assert.Equal(t, "c-rebuilt", converged.Services["web"].Container) | 244 | + | assert.Equal(t, "c-old", converged.Services["web"].Container) |
| 250 | assert.Equal(t, "rev-0", converged.Services["web"].EffectiveRevision) | 245 | assert.Equal(t, "rev-0", converged.Services["web"].EffectiveRevision) | ||
| 251 | } | 246 | } | ||
| 252 | 247 | ||||
| @@ -272,14 +267,20 @@ func TestDeploy_ReconcilesInterruptedPredecessorBeforeCreatingAReplacement(t *te | |||||
| 272 | }).Once() | 267 | }).Once() | ||
| 273 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | 268 | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | ||
| 274 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | 269 | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | ||
| 275 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(domain.ErrContainerNotFound).Once() | |||
| 276 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(domain.ErrContainerNotFound).Once() | |||
| 277 | expectNetworkProvision(runtime, "app-blog", 1) | 270 | expectNetworkProvision(runtime, "app-blog", 1) | ||
| 278 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn( | 271 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn( | ||
| 279 | func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | 272 | func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | ||
| 280 | order = append(order, "create-new-generation") | 273 | order = append(order, "create-new-generation") | ||
| 281 | return &domain.Container{ID: "c-next", Name: "web"}, nil | 274 | return &domain.Container{ID: "c-next", Name: "web"}, nil | ||
| 282 | }).Once() | 275 | }).Once() | ||
| 276 | + | // The overlap replacement retires the superseded container only after | |||
| 277 | + | // the new generation is published. | |||
| 278 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).RunAndReturn( | |||
| 279 | + | func(context.Context, string, time.Duration) error { | |||
| 280 | + | order = append(order, "retire-superseded") | |||
| 281 | + | return nil | |||
| 282 | + | }).Once() | |||
| 283 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 283 | runtime.EXPECT().StartContainer(mock.Anything, "c-next").Return(nil).Once() | 284 | runtime.EXPECT().StartContainer(mock.Anything, "c-next").Return(nil).Once() | ||
| 284 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-next", mock.Anything).Return( | 285 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-next", mock.Anything).Return( | ||
| 285 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18082, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | 286 | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18082, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | ||
| @@ -295,7 +296,7 @@ func TestDeploy_ReconcilesInterruptedPredecessorBeforeCreatingAReplacement(t *te | |||||
| 295 | require.NoError(t, err) | 296 | require.NoError(t, err) | ||
| 296 | require.NotNil(t, result) | 297 | require.NotNil(t, result) | ||
| 297 | assert.Equal(t, "deployed", result.Services["web"].Result) | 298 | assert.Equal(t, "deployed", result.Services["web"].Result) | ||
| 298 | - | assert.Equal(t, []string{"remove-leftover-candidate", "create-new-generation"}, order) | 299 | + | assert.Equal(t, []string{"remove-leftover-candidate", "create-new-generation", "retire-superseded"}, order) |
| 299 | 300 | ||||
| 300 | active, ok, err := store.LoadActive(ctx, "blog") | 301 | active, ok, err := store.LoadActive(ctx, "blog") | ||
| 301 | require.NoError(t, err) | 302 | require.NoError(t, err) | ||
M internal/usecase/deployment/deploy_mock_test.go +92 -29
| @@ -246,25 +246,99 @@ func TestDeploy_Mockery_HTTPSuccessRecordsLoopbackBinds(t *testing.T) { | |||
| 246 | 246 | runtime.AssertNotCalled(t, "GetContainerNetworkInfo", mock.Anything, mock.Anything) | |
| 247 | 247 | } | |
| 248 | 248 | ||
| 249 | - | // TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish proves the | |
| 250 | - | // sequential ordering: the superseded container is stopped and removed | |
| 251 | - | // before the replacement is created, and ACTIVE names the replacement only | |
| 252 | - | // after it exists. A public HTTP service without volumes is replaced exactly | |
| 253 | - | // like every other service. | |
| 254 | - | func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | |
| 249 | + | // TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration proves | |
| 250 | + | // the overlap ordering of a stateless public HTTP service: the replacement is | |
| 251 | + | // created while the superseded container keeps running, ACTIVE names the | |
| 252 | + | // replacement next, and the superseded container is stopped and removed only | |
| 253 | + | // afterwards, releasing its backend claims. | |
| 254 | + | func TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration(t *testing.T) { | |
| 255 | 255 | ctx := context.Background() | |
| 256 | 256 | state, runtime, images, secrets := mockDeps(t) | |
| 257 | 257 | rev := mockRevision() | |
| 258 | 258 | ||
| 259 | 259 | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 260 | 260 | expectNetworkProvision(runtime, "app-blog", 1) | |
| 261 | - | state.EXPECT().LoadActive(mock.Anything, "blog").Return(domain.AppActive{ | |
| 261 | + | oldActive := domain.AppActive{ | |
| 262 | + | App: "blog", | |
| 263 | + | Services: map[string]domain.AppEffectiveService{ | |
| 264 | + | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", BackendBinds: map[int]int{8080: 17000}}, | |
| 265 | + | }, | |
| 266 | + | } | |
| 267 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | |
| 268 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 269 | + | ||
| 270 | + | var createdAt, publishedAt, stoppedAt int | |
| 271 | + | var step int | |
| 272 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn(func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | |
| 273 | + | step++ | |
| 274 | + | createdAt = step | |
| 275 | + | return &domain.Container{ID: "c-new", Name: "web"}, nil | |
| 276 | + | }).Once() | |
| 277 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 278 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 279 | + | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.MatchedBy(func(claims []domain.AppListenerReservation) bool { | |
| 280 | + | return len(claims) == 1 && claims[0].Port == 18080 && claims[0].Owner == domain.OwnerGordonBackend && claims[0].ContainerID == "c-new" | |
| 281 | + | })).Return(nil).Once() | |
| 282 | + | var published domain.AppActive | |
| 283 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, active domain.AppActive) error { | |
| 284 | + | step++ | |
| 285 | + | publishedAt = step | |
| 286 | + | published = active | |
| 287 | + | return nil | |
| 288 | + | }).Once() | |
| 289 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 290 | + | 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 | + | step++ | |
| 293 | + | stoppedAt = step | |
| 294 | + | return nil | |
| 295 | + | }).Once() | |
| 296 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 297 | + | // The superseded container's loopback claims are released once it is gone. | |
| 298 | + | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |
| 299 | + | ||
| 300 | + | svc := deployment.NewService( | |
| 301 | + | deployment.Deps{State: state, Runtime: runtime, Images: images, Secrets: secrets}, | |
| 302 | + | zerowrap.Default(), | |
| 303 | + | ).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 304 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 305 | + | func(context.Context, string) error { return nil }, | |
| 306 | + | )) | |
| 307 | + | ||
| 308 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 309 | + | require.NoError(t, err) | |
| 310 | + | require.NotNil(t, result) | |
| 311 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 312 | + | assert.Less(t, createdAt, publishedAt, "ACTIVE is published after the replacement exists") | |
| 313 | + | assert.Less(t, publishedAt, stoppedAt, "the superseded generation is stopped only after the switch") | |
| 314 | + | assert.Equal(t, "c-new", published.Services["web"].Container) | |
| 315 | + | assert.Equal(t, map[int]int{8080: 18080}, published.Services["web"].BackendBinds, | |
| 316 | + | "ACTIVE no longer references the superseded container's binds") | |
| 317 | + | assert.Empty(t, result.CleanupWarnings) | |
| 318 | + | } | |
| 319 | + | ||
| 320 | + | // TestDeploy_Mockery_HTTPSequentialWhenServiceHasVolume proves a service with a | |
| 321 | + | // volume keeps the sequential ordering: the superseded container is stopped | |
| 322 | + | // and removed before the replacement is created, and ACTIVE names the | |
| 323 | + | // replacement only after it exists. | |
| 324 | + | func TestDeploy_Mockery_HTTPSequentialWhenServiceHasVolume(t *testing.T) { | |
| 325 | + | ctx := context.Background() | |
| 326 | + | state, runtime, images, secrets := mockDeps(t) | |
| 327 | + | rev := statefulMockRevision() | |
| 328 | + | ||
| 329 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 330 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |
| 331 | + | runtime.EXPECT().CreateVolume(mock.Anything, mock.Anything, mock.Anything).Return(nil) | |
| 332 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 333 | + | active := domain.AppActive{ | |
| 262 | 334 | App: "blog", | |
| 263 | 335 | Services: map[string]domain.AppEffectiveService{ | |
| 264 | 336 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old"}, | |
| 265 | 337 | }, | |
| 266 | - | }, true, nil) | |
| 338 | + | } | |
| 339 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |
| 267 | 340 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 341 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |
| 268 | 342 | ||
| 269 | 343 | var replacedAt, createdAt, publishedAt int | |
| 270 | 344 | var step int | |
| @@ -282,15 +356,7 @@ func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | |||
| 282 | 356 | }).Once() | |
| 283 | 357 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 284 | 358 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 285 | - | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.MatchedBy(func(claims []domain.AppListenerReservation) bool { | |
| 286 | - | return len(claims) == 1 && claims[0].Port == 18080 && claims[0].Owner == domain.OwnerGordonBackend && claims[0].ContainerID == "c-new" | |
| 287 | - | })).Return(nil).Once() | |
| 288 | - | state.EXPECT().LoadActive(mock.Anything, "blog").Return(domain.AppActive{ | |
| 289 | - | App: "blog", | |
| 290 | - | Services: map[string]domain.AppEffectiveService{ | |
| 291 | - | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old"}, | |
| 292 | - | }, | |
| 293 | - | }, true, nil) | |
| 359 | + | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() | |
| 294 | 360 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn(func(context.Context, domain.AppActive) error { | |
| 295 | 361 | step++ | |
| 296 | 362 | publishedAt = step | |
| @@ -318,11 +384,11 @@ func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | |||
| 318 | 384 | assert.Empty(t, result.CleanupWarnings) | |
| 319 | 385 | } | |
| 320 | 386 | ||
| 321 | - | // TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration proves a | |
| 322 | - | // failed replacement never falls back to the generation it already retired: | |
| 323 | - | // the superseded container stays gone, the unready candidate is removed, and | |
| 324 | - | // nothing is published in its place. | |
| 325 | - | func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) { | |
| 387 | + | // TestDeploy_Mockery_HTTPReadinessFailureKeepsOldGenerationServing proves a | |
| 388 | + | // failed overlap replacement leaves the generation that serves alone: the | |
| 389 | + | // superseded container is neither stopped nor removed nor released, the | |
| 390 | + | // unready candidate is removed, and nothing is published in its place. | |
| 391 | + | func TestDeploy_Mockery_HTTPReadinessFailureKeepsOldGenerationServing(t *testing.T) { | |
| 326 | 392 | ctx := context.Background() | |
| 327 | 393 | state, runtime, images, secrets := mockDeps(t) | |
| 328 | 394 | rev := mockRevision() | |
| @@ -337,11 +403,6 @@ func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) | |||
| 337 | 403 | }, true, nil) | |
| 338 | 404 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 339 | 405 | ||
| 340 | - | // The superseded generation is retired before the replacement is created, | |
| 341 | - | // then the replacement fails readiness fast. | |
| 342 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 343 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 344 | - | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |
| 345 | 406 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |
| 346 | 407 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 347 | 408 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| @@ -369,10 +430,12 @@ func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) | |||
| 369 | 430 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | |
| 370 | 431 | require.Len(t, result.Services["web"].Diagnostics, 1, "failure diagnostics are kept separately") | |
| 371 | 432 | assert.Equal(t, "boom", result.Services["web"].Diagnostics[0]) | |
| 372 | - | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-new", mock.Anything) | |
| 433 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 434 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |
| 373 | 435 | runtime.AssertNotCalled(t, "RemoveVolume", mock.Anything, mock.Anything, mock.Anything) | |
| 374 | 436 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | |
| 375 | - | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | |
| 437 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |
| 438 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |
| 376 | 439 | } | |
| 377 | 440 | ||
| 378 | 441 | func TestDeploy_Mockery_VolumeReplacementMarksUnsafe(t *testing.T) { | |
| @@ -246,25 +246,99 @@ func TestDeploy_Mockery_HTTPSuccessRecordsLoopbackBinds(t *testing.T) { | |||||
| 246 | runtime.AssertNotCalled(t, "GetContainerNetworkInfo", mock.Anything, mock.Anything) | 246 | runtime.AssertNotCalled(t, "GetContainerNetworkInfo", mock.Anything, mock.Anything) | ||
| 247 | } | 247 | } | ||
| 248 | 248 | ||||
| 249 | - | // TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish proves the | 249 | + | // TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration proves |
| 250 | - | // sequential ordering: the superseded container is stopped and removed | 250 | + | // the overlap ordering of a stateless public HTTP service: the replacement is |
| 251 | - | // before the replacement is created, and ACTIVE names the replacement only | 251 | + | // created while the superseded container keeps running, ACTIVE names the |
| 252 | - | // after it exists. A public HTTP service without volumes is replaced exactly | 252 | + | // replacement next, and the superseded container is stopped and removed only |
| 253 | - | // like every other service. | 253 | + | // afterwards, releasing its backend claims. |
| 254 | - | func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | 254 | + | func TestDeploy_Mockery_HTTPOverlapPublishesBeforeRetiringOldGeneration(t *testing.T) { |
| 255 | ctx := context.Background() | 255 | ctx := context.Background() | ||
| 256 | state, runtime, images, secrets := mockDeps(t) | 256 | state, runtime, images, secrets := mockDeps(t) | ||
| 257 | rev := mockRevision() | 257 | rev := mockRevision() | ||
| 258 | 258 | ||||
| 259 | expectDeployPreflight(state, runtime, images, secrets, rev) | 259 | expectDeployPreflight(state, runtime, images, secrets, rev) | ||
| 260 | expectNetworkProvision(runtime, "app-blog", 1) | 260 | expectNetworkProvision(runtime, "app-blog", 1) | ||
| 261 | - | state.EXPECT().LoadActive(mock.Anything, "blog").Return(domain.AppActive{ | 261 | + | oldActive := domain.AppActive{ |
| 262 | + | App: "blog", | |||
| 263 | + | Services: map[string]domain.AppEffectiveService{ | |||
| 264 | + | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", BackendBinds: map[int]int{8080: 17000}}, | |||
| 265 | + | }, | |||
| 266 | + | } | |||
| 267 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | |||
| 268 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 269 | + | ||||
| 270 | + | var createdAt, publishedAt, stoppedAt int | |||
| 271 | + | var step int | |||
| 272 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).RunAndReturn(func(context.Context, *domain.ContainerConfig) (*domain.Container, error) { | |||
| 273 | + | step++ | |||
| 274 | + | createdAt = step | |||
| 275 | + | return &domain.Container{ID: "c-new", Name: "web"}, nil | |||
| 276 | + | }).Once() | |||
| 277 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |||
| 278 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |||
| 279 | + | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.MatchedBy(func(claims []domain.AppListenerReservation) bool { | |||
| 280 | + | return len(claims) == 1 && claims[0].Port == 18080 && claims[0].Owner == domain.OwnerGordonBackend && claims[0].ContainerID == "c-new" | |||
| 281 | + | })).Return(nil).Once() | |||
| 282 | + | var published domain.AppActive | |||
| 283 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, active domain.AppActive) error { | |||
| 284 | + | step++ | |||
| 285 | + | publishedAt = step | |||
| 286 | + | published = active | |||
| 287 | + | return nil | |||
| 288 | + | }).Once() | |||
| 289 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |||
| 290 | + | 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 | + | step++ | |||
| 293 | + | stoppedAt = step | |||
| 294 | + | return nil | |||
| 295 | + | }).Once() | |||
| 296 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 297 | + | // The superseded container's loopback claims are released once it is gone. | |||
| 298 | + | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |||
| 299 | + | ||||
| 300 | + | svc := deployment.NewService( | |||
| 301 | + | deployment.Deps{State: state, Runtime: runtime, Images: images, Secrets: secrets}, | |||
| 302 | + | zerowrap.Default(), | |||
| 303 | + | ).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 304 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 305 | + | func(context.Context, string) error { return nil }, | |||
| 306 | + | )) | |||
| 307 | + | ||||
| 308 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 309 | + | require.NoError(t, err) | |||
| 310 | + | require.NotNil(t, result) | |||
| 311 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 312 | + | assert.Less(t, createdAt, publishedAt, "ACTIVE is published after the replacement exists") | |||
| 313 | + | assert.Less(t, publishedAt, stoppedAt, "the superseded generation is stopped only after the switch") | |||
| 314 | + | assert.Equal(t, "c-new", published.Services["web"].Container) | |||
| 315 | + | assert.Equal(t, map[int]int{8080: 18080}, published.Services["web"].BackendBinds, | |||
| 316 | + | "ACTIVE no longer references the superseded container's binds") | |||
| 317 | + | assert.Empty(t, result.CleanupWarnings) | |||
| 318 | + | } | |||
| 319 | + | ||||
| 320 | + | // TestDeploy_Mockery_HTTPSequentialWhenServiceHasVolume proves a service with a | |||
| 321 | + | // volume keeps the sequential ordering: the superseded container is stopped | |||
| 322 | + | // and removed before the replacement is created, and ACTIVE names the | |||
| 323 | + | // replacement only after it exists. | |||
| 324 | + | func TestDeploy_Mockery_HTTPSequentialWhenServiceHasVolume(t *testing.T) { | |||
| 325 | + | ctx := context.Background() | |||
| 326 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 327 | + | rev := statefulMockRevision() | |||
| 328 | + | ||||
| 329 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 330 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |||
| 331 | + | runtime.EXPECT().CreateVolume(mock.Anything, mock.Anything, mock.Anything).Return(nil) | |||
| 332 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 333 | + | active := domain.AppActive{ | |||
| 262 | App: "blog", | 334 | App: "blog", | ||
| 263 | Services: map[string]domain.AppEffectiveService{ | 335 | Services: map[string]domain.AppEffectiveService{ | ||
| 264 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old"}, | 336 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old"}, | ||
| 265 | }, | 337 | }, | ||
| 266 | - | }, true, nil) | 338 | + | } |
| 339 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(active, true, nil) | |||
| 267 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 340 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 341 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 268 | 342 | ||||
| 269 | var replacedAt, createdAt, publishedAt int | 343 | var replacedAt, createdAt, publishedAt int | ||
| 270 | var step int | 344 | var step int | ||
| @@ -282,15 +356,7 @@ func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | |||||
| 282 | }).Once() | 356 | }).Once() | ||
| 283 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | 357 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | ||
| 284 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | 358 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | ||
| 285 | - | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.MatchedBy(func(claims []domain.AppListenerReservation) bool { | 359 | + | state.EXPECT().RegisterBackendBinds(mock.Anything, mock.Anything).Return(nil).Once() |
| 286 | - | return len(claims) == 1 && claims[0].Port == 18080 && claims[0].Owner == domain.OwnerGordonBackend && claims[0].ContainerID == "c-new" | |||
| 287 | - | })).Return(nil).Once() | |||
| 288 | - | state.EXPECT().LoadActive(mock.Anything, "blog").Return(domain.AppActive{ | |||
| 289 | - | App: "blog", | |||
| 290 | - | Services: map[string]domain.AppEffectiveService{ | |||
| 291 | - | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old"}, | |||
| 292 | - | }, | |||
| 293 | - | }, true, nil) | |||
| 294 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn(func(context.Context, domain.AppActive) error { | 360 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn(func(context.Context, domain.AppActive) error { | ||
| 295 | step++ | 361 | step++ | ||
| 296 | publishedAt = step | 362 | publishedAt = step | ||
| @@ -318,11 +384,11 @@ func TestDeploy_Mockery_HTTPReplacesOldGenerationBeforePublish(t *testing.T) { | |||||
| 318 | assert.Empty(t, result.CleanupWarnings) | 384 | assert.Empty(t, result.CleanupWarnings) | ||
| 319 | } | 385 | } | ||
| 320 | 386 | ||||
| 321 | - | // TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration proves a | 387 | + | // TestDeploy_Mockery_HTTPReadinessFailureKeepsOldGenerationServing proves a |
| 322 | - | // failed replacement never falls back to the generation it already retired: | 388 | + | // failed overlap replacement leaves the generation that serves alone: the |
| 323 | - | // the superseded container stays gone, the unready candidate is removed, and | 389 | + | // superseded container is neither stopped nor removed nor released, the |
| 324 | - | // nothing is published in its place. | 390 | + | // unready candidate is removed, and nothing is published in its place. |
| 325 | - | func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) { | 391 | + | func TestDeploy_Mockery_HTTPReadinessFailureKeepsOldGenerationServing(t *testing.T) { |
| 326 | ctx := context.Background() | 392 | ctx := context.Background() | ||
| 327 | state, runtime, images, secrets := mockDeps(t) | 393 | state, runtime, images, secrets := mockDeps(t) | ||
| 328 | rev := mockRevision() | 394 | rev := mockRevision() | ||
| @@ -337,11 +403,6 @@ func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) | |||||
| 337 | }, true, nil) | 403 | }, true, nil) | ||
| 338 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 404 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 339 | 405 | ||||
| 340 | - | // The superseded generation is retired before the replacement is created, | |||
| 341 | - | // then the replacement fails readiness fast. | |||
| 342 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 343 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 344 | - | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |||
| 345 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | 406 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | ||
| 346 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | 407 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | ||
| 347 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | 408 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return([]domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18080, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | ||
| @@ -369,10 +430,12 @@ func TestDeploy_Mockery_HTTPReadinessFailureLeavesNoOldGeneration(t *testing.T) | |||||
| 369 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | 430 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | ||
| 370 | require.Len(t, result.Services["web"].Diagnostics, 1, "failure diagnostics are kept separately") | 431 | require.Len(t, result.Services["web"].Diagnostics, 1, "failure diagnostics are kept separately") | ||
| 371 | assert.Equal(t, "boom", result.Services["web"].Diagnostics[0]) | 432 | assert.Equal(t, "boom", result.Services["web"].Diagnostics[0]) | ||
| 372 | - | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-new", mock.Anything) | 433 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) |
| 434 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |||
| 373 | runtime.AssertNotCalled(t, "RemoveVolume", mock.Anything, mock.Anything, mock.Anything) | 435 | runtime.AssertNotCalled(t, "RemoveVolume", mock.Anything, mock.Anything, mock.Anything) | ||
| 374 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | 436 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | ||
| 375 | - | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | 437 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) |
| 438 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |||
| 376 | } | 439 | } | ||
| 377 | 440 | ||||
| 378 | func TestDeploy_Mockery_VolumeReplacementMarksUnsafe(t *testing.T) { | 441 | func TestDeploy_Mockery_VolumeReplacementMarksUnsafe(t *testing.T) { | ||
M internal/usecase/deployment/execute.go +141 -30
| @@ -20,12 +20,16 @@ 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. Every service is replaced sequentially — withdraw | |
| 23 | + | // sorted name order. A service without volumes, binds, or devices is | |
| 24 | + | // replaced with a short overlap: its replacement starts and becomes ready | |
| 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 | |
| 27 | + | // interrupted. Every other service is replaced sequentially — withdraw | |
| 24 | 28 | // traffic, retire the superseded container, start the replacement and wait | |
| 25 | - | // for its readiness probe — so a deployment may briefly interrupt the | |
| 26 | - | // service, and two Gordon-managed generations of one service never run at | |
| 27 | - | // the same time. Volumes are never deleted: no RemoveVolume call, no | |
| 28 | - | // volume-deletion flags on container removal. | |
| 29 | + | // for its readiness probe — so a deployment may briefly interrupt it and two | |
| 30 | + | // Gordon-managed generations of a single-writer service never run at the same | |
| 31 | + | // time. Volumes are never deleted: no RemoveVolume call, no volume-deletion | |
| 32 | + | // flags on container removal. | |
| 29 | 33 | // | |
| 30 | 34 | // It is the synchronous composition of the two engine phases: StartDeploy | |
| 31 | 35 | // claims the key and journals the operation, then ExecuteDeploy runs the | |
| @@ -147,7 +151,7 @@ func (s *Service) startDeployLocked(ctx context.Context, input DeployInput) (*St | |||
| 147 | 151 | ||
| 148 | 152 | // ExecuteDeploy performs the second phase of a deploy: it reacquires the app | |
| 149 | 153 | // lock, loads and verifies the claimed journal by app/op identity, runs the | |
| 150 | - | // image pull/pin and the sequential replacement, and records the terminal | |
| 154 | + | // image pull/pin and the per-service replacement, and records the terminal | |
| 151 | 155 | // outcome. A claim that is already terminal, or that does not match its | |
| 152 | 156 | // identity, is never executed: it replays its stored outcome instead. An | |
| 153 | 157 | // operation that stops mid-execution (shutdown or cancellation) leaves its | |
| @@ -305,6 +309,23 @@ func (s *Service) claimRevision(ctx context.Context, claim DeployClaim, op domai | |||
| 305 | 309 | // step is recorded as succeeded only after the runtime effect, the ACTIVE | |
| 306 | 310 | // publication, and the traffic apply all succeeded, so a journaled success | |
| 307 | 311 | // is never ahead of what is routed. | |
| 312 | + | // | |
| 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. | |
| 318 | + | // | |
| 319 | + | // If the publication or the traffic apply of an overlap step fails, the | |
| 320 | + | // 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. | |
| 308 | 329 | func (s *Service) runServiceStep( | |
| 309 | 330 | ctx context.Context, | |
| 310 | 331 | app, revision string, | |
| @@ -331,6 +352,10 @@ func (s *Service) runServiceStep( | |||
| 331 | 352 | Before: before, State: domain.AppStepPending, | |
| 332 | 353 | } | |
| 333 | 354 | var svcResult ServiceResult | |
| 355 | + | overlap := false | |
| 356 | + | // The grace of the generation being superseded, read before anything | |
| 357 | + | // publishes a record of its replacement. | |
| 358 | + | beforeGrace := activeStopGrace(active, p.name) | |
| 334 | 359 | if current, ok := s.unchangedService(ctx, app, p, active); ok { | |
| 335 | 360 | // Same image, spec, environment, and secret values already running: | |
| 336 | 361 | // keep the container. | |
| @@ -341,8 +366,12 @@ func (s *Service) runServiceStep( | |||
| 341 | 366 | RestartUnsafe: singleWriterRequired(p.spec), | |
| 342 | 367 | } | |
| 343 | 368 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | |
| 369 | + | } else if before != "" && overlapEligible(p.spec) { | |
| 370 | + | overlap = true | |
| 371 | + | step.Overlap = true | |
| 372 | + | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, s.journalCandidate(op, index+1)) | |
| 344 | 373 | } else { | |
| 345 | - | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | |
| 374 | + | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) | |
| 346 | 375 | } | |
| 347 | 376 | result.Services[p.name] = svcResult | |
| 348 | 377 | // A recorded candidate is authoritative while the step is unresolved: a | |
| @@ -367,22 +396,19 @@ func (s *Service) runServiceStep( | |||
| 367 | 396 | return fail(fmt.Errorf("deployment: service %q failed at %s: %s: %w", | |
| 368 | 397 | p.name, stepID, svcResult.Error, domain.ErrAppStateConflict)) | |
| 369 | 398 | } | |
| 370 | - | // Publish the per-service effective state: the new binds become the | |
| 371 | - | // service's recorded backend before the proxy is repointed at them. | |
| 372 | - | if err := s.publishService(ctx, app, revision, p, svcResult, op.Op); err != nil { | |
| 373 | - | svcResult.Result = "failed" | |
| 374 | - | svcResult.Error = err.Error() | |
| 399 | + | svcResult, err := s.publishAndRoute(ctx, app, revision, p, svcResult, op.Op) | |
| 400 | + | if err != nil { | |
| 401 | + | if overlap { | |
| 402 | + | svcResult.CleanupWarnings = append(svcResult.CleanupWarnings, CleanupWarning{ | |
| 403 | + | Service: p.name, Leftover: before, | |
| 404 | + | Detail: "superseded container kept running: the new generation was not confirmed published and routed", | |
| 405 | + | }) | |
| 406 | + | } | |
| 375 | 407 | result.Services[p.name] = svcResult | |
| 376 | 408 | return fail(err) | |
| 377 | 409 | } | |
| 378 | - | // Rebuild and publish the traffic graph. ACTIVE already names the new | |
| 379 | - | // container, but the operation is a failure until routing accepted it, | |
| 380 | - | // so a rejected graph must never be journaled as success. | |
| 381 | - | if err := s.refreshTraffic(ctx, app); err != nil { | |
| 382 | - | svcResult.Result = "failed" | |
| 383 | - | svcResult.Error = err.Error() | |
| 384 | - | result.Services[p.name] = svcResult | |
| 385 | - | return fail(err) | |
| 410 | + | if overlap { | |
| 411 | + | svcResult, step.BeforeRetired = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) | |
| 386 | 412 | } | |
| 387 | 413 | result.Services[p.name] = svcResult | |
| 388 | 414 | step.State = domain.AppStepSucceeded | |
| @@ -393,6 +419,44 @@ func (s *Service) runServiceStep( | |||
| 393 | 419 | return nil | |
| 394 | 420 | } | |
| 395 | 421 | ||
| 422 | + | // publishAndRoute publishes the per-service effective state, so the new | |
| 423 | + | // binds become the service's recorded backend, then rebuilds the traffic | |
| 424 | + | // graph so the proxy is repointed at them. ACTIVE already names the new | |
| 425 | + | // container when the graph is applied, but the operation is a failure until | |
| 426 | + | // routing accepted it, so a rejected graph is returned as a failed result and | |
| 427 | + | // never journaled as success. | |
| 428 | + | func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p pinnedService, svcResult ServiceResult, opID string) (ServiceResult, error) { | |
| 429 | + | if err := s.publishService(ctx, app, revision, p, svcResult, opID); err != nil { | |
| 430 | + | svcResult.Result = "failed" | |
| 431 | + | svcResult.Error = err.Error() | |
| 432 | + | return svcResult, err | |
| 433 | + | } | |
| 434 | + | if err := s.refreshTraffic(ctx, app); err != nil { | |
| 435 | + | svcResult.Result = "failed" | |
| 436 | + | svcResult.Error = err.Error() | |
| 437 | + | return svcResult, err | |
| 438 | + | } | |
| 439 | + | return svcResult, nil | |
| 440 | + | } | |
| 441 | + | ||
| 442 | + | // retireSuperseded stops the container an overlap replacement superseded, | |
| 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 | |
| 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) | |
| 451 | + | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | |
| 452 | + | if !retired.Gone && len(retired.Warnings) == 0 { | |
| 453 | + | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | |
| 454 | + | Service: service, Leftover: before, Detail: "superseded container not confirmed gone", | |
| 455 | + | }) | |
| 456 | + | } | |
| 457 | + | return result, retired.Gone | |
| 458 | + | } | |
| 459 | + | ||
| 396 | 460 | // unchangedService reports whether the service already runs exactly what p | |
| 397 | 461 | // would create, in a healthy published state: the same digest, service spec, | |
| 398 | 462 | // app environment, and shared networks, in a running container that is not | |
| @@ -579,7 +643,10 @@ func (s *Service) journalCandidate(op *domain.AppOperation, index int) candidate | |||
| 579 | 643 | ||
| 580 | 644 | // deployService replaces one service sequentially and returns its terminal | |
| 581 | 645 | // result. before is the ACTIVE container being superseded (empty on a first | |
| 582 | - | // deployment); beforeGrace is its effective stop grace. | |
| 646 | + | // deployment); beforeGrace is its effective stop grace. It serves every | |
| 647 | + | // service that is not overlap-eligible, a first deployment, and the | |
| 648 | + | // rebuild of a missing generation by start and restart; deployOverlap is the | |
| 649 | + | // overlapping alternative used by a deploy. | |
| 583 | 650 | // | |
| 584 | 651 | // The order is fixed: withdraw from traffic and confirm it, durably inhibit | |
| 585 | 652 | // a single-writer generation, retire the superseded container and confirm | |
| @@ -591,11 +658,8 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 591 | 658 | // Revalidate the current authorization and source before withdrawing or | |
| 592 | 659 | // retiring the serving generation. A reload between preflight and this | |
| 593 | 660 | // service step must fail without mutating the existing workload. | |
| 594 | - | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { | |
| 595 | - | return s.failResult(revision, before, "", err) | |
| 596 | - | } | |
| 597 | - | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | |
| 598 | - | return s.failResult(revision, before, "", err) | |
| 661 | + | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | |
| 662 | + | return failed | |
| 599 | 663 | } | |
| 600 | 664 | // Withdraw the service from traffic first and confirm it: a withdrawal | |
| 601 | 665 | // that cannot be applied must block every container mutation, so no | |
| @@ -629,6 +693,42 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 629 | 693 | return s.failResult(revision, before, "", fmt.Errorf("deployment: retire superseded container %s: %s", before, cleanupDetail(retired))) | |
| 630 | 694 | } | |
| 631 | 695 | } | |
| 696 | + | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | |
| 697 | + | } | |
| 698 | + | ||
| 699 | + | // 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 | |
| 706 | + | // published and routed (retireSuperseded). | |
| 707 | + | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | |
| 708 | + | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | |
| 709 | + | return failed | |
| 710 | + | } | |
| 711 | + | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | |
| 712 | + | } | |
| 713 | + | ||
| 714 | + | // revalidateService re-resolves the binds and devices of a service against | |
| 715 | + | // the current policy before any workload mutation. It returns the failed | |
| 716 | + | // result and false when either was revoked since preflight. | |
| 717 | + | func (s *Service) revalidateService(app, revision string, p pinnedService, before string) (ServiceResult, bool) { | |
| 718 | + | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { | |
| 719 | + | return s.failResult(revision, before, "", err), false | |
| 720 | + | } | |
| 721 | + | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | |
| 722 | + | return s.failResult(revision, before, "", err), false | |
| 723 | + | } | |
| 724 | + | return ServiceResult{}, true | |
| 725 | + | } | |
| 726 | + | ||
| 727 | + | // startCandidate creates and starts the replacement generation and waits for | |
| 728 | + | // its readiness probe. A candidate that fails readiness is removed after its | |
| 729 | + | // redacted diagnostics were captured; whatever generation it supersedes is | |
| 730 | + | // not touched here. | |
| 731 | + | func (s *Service) startCandidate(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | |
| 632 | 732 | candidate, err := s.createAndStart(ctx, app, revision, p, opID, journal) | |
| 633 | 733 | if err != nil { | |
| 634 | 734 | result := s.failResult(revision, before, "", err) | |
| @@ -642,10 +742,10 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 642 | 742 | created := candidate.Container | |
| 643 | 743 | if err := s.waitServiceReady(ctx, app, created.ID, deploymentReadiness(p.spec), candidate.TCPBinds); err != nil { | |
| 644 | 744 | // Capture redacted diagnostics while the failed replacement still | |
| 645 | - | // exists, then remove only that candidate. The superseded container | |
| 646 | - | // is already gone and is never recreated. Cleanup is best effort: a | |
| 647 | - | // canceled context can leave the candidate behind, which is then | |
| 648 | - | // reported as a leftover warning instead of a false success. | |
| 745 | + | // exists, then remove only that candidate. A superseded container | |
| 746 | + | // that was already retired is never recreated. Cleanup is best | |
| 747 | + | // effort: a canceled context can leave the candidate behind, which is | |
| 748 | + | // then reported as a leftover warning instead of a false success. | |
| 649 | 749 | tail := s.redactDiagnostics(ctx, app, p, s.logTail(ctx, created.ID)) | |
| 650 | 750 | cleanup := s.retireCandidate(ctx, app, p.name, created.ID) | |
| 651 | 751 | return ServiceResult{ | |
| @@ -673,10 +773,21 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||
| 673 | 773 | // singleWriterRequired reports services that must never have two generations | |
| 674 | 774 | // running concurrently. Persistent volumes and any bind (especially a | |
| 675 | 775 | // 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. | |
| 676 | 778 | func singleWriterRequired(spec domain.AppService) bool { | |
| 677 | 779 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | |
| 678 | 780 | } | |
| 679 | 781 | ||
| 782 | + | // 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. | |
| 787 | + | func overlapEligible(spec domain.AppService) bool { | |
| 788 | + | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 | |
| 789 | + | } | |
| 790 | + | ||
| 680 | 791 | // refreshTraffic rebuilds the proxy host index and applies the full | |
| 681 | 792 | // HTTP/L4 graph after activation. A failure is fatal to the caller: | |
| 682 | 793 | // ACTIVE is published but the new service is not routable, so the | |
| @@ -20,12 +20,16 @@ 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. Every service is replaced sequentially — withdraw | 23 | + | // sorted name order. A service without volumes, binds, or devices is |
| 24 | + | // replaced with a short overlap: its replacement starts and becomes ready | |||
| 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 | |||
| 27 | + | // interrupted. Every other service is replaced sequentially — withdraw | |||
| 24 | // traffic, retire the superseded container, start the replacement and wait | 28 | // traffic, retire the superseded container, start the replacement and wait | ||
| 25 | - | // for its readiness probe — so a deployment may briefly interrupt the | 29 | + | // for its readiness probe — so a deployment may briefly interrupt it and two |
| 26 | - | // service, and two Gordon-managed generations of one service never run at | 30 | + | // Gordon-managed generations of a single-writer service never run at the same |
| 27 | - | // the same time. Volumes are never deleted: no RemoveVolume call, no | 31 | + | // time. Volumes are never deleted: no RemoveVolume call, no volume-deletion |
| 28 | - | // volume-deletion flags on container removal. | 32 | + | // flags on container removal. |
| 29 | // | 33 | // | ||
| 30 | // It is the synchronous composition of the two engine phases: StartDeploy | 34 | // It is the synchronous composition of the two engine phases: StartDeploy | ||
| 31 | // claims the key and journals the operation, then ExecuteDeploy runs the | 35 | // claims the key and journals the operation, then ExecuteDeploy runs the | ||
| @@ -147,7 +151,7 @@ func (s *Service) startDeployLocked(ctx context.Context, input DeployInput) (*St | |||||
| 147 | 151 | ||||
| 148 | // ExecuteDeploy performs the second phase of a deploy: it reacquires the app | 152 | // ExecuteDeploy performs the second phase of a deploy: it reacquires the app | ||
| 149 | // lock, loads and verifies the claimed journal by app/op identity, runs the | 153 | // lock, loads and verifies the claimed journal by app/op identity, runs the | ||
| 150 | - | // image pull/pin and the sequential replacement, and records the terminal | 154 | + | // image pull/pin and the per-service replacement, and records the terminal |
| 151 | // outcome. A claim that is already terminal, or that does not match its | 155 | // outcome. A claim that is already terminal, or that does not match its | ||
| 152 | // identity, is never executed: it replays its stored outcome instead. An | 156 | // identity, is never executed: it replays its stored outcome instead. An | ||
| 153 | // operation that stops mid-execution (shutdown or cancellation) leaves its | 157 | // operation that stops mid-execution (shutdown or cancellation) leaves its | ||
| @@ -305,6 +309,23 @@ func (s *Service) claimRevision(ctx context.Context, claim DeployClaim, op domai | |||||
| 305 | // step is recorded as succeeded only after the runtime effect, the ACTIVE | 309 | // step is recorded as succeeded only after the runtime effect, the ACTIVE | ||
| 306 | // publication, and the traffic apply all succeeded, so a journaled success | 310 | // publication, and the traffic apply all succeeded, so a journaled success | ||
| 307 | // is never ahead of what is routed. | 311 | // is never ahead of what is routed. | ||
| 312 | + | // | |||
| 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. | |||
| 318 | + | // | |||
| 319 | + | // If the publication or the traffic apply of an overlap step fails, the | |||
| 320 | + | // 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. | |||
| 308 | func (s *Service) runServiceStep( | 329 | func (s *Service) runServiceStep( | ||
| 309 | ctx context.Context, | 330 | ctx context.Context, | ||
| 310 | app, revision string, | 331 | app, revision string, | ||
| @@ -331,6 +352,10 @@ func (s *Service) runServiceStep( | |||||
| 331 | Before: before, State: domain.AppStepPending, | 352 | Before: before, State: domain.AppStepPending, | ||
| 332 | } | 353 | } | ||
| 333 | var svcResult ServiceResult | 354 | var svcResult ServiceResult | ||
| 355 | + | overlap := false | |||
| 356 | + | // The grace of the generation being superseded, read before anything | |||
| 357 | + | // publishes a record of its replacement. | |||
| 358 | + | beforeGrace := activeStopGrace(active, p.name) | |||
| 334 | if current, ok := s.unchangedService(ctx, app, p, active); ok { | 359 | if current, ok := s.unchangedService(ctx, app, p, active); ok { | ||
| 335 | // Same image, spec, environment, and secret values already running: | 360 | // Same image, spec, environment, and secret values already running: | ||
| 336 | // keep the container. | 361 | // keep the container. | ||
| @@ -341,8 +366,12 @@ func (s *Service) runServiceStep( | |||||
| 341 | RestartUnsafe: singleWriterRequired(p.spec), | 366 | RestartUnsafe: singleWriterRequired(p.spec), | ||
| 342 | } | 367 | } | ||
| 343 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | 368 | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | ||
| 369 | + | } else if before != "" && overlapEligible(p.spec) { | |||
| 370 | + | overlap = true | |||
| 371 | + | step.Overlap = true | |||
| 372 | + | svcResult = s.deployOverlap(ctx, app, revision, p, op.Op, before, s.journalCandidate(op, index+1)) | |||
| 344 | } else { | 373 | } else { | ||
| 345 | - | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | 374 | + | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, beforeGrace, s.journalCandidate(op, index+1)) |
| 346 | } | 375 | } | ||
| 347 | result.Services[p.name] = svcResult | 376 | result.Services[p.name] = svcResult | ||
| 348 | // A recorded candidate is authoritative while the step is unresolved: a | 377 | // A recorded candidate is authoritative while the step is unresolved: a | ||
| @@ -367,22 +396,19 @@ func (s *Service) runServiceStep( | |||||
| 367 | return fail(fmt.Errorf("deployment: service %q failed at %s: %s: %w", | 396 | return fail(fmt.Errorf("deployment: service %q failed at %s: %s: %w", | ||
| 368 | p.name, stepID, svcResult.Error, domain.ErrAppStateConflict)) | 397 | p.name, stepID, svcResult.Error, domain.ErrAppStateConflict)) | ||
| 369 | } | 398 | } | ||
| 370 | - | // Publish the per-service effective state: the new binds become the | 399 | + | svcResult, err := s.publishAndRoute(ctx, app, revision, p, svcResult, op.Op) |
| 371 | - | // service's recorded backend before the proxy is repointed at them. | 400 | + | if err != nil { |
| 372 | - | if err := s.publishService(ctx, app, revision, p, svcResult, op.Op); err != nil { | 401 | + | if overlap { |
| 373 | - | svcResult.Result = "failed" | 402 | + | svcResult.CleanupWarnings = append(svcResult.CleanupWarnings, CleanupWarning{ |
| 374 | - | svcResult.Error = err.Error() | 403 | + | Service: p.name, Leftover: before, |
| 404 | + | Detail: "superseded container kept running: the new generation was not confirmed published and routed", | |||
| 405 | + | }) | |||
| 406 | + | } | |||
| 375 | result.Services[p.name] = svcResult | 407 | result.Services[p.name] = svcResult | ||
| 376 | return fail(err) | 408 | return fail(err) | ||
| 377 | } | 409 | } | ||
| 378 | - | // Rebuild and publish the traffic graph. ACTIVE already names the new | 410 | + | if overlap { |
| 379 | - | // container, but the operation is a failure until routing accepted it, | 411 | + | svcResult, step.BeforeRetired = s.retireSuperseded(ctx, app, p.name, before, beforeGrace, svcResult) |
| 380 | - | // so a rejected graph must never be journaled as success. | |||
| 381 | - | if err := s.refreshTraffic(ctx, app); err != nil { | |||
| 382 | - | svcResult.Result = "failed" | |||
| 383 | - | svcResult.Error = err.Error() | |||
| 384 | - | result.Services[p.name] = svcResult | |||
| 385 | - | return fail(err) | |||
| 386 | } | 412 | } | ||
| 387 | result.Services[p.name] = svcResult | 413 | result.Services[p.name] = svcResult | ||
| 388 | step.State = domain.AppStepSucceeded | 414 | step.State = domain.AppStepSucceeded | ||
| @@ -393,6 +419,44 @@ func (s *Service) runServiceStep( | |||||
| 393 | return nil | 419 | return nil | ||
| 394 | } | 420 | } | ||
| 395 | 421 | ||||
| 422 | + | // publishAndRoute publishes the per-service effective state, so the new | |||
| 423 | + | // binds become the service's recorded backend, then rebuilds the traffic | |||
| 424 | + | // graph so the proxy is repointed at them. ACTIVE already names the new | |||
| 425 | + | // container when the graph is applied, but the operation is a failure until | |||
| 426 | + | // routing accepted it, so a rejected graph is returned as a failed result and | |||
| 427 | + | // never journaled as success. | |||
| 428 | + | func (s *Service) publishAndRoute(ctx context.Context, app, revision string, p pinnedService, svcResult ServiceResult, opID string) (ServiceResult, error) { | |||
| 429 | + | if err := s.publishService(ctx, app, revision, p, svcResult, opID); err != nil { | |||
| 430 | + | svcResult.Result = "failed" | |||
| 431 | + | svcResult.Error = err.Error() | |||
| 432 | + | return svcResult, err | |||
| 433 | + | } | |||
| 434 | + | if err := s.refreshTraffic(ctx, app); err != nil { | |||
| 435 | + | svcResult.Result = "failed" | |||
| 436 | + | svcResult.Error = err.Error() | |||
| 437 | + | return svcResult, err | |||
| 438 | + | } | |||
| 439 | + | return svcResult, nil | |||
| 440 | + | } | |||
| 441 | + | ||||
| 442 | + | // retireSuperseded stops the container an overlap replacement superseded, | |||
| 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 | |||
| 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) | |||
| 451 | + | result.CleanupWarnings = append(result.CleanupWarnings, retired.Warnings...) | |||
| 452 | + | if !retired.Gone && len(retired.Warnings) == 0 { | |||
| 453 | + | result.CleanupWarnings = append(result.CleanupWarnings, CleanupWarning{ | |||
| 454 | + | Service: service, Leftover: before, Detail: "superseded container not confirmed gone", | |||
| 455 | + | }) | |||
| 456 | + | } | |||
| 457 | + | return result, retired.Gone | |||
| 458 | + | } | |||
| 459 | + | ||||
| 396 | // unchangedService reports whether the service already runs exactly what p | 460 | // unchangedService reports whether the service already runs exactly what p | ||
| 397 | // would create, in a healthy published state: the same digest, service spec, | 461 | // would create, in a healthy published state: the same digest, service spec, | ||
| 398 | // app environment, and shared networks, in a running container that is not | 462 | // app environment, and shared networks, in a running container that is not | ||
| @@ -579,7 +643,10 @@ func (s *Service) journalCandidate(op *domain.AppOperation, index int) candidate | |||||
| 579 | 643 | ||||
| 580 | // deployService replaces one service sequentially and returns its terminal | 644 | // deployService replaces one service sequentially and returns its terminal | ||
| 581 | // result. before is the ACTIVE container being superseded (empty on a first | 645 | // result. before is the ACTIVE container being superseded (empty on a first | ||
| 582 | - | // deployment); beforeGrace is its effective stop grace. | 646 | + | // deployment); beforeGrace is its effective stop grace. It serves every |
| 647 | + | // service that is not overlap-eligible, a first deployment, and the | |||
| 648 | + | // rebuild of a missing generation by start and restart; deployOverlap is the | |||
| 649 | + | // overlapping alternative used by a deploy. | |||
| 583 | // | 650 | // | ||
| 584 | // The order is fixed: withdraw from traffic and confirm it, durably inhibit | 651 | // The order is fixed: withdraw from traffic and confirm it, durably inhibit | ||
| 585 | // a single-writer generation, retire the superseded container and confirm | 652 | // a single-writer generation, retire the superseded container and confirm | ||
| @@ -591,11 +658,8 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 591 | // Revalidate the current authorization and source before withdrawing or | 658 | // Revalidate the current authorization and source before withdrawing or | ||
| 592 | // retiring the serving generation. A reload between preflight and this | 659 | // retiring the serving generation. A reload between preflight and this | ||
| 593 | // service step must fail without mutating the existing workload. | 660 | // service step must fail without mutating the existing workload. | ||
| 594 | - | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { | 661 | + | if failed, ok := s.revalidateService(app, revision, p, before); !ok { |
| 595 | - | return s.failResult(revision, before, "", err) | 662 | + | return failed |
| 596 | - | } | |||
| 597 | - | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | |||
| 598 | - | return s.failResult(revision, before, "", err) | |||
| 599 | } | 663 | } | ||
| 600 | // Withdraw the service from traffic first and confirm it: a withdrawal | 664 | // Withdraw the service from traffic first and confirm it: a withdrawal | ||
| 601 | // that cannot be applied must block every container mutation, so no | 665 | // that cannot be applied must block every container mutation, so no | ||
| @@ -629,6 +693,42 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 629 | return s.failResult(revision, before, "", fmt.Errorf("deployment: retire superseded container %s: %s", before, cleanupDetail(retired))) | 693 | return s.failResult(revision, before, "", fmt.Errorf("deployment: retire superseded container %s: %s", before, cleanupDetail(retired))) | ||
| 630 | } | 694 | } | ||
| 631 | } | 695 | } | ||
| 696 | + | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | |||
| 697 | + | } | |||
| 698 | + | ||||
| 699 | + | // 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 | |||
| 706 | + | // published and routed (retireSuperseded). | |||
| 707 | + | func (s *Service) deployOverlap(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | |||
| 708 | + | if failed, ok := s.revalidateService(app, revision, p, before); !ok { | |||
| 709 | + | return failed | |||
| 710 | + | } | |||
| 711 | + | return s.startCandidate(ctx, app, revision, p, opID, before, journal) | |||
| 712 | + | } | |||
| 713 | + | ||||
| 714 | + | // revalidateService re-resolves the binds and devices of a service against | |||
| 715 | + | // the current policy before any workload mutation. It returns the failed | |||
| 716 | + | // result and false when either was revoked since preflight. | |||
| 717 | + | func (s *Service) revalidateService(app, revision string, p pinnedService, before string) (ServiceResult, bool) { | |||
| 718 | + | if _, err := s.resolveServiceBinds(app, p.spec); err != nil { | |||
| 719 | + | return s.failResult(revision, before, "", err), false | |||
| 720 | + | } | |||
| 721 | + | if _, err := s.resolveServiceDevices(app, p.spec); err != nil { | |||
| 722 | + | return s.failResult(revision, before, "", err), false | |||
| 723 | + | } | |||
| 724 | + | return ServiceResult{}, true | |||
| 725 | + | } | |||
| 726 | + | ||||
| 727 | + | // startCandidate creates and starts the replacement generation and waits for | |||
| 728 | + | // its readiness probe. A candidate that fails readiness is removed after its | |||
| 729 | + | // redacted diagnostics were captured; whatever generation it supersedes is | |||
| 730 | + | // not touched here. | |||
| 731 | + | func (s *Service) startCandidate(ctx context.Context, app, revision string, p pinnedService, opID, before string, journal candidateJournal) ServiceResult { | |||
| 632 | candidate, err := s.createAndStart(ctx, app, revision, p, opID, journal) | 732 | candidate, err := s.createAndStart(ctx, app, revision, p, opID, journal) | ||
| 633 | if err != nil { | 733 | if err != nil { | ||
| 634 | result := s.failResult(revision, before, "", err) | 734 | result := s.failResult(revision, before, "", err) | ||
| @@ -642,10 +742,10 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 642 | created := candidate.Container | 742 | created := candidate.Container | ||
| 643 | if err := s.waitServiceReady(ctx, app, created.ID, deploymentReadiness(p.spec), candidate.TCPBinds); err != nil { | 743 | if err := s.waitServiceReady(ctx, app, created.ID, deploymentReadiness(p.spec), candidate.TCPBinds); err != nil { | ||
| 644 | // Capture redacted diagnostics while the failed replacement still | 744 | // Capture redacted diagnostics while the failed replacement still | ||
| 645 | - | // exists, then remove only that candidate. The superseded container | 745 | + | // exists, then remove only that candidate. A superseded container |
| 646 | - | // is already gone and is never recreated. Cleanup is best effort: a | 746 | + | // that was already retired is never recreated. Cleanup is best |
| 647 | - | // canceled context can leave the candidate behind, which is then | 747 | + | // effort: a canceled context can leave the candidate behind, which is |
| 648 | - | // reported as a leftover warning instead of a false success. | 748 | + | // then reported as a leftover warning instead of a false success. |
| 649 | tail := s.redactDiagnostics(ctx, app, p, s.logTail(ctx, created.ID)) | 749 | tail := s.redactDiagnostics(ctx, app, p, s.logTail(ctx, created.ID)) | ||
| 650 | cleanup := s.retireCandidate(ctx, app, p.name, created.ID) | 750 | cleanup := s.retireCandidate(ctx, app, p.name, created.ID) | ||
| 651 | return ServiceResult{ | 751 | return ServiceResult{ | ||
| @@ -673,10 +773,21 @@ func (s *Service) deployService(ctx context.Context, app, revision string, p pin | |||||
| 673 | // singleWriterRequired reports services that must never have two generations | 773 | // singleWriterRequired reports services that must never have two generations | ||
| 674 | // running concurrently. Persistent volumes and any bind (especially a | 774 | // running concurrently. Persistent volumes and any bind (especially a | ||
| 675 | // writable one) may be written by both, so they are treated identically. | 775 | // 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. | |||
| 676 | func singleWriterRequired(spec domain.AppService) bool { | 778 | func singleWriterRequired(spec domain.AppService) bool { | ||
| 677 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | 779 | return len(spec.Volumes) > 0 || len(spec.Binds) > 0 | ||
| 678 | } | 780 | } | ||
| 679 | 781 | ||||
| 782 | + | // 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. | |||
| 787 | + | func overlapEligible(spec domain.AppService) bool { | |||
| 788 | + | return len(spec.Volumes) == 0 && len(spec.Binds) == 0 && len(spec.Devices) == 0 | |||
| 789 | + | } | |||
| 790 | + | ||||
| 680 | // refreshTraffic rebuilds the proxy host index and applies the full | 791 | // refreshTraffic rebuilds the proxy host index and applies the full | ||
| 681 | // HTTP/L4 graph after activation. A failure is fatal to the caller: | 792 | // HTTP/L4 graph after activation. A failure is fatal to the caller: | ||
| 682 | // ACTIVE is published but the new service is not routable, so the | 793 | // ACTIVE is published but the new service is not routable, so the | ||
M internal/usecase/deployment/interrupted.go +156 -42
| @@ -3,6 +3,8 @@ package deployment | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "reflect" | |
| 7 | + | "slices" | |
| 6 | 8 | ||
| 7 | 9 | "github.com/bnema/zerowrap" | |
| 8 | 10 | ||
| @@ -88,18 +90,27 @@ func (s *Service) reconcileInterruptedDeploy(ctx context.Context, app string) er | |||
| 88 | 90 | ||
| 89 | 91 | // reconcileNeedsContainer reports whether one step still needs container | |
| 90 | 92 | // work: a step that recorded a candidate. A terminal operation only retries | |
| 91 | - | // the step that recorded a failed candidate; a pending step exists only in an | |
| 92 | - | // interrupted operation. | |
| 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. | |
| 93 | 96 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | |
| 94 | 97 | if step.After == "" { | |
| 95 | 98 | return false | |
| 96 | 99 | } | |
| 97 | 100 | if terminal { | |
| 98 | - | return step.State == domain.AppStepFailed | |
| 101 | + | return step.State == domain.AppStepFailed || supersededPending(step) | |
| 99 | 102 | } | |
| 100 | 103 | return true | |
| 101 | 104 | } | |
| 102 | 105 | ||
| 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 | + | ||
| 103 | 114 | // convergeInterruptedSteps retries the recorded candidate of every step that | |
| 104 | 115 | // needs container work and accumulates the bounded leftovers in the journal. | |
| 105 | 116 | // It reports whether the journal changed and fails closed on the first | |
| @@ -111,18 +122,19 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 111 | 122 | if !reconcileNeedsContainer(*step, terminal) { | |
| 112 | 123 | continue | |
| 113 | 124 | } | |
| 125 | + | recorded := *step | |
| 114 | 126 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | |
| 115 | - | if step.After == "" { | |
| 116 | - | // Converging the candidate clears its recorded After, so the | |
| 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 | |
| 117 | 130 | // journal changed even when nothing was left over to report: the | |
| 118 | 131 | // converged step must be persisted, or every later pass reconciles | |
| 119 | 132 | // it again. | |
| 120 | 133 | changed = true | |
| 121 | 134 | } | |
| 122 | - | if len(warnings) > 0 { | |
| 135 | + | if appendNewWarnings(op, warnings) { | |
| 123 | 136 | // A candidate that will not go away is operator-visible instead of | |
| 124 | 137 | // staying untracked. | |
| 125 | - | op.Warnings = append(op.Warnings, journalWarnings(warnings)...) | |
| 126 | 138 | changed = true | |
| 127 | 139 | } | |
| 128 | 140 | if convErr != nil { | |
| @@ -144,10 +156,18 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 144 | 156 | // rebuild the generation recorded in ACTIVE without two generations running | |
| 145 | 157 | // at once. An interrupted operation's step is marked failed. | |
| 146 | 158 | // | |
| 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 | + | // | |
| 147 | 166 | // A candidate that cannot be removed is returned as an error, not a | |
| 148 | 167 | // recoverable warning: the orphan may still run, so every later mutation must | |
| 149 | 168 | // fail closed instead of proceeding. The returned warnings are the leftovers | |
| 150 | - | // of that removal. | |
| 169 | + | // of that removal. A superseded container that cannot be retired is only a | |
| 170 | + | // warning (see convergeSuperseded). | |
| 151 | 171 | // | |
| 152 | 172 | // Success clears the step's recorded After and keeps the removed (or kept) | |
| 153 | 173 | // container id in Detail, so the audit trail survives while the converged step | |
| @@ -156,52 +176,146 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||
| 156 | 176 | // | |
| 157 | 177 | // Once the candidate is converged, the inhibition the replacement wrote for | |
| 158 | 178 | // the superseded container is cleared. A step carrying both Before and After | |
| 159 | - | // records that the superseded container was confirmed gone before the | |
| 160 | - | // candidate was created (deployService retires it first), so the marker | |
| 161 | - | // protects nothing and would otherwise refuse the boot/start/restart rebuild | |
| 162 | - | // from ACTIVE. | |
| 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. | |
| 163 | 184 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | |
| 164 | 185 | candidate := step.After | |
| 165 | - | if publishedContainer(active, candidate) { | |
| 166 | - | // The published generation: keep it. A step that already succeeded is | |
| 167 | - | // left as recorded; a routing apply that never ran is republished by | |
| 168 | - | // the periodic recovery pass. | |
| 169 | - | if !terminal && step.State != domain.AppStepSucceeded { | |
| 170 | - | step.State = domain.AppStepFailed | |
| 171 | - | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" | |
| 172 | - | } | |
| 173 | - | step.Detail = "converged: published container " + candidate + " kept" | |
| 174 | - | } else { | |
| 175 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) | |
| 176 | - | if !retired.Gone { | |
| 177 | - | if !terminal { | |
| 178 | - | step.State = domain.AppStepFailed | |
| 179 | - | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) | |
| 180 | - | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") | |
| 181 | - | } | |
| 182 | - | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) | |
| 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 | |
| 183 | 204 | } | |
| 184 | - | if !terminal { | |
| 185 | - | step.State = domain.AppStepFailed | |
| 186 | - | step.Error = "interrupted before publication; the unpublished candidate was removed" | |
| 187 | - | } | |
| 188 | - | step.Detail = "converged: removed unpublished candidate " + candidate | |
| 189 | 205 | } | |
| 190 | 206 | // The candidate is gone (or published): the replacement-pending | |
| 191 | 207 | // inhibition of the superseded container is stale. Dropping it lets | |
| 192 | 208 | // boot/start/restart rebuild that generation from ACTIVE; the single | |
| 193 | 209 | // writer is preserved because the superseded container was proven gone | |
| 194 | - | // before the candidate existed. | |
| 195 | - | if step.Service != "" && step.Before != "" && step.Before != candidate { | |
| 210 | + | // before the candidate existed. An overlap step wrote no inhibition. | |
| 211 | + | if !step.Overlap && step.Service != "" && step.Before != "" && step.Before != candidate { | |
| 196 | 212 | if err := s.clearRecoveryInhibition(ctx, app, step.Service, step.Before); err != nil { | |
| 197 | 213 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | |
| 198 | - | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}}, nil | |
| 214 | + | return append(warnings, CleanupWarning{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}), nil | |
| 199 | 215 | } | |
| 200 | 216 | } | |
| 201 | - | // Only reconciliation removes a recorded candidate: clearing After here is | |
| 202 | - | // what marks the step converged. | |
| 203 | - | step.After = "" | |
| 204 | - | return nil, nil | |
| 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 | |
| 239 | + | } | |
| 240 | + | ||
| 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) | |
| 292 | + | } | |
| 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") | |
| 310 | + | } | |
| 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 | + | } | |
| 317 | + | step.Detail = "converged: removed unpublished candidate " + candidate | |
| 318 | + | return retired.Warnings, nil | |
| 205 | 319 | } | |
| 206 | 320 | ||
| 207 | 321 | // interruptedOutcome classifies an interrupted operation: a failure, unless | |
| @@ -3,6 +3,8 @@ package deployment | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "reflect" | |||
| 7 | + | "slices" | |||
| 6 | 8 | ||||
| 7 | "github.com/bnema/zerowrap" | 9 | "github.com/bnema/zerowrap" | ||
| 8 | 10 | ||||
| @@ -88,18 +90,27 @@ func (s *Service) reconcileInterruptedDeploy(ctx context.Context, app string) er | |||||
| 88 | 90 | ||||
| 89 | // reconcileNeedsContainer reports whether one step still needs container | 91 | // reconcileNeedsContainer reports whether one step still needs container | ||
| 90 | // work: a step that recorded a candidate. A terminal operation only retries | 92 | // work: a step that recorded a candidate. A terminal operation only retries | ||
| 91 | - | // the step that recorded a failed candidate; a pending step exists only in an | 93 | + | // the step that recorded a failed candidate, or an overlap step whose |
| 92 | - | // interrupted operation. | 94 | + | // superseded container was not confirmed retired; a pending step exists only |
| 95 | + | // in an interrupted operation. | |||
| 93 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | 96 | func reconcileNeedsContainer(step domain.AppOperationStep, terminal bool) bool { | ||
| 94 | if step.After == "" { | 97 | if step.After == "" { | ||
| 95 | return false | 98 | return false | ||
| 96 | } | 99 | } | ||
| 97 | if terminal { | 100 | if terminal { | ||
| 98 | - | return step.State == domain.AppStepFailed | 101 | + | return step.State == domain.AppStepFailed || supersededPending(step) |
| 99 | } | 102 | } | ||
| 100 | return true | 103 | return true | ||
| 101 | } | 104 | } | ||
| 102 | 105 | ||||
| 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 | + | ||||
| 103 | // convergeInterruptedSteps retries the recorded candidate of every step that | 114 | // convergeInterruptedSteps retries the recorded candidate of every step that | ||
| 104 | // needs container work and accumulates the bounded leftovers in the journal. | 115 | // needs container work and accumulates the bounded leftovers in the journal. | ||
| 105 | // It reports whether the journal changed and fails closed on the first | 116 | // It reports whether the journal changed and fails closed on the first | ||
| @@ -111,18 +122,19 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 111 | if !reconcileNeedsContainer(*step, terminal) { | 122 | if !reconcileNeedsContainer(*step, terminal) { | ||
| 112 | continue | 123 | continue | ||
| 113 | } | 124 | } | ||
| 125 | + | recorded := *step | |||
| 114 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | 126 | warnings, convErr := s.convergeCandidate(ctx, app, active, step, terminal, log) | ||
| 115 | - | if step.After == "" { | 127 | + | if !reflect.DeepEqual(recorded, *step) { |
| 116 | - | // Converging the candidate clears its recorded After, so the | 128 | + | // Converging the candidate changes the recorded step (it clears |
| 129 | + | // After, or marks the superseded container retired), so the | |||
| 117 | // journal changed even when nothing was left over to report: the | 130 | // journal changed even when nothing was left over to report: the | ||
| 118 | // converged step must be persisted, or every later pass reconciles | 131 | // converged step must be persisted, or every later pass reconciles | ||
| 119 | // it again. | 132 | // it again. | ||
| 120 | changed = true | 133 | changed = true | ||
| 121 | } | 134 | } | ||
| 122 | - | if len(warnings) > 0 { | 135 | + | if appendNewWarnings(op, warnings) { |
| 123 | // A candidate that will not go away is operator-visible instead of | 136 | // A candidate that will not go away is operator-visible instead of | ||
| 124 | // staying untracked. | 137 | // staying untracked. | ||
| 125 | - | op.Warnings = append(op.Warnings, journalWarnings(warnings)...) | |||
| 126 | changed = true | 138 | changed = true | ||
| 127 | } | 139 | } | ||
| 128 | if convErr != nil { | 140 | if convErr != nil { | ||
| @@ -144,10 +156,18 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 144 | // rebuild the generation recorded in ACTIVE without two generations running | 156 | // rebuild the generation recorded in ACTIVE without two generations running | ||
| 145 | // at once. An interrupted operation's step is marked failed. | 157 | // at once. An interrupted operation's step is marked failed. | ||
| 146 | // | 158 | // | ||
| 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 | + | // | |||
| 147 | // A candidate that cannot be removed is returned as an error, not a | 166 | // A candidate that cannot be removed is returned as an error, not a | ||
| 148 | // recoverable warning: the orphan may still run, so every later mutation must | 167 | // recoverable warning: the orphan may still run, so every later mutation must | ||
| 149 | // fail closed instead of proceeding. The returned warnings are the leftovers | 168 | // fail closed instead of proceeding. The returned warnings are the leftovers | ||
| 150 | - | // of that removal. | 169 | + | // of that removal. A superseded container that cannot be retired is only a |
| 170 | + | // warning (see convergeSuperseded). | |||
| 151 | // | 171 | // | ||
| 152 | // Success clears the step's recorded After and keeps the removed (or kept) | 172 | // Success clears the step's recorded After and keeps the removed (or kept) | ||
| 153 | // container id in Detail, so the audit trail survives while the converged step | 173 | // container id in Detail, so the audit trail survives while the converged step | ||
| @@ -156,52 +176,146 @@ func (s *Service) convergeInterruptedSteps(ctx context.Context, app string, op * | |||||
| 156 | // | 176 | // | ||
| 157 | // Once the candidate is converged, the inhibition the replacement wrote for | 177 | // Once the candidate is converged, the inhibition the replacement wrote for | ||
| 158 | // the superseded container is cleared. A step carrying both Before and After | 178 | // the superseded container is cleared. A step carrying both Before and After | ||
| 159 | - | // records that the superseded container was confirmed gone before the | 179 | + | // of a sequential replacement records that the superseded container was |
| 160 | - | // candidate was created (deployService retires it first), so the marker | 180 | + | // confirmed gone before the candidate was created (deployService retires it |
| 161 | - | // protects nothing and would otherwise refuse the boot/start/restart rebuild | 181 | + | // first), so the marker protects nothing and would otherwise refuse the |
| 162 | - | // from ACTIVE. | 182 | + | // boot/start/restart rebuild from ACTIVE. An overlap step wrote no |
| 183 | + | // inhibition. | |||
| 163 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | 184 | func (s *Service) convergeCandidate(ctx context.Context, app string, active domain.AppActive, step *domain.AppOperationStep, terminal bool, log zerowrap.Logger) ([]CleanupWarning, error) { | ||
| 164 | candidate := step.After | 185 | candidate := step.After | ||
| 165 | - | if publishedContainer(active, candidate) { | 186 | + | var warnings []CleanupWarning |
| 166 | - | // The published generation: keep it. A step that already succeeded is | 187 | + | // retry keeps the step eligible for a later pass: the candidate is kept |
| 167 | - | // left as recorded; a routing apply that never ran is republished by | 188 | + | // and its superseded container is not gone yet. |
| 168 | - | // the periodic recovery pass. | 189 | + | retry := false |
| 169 | - | if !terminal && step.State != domain.AppStepSucceeded { | 190 | + | switch { |
| 170 | - | step.State = domain.AppStepFailed | 191 | + | case publishedContainer(active, candidate): |
| 171 | - | step.Error = "interrupted before traffic publication; the published container is kept and routing is republished" | 192 | + | warnings = s.convergePublished(ctx, app, active, step, terminal, log) |
| 172 | - | } | 193 | + | retry = supersededPending(*step) |
| 173 | - | step.Detail = "converged: published container " + candidate + " kept" | 194 | + | case terminal && step.State == domain.AppStepSucceeded: |
| 174 | - | } else { | 195 | + | // A succeeded overlap step whose candidate is no longer published |
| 175 | - | retired := s.retireContainer(ctx, app, retireOptions{Service: step.Service, Force: true}, candidate) | 196 | + | // (ACTIVE moved on): only its superseded container is left to settle. |
| 176 | - | if !retired.Gone { | 197 | + | step.Detail = "converged: container " + candidate + " is no longer published" |
| 177 | - | if !terminal { | 198 | + | warnings = s.convergeSuperseded(ctx, app, active, step, false, log) |
| 178 | - | step.State = domain.AppStepFailed | 199 | + | retry = supersededPending(*step) |
| 179 | - | step.Error = "interrupted before publication; the unpublished candidate could not be removed: " + cleanupDetail(retired) | 200 | + | default: |
| 180 | - | log.Warn().Str("app", app).Str("container", candidate).Msg("deployment: unpublished candidate could not be removed") | 201 | + | var err error |
| 181 | - | } | 202 | + | if warnings, err = s.removeUnpublished(ctx, app, step, terminal, log); err != nil { |
| 182 | - | return retired.Warnings, fmt.Errorf("deployment: candidate %s of service %q could not be removed: %s", candidate, step.Service, cleanupDetail(retired)) | 203 | + | return warnings, err |
| 183 | } | 204 | } | ||
| 184 | - | if !terminal { | |||
| 185 | - | step.State = domain.AppStepFailed | |||
| 186 | - | step.Error = "interrupted before publication; the unpublished candidate was removed" | |||
| 187 | - | } | |||
| 188 | - | step.Detail = "converged: removed unpublished candidate " + candidate | |||
| 189 | } | 205 | } | ||
| 190 | // The candidate is gone (or published): the replacement-pending | 206 | // The candidate is gone (or published): the replacement-pending | ||
| 191 | // inhibition of the superseded container is stale. Dropping it lets | 207 | // inhibition of the superseded container is stale. Dropping it lets | ||
| 192 | // boot/start/restart rebuild that generation from ACTIVE; the single | 208 | // boot/start/restart rebuild that generation from ACTIVE; the single | ||
| 193 | // writer is preserved because the superseded container was proven gone | 209 | // writer is preserved because the superseded container was proven gone | ||
| 194 | - | // before the candidate existed. | 210 | + | // before the candidate existed. An overlap step wrote no inhibition. |
| 195 | - | if step.Service != "" && step.Before != "" && step.Before != candidate { | 211 | + | if !step.Overlap && step.Service != "" && step.Before != "" && step.Before != candidate { |
| 196 | if err := s.clearRecoveryInhibition(ctx, app, step.Service, step.Before); err != nil { | 212 | if err := s.clearRecoveryInhibition(ctx, app, step.Service, step.Before); err != nil { | ||
| 197 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | 213 | log.Warn().Err(err).Str("app", app).Str("container", step.Before).Msg("deployment: clear stale replacement inhibition") | ||
| 198 | - | return []CleanupWarning{{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}}, nil | 214 | + | return append(warnings, CleanupWarning{Service: step.Service, Leftover: step.Before, Detail: "clear recovery inhibition: " + err.Error()}), nil |
| 199 | } | 215 | } | ||
| 200 | } | 216 | } | ||
| 201 | - | // Only reconciliation removes a recorded candidate: clearing After here is | 217 | + | if !retry && (!terminal || step.State != domain.AppStepSucceeded) { |
| 202 | - | // what marks the step converged. | 218 | + | // Only reconciliation removes a recorded candidate: clearing After |
| 203 | - | step.After = "" | 219 | + | // here is what marks the step converged. The After of a succeeded |
| 204 | - | return nil, nil | 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 | |||
| 239 | + | } | |||
| 240 | + | ||||
| 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) | |||
| 292 | + | } | |||
| 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") | |||
| 310 | + | } | |||
| 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 | + | } | |||
| 317 | + | step.Detail = "converged: removed unpublished candidate " + candidate | |||
| 318 | + | return retired.Warnings, nil | |||
| 205 | } | 319 | } | ||
| 206 | 320 | ||||
| 207 | // interruptedOutcome classifies an interrupted operation: a failure, unless | 321 | // interruptedOutcome classifies an interrupted operation: a failure, unless | ||
A internal/usecase/deployment/overlap_replacement_test.go +285 -0
| @@ -0,0 +1,285 @@ | |||
| 1 | + | package deployment_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "os" | |
| 6 | + | "path/filepath" | |
| 7 | + | "testing" | |
| 8 | + | "time" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/zerowrap" | |
| 11 | + | "github.com/stretchr/testify/assert" | |
| 12 | + | "github.com/stretchr/testify/mock" | |
| 13 | + | "github.com/stretchr/testify/require" | |
| 14 | + | ||
| 15 | + | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | |
| 16 | + | "github.com/bnema/gordon/internal/domain" | |
| 17 | + | "github.com/bnema/gordon/internal/usecase/deployment" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | // TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder proves a service | |
| 21 | + | // with a volume, a bind, or a device is never run beside its predecessor: the | |
| 22 | + | // service is withdrawn, the superseded container is stopped and removed, and | |
| 23 | + | // only then is the replacement created. | |
| 24 | + | func TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder(t *testing.T) { | |
| 25 | + | root := t.TempDir() | |
| 26 | + | source := filepath.Join(root, "binds", "config") | |
| 27 | + | require.NoError(t, os.MkdirAll(source, 0o755)) | |
| 28 | + | ||
| 29 | + | cases := []struct { | |
| 30 | + | name string | |
| 31 | + | mutate func(*domain.AppService) | |
| 32 | + | inhibits bool | |
| 33 | + | cdiSupported bool | |
| 34 | + | }{ | |
| 35 | + | {"volume", func(s *domain.AppService) { s.Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} }, true, false}, | |
| 36 | + | {"bind", func(s *domain.AppService) { | |
| 37 | + | s.Binds = []domain.AppBind{{Name: "config", Path: "/etc/app.conf", ReadOnly: true}} | |
| 38 | + | }, true, false}, | |
| 39 | + | {"device", func(s *domain.AppService) { s.Devices = []string{"test_gpu"} }, false, true}, | |
| 40 | + | } | |
| 41 | + | for _, tc := range cases { | |
| 42 | + | t.Run(tc.name, func(t *testing.T) { | |
| 43 | + | ctx := context.Background() | |
| 44 | + | state, runtime, images, secrets := mockDeps(t) | |
| 45 | + | rev := mockRevision() | |
| 46 | + | tc.mutate(&rev.Spec.Services[0]) | |
| 47 | + | ||
| 48 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 49 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil).Maybe() | |
| 50 | + | runtime.EXPECT().CreateVolume(mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() | |
| 51 | + | if tc.cdiSupported { | |
| 52 | + | runtime.EXPECT().SupportsCDIDevices(mock.Anything).Return(nil) | |
| 53 | + | } | |
| 54 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 55 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 56 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 57 | + | if tc.inhibits { | |
| 58 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | var order []string | |
| 62 | + | expectRetireOld(state, runtime, &order, "c-old") | |
| 63 | + | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |
| 64 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn( | |
| 65 | + | func(context.Context, domain.AppActive) error { | |
| 66 | + | recordEvent(&order, "publish-active") | |
| 67 | + | return nil | |
| 68 | + | }).Once() | |
| 69 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 70 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 71 | + | ||
| 72 | + | svc := deployment.NewService(deployment.Deps{ | |
| 73 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |
| 74 | + | Traffic: &orderedTraffic{events: &order}, | |
| 75 | + | }, zerowrap.Default()). | |
| 76 | + | WithBindPolicies(map[string]domain.AppBindPolicy{"config": { | |
| 77 | + | Name: "config", Source: source, Root: root, AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}, | |
| 78 | + | }}). | |
| 79 | + | WithDevicePolicies(map[string]domain.AppDevicePolicy{"test_gpu": { | |
| 80 | + | Name: "test_gpu", CDI: []string{"example.com/gpu=GPU-test-uuid"}, | |
| 81 | + | AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}, | |
| 82 | + | }}). | |
| 83 | + | WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 84 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 85 | + | func(context.Context, string) error { return nil }, | |
| 86 | + | )) | |
| 87 | + | ||
| 88 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 89 | + | require.NoError(t, err) | |
| 90 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 91 | + | assert.Equal(t, | |
| 92 | + | []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "publish-active", "traffic"}, | |
| 93 | + | order, "the superseded container is gone before the replacement is created") | |
| 94 | + | }) | |
| 95 | + | } | |
| 96 | + | } | |
| 97 | + | ||
| 98 | + | // TestDeploy_OverlapPublicationFailureKeepsOldContainer proves a failed ACTIVE | |
| 99 | + | // publication of an overlap replacement never stops the superseded container, | |
| 100 | + | // which still serves because ACTIVE and routing were not switched. The | |
| 101 | + | // candidate stays journaled as the step's After, so reconciliation removes it, | |
| 102 | + | // and the failure is reported with a leftover warning on the old container. | |
| 103 | + | func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |
| 104 | + | ctx := context.Background() | |
| 105 | + | state, runtime, images, secrets := mockDeps(t) | |
| 106 | + | rev := mockRevision() | |
| 107 | + | ||
| 108 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 109 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 110 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 111 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 112 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 113 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(assert.AnError).Once() | |
| 114 | + | ||
| 115 | + | var events []string | |
| 116 | + | svc := deployment.NewService(deployment.Deps{ | |
| 117 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |
| 118 | + | Traffic: &orderedTraffic{events: &events}, | |
| 119 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 120 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 121 | + | func(context.Context, string) error { return nil }, | |
| 122 | + | )) | |
| 123 | + | ||
| 124 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 125 | + | require.Error(t, err) | |
| 126 | + | assert.Equal(t, "failed", result.Services["web"].Result) | |
| 127 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |
| 128 | + | require.Len(t, result.Services["web"].CleanupWarnings, 1) | |
| 129 | + | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | |
| 130 | + | assert.Empty(t, events, "the proxy graph was neither withdrawn nor rebuilt") | |
| 131 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 132 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 133 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |
| 134 | + | } | |
| 135 | + | ||
| 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) { | |
| 145 | + | t.Helper() | |
| 146 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |
| 147 | + | 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}}, | |
| 149 | + | }})) | |
| 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)) | |
| 158 | + | } | |
| 159 | + | ||
| 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) { | |
| 165 | + | ctx := context.Background() | |
| 166 | + | store := newTestStore(t) | |
| 167 | + | images := outmocks.NewMockImageResolver(t) | |
| 168 | + | 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 | + | }) | |
| 173 | + | ||
| 174 | + | var order []string | |
| 175 | + | 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) | |
| 187 | + | ||
| 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)) | |
| 193 | + | ||
| 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) | |
| 197 | + | ||
| 198 | + | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |
| 199 | + | 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") | |
| 206 | + | active, _, err := store.LoadActive(ctx, "blog") | |
| 207 | + | require.NoError(t, err) | |
| 208 | + | 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 | + | } | |
| 214 | + | ||
| 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) { | |
| 220 | + | ctx := context.Background() | |
| 221 | + | store := newTestStore(t) | |
| 222 | + | images := outmocks.NewMockImageResolver(t) | |
| 223 | + | 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 | + | }) | |
| 228 | + | ||
| 229 | + | 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()) | |
| 237 | + | ||
| 238 | + | require.NoError(t, svc.ReconcileBoot(ctx), "a leftover old container does not block reconciliation") | |
| 239 | + | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |
| 240 | + | 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") | |
| 252 | + | require.NoError(t, err) | |
| 253 | + | step, _ = opStep(op, "service.web.replace") | |
| 254 | + | assert.True(t, step.BeforeRetired) | |
| 255 | + | } | |
| 256 | + | ||
| 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) { | |
| 261 | + | ctx := context.Background() | |
| 262 | + | store := newTestStore(t) | |
| 263 | + | images := outmocks.NewMockImageResolver(t) | |
| 264 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 265 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |
| 266 | + | 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()}, | |
| 268 | + | }})) | |
| 269 | + | 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 | + | }}, | |
| 275 | + | })) | |
| 276 | + | ||
| 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()) | |
| 281 | + | require.NoError(t, svc.ReconcileBoot(ctx)) | |
| 282 | + | ||
| 283 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |
| 284 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |
| 285 | + | } | |
| @@ -0,0 +1,285 @@ | |||||
| 1 | + | package deployment_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "os" | |||
| 6 | + | "path/filepath" | |||
| 7 | + | "testing" | |||
| 8 | + | "time" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/zerowrap" | |||
| 11 | + | "github.com/stretchr/testify/assert" | |||
| 12 | + | "github.com/stretchr/testify/mock" | |||
| 13 | + | "github.com/stretchr/testify/require" | |||
| 14 | + | ||||
| 15 | + | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | |||
| 16 | + | "github.com/bnema/gordon/internal/domain" | |||
| 17 | + | "github.com/bnema/gordon/internal/usecase/deployment" | |||
| 18 | + | ) | |||
| 19 | + | ||||
| 20 | + | // TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder proves a service | |||
| 21 | + | // with a volume, a bind, or a device is never run beside its predecessor: the | |||
| 22 | + | // service is withdrawn, the superseded container is stopped and removed, and | |||
| 23 | + | // only then is the replacement created. | |||
| 24 | + | func TestDeploy_NonOverlapEligibleServicesKeepSequentialOrder(t *testing.T) { | |||
| 25 | + | root := t.TempDir() | |||
| 26 | + | source := filepath.Join(root, "binds", "config") | |||
| 27 | + | require.NoError(t, os.MkdirAll(source, 0o755)) | |||
| 28 | + | ||||
| 29 | + | cases := []struct { | |||
| 30 | + | name string | |||
| 31 | + | mutate func(*domain.AppService) | |||
| 32 | + | inhibits bool | |||
| 33 | + | cdiSupported bool | |||
| 34 | + | }{ | |||
| 35 | + | {"volume", func(s *domain.AppService) { s.Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} }, true, false}, | |||
| 36 | + | {"bind", func(s *domain.AppService) { | |||
| 37 | + | s.Binds = []domain.AppBind{{Name: "config", Path: "/etc/app.conf", ReadOnly: true}} | |||
| 38 | + | }, true, false}, | |||
| 39 | + | {"device", func(s *domain.AppService) { s.Devices = []string{"test_gpu"} }, false, true}, | |||
| 40 | + | } | |||
| 41 | + | for _, tc := range cases { | |||
| 42 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 43 | + | ctx := context.Background() | |||
| 44 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 45 | + | rev := mockRevision() | |||
| 46 | + | tc.mutate(&rev.Spec.Services[0]) | |||
| 47 | + | ||||
| 48 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 49 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil).Maybe() | |||
| 50 | + | runtime.EXPECT().CreateVolume(mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() | |||
| 51 | + | if tc.cdiSupported { | |||
| 52 | + | runtime.EXPECT().SupportsCDIDevices(mock.Anything).Return(nil) | |||
| 53 | + | } | |||
| 54 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 55 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |||
| 56 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 57 | + | if tc.inhibits { | |||
| 58 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | var order []string | |||
| 62 | + | expectRetireOld(state, runtime, &order, "c-old") | |||
| 63 | + | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |||
| 64 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn( | |||
| 65 | + | func(context.Context, domain.AppActive) error { | |||
| 66 | + | recordEvent(&order, "publish-active") | |||
| 67 | + | return nil | |||
| 68 | + | }).Once() | |||
| 69 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |||
| 70 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |||
| 71 | + | ||||
| 72 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 73 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 74 | + | Traffic: &orderedTraffic{events: &order}, | |||
| 75 | + | }, zerowrap.Default()). | |||
| 76 | + | WithBindPolicies(map[string]domain.AppBindPolicy{"config": { | |||
| 77 | + | Name: "config", Source: source, Root: root, AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}, | |||
| 78 | + | }}). | |||
| 79 | + | WithDevicePolicies(map[string]domain.AppDevicePolicy{"test_gpu": { | |||
| 80 | + | Name: "test_gpu", CDI: []string{"example.com/gpu=GPU-test-uuid"}, | |||
| 81 | + | AllowedApps: []string{"blog"}, AllowedServices: []string{"web"}, | |||
| 82 | + | }}). | |||
| 83 | + | WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 84 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 85 | + | func(context.Context, string) error { return nil }, | |||
| 86 | + | )) | |||
| 87 | + | ||||
| 88 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 89 | + | require.NoError(t, err) | |||
| 90 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 91 | + | assert.Equal(t, | |||
| 92 | + | []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "publish-active", "traffic"}, | |||
| 93 | + | order, "the superseded container is gone before the replacement is created") | |||
| 94 | + | }) | |||
| 95 | + | } | |||
| 96 | + | } | |||
| 97 | + | ||||
| 98 | + | // TestDeploy_OverlapPublicationFailureKeepsOldContainer proves a failed ACTIVE | |||
| 99 | + | // publication of an overlap replacement never stops the superseded container, | |||
| 100 | + | // which still serves because ACTIVE and routing were not switched. The | |||
| 101 | + | // candidate stays journaled as the step's After, so reconciliation removes it, | |||
| 102 | + | // and the failure is reported with a leftover warning on the old container. | |||
| 103 | + | func TestDeploy_OverlapPublicationFailureKeepsOldContainer(t *testing.T) { | |||
| 104 | + | ctx := context.Background() | |||
| 105 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 106 | + | rev := mockRevision() | |||
| 107 | + | ||||
| 108 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 109 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 110 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |||
| 111 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 112 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |||
| 113 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(assert.AnError).Once() | |||
| 114 | + | ||||
| 115 | + | var events []string | |||
| 116 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 117 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 118 | + | Traffic: &orderedTraffic{events: &events}, | |||
| 119 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 120 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 121 | + | func(context.Context, string) error { return nil }, | |||
| 122 | + | )) | |||
| 123 | + | ||||
| 124 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 125 | + | require.Error(t, err) | |||
| 126 | + | assert.Equal(t, "failed", result.Services["web"].Result) | |||
| 127 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |||
| 128 | + | require.Len(t, result.Services["web"].CleanupWarnings, 1) | |||
| 129 | + | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | |||
| 130 | + | assert.Empty(t, events, "the proxy graph was neither withdrawn nor rebuilt") | |||
| 131 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 132 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 133 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 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) { | |||
| 145 | + | t.Helper() | |||
| 146 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |||
| 147 | + | 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}}, | |||
| 149 | + | }})) | |||
| 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)) | |||
| 158 | + | } | |||
| 159 | + | ||||
| 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) { | |||
| 165 | + | ctx := context.Background() | |||
| 166 | + | store := newTestStore(t) | |||
| 167 | + | images := outmocks.NewMockImageResolver(t) | |||
| 168 | + | 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 | + | }) | |||
| 173 | + | ||||
| 174 | + | var order []string | |||
| 175 | + | 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) | |||
| 187 | + | ||||
| 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)) | |||
| 193 | + | ||||
| 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) | |||
| 197 | + | ||||
| 198 | + | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |||
| 199 | + | 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") | |||
| 206 | + | active, _, err := store.LoadActive(ctx, "blog") | |||
| 207 | + | require.NoError(t, err) | |||
| 208 | + | 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 | + | } | |||
| 214 | + | ||||
| 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) { | |||
| 220 | + | ctx := context.Background() | |||
| 221 | + | store := newTestStore(t) | |||
| 222 | + | images := outmocks.NewMockImageResolver(t) | |||
| 223 | + | 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 | + | }) | |||
| 228 | + | ||||
| 229 | + | 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()) | |||
| 237 | + | ||||
| 238 | + | require.NoError(t, svc.ReconcileBoot(ctx), "a leftover old container does not block reconciliation") | |||
| 239 | + | op, err := store.LoadOperation(ctx, "blog", "op-overlap") | |||
| 240 | + | 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") | |||
| 252 | + | require.NoError(t, err) | |||
| 253 | + | step, _ = opStep(op, "service.web.replace") | |||
| 254 | + | assert.True(t, step.BeforeRetired) | |||
| 255 | + | } | |||
| 256 | + | ||||
| 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) { | |||
| 261 | + | ctx := context.Background() | |||
| 262 | + | store := newTestStore(t) | |||
| 263 | + | images := outmocks.NewMockImageResolver(t) | |||
| 264 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 265 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog", Stopped: true})) | |||
| 266 | + | 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()}, | |||
| 268 | + | }})) | |||
| 269 | + | 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 | + | }}, | |||
| 275 | + | })) | |||
| 276 | + | ||||
| 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()) | |||
| 281 | + | require.NoError(t, svc.ReconcileBoot(ctx)) | |||
| 282 | + | ||||
| 283 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-old", mock.Anything) | |||
| 284 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |||
| 285 | + | } | |||
M internal/usecase/deployment/retirement_test.go +3 -1
| @@ -140,7 +140,7 @@ func TestRemove_FailedRetirementKeepsClaimsAndActiveState(t *testing.T) { | |||
| 140 | 140 | func TestDeploy_RetirementFailureBlocksReplacement(t *testing.T) { | |
| 141 | 141 | ctx := context.Background() | |
| 142 | 142 | state, runtime, images, secrets := mockDeps(t) | |
| 143 | - | rev := mockRevision() | |
| 143 | + | rev := statefulMockRevision() | |
| 144 | 144 | ||
| 145 | 145 | oldActive := domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 146 | 146 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: graceSpec(time.Second)}, | |
| @@ -156,6 +156,8 @@ func TestDeploy_RetirementFailureBlocksReplacement(t *testing.T) { | |||
| 156 | 156 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog", ID: "app-blog"}, nil) | |
| 157 | 157 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | |
| 158 | 158 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 159 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |
| 160 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |
| 159 | 161 | // The superseded container is retired with its own effective grace, and | |
| 160 | 162 | // its removal fails: the replacement must not start. | |
| 161 | 163 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", time.Second).Return(nil).Once() | |
| @@ -140,7 +140,7 @@ func TestRemove_FailedRetirementKeepsClaimsAndActiveState(t *testing.T) { | |||||
| 140 | func TestDeploy_RetirementFailureBlocksReplacement(t *testing.T) { | 140 | func TestDeploy_RetirementFailureBlocksReplacement(t *testing.T) { | ||
| 141 | ctx := context.Background() | 141 | ctx := context.Background() | ||
| 142 | state, runtime, images, secrets := mockDeps(t) | 142 | state, runtime, images, secrets := mockDeps(t) | ||
| 143 | - | rev := mockRevision() | 143 | + | rev := statefulMockRevision() |
| 144 | 144 | ||||
| 145 | oldActive := domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | 145 | oldActive := domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | ||
| 146 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: graceSpec(time.Second)}, | 146 | "web": {EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: graceSpec(time.Second)}, | ||
| @@ -156,6 +156,8 @@ func TestDeploy_RetirementFailureBlocksReplacement(t *testing.T) { | |||||
| 156 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog", ID: "app-blog"}, nil) | 156 | state.EXPECT().LoadOwnership(mock.Anything, "blog").Return(domain.AppOwnership{App: "blog", ID: "app-blog"}, nil) | ||
| 157 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | 157 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | ||
| 158 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 158 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 159 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |||
| 160 | + | state.EXPECT().SaveRecoveryInhibition(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 159 | // The superseded container is retired with its own effective grace, and | 161 | // The superseded container is retired with its own effective grace, and | ||
| 160 | // its removal fails: the replacement must not start. | 162 | // its removal fails: the replacement must not start. | ||
| 161 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", time.Second).Return(nil).Once() | 163 | runtime.EXPECT().StopContainer(mock.Anything, "c-old", time.Second).Return(nil).Once() | ||
M internal/usecase/deployment/sequential_replacement_test.go +110 -22
| @@ -137,12 +137,13 @@ func expectRetireOld( | |||
| 137 | 137 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", containerID).Return(nil).Once() | |
| 138 | 138 | } | |
| 139 | 139 | ||
| 140 | - | // TestDeploy_SequentialReplacementNeverOverlapsGenerations proves the | |
| 141 | - | // replacement order for a public HTTP service without volumes: withdrawal is | |
| 142 | - | // confirmed first, the superseded container is stopped and removed before the | |
| 143 | - | // replacement is created, readiness runs before ACTIVE is published, and the | |
| 144 | - | // graph is applied last. | |
| 145 | - | func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | |
| 140 | + | // TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring proves the | |
| 141 | + | // replacement order for a public HTTP service without volumes, binds, or | |
| 142 | + | // devices: the candidate is created and ready while the superseded container | |
| 143 | + | // keeps serving, ACTIVE and the proxy graph are switched to it, and only then | |
| 144 | + | // is the superseded container stopped and removed. The old container is never | |
| 145 | + | // withdrawn from traffic and never inhibited. | |
| 146 | + | func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing.T) { | |
| 146 | 147 | ctx := context.Background() | |
| 147 | 148 | state, runtime, images, secrets := mockDeps(t) | |
| 148 | 149 | rev := mockRevision() | |
| @@ -153,16 +154,18 @@ func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | |||
| 153 | 154 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 154 | 155 | ||
| 155 | 156 | var order []string | |
| 156 | - | expectRetireOld(state, runtime, &order, "c-old") | |
| 157 | 157 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |
| 158 | 158 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 159 | - | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn( | |
| 159 | + | state.EXPECT().SaveActive(mock.Anything, mock.MatchedBy(func(active domain.AppActive) bool { | |
| 160 | + | return active.Services["web"].Container == "c-new" && active.Services["web"].BackendBinds[8080] == 18080 | |
| 161 | + | })).RunAndReturn( | |
| 160 | 162 | func(context.Context, domain.AppActive) error { | |
| 161 | 163 | recordEvent(&order, "publish-active") | |
| 162 | 164 | return nil | |
| 163 | 165 | }).Once() | |
| 164 | 166 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 165 | 167 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 168 | + | expectRetireOld(state, runtime, &order, "c-old") | |
| 166 | 169 | ||
| 167 | 170 | var probedURL string | |
| 168 | 171 | traffic := &orderedTraffic{events: &order} | |
| @@ -183,8 +186,83 @@ func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | |||
| 183 | 186 | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 184 | 187 | assert.Contains(t, probedURL, "127.0.0.1:18080", "readiness dials the replacement's fresh loopback bind") | |
| 185 | 188 | assert.Equal(t, | |
| 186 | - | []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "ready", "publish-active", "traffic"}, | |
| 187 | - | order) | |
| 189 | + | []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") | |
| 191 | + | assert.Empty(t, result.CleanupWarnings) | |
| 192 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | // TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace proves the | |
| 196 | + | // superseded container is stopped with the grace of the generation being | |
| 197 | + | // stopped (stop_grace from ACTIVE), not the grace of its replacement. | |
| 198 | + | func TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace(t *testing.T) { | |
| 199 | + | ctx := context.Background() | |
| 200 | + | state, runtime, images, secrets := mockDeps(t) | |
| 201 | + | rev := mockRevision() | |
| 202 | + | rev.Spec.Services[0].StopGrace = 3 * time.Second | |
| 203 | + | ||
| 204 | + | oldActive := replaceableActive() | |
| 205 | + | oldActive.Services["web"] = domain.AppEffectiveService{ | |
| 206 | + | EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: graceSpec(45 * time.Second), | |
| 207 | + | } | |
| 208 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 209 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 210 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | |
| 211 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 212 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 213 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |
| 214 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 215 | + | 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() | |
| 217 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 218 | + | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |
| 219 | + | ||
| 220 | + | svc := deployment.NewService(deployment.Deps{ | |
| 221 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: &orderedTraffic{}, | |
| 222 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 223 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 224 | + | func(context.Context, string) error { return nil }, | |
| 225 | + | )) | |
| 226 | + | ||
| 227 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 228 | + | require.NoError(t, err) | |
| 229 | + | } | |
| 230 | + | ||
| 231 | + | // TestDeploy_OverlapOldRetirementFailureIsACleanupWarning proves a superseded | |
| 232 | + | // container that cannot be removed does not fail a deploy whose new generation | |
| 233 | + | // already serves: the leftover is a cleanup warning on the service and the | |
| 234 | + | // journal, and the step stays eligible for reconciliation. | |
| 235 | + | func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |
| 236 | + | ctx := context.Background() | |
| 237 | + | state, runtime, images, secrets := mockDeps(t) | |
| 238 | + | rev := mockRevision() | |
| 239 | + | ||
| 240 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 241 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 242 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 243 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 244 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 245 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |
| 246 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |
| 247 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |
| 248 | + | 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() | |
| 250 | + | ||
| 251 | + | svc := deployment.NewService(deployment.Deps{ | |
| 252 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: &orderedTraffic{}, | |
| 253 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 254 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 255 | + | func(context.Context, string) error { return nil }, | |
| 256 | + | )) | |
| 257 | + | ||
| 258 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |
| 259 | + | require.NoError(t, err, "the new generation serves: a retirement leftover is not a deploy failure") | |
| 260 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 261 | + | require.Len(t, result.CleanupWarnings, 1) | |
| 262 | + | assert.Equal(t, "c-old", result.CleanupWarnings[0].Leftover) | |
| 263 | + | assert.Equal(t, "web", result.CleanupWarnings[0].Service) | |
| 264 | + | assert.Contains(t, result.CleanupWarnings[0].Detail, "remove") | |
| 265 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |
| 188 | 266 | } | |
| 189 | 267 | ||
| 190 | 268 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | |
| @@ -219,15 +297,25 @@ func TestDeploy_PreflightFailureLeavesTheWorkloadUntouched(t *testing.T) { | |||
| 219 | 297 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | |
| 220 | 298 | } | |
| 221 | 299 | ||
| 300 | + | // statefulMockRevision is mockRevision with a persistent volume, so the | |
| 301 | + | // service keeps the sequential replacement. | |
| 302 | + | func statefulMockRevision() domain.AppDesiredRevision { | |
| 303 | + | rev := mockRevision() | |
| 304 | + | rev.Spec.Services[0].Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} | |
| 305 | + | return rev | |
| 306 | + | } | |
| 307 | + | ||
| 222 | 308 | // TestDeploy_WithdrawalFailureBlocksMutation proves a traffic withdrawal that | |
| 223 | - | // cannot be applied leaves the running generation untouched: no container is | |
| 224 | - | // stopped, removed, created, or started, and no ACTIVE state is published. | |
| 309 | + | // cannot be applied leaves the running generation of a single-writer service | |
| 310 | + | // untouched: no container is stopped, removed, created, or started, and no | |
| 311 | + | // ACTIVE state is published. | |
| 225 | 312 | func TestDeploy_WithdrawalFailureBlocksMutation(t *testing.T) { | |
| 226 | 313 | ctx := context.Background() | |
| 227 | 314 | state, runtime, images, secrets := mockDeps(t) | |
| 228 | - | rev := mockRevision() | |
| 315 | + | rev := statefulMockRevision() | |
| 229 | 316 | ||
| 230 | 317 | expectDeployPreflight(state, runtime, images, secrets, rev) | |
| 318 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |
| 231 | 319 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 232 | 320 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 233 | 321 | ||
| @@ -304,7 +392,8 @@ func TestDeploy_FirstDeploymentRetiresNothing(t *testing.T) { | |||
| 304 | 392 | // TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate proves a | |
| 305 | 393 | // replacement that is not ready is journaled as failed, that ACTIVE is never | |
| 306 | 394 | // published for it, and that the unready candidate is removed. The superseded | |
| 307 | - | // container is already gone and is never recreated. | |
| 395 | + | // container keeps serving: it is never withdrawn, stopped, removed, or | |
| 396 | + | // inhibited. | |
| 308 | 397 | func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) { | |
| 309 | 398 | ctx := context.Background() | |
| 310 | 399 | state, runtime, images, secrets := mockDeps(t) | |
| @@ -317,7 +406,6 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||
| 317 | 406 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 318 | 407 | ||
| 319 | 408 | var order []string | |
| 320 | - | expectRetireOld(state, runtime, &order, "c-old") | |
| 321 | 409 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | |
| 322 | 410 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | |
| 323 | 411 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).RunAndReturn( | |
| @@ -350,10 +438,10 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||
| 350 | 438 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | |
| 351 | 439 | assert.Equal(t, "c-new", result.Services["web"].After) | |
| 352 | 440 | ||
| 353 | - | // ACTIVE was never repointed and the replaced generation's inhibition was | |
| 354 | - | // never cleared: nothing claims the old workload is available again. | |
| 441 | + | // ACTIVE was never repointed and the old container was never touched. | |
| 355 | 442 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | |
| 356 | 443 | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | |
| 444 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |
| 357 | 445 | ||
| 358 | 446 | require.NotEmpty(t, saved) | |
| 359 | 447 | last := saved[len(saved)-1] | |
| @@ -362,9 +450,11 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||
| 362 | 450 | require.True(t, ok) | |
| 363 | 451 | assert.Equal(t, domain.AppStepFailed, step.State) | |
| 364 | 452 | ||
| 365 | - | assert.Equal(t, "remove-candidate", order[len(order)-1], "the unready candidate is the last thing removed") | |
| 366 | - | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-new", mock.Anything) | |
| 367 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", true) | |
| 453 | + | assert.Equal(t, []string{"create-new", "start-new", "remove-candidate"}, order, | |
| 454 | + | "the old generation is never withdrawn: no withdraw or traffic event, the unready candidate is removed") | |
| 455 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 456 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |
| 457 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |
| 368 | 458 | } | |
| 369 | 459 | ||
| 370 | 460 | // TestDeploy_CandidateCleanupFailureStaysVisible proves a candidate that | |
| @@ -381,7 +471,6 @@ func TestDeploy_CandidateCleanupFailureStaysVisible(t *testing.T) { | |||
| 381 | 471 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 382 | 472 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 383 | 473 | ||
| 384 | - | expectRetireOld(state, runtime, nil, "c-old") | |
| 385 | 474 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 386 | 475 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | |
| 387 | 476 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(assert.AnError).Once() | |
| @@ -424,7 +513,6 @@ func TestDeploy_CancellationDuringReadinessIsNotReportedAsSuccess(t *testing.T) | |||
| 424 | 513 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |
| 425 | 514 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |
| 426 | 515 | ||
| 427 | - | expectRetireOld(state, runtime, nil, "c-old") | |
| 428 | 516 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |
| 429 | 517 | // The runtime refuses to remove the candidate with a canceled context. | |
| 430 | 518 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(context.Canceled).Once() | |
| @@ -137,12 +137,13 @@ func expectRetireOld( | |||||
| 137 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", containerID).Return(nil).Once() | 137 | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", containerID).Return(nil).Once() | ||
| 138 | } | 138 | } | ||
| 139 | 139 | ||||
| 140 | - | // TestDeploy_SequentialReplacementNeverOverlapsGenerations proves the | 140 | + | // TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring proves the |
| 141 | - | // replacement order for a public HTTP service without volumes: withdrawal is | 141 | + | // replacement order for a public HTTP service without volumes, binds, or |
| 142 | - | // confirmed first, the superseded container is stopped and removed before the | 142 | + | // devices: the candidate is created and ready while the superseded container |
| 143 | - | // replacement is created, readiness runs before ACTIVE is published, and the | 143 | + | // keeps serving, ACTIVE and the proxy graph are switched to it, and only then |
| 144 | - | // graph is applied last. | 144 | + | // is the superseded container stopped and removed. The old container is never |
| 145 | - | func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | 145 | + | // withdrawn from traffic and never inhibited. |
| 146 | + | func TestDeploy_StatelessReplacementOverlapsAndSwitchesBeforeRetiring(t *testing.T) { | |||
| 146 | ctx := context.Background() | 147 | ctx := context.Background() | ||
| 147 | state, runtime, images, secrets := mockDeps(t) | 148 | state, runtime, images, secrets := mockDeps(t) | ||
| 148 | rev := mockRevision() | 149 | rev := mockRevision() | ||
| @@ -153,16 +154,18 @@ func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | |||||
| 153 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 154 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 154 | 155 | ||||
| 155 | var order []string | 156 | var order []string | ||
| 156 | - | expectRetireOld(state, runtime, &order, "c-old") | |||
| 157 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | 157 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | ||
| 158 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | 158 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | ||
| 159 | - | state.EXPECT().SaveActive(mock.Anything, mock.Anything).RunAndReturn( | 159 | + | state.EXPECT().SaveActive(mock.Anything, mock.MatchedBy(func(active domain.AppActive) bool { |
| 160 | + | return active.Services["web"].Container == "c-new" && active.Services["web"].BackendBinds[8080] == 18080 | |||
| 161 | + | })).RunAndReturn( | |||
| 160 | func(context.Context, domain.AppActive) error { | 162 | func(context.Context, domain.AppActive) error { | ||
| 161 | recordEvent(&order, "publish-active") | 163 | recordEvent(&order, "publish-active") | ||
| 162 | return nil | 164 | return nil | ||
| 163 | }).Once() | 165 | }).Once() | ||
| 164 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | 166 | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | ||
| 165 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | 167 | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | ||
| 168 | + | expectRetireOld(state, runtime, &order, "c-old") | |||
| 166 | 169 | ||||
| 167 | var probedURL string | 170 | var probedURL string | ||
| 168 | traffic := &orderedTraffic{events: &order} | 171 | traffic := &orderedTraffic{events: &order} | ||
| @@ -183,8 +186,83 @@ func TestDeploy_SequentialReplacementNeverOverlapsGenerations(t *testing.T) { | |||||
| 183 | assert.Equal(t, "deployed", result.Services["web"].Result) | 186 | assert.Equal(t, "deployed", result.Services["web"].Result) | ||
| 184 | assert.Contains(t, probedURL, "127.0.0.1:18080", "readiness dials the replacement's fresh loopback bind") | 187 | assert.Contains(t, probedURL, "127.0.0.1:18080", "readiness dials the replacement's fresh loopback bind") | ||
| 185 | assert.Equal(t, | 188 | assert.Equal(t, | ||
| 186 | - | []string{"withdraw", "stop-old", "remove-old", "create-new", "start-new", "ready", "publish-active", "traffic"}, | 189 | + | []string{"create-new", "start-new", "ready", "publish-active", "traffic", "stop-old", "remove-old"}, |
| 187 | - | order) | 190 | + | order, "the old container serves until the new one is published and routed; it is never withdrawn") |
| 191 | + | assert.Empty(t, result.CleanupWarnings) | |||
| 192 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |||
| 193 | + | } | |||
| 194 | + | ||||
| 195 | + | // TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace proves the | |||
| 196 | + | // superseded container is stopped with the grace of the generation being | |||
| 197 | + | // stopped (stop_grace from ACTIVE), not the grace of its replacement. | |||
| 198 | + | func TestDeploy_OverlapRetiresOldGenerationWithItsOwnStopGrace(t *testing.T) { | |||
| 199 | + | ctx := context.Background() | |||
| 200 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 201 | + | rev := mockRevision() | |||
| 202 | + | rev.Spec.Services[0].StopGrace = 3 * time.Second | |||
| 203 | + | ||||
| 204 | + | oldActive := replaceableActive() | |||
| 205 | + | oldActive.Services["web"] = domain.AppEffectiveService{ | |||
| 206 | + | EffectiveRevision: "rev-0", Image: "img:0", Container: "c-old", Spec: graceSpec(45 * time.Second), | |||
| 207 | + | } | |||
| 208 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 209 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 210 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(oldActive, true, nil) | |||
| 211 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 212 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |||
| 213 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 214 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |||
| 215 | + | 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() | |||
| 217 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 218 | + | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |||
| 219 | + | ||||
| 220 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 221 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: &orderedTraffic{}, | |||
| 222 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 223 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 224 | + | func(context.Context, string) error { return nil }, | |||
| 225 | + | )) | |||
| 226 | + | ||||
| 227 | + | _, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 228 | + | require.NoError(t, err) | |||
| 229 | + | } | |||
| 230 | + | ||||
| 231 | + | // TestDeploy_OverlapOldRetirementFailureIsACleanupWarning proves a superseded | |||
| 232 | + | // container that cannot be removed does not fail a deploy whose new generation | |||
| 233 | + | // already serves: the leftover is a cleanup warning on the service and the | |||
| 234 | + | // journal, and the step stays eligible for reconciliation. | |||
| 235 | + | func TestDeploy_OverlapOldRetirementFailureIsACleanupWarning(t *testing.T) { | |||
| 236 | + | ctx := context.Background() | |||
| 237 | + | state, runtime, images, secrets := mockDeps(t) | |||
| 238 | + | rev := mockRevision() | |||
| 239 | + | ||||
| 240 | + | expectDeployPreflight(state, runtime, images, secrets, rev) | |||
| 241 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 242 | + | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | |||
| 243 | + | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | |||
| 244 | + | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | |||
| 245 | + | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 246 | + | state.EXPECT().SaveOwnership(mock.Anything, mock.Anything).Return(nil) | |||
| 247 | + | state.EXPECT().ClearRecoveryInhibition(mock.Anything, "blog", "web", "c-old").Return(nil).Once() | |||
| 248 | + | 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() | |||
| 250 | + | ||||
| 251 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 252 | + | State: state, Runtime: runtime, Images: images, Secrets: secrets, Traffic: &orderedTraffic{}, | |||
| 253 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 254 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 255 | + | func(context.Context, string) error { return nil }, | |||
| 256 | + | )) | |||
| 257 | + | ||||
| 258 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog"}) | |||
| 259 | + | require.NoError(t, err, "the new generation serves: a retirement leftover is not a deploy failure") | |||
| 260 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 261 | + | require.Len(t, result.CleanupWarnings, 1) | |||
| 262 | + | assert.Equal(t, "c-old", result.CleanupWarnings[0].Leftover) | |||
| 263 | + | assert.Equal(t, "web", result.CleanupWarnings[0].Service) | |||
| 264 | + | assert.Contains(t, result.CleanupWarnings[0].Detail, "remove") | |||
| 265 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |||
| 188 | } | 266 | } | ||
| 189 | 267 | ||||
| 190 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | 268 | // TestDeploy_PreflightFailureLeavesTheWorkloadUntouched proves preflight runs | ||
| @@ -219,15 +297,25 @@ func TestDeploy_PreflightFailureLeavesTheWorkloadUntouched(t *testing.T) { | |||||
| 219 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | 297 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | ||
| 220 | } | 298 | } | ||
| 221 | 299 | ||||
| 300 | + | // statefulMockRevision is mockRevision with a persistent volume, so the | |||
| 301 | + | // service keeps the sequential replacement. | |||
| 302 | + | func statefulMockRevision() domain.AppDesiredRevision { | |||
| 303 | + | rev := mockRevision() | |||
| 304 | + | rev.Spec.Services[0].Volumes = []domain.AppVolume{{Name: "data", Path: "/data"}} | |||
| 305 | + | return rev | |||
| 306 | + | } | |||
| 307 | + | ||||
| 222 | // TestDeploy_WithdrawalFailureBlocksMutation proves a traffic withdrawal that | 308 | // TestDeploy_WithdrawalFailureBlocksMutation proves a traffic withdrawal that | ||
| 223 | - | // cannot be applied leaves the running generation untouched: no container is | 309 | + | // cannot be applied leaves the running generation of a single-writer service |
| 224 | - | // stopped, removed, created, or started, and no ACTIVE state is published. | 310 | + | // untouched: no container is stopped, removed, created, or started, and no |
| 311 | + | // ACTIVE state is published. | |||
| 225 | func TestDeploy_WithdrawalFailureBlocksMutation(t *testing.T) { | 312 | func TestDeploy_WithdrawalFailureBlocksMutation(t *testing.T) { | ||
| 226 | ctx := context.Background() | 313 | ctx := context.Background() | ||
| 227 | state, runtime, images, secrets := mockDeps(t) | 314 | state, runtime, images, secrets := mockDeps(t) | ||
| 228 | - | rev := mockRevision() | 315 | + | rev := statefulMockRevision() |
| 229 | 316 | ||||
| 230 | expectDeployPreflight(state, runtime, images, secrets, rev) | 317 | expectDeployPreflight(state, runtime, images, secrets, rev) | ||
| 318 | + | runtime.EXPECT().VolumeExists(mock.Anything, mock.Anything).Return(false, nil) | |||
| 231 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | 319 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | ||
| 232 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 320 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 233 | 321 | ||||
| @@ -304,7 +392,8 @@ func TestDeploy_FirstDeploymentRetiresNothing(t *testing.T) { | |||||
| 304 | // TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate proves a | 392 | // TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate proves a | ||
| 305 | // replacement that is not ready is journaled as failed, that ACTIVE is never | 393 | // replacement that is not ready is journaled as failed, that ACTIVE is never | ||
| 306 | // published for it, and that the unready candidate is removed. The superseded | 394 | // published for it, and that the unready candidate is removed. The superseded | ||
| 307 | - | // container is already gone and is never recreated. | 395 | + | // container keeps serving: it is never withdrawn, stopped, removed, or |
| 396 | + | // inhibited. | |||
| 308 | func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) { | 397 | func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) { | ||
| 309 | ctx := context.Background() | 398 | ctx := context.Background() | ||
| 310 | state, runtime, images, secrets := mockDeps(t) | 399 | state, runtime, images, secrets := mockDeps(t) | ||
| @@ -317,7 +406,6 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||||
| 317 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 406 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 318 | 407 | ||||
| 319 | var order []string | 408 | var order []string | ||
| 320 | - | expectRetireOld(state, runtime, &order, "c-old") | |||
| 321 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | 409 | expectSequentialCreate(state, runtime, &order, "c-new", 18080) | ||
| 322 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | 410 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | ||
| 323 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).RunAndReturn( | 411 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).RunAndReturn( | ||
| @@ -350,10 +438,10 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||||
| 350 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | 438 | assert.NotContains(t, result.Services["web"].Error, "logs:", "raw logs must never be embedded in the public error") | ||
| 351 | assert.Equal(t, "c-new", result.Services["web"].After) | 439 | assert.Equal(t, "c-new", result.Services["web"].After) | ||
| 352 | 440 | ||||
| 353 | - | // ACTIVE was never repointed and the replaced generation's inhibition was | 441 | + | // ACTIVE was never repointed and the old container was never touched. |
| 354 | - | // never cleared: nothing claims the old workload is available again. | |||
| 355 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | 442 | state.AssertNotCalled(t, "SaveActive", mock.Anything, mock.Anything) | ||
| 356 | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | 443 | state.AssertNotCalled(t, "ClearRecoveryInhibition", mock.Anything, mock.Anything, mock.Anything, mock.Anything) | ||
| 444 | + | state.AssertNotCalled(t, "SaveRecoveryInhibition", mock.Anything, mock.Anything) | |||
| 357 | 445 | ||||
| 358 | require.NotEmpty(t, saved) | 446 | require.NotEmpty(t, saved) | ||
| 359 | last := saved[len(saved)-1] | 447 | last := saved[len(saved)-1] | ||
| @@ -362,9 +450,11 @@ func TestDeploy_ReadinessFailureJournalsFailureAndRemovesCandidate(t *testing.T) | |||||
| 362 | require.True(t, ok) | 450 | require.True(t, ok) | ||
| 363 | assert.Equal(t, domain.AppStepFailed, step.State) | 451 | assert.Equal(t, domain.AppStepFailed, step.State) | ||
| 364 | 452 | ||||
| 365 | - | assert.Equal(t, "remove-candidate", order[len(order)-1], "the unready candidate is the last thing removed") | 453 | + | assert.Equal(t, []string{"create-new", "start-new", "remove-candidate"}, order, |
| 366 | - | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, "c-new", mock.Anything) | 454 | + | "the old generation is never withdrawn: no withdraw or traffic event, the unready candidate is removed") |
| 367 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", true) | 455 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) |
| 456 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-old", mock.Anything) | |||
| 457 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, "blog", "c-old") | |||
| 368 | } | 458 | } | ||
| 369 | 459 | ||||
| 370 | // TestDeploy_CandidateCleanupFailureStaysVisible proves a candidate that | 460 | // TestDeploy_CandidateCleanupFailureStaysVisible proves a candidate that | ||
| @@ -381,7 +471,6 @@ func TestDeploy_CandidateCleanupFailureStaysVisible(t *testing.T) { | |||||
| 381 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | 471 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | ||
| 382 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 472 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 383 | 473 | ||||
| 384 | - | expectRetireOld(state, runtime, nil, "c-old") | |||
| 385 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | 474 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | ||
| 386 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | 475 | runtime.EXPECT().GetContainerLogs(mock.Anything, "c-new", false).Return(nil, assert.AnError).Once() | ||
| 387 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(assert.AnError).Once() | 476 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(assert.AnError).Once() | ||
| @@ -424,7 +513,6 @@ func TestDeploy_CancellationDuringReadinessIsNotReportedAsSuccess(t *testing.T) | |||||
| 424 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | 513 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(replaceableActive(), true, nil) | ||
| 425 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | 514 | state.EXPECT().LoadDesired(mock.Anything, "blog").Return(rev, true, nil) | ||
| 426 | 515 | ||||
| 427 | - | expectRetireOld(state, runtime, nil, "c-old") | |||
| 428 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | 516 | expectSequentialCreate(state, runtime, nil, "c-new", 18080) | ||
| 429 | // The runtime refuses to remove the candidate with a canceled context. | 517 | // The runtime refuses to remove the candidate with a canceled context. | ||
| 430 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(context.Canceled).Once() | 518 | runtime.EXPECT().RemoveContainer(mock.Anything, "c-new", true).Return(context.Canceled).Once() | ||
M internal/usecase/deployment/traffic_failure_test.go +17 -9
| @@ -50,11 +50,8 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||
| 50 | 50 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil) | |
| 51 | 51 | ||
| 52 | 52 | expectNetworkProvision(runtime, "app-blog", 1) | |
| 53 | - | // The superseded generation is confirmed gone before the replacement is | |
| 54 | - | // created: a deploy never overlaps two generations of one service. | |
| 55 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 56 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 57 | - | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |
| 53 | + | // The superseded container serves beside the candidate and must survive | |
| 54 | + | // the failed switch: no stop, no removal, no claim release. | |
| 58 | 55 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |
| 59 | 56 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 60 | 57 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| @@ -88,10 +85,21 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||
| 88 | 85 | step, ok := opStep(last, "service.web.replace") | |
| 89 | 86 | require.True(t, ok) | |
| 90 | 87 | assert.Equal(t, domain.AppStepFailed, step.State, "the service step must not claim success before traffic applied") | |
| 91 | - | // The superseded container is gone: a rejected graph is reported as a | |
| 92 | - | // failure of the new generation, never as a return to the old one. | |
| 93 | - | runtime.AssertCalled(t, "RemoveContainer", mock.Anything, "c-old", false) | |
| 94 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-new", mock.Anything) | |
| 88 | + | // ACTIVE already names c-new while the proxy graph was refused, so the | |
| 89 | + | // superseded container may still be the one a stale graph reaches: it is | |
| 90 | + | // kept running. Both generations stay traceable in the journal for | |
| 91 | + | // reconciliation, and the leftover is reported. | |
| 92 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 93 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 94 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |
| 95 | + | assert.Equal(t, "c-old", step.Before) | |
| 96 | + | 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) | |
| 100 | + | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | |
| 101 | + | require.Len(t, last.Warnings, 1) | |
| 102 | + | assert.Equal(t, "c-old", last.Warnings[0].Leftover) | |
| 95 | 103 | } | |
| 96 | 104 | ||
| 97 | 105 | // TestStop_TrafficFailureRecordsTerminalFailure proves a rejected graph | |
| @@ -50,11 +50,8 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||||
| 50 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil) | 50 | state.EXPECT().SaveActive(mock.Anything, mock.Anything).Return(nil) | ||
| 51 | 51 | ||||
| 52 | expectNetworkProvision(runtime, "app-blog", 1) | 52 | expectNetworkProvision(runtime, "app-blog", 1) | ||
| 53 | - | // The superseded generation is confirmed gone before the replacement is | 53 | + | // The superseded container serves beside the candidate and must survive |
| 54 | - | // created: a deploy never overlaps two generations of one service. | 54 | + | // the failed switch: no stop, no removal, no claim release. |
| 55 | - | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 56 | - | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 57 | - | state.EXPECT().ReleaseBackendBinds(mock.Anything, "blog", "c-old").Return(nil).Once() | |||
| 58 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | 55 | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | ||
| 59 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | 56 | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | ||
| 60 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | 57 | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | ||
| @@ -88,10 +85,21 @@ func TestDeploy_TrafficFailureRecordsFailedServiceStep(t *testing.T) { | |||||
| 88 | step, ok := opStep(last, "service.web.replace") | 85 | step, ok := opStep(last, "service.web.replace") | ||
| 89 | require.True(t, ok) | 86 | require.True(t, ok) | ||
| 90 | assert.Equal(t, domain.AppStepFailed, step.State, "the service step must not claim success before traffic applied") | 87 | assert.Equal(t, domain.AppStepFailed, step.State, "the service step must not claim success before traffic applied") | ||
| 91 | - | // The superseded container is gone: a rejected graph is reported as a | 88 | + | // ACTIVE already names c-new while the proxy graph was refused, so the |
| 92 | - | // failure of the new generation, never as a return to the old one. | 89 | + | // superseded container may still be the one a stale graph reaches: it is |
| 93 | - | runtime.AssertCalled(t, "RemoveContainer", mock.Anything, "c-old", false) | 90 | + | // kept running. Both generations stay traceable in the journal for |
| 94 | - | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, "c-new", mock.Anything) | 91 | + | // reconciliation, and the leftover is reported. |
| 92 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 93 | + | runtime.AssertNotCalled(t, "RemoveContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 94 | + | state.AssertNotCalled(t, "ReleaseBackendBinds", mock.Anything, mock.Anything, mock.Anything) | |||
| 95 | + | assert.Equal(t, "c-old", step.Before) | |||
| 96 | + | 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) | |||
| 100 | + | assert.Equal(t, "c-old", result.Services["web"].CleanupWarnings[0].Leftover) | |||
| 101 | + | require.Len(t, last.Warnings, 1) | |||
| 102 | + | assert.Equal(t, "c-old", last.Warnings[0].Leftover) | |||
| 95 | } | 103 | } | ||
| 96 | 104 | ||||
| 97 | // TestStop_TrafficFailureRecordsTerminalFailure proves a rejected graph | 105 | // TestStop_TrafficFailureRecordsTerminalFailure proves a rejected graph | ||