fix(apps): skip unchanged services on deploy, apply secret changes on deploy (#294)
* fix(apps): name the diverging path when a targeted deploy is refused * fix(apps): skip unchanged services on deploy and recreate containers on restart * fix(apps): keep deploy skip and restart safe for withdrawn services and missing secrets * fix(apps): apply secret changes on deploy and keep restart in place
- commit
- 7d43d7c28040f814d9152a47e6176668bfcc7ace
- author
- bnema · · 2026-09-26 01:16 UTC
- committer
- GitHub · · 2026-09-26 01:16 UTC
- parent
- 9940049
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown
13 files · +458 -26
M docs/cli/apps.md +16 -2
| @@ -211,6 +211,10 @@ Reads values in one of three ways: | |||
| 211 | 211 | `--key` requires `--stdin` and cannot be combined with `KEY=VALUE` arguments. | |
| 212 | 212 | An empty value is rejected, so `KEY=` is invalid. | |
| 213 | 213 | ||
| 214 | + | Running containers keep the values they were created with. Apply new values | |
| 215 | + | with `gordon apps deploy APP --service SVC`: deploy sees the changed secret and | |
| 216 | + | recreates the service. `restart` does not apply new values. | |
| 217 | + | ||
| 214 | 218 | #### Flags | |
| 215 | 219 | ||
| 216 | 220 | | Flag | Description | | |
| @@ -262,6 +266,13 @@ service. | |||
| 262 | 266 | | `--all` | Deploy every service | | |
| 263 | 267 | | `--json` | Output as JSON | | |
| 264 | 268 | ||
| 269 | + | Deploy is the only command that applies changes. It recreates a service | |
| 270 | + | when its image digest, spec, app environment, or secret values changed. A | |
| 271 | + | new image behind the same tag (for example `latest`) has a new digest and is | |
| 272 | + | deployed. A service already running with all of these unchanged keeps its | |
| 273 | + | container and is reported `unchanged`. Services with host binds or devices | |
| 274 | + | are always replaced, so bind and device policy changes apply on deploy. | |
| 275 | + | ||
| 265 | 276 | Fail-fast across services: | |
| 266 | 277 | the first failure stops the deploy, successful services are preserved, | |
| 267 | 278 | later services stay unchanged. | |
| @@ -282,8 +293,11 @@ local polling (the daemon-side operation keeps running) and prints the | |||
| 282 | 293 | gordon apps restart APP [--service SVC | --all] [--json] | |
| 283 | 294 | ``` | |
| 284 | 295 | ||
| 285 | - | Restarts from pinned digests without re-resolution. Like `deploy`, an app | |
| 286 | - | with several services needs `--service NAME` or `--all`. | |
| 296 | + | Restarts the same container. Nothing is applied: new secret values, env, | |
| 297 | + | image, or config need `gordon apps deploy`. Traffic is withdrawn, the | |
| 298 | + | container restarts, readiness is checked, and traffic returns. If the | |
| 299 | + | container no longer exists, restart rebuilds it from its pinned digest. An | |
| 300 | + | app with several services needs `--service NAME` or `--all`. | |
| 287 | 301 | ||
| 288 | 302 | --- | |
| 289 | 303 | ||
| @@ -211,6 +211,10 @@ Reads values in one of three ways: | |||||
| 211 | `--key` requires `--stdin` and cannot be combined with `KEY=VALUE` arguments. | 211 | `--key` requires `--stdin` and cannot be combined with `KEY=VALUE` arguments. | ||
| 212 | An empty value is rejected, so `KEY=` is invalid. | 212 | An empty value is rejected, so `KEY=` is invalid. | ||
| 213 | 213 | ||||
| 214 | + | Running containers keep the values they were created with. Apply new values | |||
| 215 | + | with `gordon apps deploy APP --service SVC`: deploy sees the changed secret and | |||
| 216 | + | recreates the service. `restart` does not apply new values. | |||
| 217 | + | ||||
| 214 | #### Flags | 218 | #### Flags | ||
| 215 | 219 | ||||
| 216 | | Flag | Description | | 220 | | Flag | Description | | ||
| @@ -262,6 +266,13 @@ service. | |||||
| 262 | | `--all` | Deploy every service | | 266 | | `--all` | Deploy every service | | ||
| 263 | | `--json` | Output as JSON | | 267 | | `--json` | Output as JSON | | ||
| 264 | 268 | ||||
| 269 | + | Deploy is the only command that applies changes. It recreates a service | |||
| 270 | + | when its image digest, spec, app environment, or secret values changed. A | |||
| 271 | + | new image behind the same tag (for example `latest`) has a new digest and is | |||
| 272 | + | deployed. A service already running with all of these unchanged keeps its | |||
| 273 | + | container and is reported `unchanged`. Services with host binds or devices | |||
| 274 | + | are always replaced, so bind and device policy changes apply on deploy. | |||
| 275 | + | ||||
| 265 | Fail-fast across services: | 276 | Fail-fast across services: | ||
| 266 | the first failure stops the deploy, successful services are preserved, | 277 | the first failure stops the deploy, successful services are preserved, | ||
| 267 | later services stay unchanged. | 278 | later services stay unchanged. | ||
| @@ -282,8 +293,11 @@ local polling (the daemon-side operation keeps running) and prints the | |||||
| 282 | gordon apps restart APP [--service SVC | --all] [--json] | 293 | gordon apps restart APP [--service SVC | --all] [--json] | ||
| 283 | ``` | 294 | ``` | ||
| 284 | 295 | ||||
| 285 | - | Restarts from pinned digests without re-resolution. Like `deploy`, an app | 296 | + | Restarts the same container. Nothing is applied: new secret values, env, |
| 286 | - | with several services needs `--service NAME` or `--all`. | 297 | + | image, or config need `gordon apps deploy`. Traffic is withdrawn, the |
| 298 | + | container restarts, readiness is checked, and traffic returns. If the | |||
| 299 | + | container no longer exists, restart rebuilds it from its pinned digest. An | |||
| 300 | + | app with several services needs `--service NAME` or `--all`. | |||
| 287 | 301 | ||||
| 288 | --- | 302 | --- | ||
| 289 | 303 | ||||
M docs/concepts.md +4 -2
| @@ -82,7 +82,9 @@ Deployment stops at the first service failure: services already deployed in that | |||
| 82 | 82 | ||
| 83 | 83 | A deploy interrupted after the replacement container was created but before it was published leaves the failure explicit: the candidate is recorded in the deployment journal, and Gordon removes it before it creates or rebuilds any generation of that app — at boot and before every mutation — so two generations of one service never run together. An interrupted operation is finalized instead of staying in flight: as a failure, or as the success it was when every step had already completed. If the recorded candidate cannot be removed, reconciliation fails closed: the operation is not finalized, stays the latest journal, and every later mutation is refused so a newer operation can never mask the orphan. The next recovery pass republishes routing within its 15-second cadence if the final traffic publication was the step that failed. | |
| 84 | 84 | ||
| 85 | - | `gordon apps restart` is separate from deploy and stays in place: it withdraws traffic, restarts the same pinned container, verifies readiness, and republishes traffic. No second container is created. When the recorded container is gone, restart rebuilds the service from its pinned `ACTIVE` digest and publishes it. | |
| 85 | + | Deploy is the only command that applies changes: it recreates a service whose image digest, spec, app environment, or secret values changed, and keeps an unchanged one, reported `unchanged`. | |
| 86 | + | ||
| 87 | + | `gordon apps restart` restarts the same container and applies nothing: it withdraws traffic, restarts the pinned container, verifies readiness, and republishes traffic. No second container is created. When the recorded container is gone, restart rebuilds the service from its pinned `ACTIVE` digest and publishes it. | |
| 86 | 88 | ||
| 87 | 89 | ## Deletion and Cleanup Lifecycle | |
| 88 | 90 | ||
| @@ -155,7 +157,7 @@ Service-specific values use `secrets` even when non-confidential: | |||
| 155 | 157 | DATABASE_URL = "database-url" | |
| 156 | 158 | ``` | |
| 157 | 159 | ||
| 158 | - | Secret values stay in pass under `gordon/apps/<uuid>/<service>/<name>`, keyed by the stable internal UUID so a removed app's secrets are never adopted by a new app reusing the name. Secret updates affect the next deploy/restart, not running containers. Write values with `gordon apps secrets set`; only key names are ever echoed back, never values. | |
| 160 | + | Secret values stay in pass under `gordon/apps/<uuid>/<service>/<name>`, keyed by the stable internal UUID so a removed app's secrets are never adopted by a new app reusing the name. Running containers keep the values they were created with; `gordon apps deploy` applies new values. `restart` does not. Write values with `gordon apps secrets set`; only key names are ever echoed back, never values. | |
| 159 | 161 | ||
| 160 | 162 | ## Installation Reload | |
| 161 | 163 | ||
| @@ -82,7 +82,9 @@ Deployment stops at the first service failure: services already deployed in that | |||||
| 82 | 82 | ||||
| 83 | A deploy interrupted after the replacement container was created but before it was published leaves the failure explicit: the candidate is recorded in the deployment journal, and Gordon removes it before it creates or rebuilds any generation of that app — at boot and before every mutation — so two generations of one service never run together. An interrupted operation is finalized instead of staying in flight: as a failure, or as the success it was when every step had already completed. If the recorded candidate cannot be removed, reconciliation fails closed: the operation is not finalized, stays the latest journal, and every later mutation is refused so a newer operation can never mask the orphan. The next recovery pass republishes routing within its 15-second cadence if the final traffic publication was the step that failed. | 83 | A deploy interrupted after the replacement container was created but before it was published leaves the failure explicit: the candidate is recorded in the deployment journal, and Gordon removes it before it creates or rebuilds any generation of that app — at boot and before every mutation — so two generations of one service never run together. An interrupted operation is finalized instead of staying in flight: as a failure, or as the success it was when every step had already completed. If the recorded candidate cannot be removed, reconciliation fails closed: the operation is not finalized, stays the latest journal, and every later mutation is refused so a newer operation can never mask the orphan. The next recovery pass republishes routing within its 15-second cadence if the final traffic publication was the step that failed. | ||
| 84 | 84 | ||||
| 85 | - | `gordon apps restart` is separate from deploy and stays in place: it withdraws traffic, restarts the same pinned container, verifies readiness, and republishes traffic. No second container is created. When the recorded container is gone, restart rebuilds the service from its pinned `ACTIVE` digest and publishes it. | 85 | + | Deploy is the only command that applies changes: it recreates a service whose image digest, spec, app environment, or secret values changed, and keeps an unchanged one, reported `unchanged`. |
| 86 | + | ||||
| 87 | + | `gordon apps restart` restarts the same container and applies nothing: it withdraws traffic, restarts the pinned container, verifies readiness, and republishes traffic. No second container is created. When the recorded container is gone, restart rebuilds the service from its pinned `ACTIVE` digest and publishes it. | |||
| 86 | 88 | ||||
| 87 | ## Deletion and Cleanup Lifecycle | 89 | ## Deletion and Cleanup Lifecycle | ||
| 88 | 90 | ||||
| @@ -155,7 +157,7 @@ Service-specific values use `secrets` even when non-confidential: | |||||
| 155 | DATABASE_URL = "database-url" | 157 | DATABASE_URL = "database-url" | ||
| 156 | ``` | 158 | ``` | ||
| 157 | 159 | ||||
| 158 | - | Secret values stay in pass under `gordon/apps/<uuid>/<service>/<name>`, keyed by the stable internal UUID so a removed app's secrets are never adopted by a new app reusing the name. Secret updates affect the next deploy/restart, not running containers. Write values with `gordon apps secrets set`; only key names are ever echoed back, never values. | 160 | + | Secret values stay in pass under `gordon/apps/<uuid>/<service>/<name>`, keyed by the stable internal UUID so a removed app's secrets are never adopted by a new app reusing the name. Running containers keep the values they were created with; `gordon apps deploy` applies new values. `restart` does not. Write values with `gordon apps secrets set`; only key names are ever echoed back, never values. |
| 159 | 161 | ||||
| 160 | ## Installation Reload | 162 | ## Installation Reload | ||
| 161 | 163 | ||||
M docs/config/apps.md +1 -1
| @@ -108,7 +108,7 @@ Image registry names and digest syntax are validated during manifest apply, reso | |||
| 108 | 108 | ## Environment and Secrets | |
| 109 | 109 | ||
| 110 | 110 | - `[env]` is app-wide public env injected into all services. Each key must be disjoint from every `[services.<name>.secrets]` key in the app. | |
| 111 | - | - `[services.<name>.secrets]` maps ENV var name to service-local secret name. Values are written with `gordon apps secrets set` and stay in pass under `gordon/apps/<uuid>/<service>/<name>`. Secret updates affect the next deploy/restart, not running containers. | |
| 111 | + | - `[services.<name>.secrets]` maps ENV var name to service-local secret name. Values are written with `gordon apps secrets set` and stay in pass under `gordon/apps/<uuid>/<service>/<name>`. Running containers keep the values they were created with; `gordon apps deploy` applies new values. `restart` does not. | |
| 112 | 112 | - There is no `[services.<name>.env]` key — service-specific values must use secrets. | |
| 113 | 113 | ||
| 114 | 114 | ## Volumes and Databases | |
| @@ -108,7 +108,7 @@ Image registry names and digest syntax are validated during manifest apply, reso | |||||
| 108 | ## Environment and Secrets | 108 | ## Environment and Secrets | ||
| 109 | 109 | ||||
| 110 | - `[env]` is app-wide public env injected into all services. Each key must be disjoint from every `[services.<name>.secrets]` key in the app. | 110 | - `[env]` is app-wide public env injected into all services. Each key must be disjoint from every `[services.<name>.secrets]` key in the app. | ||
| 111 | - | - `[services.<name>.secrets]` maps ENV var name to service-local secret name. Values are written with `gordon apps secrets set` and stay in pass under `gordon/apps/<uuid>/<service>/<name>`. Secret updates affect the next deploy/restart, not running containers. | 111 | + | - `[services.<name>.secrets]` maps ENV var name to service-local secret name. Values are written with `gordon apps secrets set` and stay in pass under `gordon/apps/<uuid>/<service>/<name>`. Running containers keep the values they were created with; `gordon apps deploy` applies new values. `restart` does not. |
| 112 | - There is no `[services.<name>.env]` key — service-specific values must use secrets. | 112 | - There is no `[services.<name>.env]` key — service-specific values must use secrets. | ||
| 113 | 113 | ||||
| 114 | ## Volumes and Databases | 114 | ## Volumes and Databases | ||
M internal/adapters/in/cli/apps.go +7 -1
| @@ -655,7 +655,10 @@ func runAppsSecretsSetMode(ctx context.Context, plane ControlPlane, stdin io.Rea | |||
| 655 | 655 | if jsonOut { | |
| 656 | 656 | return writeJSON(out, map[string]any{"app": app, "service": service, "keys": keys}) | |
| 657 | 657 | } | |
| 658 | - | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Set %d secret(s) for %s/%s: %s", len(keys), app, service, strings.Join(keys, ", ")))) | |
| 658 | + | if err := cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Set %d secret(s) for %s/%s: %s", len(keys), app, service, strings.Join(keys, ", ")))); err != nil { | |
| 659 | + | return err | |
| 660 | + | } | |
| 661 | + | return cliWriteLine(out, cliRenderMuted(fmt.Sprintf("Running containers keep their old values. Apply them with: gordon apps deploy %s --service %s", app, service))) | |
| 659 | 662 | } | |
| 660 | 663 | ||
| 661 | 664 | // newAppsSecretsDeleteCmd creates `apps secrets delete`. | |
| @@ -998,6 +1001,9 @@ func renderDeployServices(out io.Writer, resp *dto.AppDeployResponse) error { | |||
| 998 | 1001 | for _, name := range names { | |
| 999 | 1002 | svc := resp.Services[name] | |
| 1000 | 1003 | detail := svc.Result + " " + svc.EffectiveRevision | |
| 1004 | + | if svc.Result == domain.AppServiceUnchanged { | |
| 1005 | + | detail += " (already running this image, config, and secrets)" | |
| 1006 | + | } | |
| 1001 | 1007 | if svc.RestartUnsafe { | |
| 1002 | 1008 | detail += " restart_unsafe" | |
| 1003 | 1009 | } | |
| @@ -655,7 +655,10 @@ func runAppsSecretsSetMode(ctx context.Context, plane ControlPlane, stdin io.Rea | |||||
| 655 | if jsonOut { | 655 | if jsonOut { | ||
| 656 | return writeJSON(out, map[string]any{"app": app, "service": service, "keys": keys}) | 656 | return writeJSON(out, map[string]any{"app": app, "service": service, "keys": keys}) | ||
| 657 | } | 657 | } | ||
| 658 | - | return cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Set %d secret(s) for %s/%s: %s", len(keys), app, service, strings.Join(keys, ", ")))) | 658 | + | if err := cliWriteLine(out, cliRenderSuccess(fmt.Sprintf("Set %d secret(s) for %s/%s: %s", len(keys), app, service, strings.Join(keys, ", ")))); err != nil { |
| 659 | + | return err | |||
| 660 | + | } | |||
| 661 | + | return cliWriteLine(out, cliRenderMuted(fmt.Sprintf("Running containers keep their old values. Apply them with: gordon apps deploy %s --service %s", app, service))) | |||
| 659 | } | 662 | } | ||
| 660 | 663 | ||||
| 661 | // newAppsSecretsDeleteCmd creates `apps secrets delete`. | 664 | // newAppsSecretsDeleteCmd creates `apps secrets delete`. | ||
| @@ -998,6 +1001,9 @@ func renderDeployServices(out io.Writer, resp *dto.AppDeployResponse) error { | |||||
| 998 | for _, name := range names { | 1001 | for _, name := range names { | ||
| 999 | svc := resp.Services[name] | 1002 | svc := resp.Services[name] | ||
| 1000 | detail := svc.Result + " " + svc.EffectiveRevision | 1003 | detail := svc.Result + " " + svc.EffectiveRevision | ||
| 1004 | + | if svc.Result == domain.AppServiceUnchanged { | |||
| 1005 | + | detail += " (already running this image, config, and secrets)" | |||
| 1006 | + | } | |||
| 1001 | if svc.RestartUnsafe { | 1007 | if svc.RestartUnsafe { | ||
| 1002 | detail += " restart_unsafe" | 1008 | detail += " restart_unsafe" | ||
| 1003 | } | 1009 | } | ||
M internal/adapters/in/http/admin/handler_apps.go +3 -0
| @@ -607,6 +607,9 @@ func toAppDeployResponse(app string, op *domain.AppOperation, includeDiagnostics | |||
| 607 | 607 | switch step.State { | |
| 608 | 608 | case domain.AppStepSucceeded: | |
| 609 | 609 | entry.Result = "deployed" | |
| 610 | + | if strings.HasPrefix(step.Detail, domain.AppServiceUnchanged+":") { | |
| 611 | + | entry.Result = domain.AppServiceUnchanged | |
| 612 | + | } | |
| 610 | 613 | case domain.AppStepFailed: | |
| 611 | 614 | entry.Result = "failed" | |
| 612 | 615 | entry.Error = step.Error | |
| @@ -607,6 +607,9 @@ func toAppDeployResponse(app string, op *domain.AppOperation, includeDiagnostics | |||||
| 607 | switch step.State { | 607 | switch step.State { | ||
| 608 | case domain.AppStepSucceeded: | 608 | case domain.AppStepSucceeded: | ||
| 609 | entry.Result = "deployed" | 609 | entry.Result = "deployed" | ||
| 610 | + | if strings.HasPrefix(step.Detail, domain.AppServiceUnchanged+":") { | |||
| 611 | + | entry.Result = domain.AppServiceUnchanged | |||
| 612 | + | } | |||
| 610 | case domain.AppStepFailed: | 613 | case domain.AppStepFailed: | ||
| 611 | entry.Result = "failed" | 614 | entry.Result = "failed" | ||
| 612 | entry.Error = step.Error | 615 | entry.Error = step.Error | ||
M internal/adapters/out/docker/runtime.go +1 -0
| @@ -777,6 +777,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||
| 777 | 777 | VolumeMounts: volumeMounts, | |
| 778 | 778 | Created: created, | |
| 779 | 779 | StartedAt: startedAt, | |
| 780 | + | Env: resp.Config.Env, | |
| 780 | 781 | }, nil | |
| 781 | 782 | } | |
| 782 | 783 | ||
| @@ -777,6 +777,7 @@ func (r *Runtime) InspectContainer(ctx context.Context, containerID string) (*do | |||||
| 777 | VolumeMounts: volumeMounts, | 777 | VolumeMounts: volumeMounts, | ||
| 778 | Created: created, | 778 | Created: created, | ||
| 779 | StartedAt: startedAt, | 779 | StartedAt: startedAt, | ||
| 780 | + | Env: resp.Config.Env, | |||
| 780 | }, nil | 781 | }, nil | ||
| 781 | } | 782 | } | ||
| 782 | 783 | ||||
M internal/domain/app_spec.go +6 -0
| @@ -1038,6 +1038,12 @@ func equalStringSets(a, b []string) bool { | |||
| 1038 | 1038 | return true | |
| 1039 | 1039 | } | |
| 1040 | 1040 | ||
| 1041 | + | // SameAppService reports whether two service specs are equal under the | |
| 1042 | + | // same normalized comparison DiffAppSpec uses. | |
| 1043 | + | func SameAppService(a, b AppService) bool { | |
| 1044 | + | return len(diffService(a.Name, a, b)) == 0 | |
| 1045 | + | } | |
| 1046 | + | ||
| 1041 | 1047 | // diffService compares two services field by field. | |
| 1042 | 1048 | func diffService(name string, desired, effective AppService) []string { | |
| 1043 | 1049 | var changed []string | |
| @@ -1038,6 +1038,12 @@ func equalStringSets(a, b []string) bool { | |||||
| 1038 | return true | 1038 | return true | ||
| 1039 | } | 1039 | } | ||
| 1040 | 1040 | ||||
| 1041 | + | // SameAppService reports whether two service specs are equal under the | |||
| 1042 | + | // same normalized comparison DiffAppSpec uses. | |||
| 1043 | + | func SameAppService(a, b AppService) bool { | |||
| 1044 | + | return len(diffService(a.Name, a, b)) == 0 | |||
| 1045 | + | } | |||
| 1046 | + | ||||
| 1041 | // diffService compares two services field by field. | 1047 | // diffService compares two services field by field. | ||
| 1042 | func diffService(name string, desired, effective AppService) []string { | 1048 | func diffService(name string, desired, effective AppService) []string { | ||
| 1043 | var changed []string | 1049 | var changed []string | ||
M internal/domain/app_state.go +5 -0
| @@ -30,6 +30,11 @@ const ( | |||
| 30 | 30 | AppStepNotRun = "not-run" | |
| 31 | 31 | ) | |
| 32 | 32 | ||
| 33 | + | // AppServiceUnchanged is the result of a deploy step that kept the running | |
| 34 | + | // container because its image, spec, and environment were already current. | |
| 35 | + | // A succeeded step whose Detail starts with it reports this result. | |
| 36 | + | const AppServiceUnchanged = "unchanged" | |
| 37 | + | ||
| 33 | 38 | // App operation outcomes over terminal per-service results. | |
| 34 | 39 | const ( | |
| 35 | 40 | AppOutcomeSuccess = "success" | |
| @@ -30,6 +30,11 @@ const ( | |||||
| 30 | AppStepNotRun = "not-run" | 30 | AppStepNotRun = "not-run" | ||
| 31 | ) | 31 | ) | ||
| 32 | 32 | ||||
| 33 | + | // AppServiceUnchanged is the result of a deploy step that kept the running | |||
| 34 | + | // container because its image, spec, and environment were already current. | |||
| 35 | + | // A succeeded step whose Detail starts with it reports this result. | |||
| 36 | + | const AppServiceUnchanged = "unchanged" | |||
| 37 | + | ||||
| 33 | // App operation outcomes over terminal per-service results. | 38 | // App operation outcomes over terminal per-service results. | ||
| 34 | const ( | 39 | const ( | ||
| 35 | AppOutcomeSuccess = "success" | 40 | AppOutcomeSuccess = "success" | ||
M internal/domain/container.go +3 -0
| @@ -21,6 +21,9 @@ type Container struct { | |||
| 21 | 21 | // Readiness scoping uses it so log markers from a previous | |
| 22 | 22 | // execution of the same container ID cannot satisfy a probe. | |
| 23 | 23 | StartedAt time.Time | |
| 24 | + | // Env is the container's KEY=value environment as created. It may hold | |
| 25 | + | // secret values: compare it in memory only, never persist or log it. | |
| 26 | + | Env []string | |
| 24 | 27 | } | |
| 25 | 28 | ||
| 26 | 29 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | |
| @@ -21,6 +21,9 @@ type Container struct { | |||||
| 21 | // Readiness scoping uses it so log markers from a previous | 21 | // Readiness scoping uses it so log markers from a previous | ||
| 22 | // execution of the same container ID cannot satisfy a probe. | 22 | // execution of the same container ID cannot satisfy a probe. | ||
| 23 | StartedAt time.Time | 23 | StartedAt time.Time | ||
| 24 | + | // Env is the container's KEY=value environment as created. It may hold | |||
| 25 | + | // secret values: compare it in memory only, never persist or log it. | |||
| 26 | + | Env []string | |||
| 24 | } | 27 | } | ||
| 25 | 28 | ||||
| 26 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | 29 | // ContainerVolumeMount describes a mounted volume-like resource on a container. | ||
A internal/usecase/deployment/deploy_unchanged_test.go +264 -0
| @@ -0,0 +1,264 @@ | |||
| 1 | + | package deployment_test | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/zerowrap" | |
| 9 | + | "github.com/stretchr/testify/assert" | |
| 10 | + | "github.com/stretchr/testify/mock" | |
| 11 | + | "github.com/stretchr/testify/require" | |
| 12 | + | ||
| 13 | + | "github.com/bnema/gordon/internal/adapters/out/appstate" | |
| 14 | + | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | |
| 15 | + | "github.com/bnema/gordon/internal/domain" | |
| 16 | + | "github.com/bnema/gordon/internal/usecase/deployment" | |
| 17 | + | ) | |
| 18 | + | ||
| 19 | + | // unchangedSeed shapes the ACTIVE record and the desired revision of an app | |
| 20 | + | // whose defaults match exactly (same image, digest, spec, env, binds). | |
| 21 | + | type unchangedSeed struct { | |
| 22 | + | binds map[int]int | |
| 23 | + | desiredEnv map[string]string | |
| 24 | + | inhibitedBy string | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | // seedUnchangedApp seeds an app whose ACTIVE service runs exactly what the | |
| 28 | + | // desired revision would create, unless the seed alters one input. | |
| 29 | + | func seedUnchangedApp(t *testing.T, ctx context.Context, store *appstate.Store, seeds ...unchangedSeed) { | |
| 30 | + | t.Helper() | |
| 31 | + | seed := unchangedSeed{binds: map[int]int{8080: 18080}} | |
| 32 | + | if len(seeds) > 0 { | |
| 33 | + | seed = seeds[0] | |
| 34 | + | } | |
| 35 | + | spec := webService() | |
| 36 | + | spec.Secrets = map[string]string{} | |
| 37 | + | spec.Readiness = domain.AppReadiness{Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second} | |
| 38 | + | activeRev := testRevision("blog", spec) | |
| 39 | + | activeRev.Revision = "rev-0" | |
| 40 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |
| 41 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 42 | + | "web": { | |
| 43 | + | Container: "c-old", EffectiveRevision: "rev-0", Image: spec.Image, ActivatedBy: "op-created", | |
| 44 | + | Digest: restartTestDigest, Spec: activeRev.Spec.Services[0], BackendBinds: seed.binds, | |
| 45 | + | }, | |
| 46 | + | }})) | |
| 47 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |
| 48 | + | seedRevision(t, ctx, store, "intent-0", "", activeRev) | |
| 49 | + | desired := testRevision("blog", spec) | |
| 50 | + | if seed.desiredEnv != nil { | |
| 51 | + | desired.Spec.Env = seed.desiredEnv | |
| 52 | + | } | |
| 53 | + | seedRevision(t, ctx, store, "intent-1", "rev-0", desired) | |
| 54 | + | if seed.inhibitedBy != "" { | |
| 55 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |
| 56 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitReplacementPending, Operation: seed.inhibitedBy, | |
| 57 | + | })) | |
| 58 | + | } | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | // expectReplacement wires one full replacement of c-old by c-new. | |
| 62 | + | func expectReplacement(runtime *outmocks.MockContainerRuntime) { | |
| 63 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 64 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 65 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 66 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |
| 67 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 68 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 69 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | // unchangedDeployService wires a deploy of an app whose ACTIVE service | |
| 73 | + | // already matches the desired revision. | |
| 74 | + | func unchangedDeployService(t *testing.T, seeds ...unchangedSeed) (*deployment.Service, *outmocks.MockContainerRuntime, *outmocks.MockImageResolver, context.Context, *appstate.Store) { | |
| 75 | + | t.Helper() | |
| 76 | + | ctx := context.Background() | |
| 77 | + | store := newTestStore(t) | |
| 78 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 79 | + | images := outmocks.NewMockImageResolver(t) | |
| 80 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 81 | + | seedUnchangedApp(t, ctx, store, seeds...) | |
| 82 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |
| 83 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |
| 84 | + | svc := deployment.NewService(deployment.Deps{ | |
| 85 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |
| 86 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 87 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 88 | + | func(context.Context, string) error { return nil }, | |
| 89 | + | )) | |
| 90 | + | return svc, runtime, images, ctx, store | |
| 91 | + | } | |
| 92 | + | ||
| 93 | + | // TestDeploy_SkipsServiceAlreadyRunningTheSameImage proves a deploy whose | |
| 94 | + | // resolved digest, spec, and env match the running service keeps the | |
| 95 | + | // container and reports it unchanged instead of recreating it. | |
| 96 | + | func TestDeploy_SkipsServiceAlreadyRunningTheSameImage(t *testing.T) { | |
| 97 | + | svc, runtime, _, ctx, store := unchangedDeployService(t) | |
| 98 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |
| 99 | + | Return(&domain.Container{ID: "c-old", Status: string(domain.ContainerStatusRunning)}, nil).Once() | |
| 100 | + | ||
| 101 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-same"}) | |
| 102 | + | require.NoError(t, err) | |
| 103 | + | assert.Equal(t, deployment.ServiceResultUnchanged, result.Services["web"].Result) | |
| 104 | + | assert.Equal(t, "c-old", result.Services["web"].After, "the running container is kept") | |
| 105 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |
| 106 | + | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |
| 107 | + | ||
| 108 | + | op, err := store.LoadOperation(ctx, "blog", "op-same") | |
| 109 | + | require.NoError(t, err) | |
| 110 | + | assert.Equal(t, domain.AppOutcomeSuccess, op.Outcome) | |
| 111 | + | step, found := opStep(op, "service.web.replace") | |
| 112 | + | require.True(t, found) | |
| 113 | + | assert.Equal(t, domain.AppStepSucceeded, step.State) | |
| 114 | + | assert.Contains(t, step.Detail, "unchanged: already running docker.io/example/web:1.4.2") | |
| 115 | + | ||
| 116 | + | active, ok, err := store.LoadActive(ctx, "blog") | |
| 117 | + | require.NoError(t, err) | |
| 118 | + | require.True(t, ok) | |
| 119 | + | assert.Equal(t, "c-old", active.Services["web"].Container) | |
| 120 | + | assert.Equal(t, map[int]int{8080: 18080}, active.Services["web"].BackendBinds, "the kept container stays routable") | |
| 121 | + | assert.Equal(t, "op-created", active.Services["web"].ActivatedBy, "a kept container keeps its original activation") | |
| 122 | + | assert.True(t, active.Converged, "an unchanged deploy still converges ACTIVE on the deployed revision") | |
| 123 | + | } | |
| 124 | + | ||
| 125 | + | // TestDeploy_ReplacesSameDigestWhenTheServiceIsNotServing proves the skip | |
| 126 | + | // never keeps a container that is not fully published: missing routing | |
| 127 | + | // binds (a withdrawn generation), a recovery inhibition left by a failed | |
| 128 | + | // replacement, or a changed app env all force a replacement. | |
| 129 | + | func TestDeploy_ReplacesSameDigestWhenTheServiceIsNotServing(t *testing.T) { | |
| 130 | + | tests := []struct { | |
| 131 | + | name string | |
| 132 | + | seed unchangedSeed | |
| 133 | + | }{ | |
| 134 | + | {name: "withdrawn binds", seed: unchangedSeed{}}, | |
| 135 | + | {name: "recovery inhibited", seed: unchangedSeed{binds: map[int]int{8080: 18080}, inhibitedBy: "op-failed"}}, | |
| 136 | + | {name: "app env changed", seed: unchangedSeed{binds: map[int]int{8080: 18080}, desiredEnv: map[string]string{"MODE": "prod"}}}, | |
| 137 | + | } | |
| 138 | + | for _, tc := range tests { | |
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |
| 140 | + | svc, runtime, _, ctx, _ := unchangedDeployService(t, tc.seed) | |
| 141 | + | expectReplacement(runtime) | |
| 142 | + | ||
| 143 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-replace"}) | |
| 144 | + | require.NoError(t, err) | |
| 145 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 146 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |
| 147 | + | }) | |
| 148 | + | } | |
| 149 | + | } | |
| 150 | + | ||
| 151 | + | // TestDeploy_ReplacesWhenTheRunningContainerIsNotRunning proves the skip | |
| 152 | + | // never trusts ACTIVE alone: a stopped or missing container is replaced. | |
| 153 | + | func TestDeploy_ReplacesWhenTheRunningContainerIsNotRunning(t *testing.T) { | |
| 154 | + | svc, runtime, _, ctx, _ := unchangedDeployService(t) | |
| 155 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |
| 156 | + | Return(&domain.Container{ID: "c-old", Status: "exited"}, nil).Once() | |
| 157 | + | expectReplacement(runtime) | |
| 158 | + | ||
| 159 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-exited"}) | |
| 160 | + | require.NoError(t, err) | |
| 161 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 162 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | // TestDeploy_ReplacesWhenTheDigestChanged proves a new image digest behind | |
| 166 | + | // the same tag is always deployed. | |
| 167 | + | func TestDeploy_ReplacesWhenTheDigestChanged(t *testing.T) { | |
| 168 | + | ctx := context.Background() | |
| 169 | + | store := newTestStore(t) | |
| 170 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 171 | + | images := outmocks.NewMockImageResolver(t) | |
| 172 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 173 | + | seedUnchangedApp(t, ctx, store) | |
| 174 | + | newDigest := "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" | |
| 175 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(newDigest, nil).Once() | |
| 176 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |
| 177 | + | expectNetworkProvision(runtime, "app-blog", 1) | |
| 178 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |
| 179 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |
| 180 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |
| 181 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |
| 182 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |
| 183 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |
| 184 | + | ||
| 185 | + | svc := deployment.NewService(deployment.Deps{ | |
| 186 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |
| 187 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 188 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 189 | + | func(context.Context, string) error { return nil }, | |
| 190 | + | )) | |
| 191 | + | ||
| 192 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-new-digest"}) | |
| 193 | + | require.NoError(t, err) | |
| 194 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |
| 195 | + | runtime.AssertNotCalled(t, "InspectContainer", mock.Anything, "c-old") | |
| 196 | + | ||
| 197 | + | active, ok, err := store.LoadActive(ctx, "blog") | |
| 198 | + | require.NoError(t, err) | |
| 199 | + | require.True(t, ok) | |
| 200 | + | assert.Equal(t, newDigest, active.Services["web"].Digest) | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | // TestDeploy_RecreatesWhenASecretValueChanged proves deploy applies new | |
| 204 | + | // secret values: a running container created with an old value is replaced, | |
| 205 | + | // and one created with the current value is kept. | |
| 206 | + | func TestDeploy_RecreatesWhenASecretValueChanged(t *testing.T) { | |
| 207 | + | tests := []struct { | |
| 208 | + | name string | |
| 209 | + | runningEnv []string | |
| 210 | + | wantResult string | |
| 211 | + | }{ | |
| 212 | + | {name: "secret changed", runningEnv: []string{"DB_PASSWORD=old", "PATH=/bin"}, wantResult: "deployed"}, | |
| 213 | + | {name: "secret current", runningEnv: []string{"DB_PASSWORD=new", "PATH=/bin"}, wantResult: deployment.ServiceResultUnchanged}, | |
| 214 | + | } | |
| 215 | + | for _, tc := range tests { | |
| 216 | + | t.Run(tc.name, func(t *testing.T) { | |
| 217 | + | svc, runtime, secrets, ctx := secretDeployService(t) | |
| 218 | + | secrets.EXPECT().GetSecret(mock.Anything, mock.Anything).Return("new", nil) | |
| 219 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |
| 220 | + | Return(&domain.Container{ID: "c-old", Status: string(domain.ContainerStatusRunning), Env: tc.runningEnv}, nil).Once() | |
| 221 | + | if tc.wantResult == "deployed" { | |
| 222 | + | expectReplacement(runtime) | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-secret"}) | |
| 226 | + | require.NoError(t, err) | |
| 227 | + | assert.Equal(t, tc.wantResult, result.Services["web"].Result) | |
| 228 | + | }) | |
| 229 | + | } | |
| 230 | + | } | |
| 231 | + | ||
| 232 | + | // secretDeployService wires an unchanged app whose service reads one secret. | |
| 233 | + | func secretDeployService(t *testing.T) (*deployment.Service, *outmocks.MockContainerRuntime, *outmocks.MockSecretProvider, context.Context) { | |
| 234 | + | t.Helper() | |
| 235 | + | ctx := context.Background() | |
| 236 | + | store := newTestStore(t) | |
| 237 | + | runtime := outmocks.NewMockContainerRuntime(t) | |
| 238 | + | images := outmocks.NewMockImageResolver(t) | |
| 239 | + | secrets := outmocks.NewMockSecretProvider(t) | |
| 240 | + | spec := webService() | |
| 241 | + | spec.Secrets = map[string]string{"DB_PASSWORD": "db-password"} | |
| 242 | + | spec.Readiness = domain.AppReadiness{Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second} | |
| 243 | + | activeRev := testRevision("blog", spec) | |
| 244 | + | activeRev.Revision = "rev-0" | |
| 245 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |
| 246 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |
| 247 | + | "web": { | |
| 248 | + | Container: "c-old", EffectiveRevision: "rev-0", Image: spec.Image, ActivatedBy: "op-created", | |
| 249 | + | Digest: restartTestDigest, Spec: activeRev.Spec.Services[0], BackendBinds: map[int]int{8080: 18080}, | |
| 250 | + | }, | |
| 251 | + | }})) | |
| 252 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |
| 253 | + | seedRevision(t, ctx, store, "intent-0", "", activeRev) | |
| 254 | + | seedRevision(t, ctx, store, "intent-1", "rev-0", testRevision("blog", spec)) | |
| 255 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |
| 256 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |
| 257 | + | svc := deployment.NewService(deployment.Deps{ | |
| 258 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |
| 259 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |
| 260 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |
| 261 | + | func(context.Context, string) error { return nil }, | |
| 262 | + | )) | |
| 263 | + | return svc, runtime, secrets, ctx | |
| 264 | + | } | |
| @@ -0,0 +1,264 @@ | |||||
| 1 | + | package deployment_test | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/zerowrap" | |||
| 9 | + | "github.com/stretchr/testify/assert" | |||
| 10 | + | "github.com/stretchr/testify/mock" | |||
| 11 | + | "github.com/stretchr/testify/require" | |||
| 12 | + | ||||
| 13 | + | "github.com/bnema/gordon/internal/adapters/out/appstate" | |||
| 14 | + | outmocks "github.com/bnema/gordon/internal/boundaries/out/mocks" | |||
| 15 | + | "github.com/bnema/gordon/internal/domain" | |||
| 16 | + | "github.com/bnema/gordon/internal/usecase/deployment" | |||
| 17 | + | ) | |||
| 18 | + | ||||
| 19 | + | // unchangedSeed shapes the ACTIVE record and the desired revision of an app | |||
| 20 | + | // whose defaults match exactly (same image, digest, spec, env, binds). | |||
| 21 | + | type unchangedSeed struct { | |||
| 22 | + | binds map[int]int | |||
| 23 | + | desiredEnv map[string]string | |||
| 24 | + | inhibitedBy string | |||
| 25 | + | } | |||
| 26 | + | ||||
| 27 | + | // seedUnchangedApp seeds an app whose ACTIVE service runs exactly what the | |||
| 28 | + | // desired revision would create, unless the seed alters one input. | |||
| 29 | + | func seedUnchangedApp(t *testing.T, ctx context.Context, store *appstate.Store, seeds ...unchangedSeed) { | |||
| 30 | + | t.Helper() | |||
| 31 | + | seed := unchangedSeed{binds: map[int]int{8080: 18080}} | |||
| 32 | + | if len(seeds) > 0 { | |||
| 33 | + | seed = seeds[0] | |||
| 34 | + | } | |||
| 35 | + | spec := webService() | |||
| 36 | + | spec.Secrets = map[string]string{} | |||
| 37 | + | spec.Readiness = domain.AppReadiness{Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second} | |||
| 38 | + | activeRev := testRevision("blog", spec) | |||
| 39 | + | activeRev.Revision = "rev-0" | |||
| 40 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |||
| 41 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |||
| 42 | + | "web": { | |||
| 43 | + | Container: "c-old", EffectiveRevision: "rev-0", Image: spec.Image, ActivatedBy: "op-created", | |||
| 44 | + | Digest: restartTestDigest, Spec: activeRev.Spec.Services[0], BackendBinds: seed.binds, | |||
| 45 | + | }, | |||
| 46 | + | }})) | |||
| 47 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |||
| 48 | + | seedRevision(t, ctx, store, "intent-0", "", activeRev) | |||
| 49 | + | desired := testRevision("blog", spec) | |||
| 50 | + | if seed.desiredEnv != nil { | |||
| 51 | + | desired.Spec.Env = seed.desiredEnv | |||
| 52 | + | } | |||
| 53 | + | seedRevision(t, ctx, store, "intent-1", "rev-0", desired) | |||
| 54 | + | if seed.inhibitedBy != "" { | |||
| 55 | + | require.NoError(t, store.SaveRecoveryInhibition(ctx, domain.AppRecoveryInhibition{ | |||
| 56 | + | App: "blog", Service: "web", ContainerID: "c-old", Reason: domain.AppInhibitReplacementPending, Operation: seed.inhibitedBy, | |||
| 57 | + | })) | |||
| 58 | + | } | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | // expectReplacement wires one full replacement of c-old by c-new. | |||
| 62 | + | func expectReplacement(runtime *outmocks.MockContainerRuntime) { | |||
| 63 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 64 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 65 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 66 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |||
| 67 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |||
| 68 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |||
| 69 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | // unchangedDeployService wires a deploy of an app whose ACTIVE service | |||
| 73 | + | // already matches the desired revision. | |||
| 74 | + | func unchangedDeployService(t *testing.T, seeds ...unchangedSeed) (*deployment.Service, *outmocks.MockContainerRuntime, *outmocks.MockImageResolver, context.Context, *appstate.Store) { | |||
| 75 | + | t.Helper() | |||
| 76 | + | ctx := context.Background() | |||
| 77 | + | store := newTestStore(t) | |||
| 78 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 79 | + | images := outmocks.NewMockImageResolver(t) | |||
| 80 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 81 | + | seedUnchangedApp(t, ctx, store, seeds...) | |||
| 82 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |||
| 83 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |||
| 84 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 85 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 86 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 87 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 88 | + | func(context.Context, string) error { return nil }, | |||
| 89 | + | )) | |||
| 90 | + | return svc, runtime, images, ctx, store | |||
| 91 | + | } | |||
| 92 | + | ||||
| 93 | + | // TestDeploy_SkipsServiceAlreadyRunningTheSameImage proves a deploy whose | |||
| 94 | + | // resolved digest, spec, and env match the running service keeps the | |||
| 95 | + | // container and reports it unchanged instead of recreating it. | |||
| 96 | + | func TestDeploy_SkipsServiceAlreadyRunningTheSameImage(t *testing.T) { | |||
| 97 | + | svc, runtime, _, ctx, store := unchangedDeployService(t) | |||
| 98 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |||
| 99 | + | Return(&domain.Container{ID: "c-old", Status: string(domain.ContainerStatusRunning)}, nil).Once() | |||
| 100 | + | ||||
| 101 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-same"}) | |||
| 102 | + | require.NoError(t, err) | |||
| 103 | + | assert.Equal(t, deployment.ServiceResultUnchanged, result.Services["web"].Result) | |||
| 104 | + | assert.Equal(t, "c-old", result.Services["web"].After, "the running container is kept") | |||
| 105 | + | runtime.AssertNotCalled(t, "StopContainer", mock.Anything, mock.Anything, mock.Anything) | |||
| 106 | + | runtime.AssertNotCalled(t, "CreateContainer", mock.Anything, mock.Anything) | |||
| 107 | + | ||||
| 108 | + | op, err := store.LoadOperation(ctx, "blog", "op-same") | |||
| 109 | + | require.NoError(t, err) | |||
| 110 | + | assert.Equal(t, domain.AppOutcomeSuccess, op.Outcome) | |||
| 111 | + | step, found := opStep(op, "service.web.replace") | |||
| 112 | + | require.True(t, found) | |||
| 113 | + | assert.Equal(t, domain.AppStepSucceeded, step.State) | |||
| 114 | + | assert.Contains(t, step.Detail, "unchanged: already running docker.io/example/web:1.4.2") | |||
| 115 | + | ||||
| 116 | + | active, ok, err := store.LoadActive(ctx, "blog") | |||
| 117 | + | require.NoError(t, err) | |||
| 118 | + | require.True(t, ok) | |||
| 119 | + | assert.Equal(t, "c-old", active.Services["web"].Container) | |||
| 120 | + | assert.Equal(t, map[int]int{8080: 18080}, active.Services["web"].BackendBinds, "the kept container stays routable") | |||
| 121 | + | assert.Equal(t, "op-created", active.Services["web"].ActivatedBy, "a kept container keeps its original activation") | |||
| 122 | + | assert.True(t, active.Converged, "an unchanged deploy still converges ACTIVE on the deployed revision") | |||
| 123 | + | } | |||
| 124 | + | ||||
| 125 | + | // TestDeploy_ReplacesSameDigestWhenTheServiceIsNotServing proves the skip | |||
| 126 | + | // never keeps a container that is not fully published: missing routing | |||
| 127 | + | // binds (a withdrawn generation), a recovery inhibition left by a failed | |||
| 128 | + | // replacement, or a changed app env all force a replacement. | |||
| 129 | + | func TestDeploy_ReplacesSameDigestWhenTheServiceIsNotServing(t *testing.T) { | |||
| 130 | + | tests := []struct { | |||
| 131 | + | name string | |||
| 132 | + | seed unchangedSeed | |||
| 133 | + | }{ | |||
| 134 | + | {name: "withdrawn binds", seed: unchangedSeed{}}, | |||
| 135 | + | {name: "recovery inhibited", seed: unchangedSeed{binds: map[int]int{8080: 18080}, inhibitedBy: "op-failed"}}, | |||
| 136 | + | {name: "app env changed", seed: unchangedSeed{binds: map[int]int{8080: 18080}, desiredEnv: map[string]string{"MODE": "prod"}}}, | |||
| 137 | + | } | |||
| 138 | + | for _, tc := range tests { | |||
| 139 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 140 | + | svc, runtime, _, ctx, _ := unchangedDeployService(t, tc.seed) | |||
| 141 | + | expectReplacement(runtime) | |||
| 142 | + | ||||
| 143 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-replace"}) | |||
| 144 | + | require.NoError(t, err) | |||
| 145 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 146 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |||
| 147 | + | }) | |||
| 148 | + | } | |||
| 149 | + | } | |||
| 150 | + | ||||
| 151 | + | // TestDeploy_ReplacesWhenTheRunningContainerIsNotRunning proves the skip | |||
| 152 | + | // never trusts ACTIVE alone: a stopped or missing container is replaced. | |||
| 153 | + | func TestDeploy_ReplacesWhenTheRunningContainerIsNotRunning(t *testing.T) { | |||
| 154 | + | svc, runtime, _, ctx, _ := unchangedDeployService(t) | |||
| 155 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |||
| 156 | + | Return(&domain.Container{ID: "c-old", Status: "exited"}, nil).Once() | |||
| 157 | + | expectReplacement(runtime) | |||
| 158 | + | ||||
| 159 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-exited"}) | |||
| 160 | + | require.NoError(t, err) | |||
| 161 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 162 | + | assert.Equal(t, "c-new", result.Services["web"].After) | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | // TestDeploy_ReplacesWhenTheDigestChanged proves a new image digest behind | |||
| 166 | + | // the same tag is always deployed. | |||
| 167 | + | func TestDeploy_ReplacesWhenTheDigestChanged(t *testing.T) { | |||
| 168 | + | ctx := context.Background() | |||
| 169 | + | store := newTestStore(t) | |||
| 170 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 171 | + | images := outmocks.NewMockImageResolver(t) | |||
| 172 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 173 | + | seedUnchangedApp(t, ctx, store) | |||
| 174 | + | newDigest := "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" | |||
| 175 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(newDigest, nil).Once() | |||
| 176 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |||
| 177 | + | expectNetworkProvision(runtime, "app-blog", 1) | |||
| 178 | + | runtime.EXPECT().StopContainer(mock.Anything, "c-old", mock.Anything).Return(nil).Once() | |||
| 179 | + | runtime.EXPECT().RemoveContainer(mock.Anything, "c-old", false).Return(nil).Once() | |||
| 180 | + | runtime.EXPECT().CreateContainer(mock.Anything, mock.Anything).Return(&domain.Container{ID: "c-new", Name: "web"}, nil).Once() | |||
| 181 | + | runtime.EXPECT().StartContainer(mock.Anything, "c-new").Return(nil).Once() | |||
| 182 | + | runtime.EXPECT().GetContainerBackendBinds(mock.Anything, "c-new", mock.Anything).Return( | |||
| 183 | + | []domain.ContainerBackendBind{{ContainerPort: 8080, HostPort: 18081, Protocol: domain.NetworkProtocolTCP}}, nil).Once() | |||
| 184 | + | ||||
| 185 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 186 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 187 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 188 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 189 | + | func(context.Context, string) error { return nil }, | |||
| 190 | + | )) | |||
| 191 | + | ||||
| 192 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-new-digest"}) | |||
| 193 | + | require.NoError(t, err) | |||
| 194 | + | assert.Equal(t, "deployed", result.Services["web"].Result) | |||
| 195 | + | runtime.AssertNotCalled(t, "InspectContainer", mock.Anything, "c-old") | |||
| 196 | + | ||||
| 197 | + | active, ok, err := store.LoadActive(ctx, "blog") | |||
| 198 | + | require.NoError(t, err) | |||
| 199 | + | require.True(t, ok) | |||
| 200 | + | assert.Equal(t, newDigest, active.Services["web"].Digest) | |||
| 201 | + | } | |||
| 202 | + | ||||
| 203 | + | // TestDeploy_RecreatesWhenASecretValueChanged proves deploy applies new | |||
| 204 | + | // secret values: a running container created with an old value is replaced, | |||
| 205 | + | // and one created with the current value is kept. | |||
| 206 | + | func TestDeploy_RecreatesWhenASecretValueChanged(t *testing.T) { | |||
| 207 | + | tests := []struct { | |||
| 208 | + | name string | |||
| 209 | + | runningEnv []string | |||
| 210 | + | wantResult string | |||
| 211 | + | }{ | |||
| 212 | + | {name: "secret changed", runningEnv: []string{"DB_PASSWORD=old", "PATH=/bin"}, wantResult: "deployed"}, | |||
| 213 | + | {name: "secret current", runningEnv: []string{"DB_PASSWORD=new", "PATH=/bin"}, wantResult: deployment.ServiceResultUnchanged}, | |||
| 214 | + | } | |||
| 215 | + | for _, tc := range tests { | |||
| 216 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 217 | + | svc, runtime, secrets, ctx := secretDeployService(t) | |||
| 218 | + | secrets.EXPECT().GetSecret(mock.Anything, mock.Anything).Return("new", nil) | |||
| 219 | + | runtime.EXPECT().InspectContainer(mock.Anything, "c-old"). | |||
| 220 | + | Return(&domain.Container{ID: "c-old", Status: string(domain.ContainerStatusRunning), Env: tc.runningEnv}, nil).Once() | |||
| 221 | + | if tc.wantResult == "deployed" { | |||
| 222 | + | expectReplacement(runtime) | |||
| 223 | + | } | |||
| 224 | + | ||||
| 225 | + | result, err := svc.Deploy(ctx, deployment.DeployInput{App: "blog", Op: "op-secret"}) | |||
| 226 | + | require.NoError(t, err) | |||
| 227 | + | assert.Equal(t, tc.wantResult, result.Services["web"].Result) | |||
| 228 | + | }) | |||
| 229 | + | } | |||
| 230 | + | } | |||
| 231 | + | ||||
| 232 | + | // secretDeployService wires an unchanged app whose service reads one secret. | |||
| 233 | + | func secretDeployService(t *testing.T) (*deployment.Service, *outmocks.MockContainerRuntime, *outmocks.MockSecretProvider, context.Context) { | |||
| 234 | + | t.Helper() | |||
| 235 | + | ctx := context.Background() | |||
| 236 | + | store := newTestStore(t) | |||
| 237 | + | runtime := outmocks.NewMockContainerRuntime(t) | |||
| 238 | + | images := outmocks.NewMockImageResolver(t) | |||
| 239 | + | secrets := outmocks.NewMockSecretProvider(t) | |||
| 240 | + | spec := webService() | |||
| 241 | + | spec.Secrets = map[string]string{"DB_PASSWORD": "db-password"} | |||
| 242 | + | spec.Readiness = domain.AppReadiness{Type: domain.AppReadinessHTTP, Path: "/healthz", Timeout: time.Second} | |||
| 243 | + | activeRev := testRevision("blog", spec) | |||
| 244 | + | activeRev.Revision = "rev-0" | |||
| 245 | + | require.NoError(t, store.SaveIntent(ctx, domain.AppStopIntent{App: "blog"})) | |||
| 246 | + | require.NoError(t, store.SaveActive(ctx, domain.AppActive{App: "blog", Services: map[string]domain.AppEffectiveService{ | |||
| 247 | + | "web": { | |||
| 248 | + | Container: "c-old", EffectiveRevision: "rev-0", Image: spec.Image, ActivatedBy: "op-created", | |||
| 249 | + | Digest: restartTestDigest, Spec: activeRev.Spec.Services[0], BackendBinds: map[int]int{8080: 18080}, | |||
| 250 | + | }, | |||
| 251 | + | }})) | |||
| 252 | + | require.NoError(t, store.SaveOwnership(ctx, domain.AppOwnership{App: "blog", ID: "app-blog"})) | |||
| 253 | + | seedRevision(t, ctx, store, "intent-0", "", activeRev) | |||
| 254 | + | seedRevision(t, ctx, store, "intent-1", "rev-0", testRevision("blog", spec)) | |||
| 255 | + | images.EXPECT().ResolveDigest(mock.Anything, "docker.io/example/web:1.4.2").Return(restartTestDigest, nil).Once() | |||
| 256 | + | runtime.EXPECT().InspectImageVolumes(mock.Anything, "docker.io/example/web:1.4.2").Return(nil, nil).Once() | |||
| 257 | + | svc := deployment.NewService(deployment.Deps{ | |||
| 258 | + | State: store, Runtime: runtime, Images: images, Secrets: secrets, | |||
| 259 | + | }, zerowrap.Default()).WithProbeDeps(deployment.NewTestProbeDeps(runtime, | |||
| 260 | + | func(context.Context, string, string) (int, error) { return 200, nil }, | |||
| 261 | + | func(context.Context, string) error { return nil }, | |||
| 262 | + | )) | |||
| 263 | + | return svc, runtime, secrets, ctx | |||
| 264 | + | } | |||
M internal/usecase/deployment/execute.go +118 -3
| @@ -6,6 +6,8 @@ import ( | |||
| 6 | 6 | "errors" | |
| 7 | 7 | "fmt" | |
| 8 | 8 | "io" | |
| 9 | + | "maps" | |
| 10 | + | "reflect" | |
| 9 | 11 | "sort" | |
| 10 | 12 | "strings" | |
| 11 | 13 | "time" | |
| @@ -332,7 +334,20 @@ func (s *Service) runServiceStep( | |||
| 332 | 334 | Digest: p.digest, Image: p.runtimeImage, | |
| 333 | 335 | Before: before, State: domain.AppStepPending, | |
| 334 | 336 | } | |
| 335 | - | svcResult := s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | |
| 337 | + | var svcResult ServiceResult | |
| 338 | + | if current, ok := s.unchangedService(ctx, app, p, active); ok { | |
| 339 | + | // Same image, spec, environment, and secret values already running: | |
| 340 | + | // keep the container. | |
| 341 | + | svcResult = ServiceResult{ | |
| 342 | + | Result: ServiceResultUnchanged, EffectiveRevision: revision, | |
| 343 | + | Before: before, After: before, | |
| 344 | + | BackendBinds: current.BackendBinds, UDPBackendBinds: current.UDPBackendBinds, | |
| 345 | + | RestartUnsafe: singleWriterRequired(p.spec), | |
| 346 | + | } | |
| 347 | + | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | |
| 348 | + | } else { | |
| 349 | + | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | |
| 350 | + | } | |
| 336 | 351 | result.Services[p.name] = svcResult | |
| 337 | 352 | // A recorded candidate is authoritative while the step is unresolved: a | |
| 338 | 353 | // replacement that failed after creating its container must stay traceable | |
| @@ -382,6 +397,101 @@ func (s *Service) runServiceStep( | |||
| 382 | 397 | return nil | |
| 383 | 398 | } | |
| 384 | 399 | ||
| 400 | + | // unchangedService reports whether the service already runs exactly what p | |
| 401 | + | // would create, in a healthy published state: the same digest, service spec, | |
| 402 | + | // app environment, and shared networks, in a running container that is not | |
| 403 | + | // withdrawn or recovery-inhibited and whose recorded binds cover every | |
| 404 | + | // backend port, and whose environment already holds the current secret | |
| 405 | + | // values. Services with host binds or devices are never skipped: their | |
| 406 | + | // resolved sources depend on host policy that ACTIVE does not record. | |
| 407 | + | func (s *Service) unchangedService(ctx context.Context, app string, p pinnedService, active domain.AppActive) (domain.AppEffectiveService, bool) { | |
| 408 | + | eff, ok := active.Services[p.name] | |
| 409 | + | if !ok || eff.Container == "" || eff.Digest == "" || eff.Digest != p.digest { | |
| 410 | + | return eff, false | |
| 411 | + | } | |
| 412 | + | reason := s.unchangedRefusal(ctx, app, p, eff) | |
| 413 | + | if reason != "" { | |
| 414 | + | log := zerowrap.FromCtx(ctx) | |
| 415 | + | log.Debug().Str("app", app).Str("service", p.name).Str("reason", reason). | |
| 416 | + | Msg("deployment: same digest, replacing service") | |
| 417 | + | return eff, false | |
| 418 | + | } | |
| 419 | + | return eff, true | |
| 420 | + | } | |
| 421 | + | ||
| 422 | + | // unchangedRefusal returns why a service running the same digest must still | |
| 423 | + | // be replaced, or "" when it can be kept. | |
| 424 | + | func (s *Service) unchangedRefusal(ctx context.Context, app string, p pinnedService, eff domain.AppEffectiveService) string { | |
| 425 | + | switch { | |
| 426 | + | case len(p.spec.Binds) > 0 || len(p.spec.Devices) > 0: | |
| 427 | + | return "host binds or devices" | |
| 428 | + | case !domain.SameAppService(p.spec, eff.Spec): | |
| 429 | + | return "spec changed" | |
| 430 | + | case !bindsCover(eff, backendPorts(p.spec)): | |
| 431 | + | return "backend binds missing" | |
| 432 | + | case s.publication.inhibited(app, p.name): | |
| 433 | + | return "withdrawal pending" | |
| 434 | + | } | |
| 435 | + | previous, err := s.deps.State.LoadRevision(ctx, app, eff.EffectiveRevision) | |
| 436 | + | if err != nil { | |
| 437 | + | return "load effective revision: " + err.Error() | |
| 438 | + | } | |
| 439 | + | if !maps.Equal(previous.Spec.Env, p.appEnv) { | |
| 440 | + | return "app env changed" | |
| 441 | + | } | |
| 442 | + | if !reflect.DeepEqual(domain.AppServiceSharedNetworks(previous.Spec, p.name), p.sharedNetworks) { | |
| 443 | + | return "shared networks changed" | |
| 444 | + | } | |
| 445 | + | if inhibited, err := s.recoveryInhibited(ctx, app, p.name, eff.Container); err != nil || inhibited { | |
| 446 | + | return "recovery inhibited" | |
| 447 | + | } | |
| 448 | + | container, err := s.deps.Runtime.InspectContainer(ctx, eff.Container) | |
| 449 | + | if err != nil || container.Status != string(domain.ContainerStatusRunning) { | |
| 450 | + | return "container not running" | |
| 451 | + | } | |
| 452 | + | if !s.envCurrent(ctx, app, p, container.Env) { | |
| 453 | + | return "environment or secrets changed" | |
| 454 | + | } | |
| 455 | + | return "" | |
| 456 | + | } | |
| 457 | + | ||
| 458 | + | // envCurrent reports whether the running container was created with the | |
| 459 | + | // environment p would get now, including current secret values. The | |
| 460 | + | // runtime env also holds image-declared keys, so every desired entry must | |
| 461 | + | // be present; removed keys are caught by the spec and app env comparisons. | |
| 462 | + | // Values are compared in memory only and never logged. | |
| 463 | + | func (s *Service) envCurrent(ctx context.Context, app string, p pinnedService, running []string) bool { | |
| 464 | + | desired, err := s.serviceEnv(ctx, app, p) | |
| 465 | + | if err != nil { | |
| 466 | + | return false | |
| 467 | + | } | |
| 468 | + | have := make(map[string]struct{}, len(running)) | |
| 469 | + | for _, entry := range running { | |
| 470 | + | have[entry] = struct{}{} | |
| 471 | + | } | |
| 472 | + | for _, entry := range desired { | |
| 473 | + | if _, ok := have[entry]; !ok { | |
| 474 | + | return false | |
| 475 | + | } | |
| 476 | + | } | |
| 477 | + | return true | |
| 478 | + | } | |
| 479 | + | ||
| 480 | + | // bindsCover reports whether the recorded loopback binds publish every | |
| 481 | + | // backend port the spec requires, so a kept container stays routable. | |
| 482 | + | func bindsCover(eff domain.AppEffectiveService, ports []domain.ContainerBackendPort) bool { | |
| 483 | + | for _, port := range ports { | |
| 484 | + | binds := eff.BackendBinds | |
| 485 | + | if port.Protocol == domain.NetworkProtocolUDP { | |
| 486 | + | binds = eff.UDPBackendBinds | |
| 487 | + | } | |
| 488 | + | if binds[port.ContainerPort] == 0 { | |
| 489 | + | return false | |
| 490 | + | } | |
| 491 | + | } | |
| 492 | + | return true | |
| 493 | + | } | |
| 494 | + | ||
| 385 | 495 | // failOperation records a terminal failure on an already claimed | |
| 386 | 496 | // journal, so a keyed repeat replays a terminal outcome instead of | |
| 387 | 497 | // reading a permanently in-flight claim. stepID names the failing phase | |
| @@ -1399,10 +1509,15 @@ func (s *Service) publishService(ctx context.Context, app, revision string, p pi | |||
| 1399 | 1509 | if active.Services == nil { | |
| 1400 | 1510 | active.Services = map[string]domain.AppEffectiveService{} | |
| 1401 | 1511 | } | |
| 1512 | + | activatedBy, activatedAt := opID, time.Now().UTC() | |
| 1513 | + | if previous, ok := active.Services[p.name]; ok && result.Result == ServiceResultUnchanged { | |
| 1514 | + | // A kept container keeps the activation of the operation that created it. | |
| 1515 | + | activatedBy, activatedAt = previous.ActivatedBy, previous.ActivatedAt | |
| 1516 | + | } | |
| 1402 | 1517 | active.Services[p.name] = domain.AppEffectiveService{ | |
| 1403 | 1518 | EffectiveRevision: revision, | |
| 1404 | - | ActivatedBy: opID, | |
| 1405 | - | ActivatedAt: time.Now().UTC(), | |
| 1519 | + | ActivatedBy: activatedBy, | |
| 1520 | + | ActivatedAt: activatedAt, | |
| 1406 | 1521 | Image: p.spec.Image, | |
| 1407 | 1522 | Digest: p.digest, | |
| 1408 | 1523 | Container: result.After, | |
| @@ -6,6 +6,8 @@ import ( | |||||
| 6 | "errors" | 6 | "errors" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | "io" | 8 | "io" | ||
| 9 | + | "maps" | |||
| 10 | + | "reflect" | |||
| 9 | "sort" | 11 | "sort" | ||
| 10 | "strings" | 12 | "strings" | ||
| 11 | "time" | 13 | "time" | ||
| @@ -332,7 +334,20 @@ func (s *Service) runServiceStep( | |||||
| 332 | Digest: p.digest, Image: p.runtimeImage, | 334 | Digest: p.digest, Image: p.runtimeImage, | ||
| 333 | Before: before, State: domain.AppStepPending, | 335 | Before: before, State: domain.AppStepPending, | ||
| 334 | } | 336 | } | ||
| 335 | - | svcResult := s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | 337 | + | var svcResult ServiceResult |
| 338 | + | if current, ok := s.unchangedService(ctx, app, p, active); ok { | |||
| 339 | + | // Same image, spec, environment, and secret values already running: | |||
| 340 | + | // keep the container. | |||
| 341 | + | svcResult = ServiceResult{ | |||
| 342 | + | Result: ServiceResultUnchanged, EffectiveRevision: revision, | |||
| 343 | + | Before: before, After: before, | |||
| 344 | + | BackendBinds: current.BackendBinds, UDPBackendBinds: current.UDPBackendBinds, | |||
| 345 | + | RestartUnsafe: singleWriterRequired(p.spec), | |||
| 346 | + | } | |||
| 347 | + | step.Detail = domain.AppServiceUnchanged + ": already running " + p.spec.Image + " (" + p.digest + ")" | |||
| 348 | + | } else { | |||
| 349 | + | svcResult = s.deployService(ctx, app, revision, p, op.Op, before, activeStopGrace(active, p.name), s.journalCandidate(op, index+1)) | |||
| 350 | + | } | |||
| 336 | result.Services[p.name] = svcResult | 351 | result.Services[p.name] = svcResult | ||
| 337 | // A recorded candidate is authoritative while the step is unresolved: a | 352 | // A recorded candidate is authoritative while the step is unresolved: a | ||
| 338 | // replacement that failed after creating its container must stay traceable | 353 | // replacement that failed after creating its container must stay traceable | ||
| @@ -382,6 +397,101 @@ func (s *Service) runServiceStep( | |||||
| 382 | return nil | 397 | return nil | ||
| 383 | } | 398 | } | ||
| 384 | 399 | ||||
| 400 | + | // unchangedService reports whether the service already runs exactly what p | |||
| 401 | + | // would create, in a healthy published state: the same digest, service spec, | |||
| 402 | + | // app environment, and shared networks, in a running container that is not | |||
| 403 | + | // withdrawn or recovery-inhibited and whose recorded binds cover every | |||
| 404 | + | // backend port, and whose environment already holds the current secret | |||
| 405 | + | // values. Services with host binds or devices are never skipped: their | |||
| 406 | + | // resolved sources depend on host policy that ACTIVE does not record. | |||
| 407 | + | func (s *Service) unchangedService(ctx context.Context, app string, p pinnedService, active domain.AppActive) (domain.AppEffectiveService, bool) { | |||
| 408 | + | eff, ok := active.Services[p.name] | |||
| 409 | + | if !ok || eff.Container == "" || eff.Digest == "" || eff.Digest != p.digest { | |||
| 410 | + | return eff, false | |||
| 411 | + | } | |||
| 412 | + | reason := s.unchangedRefusal(ctx, app, p, eff) | |||
| 413 | + | if reason != "" { | |||
| 414 | + | log := zerowrap.FromCtx(ctx) | |||
| 415 | + | log.Debug().Str("app", app).Str("service", p.name).Str("reason", reason). | |||
| 416 | + | Msg("deployment: same digest, replacing service") | |||
| 417 | + | return eff, false | |||
| 418 | + | } | |||
| 419 | + | return eff, true | |||
| 420 | + | } | |||
| 421 | + | ||||
| 422 | + | // unchangedRefusal returns why a service running the same digest must still | |||
| 423 | + | // be replaced, or "" when it can be kept. | |||
| 424 | + | func (s *Service) unchangedRefusal(ctx context.Context, app string, p pinnedService, eff domain.AppEffectiveService) string { | |||
| 425 | + | switch { | |||
| 426 | + | case len(p.spec.Binds) > 0 || len(p.spec.Devices) > 0: | |||
| 427 | + | return "host binds or devices" | |||
| 428 | + | case !domain.SameAppService(p.spec, eff.Spec): | |||
| 429 | + | return "spec changed" | |||
| 430 | + | case !bindsCover(eff, backendPorts(p.spec)): | |||
| 431 | + | return "backend binds missing" | |||
| 432 | + | case s.publication.inhibited(app, p.name): | |||
| 433 | + | return "withdrawal pending" | |||
| 434 | + | } | |||
| 435 | + | previous, err := s.deps.State.LoadRevision(ctx, app, eff.EffectiveRevision) | |||
| 436 | + | if err != nil { | |||
| 437 | + | return "load effective revision: " + err.Error() | |||
| 438 | + | } | |||
| 439 | + | if !maps.Equal(previous.Spec.Env, p.appEnv) { | |||
| 440 | + | return "app env changed" | |||
| 441 | + | } | |||
| 442 | + | if !reflect.DeepEqual(domain.AppServiceSharedNetworks(previous.Spec, p.name), p.sharedNetworks) { | |||
| 443 | + | return "shared networks changed" | |||
| 444 | + | } | |||
| 445 | + | if inhibited, err := s.recoveryInhibited(ctx, app, p.name, eff.Container); err != nil || inhibited { | |||
| 446 | + | return "recovery inhibited" | |||
| 447 | + | } | |||
| 448 | + | container, err := s.deps.Runtime.InspectContainer(ctx, eff.Container) | |||
| 449 | + | if err != nil || container.Status != string(domain.ContainerStatusRunning) { | |||
| 450 | + | return "container not running" | |||
| 451 | + | } | |||
| 452 | + | if !s.envCurrent(ctx, app, p, container.Env) { | |||
| 453 | + | return "environment or secrets changed" | |||
| 454 | + | } | |||
| 455 | + | return "" | |||
| 456 | + | } | |||
| 457 | + | ||||
| 458 | + | // envCurrent reports whether the running container was created with the | |||
| 459 | + | // environment p would get now, including current secret values. The | |||
| 460 | + | // runtime env also holds image-declared keys, so every desired entry must | |||
| 461 | + | // be present; removed keys are caught by the spec and app env comparisons. | |||
| 462 | + | // Values are compared in memory only and never logged. | |||
| 463 | + | func (s *Service) envCurrent(ctx context.Context, app string, p pinnedService, running []string) bool { | |||
| 464 | + | desired, err := s.serviceEnv(ctx, app, p) | |||
| 465 | + | if err != nil { | |||
| 466 | + | return false | |||
| 467 | + | } | |||
| 468 | + | have := make(map[string]struct{}, len(running)) | |||
| 469 | + | for _, entry := range running { | |||
| 470 | + | have[entry] = struct{}{} | |||
| 471 | + | } | |||
| 472 | + | for _, entry := range desired { | |||
| 473 | + | if _, ok := have[entry]; !ok { | |||
| 474 | + | return false | |||
| 475 | + | } | |||
| 476 | + | } | |||
| 477 | + | return true | |||
| 478 | + | } | |||
| 479 | + | ||||
| 480 | + | // bindsCover reports whether the recorded loopback binds publish every | |||
| 481 | + | // backend port the spec requires, so a kept container stays routable. | |||
| 482 | + | func bindsCover(eff domain.AppEffectiveService, ports []domain.ContainerBackendPort) bool { | |||
| 483 | + | for _, port := range ports { | |||
| 484 | + | binds := eff.BackendBinds | |||
| 485 | + | if port.Protocol == domain.NetworkProtocolUDP { | |||
| 486 | + | binds = eff.UDPBackendBinds | |||
| 487 | + | } | |||
| 488 | + | if binds[port.ContainerPort] == 0 { | |||
| 489 | + | return false | |||
| 490 | + | } | |||
| 491 | + | } | |||
| 492 | + | return true | |||
| 493 | + | } | |||
| 494 | + | ||||
| 385 | // failOperation records a terminal failure on an already claimed | 495 | // failOperation records a terminal failure on an already claimed | ||
| 386 | // journal, so a keyed repeat replays a terminal outcome instead of | 496 | // journal, so a keyed repeat replays a terminal outcome instead of | ||
| 387 | // reading a permanently in-flight claim. stepID names the failing phase | 497 | // reading a permanently in-flight claim. stepID names the failing phase | ||
| @@ -1399,10 +1509,15 @@ func (s *Service) publishService(ctx context.Context, app, revision string, p pi | |||||
| 1399 | if active.Services == nil { | 1509 | if active.Services == nil { | ||
| 1400 | active.Services = map[string]domain.AppEffectiveService{} | 1510 | active.Services = map[string]domain.AppEffectiveService{} | ||
| 1401 | } | 1511 | } | ||
| 1512 | + | activatedBy, activatedAt := opID, time.Now().UTC() | |||
| 1513 | + | if previous, ok := active.Services[p.name]; ok && result.Result == ServiceResultUnchanged { | |||
| 1514 | + | // A kept container keeps the activation of the operation that created it. | |||
| 1515 | + | activatedBy, activatedAt = previous.ActivatedBy, previous.ActivatedAt | |||
| 1516 | + | } | |||
| 1402 | active.Services[p.name] = domain.AppEffectiveService{ | 1517 | active.Services[p.name] = domain.AppEffectiveService{ | ||
| 1403 | EffectiveRevision: revision, | 1518 | EffectiveRevision: revision, | ||
| 1404 | - | ActivatedBy: opID, | 1519 | + | ActivatedBy: activatedBy, |
| 1405 | - | ActivatedAt: time.Now().UTC(), | 1520 | + | ActivatedAt: activatedAt, |
| 1406 | Image: p.spec.Image, | 1521 | Image: p.spec.Image, | ||
| 1407 | Digest: p.digest, | 1522 | Digest: p.digest, | ||
| 1408 | Container: result.After, | 1523 | Container: result.After, | ||
M internal/usecase/deployment/preflight_gate_test.go +15 -9
| @@ -176,17 +176,22 @@ func TestStartDeploy_TargetedDiffPolicy(t *testing.T) { | |||
| 176 | 176 | } | |
| 177 | 177 | ||
| 178 | 178 | tests := []struct { | |
| 179 | - | name string | |
| 180 | - | mutate func(*domain.AppSpec) | |
| 181 | - | wantErr bool | |
| 179 | + | name string | |
| 180 | + | mutate func(*domain.AppSpec) | |
| 181 | + | wantPath string | |
| 182 | 182 | }{ | |
| 183 | 183 | {name: "requested service image", mutate: func(spec *domain.AppSpec) { spec.Services[0].Image = "img:2" }}, | |
| 184 | - | {name: "other service", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Services[1].Image = "img:2" }}, | |
| 185 | - | {name: "environment", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Env = map[string]string{"MODE": "prod"} }}, | |
| 186 | - | {name: "network", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Networks[0].Aliases = []string{"peer"} }}, | |
| 187 | - | {name: "service added", wantErr: true, mutate: func(spec *domain.AppSpec) { | |
| 184 | + | {name: "identical spec", mutate: func(*domain.AppSpec) {}}, | |
| 185 | + | {name: "other service", wantPath: "service/worker/image", mutate: func(spec *domain.AppSpec) { spec.Services[1].Image = "img:2" }}, | |
| 186 | + | {name: "environment", wantPath: "also changes env", mutate: func(spec *domain.AppSpec) { spec.Env = map[string]string{"MODE": "prod"} }}, | |
| 187 | + | {name: "network", wantPath: "network/shared/aliases", mutate: func(spec *domain.AppSpec) { spec.Networks[0].Aliases = []string{"peer"} }}, | |
| 188 | + | {name: "service added", wantPath: "service/job (added)", mutate: func(spec *domain.AppSpec) { | |
| 188 | 189 | spec.Services = append(spec.Services, domain.AppService{Name: "job", Image: "img:1", Readiness: domain.AppReadiness{Type: "none", Timeout: 30 * time.Second}}) | |
| 189 | 190 | }}, | |
| 191 | + | {name: "service removed", wantPath: "service/worker (removed)", mutate: func(spec *domain.AppSpec) { | |
| 192 | + | spec.Services = spec.Services[:1] | |
| 193 | + | spec.Networks = nil | |
| 194 | + | }}, | |
| 190 | 195 | } | |
| 191 | 196 | for _, tc := range tests { | |
| 192 | 197 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -204,15 +209,16 @@ func TestStartDeploy_TargetedDiffPolicy(t *testing.T) { | |||
| 204 | 209 | state.EXPECT().Recover(mock.Anything).Return(nil) | |
| 205 | 210 | state.EXPECT().LoadRevision(mock.Anything, "blog", "rev-2").Return(rev, nil).Once() | |
| 206 | 211 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(base, true, nil).Once() | |
| 207 | - | if !tc.wantErr { | |
| 212 | + | if tc.wantPath == "" { | |
| 208 | 213 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil).Once() | |
| 209 | 214 | } | |
| 210 | 215 | ||
| 211 | 216 | svc := keyedService(t, state, runtime, images, secrets) | |
| 212 | 217 | started, err := svc.StartDeploy(ctx, deployment.DeployInput{App: "blog", Revision: "rev-2", Service: "web"}) | |
| 213 | 218 | ||
| 214 | - | if tc.wantErr { | |
| 219 | + | if tc.wantPath != "" { | |
| 215 | 220 | require.ErrorIs(t, err, domain.ErrAppStateConflict) | |
| 221 | + | assert.Contains(t, err.Error(), tc.wantPath) | |
| 216 | 222 | state.AssertNotCalled(t, "SaveOperation", mock.Anything, mock.Anything) | |
| 217 | 223 | } else { | |
| 218 | 224 | require.NoError(t, err) | |
| @@ -176,17 +176,22 @@ func TestStartDeploy_TargetedDiffPolicy(t *testing.T) { | |||||
| 176 | } | 176 | } | ||
| 177 | 177 | ||||
| 178 | tests := []struct { | 178 | tests := []struct { | ||
| 179 | - | name string | 179 | + | name string |
| 180 | - | mutate func(*domain.AppSpec) | 180 | + | mutate func(*domain.AppSpec) |
| 181 | - | wantErr bool | 181 | + | wantPath string |
| 182 | }{ | 182 | }{ | ||
| 183 | {name: "requested service image", mutate: func(spec *domain.AppSpec) { spec.Services[0].Image = "img:2" }}, | 183 | {name: "requested service image", mutate: func(spec *domain.AppSpec) { spec.Services[0].Image = "img:2" }}, | ||
| 184 | - | {name: "other service", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Services[1].Image = "img:2" }}, | 184 | + | {name: "identical spec", mutate: func(*domain.AppSpec) {}}, |
| 185 | - | {name: "environment", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Env = map[string]string{"MODE": "prod"} }}, | 185 | + | {name: "other service", wantPath: "service/worker/image", mutate: func(spec *domain.AppSpec) { spec.Services[1].Image = "img:2" }}, |
| 186 | - | {name: "network", wantErr: true, mutate: func(spec *domain.AppSpec) { spec.Networks[0].Aliases = []string{"peer"} }}, | 186 | + | {name: "environment", wantPath: "also changes env", mutate: func(spec *domain.AppSpec) { spec.Env = map[string]string{"MODE": "prod"} }}, |
| 187 | - | {name: "service added", wantErr: true, mutate: func(spec *domain.AppSpec) { | 187 | + | {name: "network", wantPath: "network/shared/aliases", mutate: func(spec *domain.AppSpec) { spec.Networks[0].Aliases = []string{"peer"} }}, |
| 188 | + | {name: "service added", wantPath: "service/job (added)", mutate: func(spec *domain.AppSpec) { | |||
| 188 | spec.Services = append(spec.Services, domain.AppService{Name: "job", Image: "img:1", Readiness: domain.AppReadiness{Type: "none", Timeout: 30 * time.Second}}) | 189 | spec.Services = append(spec.Services, domain.AppService{Name: "job", Image: "img:1", Readiness: domain.AppReadiness{Type: "none", Timeout: 30 * time.Second}}) | ||
| 189 | }}, | 190 | }}, | ||
| 191 | + | {name: "service removed", wantPath: "service/worker (removed)", mutate: func(spec *domain.AppSpec) { | |||
| 192 | + | spec.Services = spec.Services[:1] | |||
| 193 | + | spec.Networks = nil | |||
| 194 | + | }}, | |||
| 190 | } | 195 | } | ||
| 191 | for _, tc := range tests { | 196 | for _, tc := range tests { | ||
| 192 | t.Run(tc.name, func(t *testing.T) { | 197 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -204,15 +209,16 @@ func TestStartDeploy_TargetedDiffPolicy(t *testing.T) { | |||||
| 204 | state.EXPECT().Recover(mock.Anything).Return(nil) | 209 | state.EXPECT().Recover(mock.Anything).Return(nil) | ||
| 205 | state.EXPECT().LoadRevision(mock.Anything, "blog", "rev-2").Return(rev, nil).Once() | 210 | state.EXPECT().LoadRevision(mock.Anything, "blog", "rev-2").Return(rev, nil).Once() | ||
| 206 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(base, true, nil).Once() | 211 | state.EXPECT().LoadActive(mock.Anything, "blog").Return(base, true, nil).Once() | ||
| 207 | - | if !tc.wantErr { | 212 | + | if tc.wantPath == "" { |
| 208 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil).Once() | 213 | state.EXPECT().SaveOperation(mock.Anything, mock.Anything).Return(nil).Once() | ||
| 209 | } | 214 | } | ||
| 210 | 215 | ||||
| 211 | svc := keyedService(t, state, runtime, images, secrets) | 216 | svc := keyedService(t, state, runtime, images, secrets) | ||
| 212 | started, err := svc.StartDeploy(ctx, deployment.DeployInput{App: "blog", Revision: "rev-2", Service: "web"}) | 217 | started, err := svc.StartDeploy(ctx, deployment.DeployInput{App: "blog", Revision: "rev-2", Service: "web"}) | ||
| 213 | 218 | ||||
| 214 | - | if tc.wantErr { | 219 | + | if tc.wantPath != "" { |
| 215 | require.ErrorIs(t, err, domain.ErrAppStateConflict) | 220 | require.ErrorIs(t, err, domain.ErrAppStateConflict) | ||
| 221 | + | assert.Contains(t, err.Error(), tc.wantPath) | |||
| 216 | state.AssertNotCalled(t, "SaveOperation", mock.Anything, mock.Anything) | 222 | state.AssertNotCalled(t, "SaveOperation", mock.Anything, mock.Anything) | ||
| 217 | } else { | 223 | } else { | ||
| 218 | require.NoError(t, err) | 224 | require.NoError(t, err) | ||
M internal/usecase/deployment/service.go +15 -8
| @@ -358,6 +358,10 @@ type DeployResult struct { | |||
| 358 | 358 | Removed []string | |
| 359 | 359 | } | |
| 360 | 360 | ||
| 361 | + | // ServiceResultUnchanged marks a deploy step that kept the running | |
| 362 | + | // container because its image, spec, and environment were already current. | |
| 363 | + | const ServiceResultUnchanged = domain.AppServiceUnchanged | |
| 364 | + | ||
| 361 | 365 | // ServiceResult is one service's terminal deployment result. | |
| 362 | 366 | type ServiceResult struct { | |
| 363 | 367 | Result string | |
| @@ -628,15 +632,18 @@ func (s *Service) checkConverged(ctx context.Context, app, service string, rev d | |||
| 628 | 632 | } | |
| 629 | 633 | diff := domain.DiffAppSpec(rev.Spec, effectiveAppSpec(active)) | |
| 630 | 634 | prefix := "service/" + service + "/" | |
| 631 | - | if len(diff.Added) == 0 && len(diff.Removed) == 0 && len(diff.Changed) > 0 { | |
| 632 | - | for _, path := range diff.Changed { | |
| 633 | - | if !strings.HasPrefix(path, prefix) { | |
| 634 | - | return targetedDivergenceError(rev.Revision, path) | |
| 635 | - | } | |
| 635 | + | if len(diff.Added) > 0 { | |
| 636 | + | return targetedDivergenceError(rev.Revision, diff.Added[0]+" (added)") | |
| 637 | + | } | |
| 638 | + | if len(diff.Removed) > 0 { | |
| 639 | + | return targetedDivergenceError(rev.Revision, diff.Removed[0]+" (removed)") | |
| 640 | + | } | |
| 641 | + | for _, path := range diff.Changed { | |
| 642 | + | if !strings.HasPrefix(path, prefix) { | |
| 643 | + | return targetedDivergenceError(rev.Revision, path) | |
| 636 | 644 | } | |
| 637 | - | return nil | |
| 638 | 645 | } | |
| 639 | - | return targetedDivergenceError(rev.Revision, "application structure") | |
| 646 | + | return nil | |
| 640 | 647 | } | |
| 641 | 648 | ||
| 642 | 649 | func effectiveAppSpec(active domain.AppActive) domain.AppSpec { | |
| @@ -929,7 +936,7 @@ func ComputeOutcome(results map[string]ServiceResult) string { | |||
| 929 | 936 | failed := 0 | |
| 930 | 937 | for _, result := range results { | |
| 931 | 938 | switch result.Result { | |
| 932 | - | case "deployed": | |
| 939 | + | case "deployed", ServiceResultUnchanged: | |
| 933 | 940 | deployed++ | |
| 934 | 941 | case "failed": | |
| 935 | 942 | failed++ | |
| @@ -358,6 +358,10 @@ type DeployResult struct { | |||||
| 358 | Removed []string | 358 | Removed []string | ||
| 359 | } | 359 | } | ||
| 360 | 360 | ||||
| 361 | + | // ServiceResultUnchanged marks a deploy step that kept the running | |||
| 362 | + | // container because its image, spec, and environment were already current. | |||
| 363 | + | const ServiceResultUnchanged = domain.AppServiceUnchanged | |||
| 364 | + | ||||
| 361 | // ServiceResult is one service's terminal deployment result. | 365 | // ServiceResult is one service's terminal deployment result. | ||
| 362 | type ServiceResult struct { | 366 | type ServiceResult struct { | ||
| 363 | Result string | 367 | Result string | ||
| @@ -628,15 +632,18 @@ func (s *Service) checkConverged(ctx context.Context, app, service string, rev d | |||||
| 628 | } | 632 | } | ||
| 629 | diff := domain.DiffAppSpec(rev.Spec, effectiveAppSpec(active)) | 633 | diff := domain.DiffAppSpec(rev.Spec, effectiveAppSpec(active)) | ||
| 630 | prefix := "service/" + service + "/" | 634 | prefix := "service/" + service + "/" | ||
| 631 | - | if len(diff.Added) == 0 && len(diff.Removed) == 0 && len(diff.Changed) > 0 { | 635 | + | if len(diff.Added) > 0 { |
| 632 | - | for _, path := range diff.Changed { | 636 | + | return targetedDivergenceError(rev.Revision, diff.Added[0]+" (added)") |
| 633 | - | if !strings.HasPrefix(path, prefix) { | 637 | + | } |
| 634 | - | return targetedDivergenceError(rev.Revision, path) | 638 | + | if len(diff.Removed) > 0 { |
| 635 | - | } | 639 | + | return targetedDivergenceError(rev.Revision, diff.Removed[0]+" (removed)") |
| 640 | + | } | |||
| 641 | + | for _, path := range diff.Changed { | |||
| 642 | + | if !strings.HasPrefix(path, prefix) { | |||
| 643 | + | return targetedDivergenceError(rev.Revision, path) | |||
| 636 | } | 644 | } | ||
| 637 | - | return nil | |||
| 638 | } | 645 | } | ||
| 639 | - | return targetedDivergenceError(rev.Revision, "application structure") | 646 | + | return nil |
| 640 | } | 647 | } | ||
| 641 | 648 | ||||
| 642 | func effectiveAppSpec(active domain.AppActive) domain.AppSpec { | 649 | func effectiveAppSpec(active domain.AppActive) domain.AppSpec { | ||
| @@ -929,7 +936,7 @@ func ComputeOutcome(results map[string]ServiceResult) string { | |||||
| 929 | failed := 0 | 936 | failed := 0 | ||
| 930 | for _, result := range results { | 937 | for _, result := range results { | ||
| 931 | switch result.Result { | 938 | switch result.Result { | ||
| 932 | - | case "deployed": | 939 | + | case "deployed", ServiceResultUnchanged: |
| 933 | deployed++ | 940 | deployed++ | ||
| 934 | case "failed": | 941 | case "failed": | ||
| 935 | failed++ | 942 | failed++ | ||