mirror of
https://github.com/docker/compose.git
synced 2026-05-13 13:58:02 +00:00
fix: treat container exit code 0 as success in --wait mode
When using , containers that exit with code 0 (such as init or oneshot containers) were incorrectly treated as failures, causing the command to return exit code 1. Introduce a typed containerExitError in isServiceHealthy and handle exit code 0 in the RunningOrHealthy wait path. Services with restart policies that restart on clean exit (always, unless-stopped) are excluded via restartsOnExit0 to avoid false positives. Fixes #10596 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This commit is contained in:
parent
2b9f60ba58
commit
6f0dccbdc2
3 changed files with 182 additions and 1 deletions
|
|
@ -482,6 +482,14 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr
|
|||
case ServiceConditionRunningOrHealthy:
|
||||
isHealthy, err := s.isServiceHealthy(ctx, waitingFor, true)
|
||||
if err != nil {
|
||||
// A container that exited with code 0 can be treated as
|
||||
// successfully completed (init/oneshot containers) when the
|
||||
// service won't be restarted by Docker after a clean exit.
|
||||
var exitErr *containerExitError
|
||||
if errors.As(err, &exitErr) && exitErr.exitCode == 0 && !restartsOnExit0(project, dep) {
|
||||
s.events.On(containerEvents(waitingFor, exited)...)
|
||||
return nil
|
||||
}
|
||||
if !config.Required {
|
||||
s.events.On(containerReasonEvents(waitingFor, skippedEvent,
|
||||
fmt.Sprintf("optional dependency %q is not running or is unhealthy", dep))...)
|
||||
|
|
@ -552,6 +560,29 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr
|
|||
return err
|
||||
}
|
||||
|
||||
// restartsOnExit0 reports whether Docker will restart the named service after
|
||||
// it exits with code 0. Only restart policies "always" and "unless-stopped"
|
||||
// cause a restart on clean exit; "no", "on-failure", and unset do not.
|
||||
// For deploy.restart_policy.condition, "any" (and the non-spec but accepted
|
||||
// "always" and "unless-stopped") cause a restart on clean exit.
|
||||
func restartsOnExit0(project *types.Project, serviceName string) bool {
|
||||
service, err := project.GetService(serviceName)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
switch service.Restart {
|
||||
case types.RestartPolicyAlways, types.RestartPolicyUnlessStopped:
|
||||
return true
|
||||
}
|
||||
if service.Deploy != nil && service.Deploy.RestartPolicy != nil {
|
||||
switch service.Deploy.RestartPolicy.Condition {
|
||||
case "any", "always", "unless-stopped":
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceDependency, project *types.Project) (bool, error) {
|
||||
if dependencyConfig.Condition == types.ServiceConditionStarted {
|
||||
// already managed by InDependencyOrder
|
||||
|
|
@ -803,6 +834,18 @@ func (s *composeService) getLinks(ctx context.Context, projectName string, servi
|
|||
return links, nil
|
||||
}
|
||||
|
||||
// containerExitError is returned by isServiceHealthy when a container has exited.
|
||||
// It carries the exit code so callers can distinguish clean exits (code 0)
|
||||
// from failures without parsing error strings.
|
||||
type containerExitError struct {
|
||||
name string
|
||||
exitCode int
|
||||
}
|
||||
|
||||
func (e *containerExitError) Error() string {
|
||||
return fmt.Sprintf("container %s exited (%d)", e.name, e.exitCode)
|
||||
}
|
||||
|
||||
func (s *composeService) isServiceHealthy(ctx context.Context, containers Containers, fallbackRunning bool) (bool, error) {
|
||||
for _, c := range containers {
|
||||
res, err := s.apiClient().ContainerInspect(ctx, c.ID, client.ContainerInspectOptions{})
|
||||
|
|
@ -813,7 +856,7 @@ func (s *composeService) isServiceHealthy(ctx context.Context, containers Contai
|
|||
name := ctr.Name[1:]
|
||||
|
||||
if ctr.State.Status == container.StateExited {
|
||||
return false, fmt.Errorf("container %s exited (%d)", name, ctr.State.ExitCode)
|
||||
return false, &containerExitError{name: name, exitCode: ctr.State.ExitCode}
|
||||
}
|
||||
|
||||
noHealthcheck := ctr.Config.Healthcheck == nil || (len(ctr.Config.Healthcheck.Test) > 0 && ctr.Config.Healthcheck.Test[0] == "NONE")
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ package compose
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/netip"
|
||||
"strings"
|
||||
|
|
@ -366,6 +367,34 @@ func TestIsServiceHealthy(t *testing.T) {
|
|||
|
||||
_, err := tested.(*composeService).isServiceHealthy(ctx, containers, true)
|
||||
assert.ErrorContains(t, err, "exited")
|
||||
var exitErr *containerExitError
|
||||
assert.Assert(t, errors.As(err, &exitErr))
|
||||
assert.Equal(t, exitErr.exitCode, 1)
|
||||
})
|
||||
|
||||
t.Run("exited container with exit code 0 returns containerExitError", func(t *testing.T) {
|
||||
containerID := "test-container-id"
|
||||
containers := Containers{
|
||||
{ID: containerID},
|
||||
}
|
||||
|
||||
apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{
|
||||
Container: container.InspectResponse{
|
||||
ID: containerID,
|
||||
Name: "test-container",
|
||||
State: &container.State{
|
||||
Status: "exited",
|
||||
ExitCode: 0,
|
||||
},
|
||||
Config: &container.Config{},
|
||||
},
|
||||
}, nil)
|
||||
|
||||
_, err := tested.(*composeService).isServiceHealthy(ctx, containers, true)
|
||||
assert.Assert(t, err != nil, "expected error for exited container")
|
||||
var exitErr *containerExitError
|
||||
assert.Assert(t, errors.As(err, &exitErr))
|
||||
assert.Equal(t, exitErr.exitCode, 0)
|
||||
})
|
||||
|
||||
t.Run("healthy container with healthcheck", func(t *testing.T) {
|
||||
|
|
|
|||
109
pkg/compose/start_test.go
Normal file
109
pkg/compose/start_test.go
Normal file
|
|
@ -0,0 +1,109 @@
|
|||
/*
|
||||
Copyright 2020 Docker Compose CLI authors
|
||||
|
||||
Licensed under the Apache License, Version 2.0 (the "License");
|
||||
you may not use this file except in compliance with the License.
|
||||
You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing, software
|
||||
distributed under the License is distributed on an "AS IS" BASIS,
|
||||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
See the License for the specific language governing permissions and
|
||||
limitations under the License.
|
||||
*/
|
||||
|
||||
package compose
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/compose-spec/compose-go/v2/types"
|
||||
"gotest.tools/v3/assert"
|
||||
)
|
||||
|
||||
func TestRestartsOnExit0(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
service types.ServiceConfig
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "no restart policy",
|
||||
service: types.ServiceConfig{Name: "init"},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "restart no",
|
||||
service: types.ServiceConfig{Name: "init", Restart: types.RestartPolicyNo},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "restart on-failure",
|
||||
service: types.ServiceConfig{Name: "init", Restart: types.RestartPolicyOnFailure},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "restart always",
|
||||
service: types.ServiceConfig{Name: "web", Restart: types.RestartPolicyAlways},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "restart unless-stopped",
|
||||
service: types.ServiceConfig{Name: "web", Restart: types.RestartPolicyUnlessStopped},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "deploy restart policy condition any",
|
||||
service: types.ServiceConfig{
|
||||
Name: "web",
|
||||
Deploy: &types.DeployConfig{
|
||||
RestartPolicy: &types.RestartPolicy{Condition: "any"},
|
||||
},
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "deploy restart policy condition on-failure",
|
||||
service: types.ServiceConfig{
|
||||
Name: "web",
|
||||
Deploy: &types.DeployConfig{
|
||||
RestartPolicy: &types.RestartPolicy{Condition: "on-failure"},
|
||||
},
|
||||
},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "deploy restart policy condition none",
|
||||
service: types.ServiceConfig{
|
||||
Name: "web",
|
||||
Deploy: &types.DeployConfig{
|
||||
RestartPolicy: &types.RestartPolicy{Condition: "none"},
|
||||
},
|
||||
},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "deploy restart policy condition unless-stopped",
|
||||
service: types.ServiceConfig{
|
||||
Name: "web",
|
||||
Deploy: &types.DeployConfig{
|
||||
RestartPolicy: &types.RestartPolicy{Condition: "unless-stopped"},
|
||||
},
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
project := &types.Project{
|
||||
Services: types.Services{
|
||||
tt.service.Name: tt.service,
|
||||
},
|
||||
}
|
||||
assert.Equal(t, restartsOnExit0(project, tt.service.Name), tt.expected)
|
||||
})
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue