Skip to content

[demo] Read the restart policy from restart only, merged by compose-go #14461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ndeloof wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[demo] Read the restart policy from restart only, merged by compose-go #14461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b31804a
demo: read the restart policy from restart only, merged by compose-go
ndeloof Nov 5, 2026
fffc26d
build: follow the compose-go PR (zero retries, carry in Normalize)
ndeloof Nov 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,5 @@ exclude (
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
)

replace github.com/compose-spec/compose-go/v2 => github.com/ndeloof/compose-go/v2 v2.0.0-20261105145003-9652c7ff3592
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg
github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4=
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE=
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4=
github.com/compose-spec/compose-go/v2 v2.16.2 h1:pTp1DqQwJzFd85GzlWsSszKA1ABrV/ySqsClujIcWA8=
github.com/compose-spec/compose-go/v2 v2.16.2/go.mod h1:Q1+qtN4vhzEjGrnqRtzx1xa8raDZQlMUe3WJxndYNiQ=
github.com/containerd/cgroups/v3 v3.1.3 h1:eUNflyMddm18+yrDmZPn3jI7C5hJ9ahABE5q6dyLYXQ=
github.com/containerd/cgroups/v3 v3.1.3/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw=
github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc=
Expand Down Expand Up @@ -183,6 +181,8 @@ github.com/morikuni/aec v1.1.0 h1:vBBl0pUnvi/Je71dsRrhMBtreIqNMYErSAbEeb8jrXQ=
github.com/morikuni/aec v1.1.0/go.mod h1:xDRgiq/iw5l+zkao76YTKzKttOp2cwPEne25HDkJnBw=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/ndeloof/compose-go/v2 v2.0.0-20261105145003-9652c7ff3592 h1:hxNAjGvDZAOvtsYOfiq5KBlwPtHLtg5deX8eKHmdVMM=
github.com/ndeloof/compose-go/v2 v2.0.0-20261105145003-9652c7ff3592/go.mod h1:hDQhfvD3lgjW9Q2Vh69LmIg2usJTnvh9D/7xByuMdxc=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.1 h1:y0fUlFfIZhPF1W537XOLg0/fcx6zcHCJwooC2xJA040=
Expand Down
37 changes: 14 additions & 23 deletions pkg/compose/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,35 +748,26 @@ func umaskOption(service types.ServiceConfig, version string) (*uint32, error) {
return service.Umask, nil
}

// getRestartPolicy reads the restart policy of the service. The loader has
// already merged deploy.restart_policy into restart, and rejected the two when
// they contradict each other, so restart is the only attribute to read.
func getRestartPolicy(service types.ServiceConfig) container.RestartPolicy {
var restart container.RestartPolicy
if service.Restart != "" {
name, num, ok := strings.Cut(service.Restart, ":")
var attempts int
if ok {
attempts, _ = strconv.Atoi(num)
}
restart = container.RestartPolicy{
Name: mapRestartPolicyCondition(name),
MaximumRetryCount: attempts,
}
if service.Restart == "" {

@docker-agent docker-agent Nov 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] getRestartPolicy silently drops deploy.restart_policy without the forked compose-go

The simplified function trusts that service.Restart already incorporates deploy.restart_policy, but that merge is performed by the forked loader (github.com/ndeloof/compose-go/v2). If this PR is merged — or if the replace directive is later removed without the upstream compose-spec/compose-go#989 also being merged — any service that specifies only deploy.restart_policy (no restart: key) will silently receive an empty container.RestartPolicy{}, meaning the container runs with no restart policy at all.

The PR description explicitly notes this risk: "Services built by hand through the SDK, which do not go through the loader, would no longer get their deploy.restart_policy read." The same applies at the Go-module level if the fork is ever swapped back for the official module without the upstream PR.

This is an inherent design constraint of the demo. The comment at lines 751–753 documents the assumption clearly, which helps a future reviewer catch it. Consider adding a //nolint or build-tag guard to prevent accidental merge, or at a minimum add a TODO(compose-spec/compose-go#989) marker so the coupling is tracked.

Confidence Score
🟢 strong 100/100

return container.RestartPolicy{}
}
if service.Deploy != nil && service.Deploy.RestartPolicy != nil {
policy := *service.Deploy.RestartPolicy
var attempts int
if policy.MaxAttempts != nil {
attempts = int(*policy.MaxAttempts)
}
restart = container.RestartPolicy{
Name: mapRestartPolicyCondition(policy.Condition),
MaximumRetryCount: attempts,
}
name, num, ok := strings.Cut(service.Restart, ":")
var attempts int
if ok {
attempts, _ = strconv.Atoi(num)
}
return container.RestartPolicy{
Name: mapRestartPolicyCondition(name),
MaximumRetryCount: attempts,
}
return restart
}

func mapRestartPolicyCondition(condition string) container.RestartPolicyMode {
// map definitions of deploy.restart_policy to engine definitions
// map the names of restart to engine definitions
switch condition {
case "none", "no":
return container.RestartPolicyDisabled
Expand Down
48 changes: 48 additions & 0 deletions pkg/compose/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,51 @@ func TestSetBlkio(t *testing.T) {
assert.Equal(t, resources.BlkioDeviceWriteBps[0].Rate, uint64(3))
assert.Assert(t, resources.BlkioDeviceWriteIOps == nil)
}

// The restart policy of a container comes from the service's restart, in which
// the loader has already merged deploy.restart_policy: a restart_policy that
// sets no condition (docker/compose#14383) must leave the policy of restart as
// it is, and the two attributes must not contradict each other.
func TestGetRestartPolicy(t *testing.T) {
loadService := func(t *testing.T, attributes string) (composetypes.ServiceConfig, error) {
t.Helper()
project, err := composeloader.LoadWithContext(t.Context(), composetypes.ConfigDetails{
ConfigFiles: []composetypes.ConfigFile{{Filename: "compose.yaml", Content: []byte("name: test\nservices:\n foo:\n image: alpine\n" + attributes)}},
Environment: map[string]string{},
})
if err != nil {
return composetypes.ServiceConfig{}, err
}
return project.Services["foo"], nil
}

for _, tc := range []struct {
name string
attributes string
want container.RestartPolicy
}{
{"restart", " restart: unless-stopped\n", container.RestartPolicy{Name: container.RestartPolicyUnlessStopped}},
{"restart with a retry count", " restart: on-failure:3\n", container.RestartPolicy{Name: container.RestartPolicyOnFailure, MaximumRetryCount: 3}},
{"restart_policy alone", " deploy: {restart_policy: {condition: on-failure, max_attempts: 5}}\n", container.RestartPolicy{Name: container.RestartPolicyOnFailure, MaximumRetryCount: 5}},
{"restart_policy any", " deploy: {restart_policy: {condition: any}}\n", container.RestartPolicy{Name: container.RestartPolicyAlways}},
{"restart_policy none", " deploy: {restart_policy: {condition: none}}\n", container.RestartPolicy{Name: container.RestartPolicyDisabled}},
{
"restart_policy completes restart",
" restart: on-failure\n deploy: {restart_policy: {max_attempts: 3}}\n",
container.RestartPolicy{Name: container.RestartPolicyOnFailure, MaximumRetryCount: 3},
},
{"restart_policy without condition keeps restart", " restart: always\n deploy: {restart_policy: {delay: 5s}}\n", container.RestartPolicy{Name: container.RestartPolicyAlways}},
{"restart_policy without condition and no restart", " deploy: {restart_policy: {delay: 5s}}\n", container.RestartPolicy{}},
} {
t.Run(tc.name, func(t *testing.T) {
service, err := loadService(t, tc.attributes)
assert.NilError(t, err)
assert.DeepEqual(t, getRestartPolicy(service), tc.want)
})
}

t.Run("contradictory restart and restart_policy", func(t *testing.T) {
_, err := loadService(t, " restart: unless-stopped\n deploy: {restart_policy: {condition: on-failure}}\n")
assert.ErrorContains(t, err, "can't set distinct values on 'restart' and 'deploy.restart_policy.condition'")
})
}
Loading