Skip to content

Commit 702eb82

Browse files
tchaptchap
committed
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.
1 parent 03ca6c6 commit 702eb82

3 files changed

Lines changed: 339 additions & 15 deletions

File tree

‎pkg/cli/debug/debug.go‎

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"os"
88
"reflect"
9+
"slices"
910
"strings"
1011
"time"
1112

@@ -43,6 +44,7 @@ import (
4344
"k8s.io/kubectl/pkg/util/interrupt"
4445
"k8s.io/kubectl/pkg/util/templates"
4546
"k8s.io/pod-security-admission/api"
47+
utilexec "k8s.io/utils/exec"
4648

4749
appsv1 "github.com/openshift/api/apps/v1"
4850
dockerv10 "github.com/openshift/api/image/docker10"
@@ -614,7 +616,7 @@ func (o *DebugOptions) RunDebug() error {
614616
}
615617
return errors.New(msg)
616618
// switch to logging output
617-
case err == krun.ErrPodCompleted, err == conditions.ErrContainerTerminated:
619+
case errors.Is(err, krun.ErrPodCompleted), errors.Is(err, conditions.ErrContainerTerminated):
618620
resultPod, ok := containerRunningEvent.Object.(*corev1.Pod)
619621
if ok {
620622
if resultPod.Status.Reason == "NodeAffinity" && len(resultPod.Spec.NodeSelector) != 0 {
@@ -630,12 +632,34 @@ func (o *DebugOptions) RunDebug() error {
630632
}
631633
}
632634
}
633-
return o.getLogs(pod)
634-
case err == conditions.ErrNonZeroExitCode:
635+
636+
if err := o.getLogs(pod); err != nil {
637+
return err
638+
}
639+
640+
// The watch event contains the terminal pod state from the API server.
641+
// Use it directly to extract the exit code.
642+
if ok {
643+
return exitCodeError(resultPod, o.ContainerName)
644+
}
645+
return nil
646+
case errors.Is(err, conditions.ErrNonZeroExitCode):
635647
if err = o.getLogs(pod); err != nil {
636648
return err
637649
}
638-
return conditions.ErrNonZeroExitCode
650+
651+
// The watch event contains the terminal pod state from the API server.
652+
// Use it directly to extract the exit code.
653+
if resultPod, ok := containerRunningEvent.Object.(*corev1.Pod); ok {
654+
if exitErr := exitCodeError(resultPod, o.ContainerName); exitErr != nil {
655+
return exitErr
656+
}
657+
}
658+
// We know the exit code was non-zero but couldn't determine the actual value.
659+
return utilexec.CodeExitError{
660+
Err: fmt.Errorf("the debug container terminated with a non-zero exit code"),
661+
Code: 1,
662+
}
639663
case err != nil:
640664
return err
641665
case !o.Attach.Stdin:
@@ -645,20 +669,14 @@ func (o *DebugOptions) RunDebug() error {
645669
lastWatchEvent, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, preconditionFunc, conditions.PodDone)
646670
if err != nil {
647671
if kapierrors.IsNotFound(err) {
648-
return nil
672+
return fmt.Errorf("the debug pod %q was deleted before completion", pod.Name)
649673
}
650674
return err
651675
}
652676

653-
resultPod, ok := lastWatchEvent.Object.(*corev1.Pod)
654-
if ok {
655-
for _, s := range append(append([]corev1.ContainerStatus{}, resultPod.Status.InitContainerStatuses...), resultPod.Status.ContainerStatuses...) {
656-
if s.Name != o.ContainerName {
657-
continue
658-
}
659-
if s.State.Terminated != nil && s.State.Terminated.ExitCode != 0 {
660-
return conditions.ErrNonZeroExitCode
661-
}
677+
if resultPod, ok := lastWatchEvent.Object.(*corev1.Pod); ok {
678+
if exitErr := exitCodeError(resultPod, o.ContainerName); exitErr != nil {
679+
return exitErr
662680
}
663681
}
664682
return nil
@@ -673,7 +691,17 @@ func (o *DebugOptions) RunDebug() error {
673691

674692
// TODO: attach can race with pod completion, allow attach to switch to logs
675693
o.Attach.ContainerName = o.ContainerName
676-
return o.Attach.Run()
694+
if err := o.Attach.Run(); err != nil {
695+
return err
696+
}
697+
698+
// After the attach session ends, check the container exit code.
699+
resultPod, err := o.CoreClient.Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
700+
if err != nil {
701+
klog.V(4).Infof("Unable to re-fetch pod %s/%s after attach: %v", pod.Namespace, pod.Name, err)
702+
return nil
703+
}
704+
return exitCodeError(resultPod, o.ContainerName)
677705
}
678706
})
679707
}
@@ -1238,6 +1266,35 @@ func (o *DebugOptions) approximatePodTemplateForObject(object runtime.Object) (*
12381266
return nil, fmt.Errorf("%v is not supported by debug", reflect.TypeOf(object))
12391267
}
12401268

1269+
// containerExitCode returns the exit code of the named container from the pod status.
1270+
// It returns -1 if the container is not found or has not terminated.
1271+
func containerExitCode(pod *corev1.Pod, containerName string) int32 {
1272+
for _, s := range slices.Concat(pod.Status.InitContainerStatuses, pod.Status.ContainerStatuses) {
1273+
if s.Name != containerName {
1274+
continue
1275+
}
1276+
if s.State.Terminated != nil {
1277+
return s.State.Terminated.ExitCode
1278+
}
1279+
return -1
1280+
}
1281+
return -1
1282+
}
1283+
1284+
// exitCodeError returns a CodeExitError with the container's actual exit code if it
1285+
// terminated with a non-zero exit code. It returns nil if the container exited
1286+
// successfully or if exit code information is not available.
1287+
func exitCodeError(pod *corev1.Pod, containerName string) error {
1288+
code := containerExitCode(pod, containerName)
1289+
if code <= 0 {
1290+
return nil
1291+
}
1292+
return utilexec.CodeExitError{
1293+
Err: fmt.Errorf("the debug container terminated with exit code %d", code),
1294+
Code: int(code),
1295+
}
1296+
}
1297+
12411298
func (o *DebugOptions) getLogs(pod *corev1.Pod) error {
12421299
return logs.LogsOptions{
12431300
Object: pod,

‎pkg/cli/debug/debug_test.go‎

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package debug
22

33
import (
4+
"errors"
45
"testing"
56

67
corev1 "k8s.io/api/core/v1"
@@ -10,6 +11,7 @@ import (
1011
"k8s.io/kubectl/pkg/cmd/attach"
1112
"k8s.io/kubectl/pkg/cmd/exec"
1213
"k8s.io/pod-security-admission/api"
14+
utilexec "k8s.io/utils/exec"
1315

1416
fakekubeclient "k8s.io/client-go/kubernetes/fake"
1517
fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake"
@@ -266,3 +268,209 @@ func TestCreateLabelMap(t *testing.T) {
266268
})
267269
}
268270
}
271+
272+
func TestContainerExitCode(t *testing.T) {
273+
tests := []struct {
274+
name string
275+
pod *corev1.Pod
276+
containerName string
277+
expectedExitCode int32
278+
}{
279+
{
280+
name: "terminated with non-zero exit code",
281+
pod: &corev1.Pod{
282+
Status: corev1.PodStatus{
283+
ContainerStatuses: []corev1.ContainerStatus{
284+
{
285+
Name: "debug",
286+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}},
287+
},
288+
},
289+
},
290+
},
291+
containerName: "debug",
292+
expectedExitCode: 42,
293+
},
294+
{
295+
name: "terminated with exit code 0",
296+
pod: &corev1.Pod{
297+
Status: corev1.PodStatus{
298+
ContainerStatuses: []corev1.ContainerStatus{
299+
{
300+
Name: "debug",
301+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}},
302+
},
303+
},
304+
},
305+
},
306+
containerName: "debug",
307+
expectedExitCode: 0,
308+
},
309+
{
310+
name: "container still running",
311+
pod: &corev1.Pod{
312+
Status: corev1.PodStatus{
313+
ContainerStatuses: []corev1.ContainerStatus{
314+
{
315+
Name: "debug",
316+
State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}},
317+
},
318+
},
319+
},
320+
},
321+
containerName: "debug",
322+
expectedExitCode: -1,
323+
},
324+
{
325+
name: "container not found",
326+
pod: &corev1.Pod{
327+
Status: corev1.PodStatus{
328+
ContainerStatuses: []corev1.ContainerStatus{
329+
{
330+
Name: "other",
331+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}},
332+
},
333+
},
334+
},
335+
},
336+
containerName: "debug",
337+
expectedExitCode: -1,
338+
},
339+
{
340+
name: "init container terminated with non-zero exit code",
341+
pod: &corev1.Pod{
342+
Status: corev1.PodStatus{
343+
InitContainerStatuses: []corev1.ContainerStatus{
344+
{
345+
Name: "init",
346+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 137}},
347+
},
348+
},
349+
},
350+
},
351+
containerName: "init",
352+
expectedExitCode: 137,
353+
},
354+
{
355+
name: "empty pod status",
356+
pod: &corev1.Pod{},
357+
containerName: "debug",
358+
expectedExitCode: -1,
359+
},
360+
}
361+
362+
for _, tt := range tests {
363+
t.Run(tt.name, func(t *testing.T) {
364+
got := containerExitCode(tt.pod, tt.containerName)
365+
if got != tt.expectedExitCode {
366+
t.Errorf("containerExitCode() = %d, want %d", got, tt.expectedExitCode)
367+
}
368+
})
369+
}
370+
}
371+
372+
func TestExitCodeError(t *testing.T) {
373+
tests := []struct {
374+
name string
375+
pod *corev1.Pod
376+
containerName string
377+
// expectedExitCode 0 is used to expect exitCodeError to return nil.
378+
expectedExitCode int
379+
}{
380+
{
381+
name: "non-zero exit code returns CodeExitError",
382+
pod: &corev1.Pod{
383+
Status: corev1.PodStatus{
384+
ContainerStatuses: []corev1.ContainerStatus{
385+
{
386+
Name: "debug",
387+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}},
388+
},
389+
},
390+
},
391+
},
392+
containerName: "debug",
393+
expectedExitCode: 42,
394+
},
395+
{
396+
name: "exit code 0 returns nil",
397+
pod: &corev1.Pod{
398+
Status: corev1.PodStatus{
399+
ContainerStatuses: []corev1.ContainerStatus{
400+
{
401+
Name: "debug",
402+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}},
403+
},
404+
},
405+
},
406+
},
407+
containerName: "debug",
408+
expectedExitCode: 0,
409+
},
410+
{
411+
name: "container not terminated returns nil",
412+
pod: &corev1.Pod{
413+
Status: corev1.PodStatus{
414+
ContainerStatuses: []corev1.ContainerStatus{
415+
{
416+
Name: "debug",
417+
State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}},
418+
},
419+
},
420+
},
421+
},
422+
containerName: "debug",
423+
expectedExitCode: 0,
424+
},
425+
{
426+
name: "container not found returns nil",
427+
pod: &corev1.Pod{
428+
Status: corev1.PodStatus{
429+
ContainerStatuses: []corev1.ContainerStatus{
430+
{
431+
Name: "other",
432+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}},
433+
},
434+
},
435+
},
436+
},
437+
containerName: "debug",
438+
expectedExitCode: 0,
439+
},
440+
{
441+
name: "exit code 127 returns correct code",
442+
pod: &corev1.Pod{
443+
Status: corev1.PodStatus{
444+
ContainerStatuses: []corev1.ContainerStatus{
445+
{
446+
Name: "debug",
447+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 127}},
448+
},
449+
},
450+
},
451+
},
452+
containerName: "debug",
453+
expectedExitCode: 127,
454+
},
455+
}
456+
457+
for _, tt := range tests {
458+
t.Run(tt.name, func(t *testing.T) {
459+
err := exitCodeError(tt.pod, tt.containerName)
460+
if tt.expectedExitCode == 0 {
461+
if err != nil {
462+
t.Errorf("expected nil, got %v", err)
463+
}
464+
return
465+
}
466+
467+
var exitErr utilexec.CodeExitError
468+
if !errors.As(err, &exitErr) {
469+
t.Fatalf("expected utilexec.CodeExitError, got %T: %v", err, err)
470+
}
471+
if exitErr.ExitStatus() != tt.expectedExitCode {
472+
t.Errorf("ExitStatus() = %d, want %d", exitErr.ExitStatus(), tt.expectedExitCode)
473+
}
474+
})
475+
}
476+
}

0 commit comments

Comments
 (0)