Skip to content

Commit

Permalink
Merge pull request moby#48552 from laurazard/exec-exit-code-race
Browse files Browse the repository at this point in the history
daemon/exec: don't overwrite exit code if set
  • Loading branch information
thaJeztah authored Sep 30, 2024
2 parents b5d04e4 + c866a7e commit 4001d07
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 2 deletions.
8 changes: 6 additions & 2 deletions daemon/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,12 @@ func (daemon *Daemon) ContainerExecStart(ctx context.Context, name string, optio
ec.Lock()
ec.Container.ExecCommands.Delete(ec.ID)
ec.Running = false
exitCode := 126
ec.ExitCode = &exitCode
if ec.ExitCode == nil {
// default to `126` (`EACCES`) if we fail to start
// the exec without setting an exit code.
exitCode := exitEaccess
ec.ExitCode = &exitCode
}
if err := ec.CloseStreams(); err != nil {
log.G(ctx).Errorf("failed to cleanup exec %s streams: %s", ec.Container.ID, err)
}
Expand Down
3 changes: 3 additions & 0 deletions hack/make/test-docker-py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ fi
# TODO(vvoland): re-enable after https://github.com/docker/docker-py/pull/3203 is included in the DOCKER_PY_COMMIT release.
PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_image_test.py::CommitTest::test_commit"
PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_image_test.py::CommitTest::test_commit_with_changes"

# TODO(laurazard): re-enable after https://github.com/docker/docker-py/pull/3290 is included in the DOCKER_PY_COMMIT release.
PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/models_containers_test.py::ContainerTest::test_exec_run_failed"
(
bundle .integration-daemon-start

Expand Down
33 changes: 33 additions & 0 deletions integration/container/exec_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,36 @@ func TestExecConsoleSize(t *testing.T) {
assert.NilError(t, err)
assert.Equal(t, strings.TrimSpace(result.Stdout()), "57 123")
}

func TestFailedExecExitCode(t *testing.T) {
testCases := []struct {
doc string
command []string
expectedExitCode int
}{
{
doc: "executable not found",
command: []string{"nonexistent"},
expectedExitCode: 127,
},
{
doc: "executable cannot be invoked",
command: []string{"/etc"},
expectedExitCode: 126,
},
}

for _, tc := range testCases {
t.Run(tc.doc, func(t *testing.T) {
ctx := setupTest(t)
apiClient := testEnv.APIClient()

cID := container.Run(ctx, t, apiClient)

result, err := container.Exec(ctx, apiClient, cID, tc.command)
assert.NilError(t, err)

assert.Equal(t, result.ExitCode, tc.expectedExitCode)
})
}
}

0 comments on commit 4001d07

Please sign in to comment.