Merge pull request #9011 from visheshgubrani/scheduler-k3s-run-exit-code

Propagate command exit code on dokku run

plugins/scheduler-k3s/functions.go

@@ -36,6 +36,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/wait"
+	utilexec "k8s.io/client-go/util/exec"
 	"k8s.io/kubernetes/pkg/client/conditions"
 	"k8s.io/utils/ptr"
 	"mvdan.cc/sh/v3/shell"
@@ -409,6 +410,24 @@ func createKubernetesNamespace(ctx context.Context, namespaceName string) error
 	return nil
 }
 
+// attachExitError surfaces the exit code of the remote command from a
+// kubernetes exec stream as an ExitCodeError, so that attached sessions
+// (dokku run in interactive mode, dokku enter) exit with the command's own
+// status rather than a generic failure, matching docker-local scheduler
+// behavior. Other errors pass through unchanged.
+func attachExitError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	var coded utilexec.CodeExitError
+	if errors.As(err, &coded) && coded.Code > 0 {
+		return &ExitCodeError{Code: coded.Code, Message: coded.Error()}
+	}
+
+	return err
+}
+
 func enterPod(ctx context.Context, input EnterPodInput) error {
 	labelSelector := []string{}
 	for k, v := range input.SelectedPod.Labels {
@@ -440,13 +459,13 @@ func enterPod(ctx context.Context, input EnterPodInput) error {
 		return fmt.Errorf("No container specified and no default container found")
 	}
 
-	return input.Clientset.ExecCommand(ctx, ExecCommandInput{
+	return attachExitError(input.Clientset.ExecCommand(ctx, ExecCommandInput{
 		Command:       input.Command,
 		ContainerName: input.SelectedContainerName,
 		Entrypoint:    input.Entrypoint,
 		Name:          input.SelectedPod.Name,
 		Namespace:     input.SelectedPod.Namespace,
-	})
+	}))
 }
 
 func extractStartCommand(input StartCommandInput) string {

plugins/scheduler-k3s/functions_test.go

@@ -1,11 +1,14 @@
 package scheduler_k3s
 
 import (
+	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
 	"github.com/dokku/dokku/plugins/common"
 	corev1 "k8s.io/api/core/v1"
+	utilexec "k8s.io/client-go/util/exec"
 )
 
 func TestNeedsImagePullSecretsPrune(t *testing.T) {
@@ -681,3 +684,42 @@ func TestResolveAppTLSConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestAttachExitError(t *testing.T) {
+	plainErr := errors.New("boom")
+	codedErr := utilexec.CodeExitError{
+		Err:  fmt.Errorf("command terminated with exit code 3"),
+		Code: 3,
+	}
+
+	if err := attachExitError(nil); err != nil {
+		t.Errorf("attachExitError(nil) = %v, want nil", err)
+	}
+
+	if err := attachExitError(plainErr); err != plainErr {
+		t.Errorf("attachExitError(plain error) = %v, want the original error", err)
+	}
+
+	err := attachExitError(codedErr)
+	if err == nil {
+		t.Fatal("attachExitError(CodeExitError) = nil, want ExitCodeError")
+	}
+	exitErr, ok := err.(*ExitCodeError)
+	if !ok {
+		t.Fatalf("attachExitError(CodeExitError) returned %T, want *ExitCodeError", err)
+	}
+	if exitErr.Code != 3 {
+		t.Errorf("Code = %d, want 3", exitErr.Code)
+	}
+	if err.Error() != "command terminated with exit code 3" {
+		t.Errorf("Error() = %q, want %q", err.Error(), "command terminated with exit code 3")
+	}
+
+	zeroCodeErr := utilexec.CodeExitError{
+		Err:  fmt.Errorf("command terminated with exit code 0"),
+		Code: 0,
+	}
+	if err := attachExitError(zeroCodeErr); err != zeroCodeErr {
+		t.Errorf("attachExitError(zero code error) = %v, want the original error", err)
+	}
+}

plugins/scheduler-k3s/triggers.go

@@ -916,6 +916,55 @@ func TriggerSchedulerProxyLogs(scheduler string, appName string, proxyType strin
 	})
 }
 
+// ExitCodeError carries the exit code of a failed user command so that the
+// scheduler trigger can exit with the command's own status rather than a
+// generic failure. It implements common.ErrWithExitCode, which
+// common.LogFailWithError honors, matching docker-local scheduler behavior.
+type ExitCodeError struct {
+	// Code is the exit code of the failed command or container
+	Code int
+
+	// Message is the error text printed to stderr
+	Message string
+}
+
+var _ common.ErrWithExitCode = (*ExitCodeError)(nil)
+
+// Error returns the error text to print to stderr
+func (e *ExitCodeError) Error() string {
+	return e.Message
+}
+
+// ExitCode returns the exit code to use in case this error bubbles up into an
+// os.Exit() call
+func (e *ExitCodeError) ExitCode() int {
+	return e.Code
+}
+
+// failedRunContainerError builds an ExitCodeError from the terminated run
+// container of a failed run pod. The container's exit code is the exit code of
+// the command the run pod executed, so it can be propagated to the caller.
+// When no terminated container with a nonzero exit code exists, a generic
+// failure error is returned instead.
+func failedRunContainerError(appName string, processType string, pod v1.Pod) error {
+	containerName := fmt.Sprintf("%s-%s", appName, processType)
+	for _, status := range pod.Status.ContainerStatuses {
+		if status.Name != containerName || status.State.Terminated == nil {
+			continue
+		}
+		exitCode := int(status.State.Terminated.ExitCode)
+		if exitCode > 0 {
+			message := fmt.Sprintf("Unable to attach as the pod has already exited with a failed exit code: %d", exitCode)
+			if messageDetail := status.State.Terminated.Message; messageDetail != "" {
+				message = fmt.Sprintf("%s (%s)", message, messageDetail)
+			}
+			return &ExitCodeError{Code: exitCode, Message: message}
+		}
+	}
+
+	return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code")
+}
+
 // TriggerSchedulerRun runs a command in an ephemeral container
 func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []string) error {
 	if scheduler != "k3s" {
@@ -1240,16 +1289,7 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
 		case v1.PodSucceeded:
 			return nil
 		case v1.PodFailed:
-			for _, status := range selectedPod.Status.ContainerStatuses {
+			return failedRunContainerError(appName, processType, selectedPod)
-				if status.Name != fmt.Sprintf("%s-%s", appName, processType) {
-					continue
-				}
-				if status.State.Terminated == nil {
-					continue
-				}
-				return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code: %s", status.State.Terminated.Message)
-			}
-			return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code")
 		}
 		return fmt.Errorf("Unable to attach as the pod is in an unknown state: %s", selectedPod.Status.Phase)
 	}
@@ -1276,17 +1316,7 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
 				return fmt.Errorf("Error streaming logs: %w", err)
 			}
 			if selectedPod.Status.Phase == v1.PodFailed {
-				for _, status := range selectedPod.Status.ContainerStatuses {
+				return failedRunContainerError(appName, processType, selectedPod)
-					if status.Name != fmt.Sprintf("%s-%s", appName, processType) {
-						continue
-					}
-					if status.State.Terminated == nil {
-						continue
-					}
-					return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code: %s", status.State.Terminated.Message)
-				}
-
-				return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code")
 			} else if selectedPod.Status.Phase == v1.PodSucceeded {
 				return nil
 			}
@@ -1307,17 +1337,7 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
 	switch selectedPod.Status.Phase {
 	case v1.PodFailed, v1.PodSucceeded:
 		if selectedPod.Status.Phase == v1.PodFailed {
-			for _, status := range selectedPod.Status.ContainerStatuses {
+			return failedRunContainerError(appName, processType, selectedPod)
-				if status.Name != fmt.Sprintf("%s-%s", appName, processType) {
-					continue
-				}
-				if status.State.Terminated == nil {
-					continue
-				}
-				return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code: %s", status.State.Terminated.Message)
-			}
-
-			return fmt.Errorf("Unable to attach as the pod has already exited with a failed exit code")
 		} else if selectedPod.Status.Phase == v1.PodSucceeded {
 			return errors.New("Unable to attach as the pod has already exited with a successful exit code")
 		}

plugins/scheduler-k3s/triggers_test.go

@@ -2,12 +2,15 @@ package scheduler_k3s
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/dokku/dokku/plugins/common"
 	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 func setupChartMigrationTest(t *testing.T) {
@@ -194,3 +197,146 @@ func TestMigrateAnnotationsLabelsToMapFormatSkipsReservedPrefixes(t *testing.T)
 	Expect(common.PropertyGet("scheduler-k3s", "--global", "chart.cert-manager.deployment")).To(Equal("irrelevant"))
 	Expect(common.PropertyGet("scheduler-k3s", "--global", "node-profile-foo.json")).To(Equal("{}"))
 }
+
+func TestExitCodeError(t *testing.T) {
+	var _ common.ErrWithExitCode = (*ExitCodeError)(nil)
+
+	err := &ExitCodeError{Code: 3, Message: "boom"}
+	if err.ExitCode() != 3 {
+		t.Errorf("ExitCode() = %d, want 3", err.ExitCode())
+	}
+	if err.Error() != "boom" {
+		t.Errorf("Error() = %q, want %q", err.Error(), "boom")
+	}
+}
+
+func runPod(appName string, processType string, containerStatuses []corev1.ContainerStatus) v1.Pod {
+	return v1.Pod{
+		Status: v1.PodStatus{
+			Phase:             v1.PodFailed,
+			ContainerStatuses: containerStatuses,
+		},
+	}
+}
+
+func terminatedContainerStatus(name string, exitCode int32, message string) corev1.ContainerStatus {
+	return corev1.ContainerStatus{
+		Name: name,
+		State: corev1.ContainerState{
+			Terminated: &corev1.ContainerStateTerminated{
+				ExitCode: exitCode,
+				Message:  message,
+			},
+		},
+	}
+}
+
+func TestFailedRunContainerError(t *testing.T) {
+	containerName := fmt.Sprintf("%s-%s", "myapp", "run")
+
+	cases := []struct {
+		name        string
+		appName     string
+		processType string
+		statuses    []corev1.ContainerStatus
+		wantCode    int
+		wantError   string
+	}{
+		{
+			name:        "terminated run container with nonzero exit code",
+			appName:     "myapp",
+			processType: "run",
+			statuses:    []corev1.ContainerStatus{terminatedContainerStatus(containerName, 3, "")},
+			wantCode:    3,
+			wantError:   "Unable to attach as the pod has already exited with a failed exit code: 3",
+		},
+		{
+			name:        "terminated run container with nonzero exit code and message",
+			appName:     "myapp",
+			processType: "run",
+			statuses:    []corev1.ContainerStatus{terminatedContainerStatus(containerName, 137, "OOMKilled")},
+			wantCode:    137,
+			wantError:   "Unable to attach as the pod has already exited with a failed exit code: 137 (OOMKilled)",
+		},
+		{
+			name:        "terminated run container with zero exit code returns generic error",
+			appName:     "myapp",
+			processType: "run",
+			statuses:    []corev1.ContainerStatus{terminatedContainerStatus(containerName, 0, "")},
+			wantCode:    -1,
+			wantError:   "Unable to attach as the pod has already exited with a failed exit code",
+		},
+		{
+			name:        "no container statuses returns generic error",
+			appName:     "myapp",
+			processType: "run",
+			statuses:    []corev1.ContainerStatus{},
+			wantCode:    -1,
+			wantError:   "Unable to attach as the pod has already exited with a failed exit code",
+		},
+		{
+			name:        "container name mismatch returns generic error",
+			appName:     "myapp",
+			processType: "run",
+			statuses:    []corev1.ContainerStatus{terminatedContainerStatus("other-app-run", 3, "")},
+			wantCode:    -1,
+			wantError:   "Unable to attach as the pod has already exited with a failed exit code",
+		},
+		{
+			name:        "unterminated matching container returns generic error",
+			appName:     "myapp",
+			processType: "run",
+			statuses: []corev1.ContainerStatus{
+				{
+					Name:  containerName,
+					State: corev1.ContainerState{},
+				},
+			},
+			wantCode:  -1,
+			wantError: "Unable to attach as the pod has already exited with a failed exit code",
+		},
+		{
+			name:        "unrelated terminated container before matching one",
+			appName:     "myapp",
+			processType: "run",
+			statuses: []corev1.ContainerStatus{
+				terminatedContainerStatus("myapp-web", 9, ""),
+				terminatedContainerStatus(containerName, 5, ""),
+			},
+			wantCode:  5,
+			wantError: "Unable to attach as the pod has already exited with a failed exit code: 5",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := failedRunContainerError(tc.appName, tc.processType, runPod(tc.appName, tc.processType, tc.statuses))
+			if tc.wantCode == -1 {
+				if err == nil {
+					t.Fatal("failedRunContainerError() = nil, want generic error")
+				}
+				if _, ok := err.(*ExitCodeError); ok {
+					t.Fatalf("failedRunContainerError() returned %T, want generic error", err)
+				}
+				if err.Error() != tc.wantError {
+					t.Errorf("failedRunContainerError() error = %q, want %q", err.Error(), tc.wantError)
+				}
+				return
+			}
+
+			if err == nil {
+				t.Fatal("failedRunContainerError() = nil, want ExitCodeError")
+			}
+			exitErr, ok := err.(*ExitCodeError)
+			if !ok {
+				t.Fatalf("failedRunContainerError() returned %T, want *ExitCodeError", err)
+			}
+			if exitErr.ExitCode() != tc.wantCode {
+				t.Errorf("ExitCode() = %d, want %d", exitErr.ExitCode(), tc.wantCode)
+			}
+			if err.Error() != tc.wantError {
+				t.Errorf("Error() = %q, want %q", err.Error(), tc.wantError)
+			}
+		})
+	}
+}

