From 03ca6c69a8e62e9e5fb88850a95d2867d03a596b Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Tue, 14 Apr 2026 20:47:53 +0200 Subject: [PATCH 1/2] Add .work into gitignore This is often created by Claude as a temp dir. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5bcb8dba23..563dfcd17f 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ .idea/ _tmp/ .vscode +.work From ef9cf523c088534dfecca0c0a1e1b76ece024b31 Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Tue, 14 Apr 2026 13:50:12 +0200 Subject: [PATCH 2/2] oc debug: Propagate exit code Make oc debug propagate the debug container exit code. This is not happening now and `oc debug` always exits with 0. --- pkg/cli/debug/debug.go | 87 ++++++++++++--- pkg/cli/debug/debug_test.go | 208 ++++++++++++++++++++++++++++++++++++ test/e2e/cli.go | 55 ++++++++++ 3 files changed, 335 insertions(+), 15 deletions(-) diff --git a/pkg/cli/debug/debug.go b/pkg/cli/debug/debug.go index cf4faf2305..fc744a8a46 100644 --- a/pkg/cli/debug/debug.go +++ b/pkg/cli/debug/debug.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "reflect" + "slices" "strings" "time" @@ -43,6 +44,7 @@ import ( "k8s.io/kubectl/pkg/util/interrupt" "k8s.io/kubectl/pkg/util/templates" "k8s.io/pod-security-admission/api" + utilexec "k8s.io/utils/exec" appsv1 "github.com/openshift/api/apps/v1" dockerv10 "github.com/openshift/api/image/docker10" @@ -614,7 +616,7 @@ func (o *DebugOptions) RunDebug() error { } return errors.New(msg) // switch to logging output - case err == krun.ErrPodCompleted, err == conditions.ErrContainerTerminated: + case errors.Is(err, krun.ErrPodCompleted), errors.Is(err, conditions.ErrContainerTerminated): resultPod, ok := containerRunningEvent.Object.(*corev1.Pod) if ok { if resultPod.Status.Reason == "NodeAffinity" && len(resultPod.Spec.NodeSelector) != 0 { @@ -630,12 +632,34 @@ func (o *DebugOptions) RunDebug() error { } } } - return o.getLogs(pod) - case err == conditions.ErrNonZeroExitCode: + + if err := o.getLogs(pod); err != nil { + return err + } + + // The watch event contains the terminal pod state from the API server. + // Use it directly to extract the exit code. + if ok { + return exitCodeError(resultPod, o.ContainerName) + } + return nil + case errors.Is(err, conditions.ErrNonZeroExitCode): if err = o.getLogs(pod); err != nil { return err } - return conditions.ErrNonZeroExitCode + + // The watch event contains the terminal pod state from the API server. + // Use it directly to extract the exit code. + if resultPod, ok := containerRunningEvent.Object.(*corev1.Pod); ok { + if exitErr := exitCodeError(resultPod, o.ContainerName); exitErr != nil { + return exitErr + } + } + // We know the exit code was non-zero but couldn't determine the actual value. + return utilexec.CodeExitError{ + Err: fmt.Errorf("the debug container terminated with a non-zero exit code"), + Code: 1, + } case err != nil: return err case !o.Attach.Stdin: @@ -645,20 +669,14 @@ func (o *DebugOptions) RunDebug() error { lastWatchEvent, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, preconditionFunc, conditions.PodDone) if err != nil { if kapierrors.IsNotFound(err) { - return nil + return fmt.Errorf("the debug pod %q was deleted before completion", pod.Name) } return err } - resultPod, ok := lastWatchEvent.Object.(*corev1.Pod) - if ok { - for _, s := range append(append([]corev1.ContainerStatus{}, resultPod.Status.InitContainerStatuses...), resultPod.Status.ContainerStatuses...) { - if s.Name != o.ContainerName { - continue - } - if s.State.Terminated != nil && s.State.Terminated.ExitCode != 0 { - return conditions.ErrNonZeroExitCode - } + if resultPod, ok := lastWatchEvent.Object.(*corev1.Pod); ok { + if exitErr := exitCodeError(resultPod, o.ContainerName); exitErr != nil { + return exitErr } } return nil @@ -673,7 +691,17 @@ func (o *DebugOptions) RunDebug() error { // TODO: attach can race with pod completion, allow attach to switch to logs o.Attach.ContainerName = o.ContainerName - return o.Attach.Run() + if err := o.Attach.Run(); err != nil { + return err + } + + // After the attach session ends, check the container exit code. + resultPod, err := o.CoreClient.Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{}) + if err != nil { + klog.V(4).Infof("Unable to re-fetch pod %s/%s after attach: %v", pod.Namespace, pod.Name, err) + return nil + } + return exitCodeError(resultPod, o.ContainerName) } }) } @@ -1238,6 +1266,35 @@ func (o *DebugOptions) approximatePodTemplateForObject(object runtime.Object) (* return nil, fmt.Errorf("%v is not supported by debug", reflect.TypeOf(object)) } +// containerExitCode returns the exit code of the named container from the pod status. +// It returns -1 if the container is not found or has not terminated. +func containerExitCode(pod *corev1.Pod, containerName string) int32 { + for _, s := range slices.Concat(pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses) { + if s.Name != containerName { + continue + } + if s.State.Terminated != nil { + return s.State.Terminated.ExitCode + } + return -1 + } + return -1 +} + +// exitCodeError returns a CodeExitError with the container's actual exit code if it +// terminated with a non-zero exit code. It returns nil if the container exited +// successfully or if exit code information is not available. +func exitCodeError(pod *corev1.Pod, containerName string) error { + code := containerExitCode(pod, containerName) + if code <= 0 { + return nil + } + return utilexec.CodeExitError{ + Err: fmt.Errorf("the debug container terminated with exit code %d", code), + Code: int(code), + } +} + func (o *DebugOptions) getLogs(pod *corev1.Pod) error { return logs.LogsOptions{ Object: pod, diff --git a/pkg/cli/debug/debug_test.go b/pkg/cli/debug/debug_test.go index 417fec7ae0..00c00c06b9 100644 --- a/pkg/cli/debug/debug_test.go +++ b/pkg/cli/debug/debug_test.go @@ -1,6 +1,7 @@ package debug import ( + "errors" "testing" corev1 "k8s.io/api/core/v1" @@ -10,6 +11,7 @@ import ( "k8s.io/kubectl/pkg/cmd/attach" "k8s.io/kubectl/pkg/cmd/exec" "k8s.io/pod-security-admission/api" + utilexec "k8s.io/utils/exec" fakekubeclient "k8s.io/client-go/kubernetes/fake" fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake" @@ -266,3 +268,209 @@ func TestCreateLabelMap(t *testing.T) { }) } } + +func TestContainerExitCode(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + containerName string + expectedExitCode int32 + }{ + { + name: "terminated with non-zero exit code", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 42, + }, + { + name: "terminated with exit code 0", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 0, + }, + { + name: "container still running", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: -1, + }, + { + name: "container not found", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "other", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: -1, + }, + { + name: "init container terminated with non-zero exit code", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + InitContainerStatuses: []corev1.ContainerStatus{ + { + Name: "init", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 137}}, + }, + }, + }, + }, + containerName: "init", + expectedExitCode: 137, + }, + { + name: "empty pod status", + pod: &corev1.Pod{}, + containerName: "debug", + expectedExitCode: -1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := containerExitCode(tt.pod, tt.containerName) + if got != tt.expectedExitCode { + t.Errorf("containerExitCode() = %d, want %d", got, tt.expectedExitCode) + } + }) + } +} + +func TestExitCodeError(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + containerName string + // expectedExitCode 0 is used to expect exitCodeError to return nil. + expectedExitCode int + }{ + { + name: "non-zero exit code returns CodeExitError", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 42, + }, + { + name: "exit code 0 returns nil", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 0, + }, + { + name: "container not terminated returns nil", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 0, + }, + { + name: "container not found returns nil", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "other", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 0, + }, + { + name: "exit code 127 returns correct code", + pod: &corev1.Pod{ + Status: corev1.PodStatus{ + ContainerStatuses: []corev1.ContainerStatus{ + { + Name: "debug", + State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 127}}, + }, + }, + }, + }, + containerName: "debug", + expectedExitCode: 127, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exitCodeError(tt.pod, tt.containerName) + if tt.expectedExitCode == 0 { + if err != nil { + t.Errorf("expected nil, got %v", err) + } + return + } + + var exitErr utilexec.CodeExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected utilexec.CodeExitError, got %T: %v", err, err) + } + if exitErr.ExitStatus() != tt.expectedExitCode { + t.Errorf("ExitStatus() = %d, want %d", exitErr.ExitStatus(), tt.expectedExitCode) + } + }) + } +} diff --git a/test/e2e/cli.go b/test/e2e/cli.go index b6b1b057e9..cc9e4dbf2c 100644 --- a/test/e2e/cli.go +++ b/test/e2e/cli.go @@ -3,6 +3,7 @@ package e2e import ( "context" "encoding/json" + "errors" "fmt" "os" "os/exec" @@ -1085,6 +1086,60 @@ var _ = g.Describe("[sig-cli] Workloads client test", func() { err = oc.AsAdmin().WithoutNamespace().Run("debug").Args("node/"+nodeList[0], "-n", project76287, "--", "sos", "help").Execute() o.Expect(err).NotTo(o.HaveOccurred()) }) + + g.It("Author:okupka-ROSA-OSD_CCS-ARO-ConnectedOnly-High-debug-exit-code-oc debug should propagate container exit code", oteginkgo.Informing(), func() { + skipIfMicroShift(oc) + skipIfDisconnected(oc) + + g.By("Create new namespace") + oc.SetupProject() + ns := oc.Namespace() + + // oc debug creates pods without restricted-compatible security context, + // so the namespace must allow privileged pods. + g.By("Set namespace as privileged namespace") + SetNamespacePrivileged(oc, ns) + + // The hello-openshift image is a minimal scratch-based image without + // standard POSIX utilities. Use --image to override with base-alpine + // which has /bin/true, /bin/false, and /bin/sh. + debugImage := "quay.io/openshifttest/base-alpine@sha256:3126e4eed4a3ebd8bf972b2453fa838200988ee07c01b2251e3ea47e4b1f245c" + + g.By("Create a simple deployment to debug") + err := oc.Run("create").Args("deploy", "exit-code-test", "--image", "quay.io/openshifttest/hello-openshift@sha256:4200f438cf2e9446f6bcff9d67ceea1f69ed07a2f83363b7fb52529f7ddd8a83", "-n", ns).Execute() + o.Expect(err).NotTo(o.HaveOccurred()) + if ok := waitForAvailableRsRunning(oc, "deployment", "exit-code-test", ns, "1"); !ok { + e2e.Failf("deployment exit-code-test did not become ready") + } + + runDebug := func(command ...string) error { + args := append([]string{"deploy/exit-code-test", "--image", debugImage, "--"}, command...) + return oc.Run("debug").Args(args...).Execute() + } + + g.By("Verify oc debug exits 0 when command succeeds") + runErr := runDebug("/bin/true") + o.Expect(runErr).NotTo(o.HaveOccurred(), "oc debug with /bin/true should exit 0") + + g.By("Verify oc debug exits non-zero when command fails") + runErr = runDebug("/bin/false") + o.Expect(runErr).To(o.HaveOccurred(), "oc debug with /bin/false should exit non-zero") + var exitErr *exec.ExitError + o.Expect(errors.As(runErr, &exitErr)).To(o.BeTrue(), "error should be an ExitError") + o.Expect(exitErr.ExitCode()).To(o.Equal(1), "exit code should be 1 for /bin/false") + + g.By("Verify oc debug propagates the actual exit code") + runErr = runDebug("/bin/sh", "-c", "exit 42") + o.Expect(runErr).To(o.HaveOccurred(), "oc debug with exit 42 should exit non-zero") + o.Expect(errors.As(runErr, &exitErr)).To(o.BeTrue(), "error should be an ExitError") + o.Expect(exitErr.ExitCode()).To(o.Equal(42), "exit code should be 42") + + g.By("Verify oc debug exits non-zero for non-existent command") + runErr = runDebug("/nonexistent-command") + o.Expect(runErr).To(o.HaveOccurred(), "oc debug with non-existent command should exit non-zero") + o.Expect(errors.As(runErr, &exitErr)).To(o.BeTrue(), "error should be an ExitError") + o.Expect(exitErr.ExitCode()).NotTo(o.Equal(0), "exit code should be non-zero for non-existent command") + }) }) // ClientVersion ...