From 4babd72186971f5b5b9df67b6c3f8fa65fcdf635 Mon Sep 17 00:00:00 2001
From: Laura Brehm <laurabrehm@hey.com>
Date: Fri, 27 Sep 2024 15:39:12 +0100
Subject: [PATCH 1/2] tests: skip docker-py exec exit code test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Temporarily skip the exec run failed exit code test in `docker-py` –
https://github.com/docker/docker-py/blob/a3652028b1ead708bd9191efb286f909ba6c2a49/tests/integration/models_containers_test.py#L356-L363

We can reenable this after the PR fixing the expected exit code in that
test is merged/released/included – https://github.com/docker/docker-py/pull/3290

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
---
 hack/make/test-docker-py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hack/make/test-docker-py b/hack/make/test-docker-py
index c3e788d0d2737..565e160dad191 100644
--- a/hack/make/test-docker-py
+++ b/hack/make/test-docker-py
@@ -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
 

From c866a7e5f8caa882641cbba59102f457221314dc Mon Sep 17 00:00:00 2001
From: Laura Brehm <laurabrehm@hey.com>
Date: Thu, 26 Sep 2024 13:51:50 +0100
Subject: [PATCH 2/2] daemon/exec: don't overwrite exit code if set

If we fail to start an exec, the deferred error-handling block in [L181-L193](https://github.com/moby/moby/blob/c7e42d855ec41c28702a1f6f6c2c28ecc7bd9424/daemon/exec.go#L181-L193)
would set the exit code to `126` (`EACCES`). However, if we get far enough along
attempting to start the exec, we set the exit code according to the error returned
from starting the task [L288-L291](https://github.com/moby/moby/blob/c7e42d855ec41c28702a1f6f6c2c28ecc7bd9424/daemon/exec.go#L288-L291).

For some situations (such as `docker exec [some-container]
missing-binary`), the 2nd block returns the correct exit code (`127`)
but that then gets overwritten by the 1st block.

This commit changes that logic to only set the default exit code `126`
if the exit code has not been set yet.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
---
 daemon/exec.go                           |  8 ++++--
 integration/container/exec_linux_test.go | 33 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/daemon/exec.go b/daemon/exec.go
index 0005c9474ce6b..1ca74b122c5a3 100644
--- a/daemon/exec.go
+++ b/daemon/exec.go
@@ -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)
 			}
diff --git a/integration/container/exec_linux_test.go b/integration/container/exec_linux_test.go
index a91d065b25c3f..dd79ab561b5bf 100644
--- a/integration/container/exec_linux_test.go
+++ b/integration/container/exec_linux_test.go
@@ -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)
+		})
+	}
+}