Skip to content

validate: reject distinct restart and deploy.restart_policy values - #990

Closed
arxhr007 wants to merge 1 commit into
compose-spec:mainfrom
arxhr007:fix/restart-policy-consistency
Closed

arxhr007 wants to merge 1 commit into
compose-spec:mainfrom
arxhr007:fix/restart-policy-consistency

Conversation

@arxhr007

@arxhr007 arxhr007 commented Nov 5, 2026

Copy link
Copy Markdown

What this PR does

Adds a consistency check for restart and deploy.restart_policy in loader/validate.go, next to the existing scale/deploy.replicas and cpus/deploy.resources.limits.cpus checks. When both attributes are set, they must not contradict each other — the values are compared after mapping to the engine vocabulary, so restart: always + condition: any is accepted as the same policy.

Attributes left unset on one side are treated as complements, not conflicts: restart: on-failure + max_attempts: 3 is allowed (the deploy policy supplies what restart did not set), and a restart_policy with only delay/window stays valid — preserving the complementary cases that motivated the merge-field-by-field fix in docker/compose.

Rejected examples:

  • restart: unless-stopped + condition: on-failure → can't set distinct values on 'restart' and 'deploy.restart_policy.condition'
  • restart: on-failure:3 + condition: any → same error (the merged always + retry count of 3 would also be rejected by the engine at container creation)
  • restart: on-failure:3 + condition: on-failure + max_attempts: 5 → can't set distinct values on 'restart' and 'deploy.restart_policy.max_attempts'

Related

Proposed by @ndeloof on docker/compose#14459 as the compose-go half of the fix for docker/compose#14383 — with the loader rejecting distinct values, the compose-side merge only has to deal with consistent or unset attributes.

Testing done

  • New spec-locking test TestRestartConsistentWithDeployRestartPolicy in loader/tests/restart_test.go: 8 cases covering vocabulary equivalence (any/always, none/no), matching condition + attempts, complementary/unset attributes, and each distinct-value rejection
  • go test ./loader/... — full loader suite passes
  • golangci-lint run — 0 issues

AI disclosure

This contribution was prepared by an AI agent (Devin, Cognition) acting on a human's behalf. The human submitter may not have independently reviewed or tested the change.

When both attributes are set they must not contradict each other:
deploy.restart_policy.condition and max_attempts are compared to the
service-level restart value after mapping them to the engine
vocabulary, so 'restart: always' and 'condition: any' are accepted as
the same policy while 'restart: unless-stopped' with
'condition: on-failure' is rejected instead of silently picking a
winner. Attributes left unset on one side are complements and remain
allowed.

Refs docker/compose#14383

Signed-off-by: arxhr007 <aaron123thomas098@gmail.com>
@arxhr007

arxhr007 commented Nov 5, 2026

Copy link
Copy Markdown
Author

Closing — #989 (filed within the same second, by @ndeloof) supersedes this: it carries the same contradiction check with identical error messages, plus the loader-side reconciliation that folds the merged outcome into restart itself, which is the better long-term design. The complementary cases I tested here (delay-only, no/none equivalence, deploy-supplied max_attempts) are all covered there too.

docker/compose#14459 stays relevant in the meantime — as noted in #989's description, it does the field-by-field merge for current compose-go versions.

@arxhr007 arxhr007 closed this Nov 5, 2026
@arxhr007
arxhr007 deleted the fix/restart-policy-consistency branch November 5, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant