diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index c1164d813..acbf8391b 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/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 { diff --git a/plugins/scheduler-k3s/functions_test.go b/plugins/scheduler-k3s/functions_test.go index aad7ab8c6..182f0de6e 100644 --- a/plugins/scheduler-k3s/functions_test.go +++ b/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) + } +} diff --git a/plugins/scheduler-k3s/triggers.go b/plugins/scheduler-k3s/triggers.go index 89d31cea9..c528400d7 100644 --- a/plugins/scheduler-k3s/triggers.go +++ b/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 { - 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 failedRunContainerError(appName, processType, selectedPod) } 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 { - 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 failedRunContainerError(appName, processType, selectedPod) } 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 { - 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 failedRunContainerError(appName, processType, selectedPod) } else if selectedPod.Status.Phase == v1.PodSucceeded { return errors.New("Unable to attach as the pod has already exited with a successful exit code") } diff --git a/plugins/scheduler-k3s/triggers_test.go b/plugins/scheduler-k3s/triggers_test.go index 3b8afa20e..c12748810 100644 --- a/plugins/scheduler-k3s/triggers_test.go +++ b/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) + } + }) + } +} diff --git a/tests/unit/run_2.bats b/tests/unit/run_2.bats index b80957028..86cf27e70 100644 --- a/tests/unit/run_2.bats +++ b/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" diff --git a/tests/unit/scheduler-k3s-dockerfile.bats b/tests/unit/scheduler-k3s-dockerfile.bats index 83636ce6f..eb8fdf1da 100644 --- a/tests/unit/scheduler-k3s-dockerfile.bats +++ b/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"