tests/unit/run_2.bats

@@ -178,6 +178,23 @@ teardown() {
   [[ "$output" != *"uid="* ]] || flunk "id command output leaked - env value was expanded"
 }
 
+@test "(run) run propagates the command exit code" {
+  run deploy_app dockerfile
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku run $TEST_APP sh -c 'exit 3'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_exit_status 3
+
+  run /bin/bash -c "dokku run $TEST_APP sh -c 'exit 0'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+}
+
 @test "(run) --ttl-seconds rejects non-numeric input; docker-options stores it verbatim" {
   run /bin/bash -c "dokku run --ttl-seconds '\$(id)' $TEST_APP echo hi"
   echo "output: $output"

tests/unit/scheduler-k3s-dockerfile.bats

@@ -156,6 +156,34 @@ teardown() {
   assert_output_contains "SECRET_KEY=fjdkslafjdk"
 }
 
+@test "(scheduler-k3s) dockerfile dokku run propagates the command exit code" {
+  if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then
+    skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN"
+  fi
+
+  INGRESS_CLASS=nginx install_k3s
+
+  run /bin/bash -c "dokku apps:create $TEST_APP"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run deploy_app dockerfile-procfile dokku@$DOKKU_DOMAIN:$TEST_APP
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet run $TEST_APP sh -c 'exit 3'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_exit_status 3
+
+  run /bin/bash -c "dokku --quiet run $TEST_APP sh -c 'exit 0'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+}
+
 @test "(scheduler-k3s) dockerfile dokku run resolves Procfile key" {
   if [[ -z "$DOCKERHUB_USERNAME" ]] || [[ -z "$DOCKERHUB_TOKEN" ]]; then
     skip "skipping due to missing docker.io credentials DOCKERHUB_USERNAME:DOCKERHUB_TOKEN"