Skip to content

Commit a00a8f4

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 a00a8f4

3 files changed

Lines changed: 348 additions & 13 deletions

File tree

‎pkg/cli/debug/debug.go‎

Lines changed: 83 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import (
4343
"k8s.io/kubectl/pkg/util/interrupt"
4444
"k8s.io/kubectl/pkg/util/templates"
4545
"k8s.io/pod-security-admission/api"
46+
utilexec "k8s.io/utils/exec"
4647

4748
appsv1 "github.com/openshift/api/apps/v1"
4849
dockerv10 "github.com/openshift/api/image/docker10"
@@ -630,12 +631,49 @@ func (o *DebugOptions) RunDebug() error {
630631
}
631632
}
632633
}
633-
return o.getLogs(pod)
634+
if logErr := o.getLogs(pod); logErr != nil {
635+
return logErr
636+
}
637+
// The watch event may have stale container status for fast-failing
638+
// containers. Re-fetch the pod to get the authoritative final state.
639+
freshPod, getErr := o.CoreClient.Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
640+
if getErr != nil {
641+
klog.V(4).Infof("Unable to re-fetch pod %s/%s: %v", pod.Namespace, pod.Name, getErr)
642+
if ok {
643+
freshPod = resultPod
644+
}
645+
}
646+
if freshPod == nil {
647+
klog.V(4).Infof("Exit code information unavailable for pod %s/%s", pod.Namespace, pod.Name)
648+
return nil
649+
}
650+
if exitErr := exitCodeError(freshPod, o.ContainerName); exitErr != nil {
651+
return exitErr
652+
}
653+
return nil
634654
case err == conditions.ErrNonZeroExitCode:
635655
if err = o.getLogs(pod); err != nil {
636656
return err
637657
}
638-
return conditions.ErrNonZeroExitCode
658+
// The watch event may have stale container status.
659+
// Re-fetch the pod to get the authoritative final state.
660+
freshPod, getErr := o.CoreClient.Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
661+
if getErr != nil {
662+
klog.V(4).Infof("Unable to re-fetch pod %s/%s: %v", pod.Namespace, pod.Name, getErr)
663+
if resultPod, ok := containerRunningEvent.Object.(*corev1.Pod); ok {
664+
freshPod = resultPod
665+
}
666+
}
667+
if freshPod != nil {
668+
if exitErr := exitCodeError(freshPod, o.ContainerName); exitErr != nil {
669+
return exitErr
670+
}
671+
}
672+
// We know the exit code was non-zero but couldn't determine the actual value.
673+
return utilexec.CodeExitError{
674+
Err: fmt.Errorf("the debug container terminated with a non-zero exit code"),
675+
Code: 1,
676+
}
639677
case err != nil:
640678
return err
641679
case !o.Attach.Stdin:
@@ -645,20 +683,14 @@ func (o *DebugOptions) RunDebug() error {
645683
lastWatchEvent, err := watchtools.UntilWithSync(ctx, lw, &corev1.Pod{}, preconditionFunc, conditions.PodDone)
646684
if err != nil {
647685
if kapierrors.IsNotFound(err) {
648-
return nil
686+
return fmt.Errorf("the debug pod %q was deleted before completion", pod.Name)
649687
}
650688
return err
651689
}
652690

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-
}
691+
if resultPod, ok := lastWatchEvent.Object.(*corev1.Pod); ok {
692+
if exitErr := exitCodeError(resultPod, o.ContainerName); exitErr != nil {
693+
return exitErr
662694
}
663695
}
664696
return nil
@@ -673,7 +705,16 @@ func (o *DebugOptions) RunDebug() error {
673705

674706
// TODO: attach can race with pod completion, allow attach to switch to logs
675707
o.Attach.ContainerName = o.ContainerName
676-
return o.Attach.Run()
708+
if attachErr := o.Attach.Run(); attachErr != nil {
709+
return attachErr
710+
}
711+
// After the attach session ends, check the container exit code.
712+
freshPod, getErr := o.CoreClient.Pods(pod.Namespace).Get(context.TODO(), pod.Name, metav1.GetOptions{})
713+
if getErr != nil {
714+
klog.V(4).Infof("Unable to re-fetch pod %s/%s after attach: %v", pod.Namespace, pod.Name, getErr)
715+
return nil
716+
}
717+
return exitCodeError(freshPod, o.ContainerName)
677718
}
678719
})
679720
}
@@ -1238,6 +1279,35 @@ func (o *DebugOptions) approximatePodTemplateForObject(object runtime.Object) (*
12381279
return nil, fmt.Errorf("%v is not supported by debug", reflect.TypeOf(object))
12391280
}
12401281

1282+
// containerExitCode returns the exit code of the named container from the pod status.
1283+
// It returns -1 if the container is not found or has not terminated.
1284+
func containerExitCode(pod *corev1.Pod, containerName string) int32 {
1285+
for _, s := range append(append([]corev1.ContainerStatus{}, pod.Status.InitContainerStatuses...), pod.Status.ContainerStatuses...) {
1286+
if s.Name != containerName {
1287+
continue
1288+
}
1289+
if s.State.Terminated != nil {
1290+
return s.State.Terminated.ExitCode
1291+
}
1292+
return -1
1293+
}
1294+
return -1
1295+
}
1296+
1297+
// exitCodeError returns a CodeExitError with the container's actual exit code if it
1298+
// terminated with a non-zero exit code. It returns nil if the container exited
1299+
// successfully or if exit code information is not available.
1300+
func exitCodeError(pod *corev1.Pod, containerName string) error {
1301+
code := containerExitCode(pod, containerName)
1302+
if code <= 0 {
1303+
return nil
1304+
}
1305+
return utilexec.CodeExitError{
1306+
Err: fmt.Errorf("the debug container terminated with exit code %d", code),
1307+
Code: int(code),
1308+
}
1309+
}
1310+
12411311
func (o *DebugOptions) getLogs(pod *corev1.Pod) error {
12421312
return logs.LogsOptions{
12431313
Object: pod,

‎pkg/cli/debug/debug_test.go‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"k8s.io/kubectl/pkg/cmd/attach"
1111
"k8s.io/kubectl/pkg/cmd/exec"
1212
"k8s.io/pod-security-admission/api"
13+
utilexec "k8s.io/utils/exec"
1314

1415
fakekubeclient "k8s.io/client-go/kubernetes/fake"
1516
fakecorev1client "k8s.io/client-go/kubernetes/typed/core/v1/fake"
@@ -266,3 +267,213 @@ func TestCreateLabelMap(t *testing.T) {
266267
})
267268
}
268269
}
270+
271+
func TestContainerExitCode(t *testing.T) {
272+
tests := []struct {
273+
name string
274+
pod *corev1.Pod
275+
containerName string
276+
expected int32
277+
}{
278+
{
279+
name: "terminated with non-zero exit code",
280+
pod: &corev1.Pod{
281+
Status: corev1.PodStatus{
282+
ContainerStatuses: []corev1.ContainerStatus{
283+
{
284+
Name: "debug",
285+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}},
286+
},
287+
},
288+
},
289+
},
290+
containerName: "debug",
291+
expected: 42,
292+
},
293+
{
294+
name: "terminated with exit code 0",
295+
pod: &corev1.Pod{
296+
Status: corev1.PodStatus{
297+
ContainerStatuses: []corev1.ContainerStatus{
298+
{
299+
Name: "debug",
300+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 0}},
301+
},
302+
},
303+
},
304+
},
305+
containerName: "debug",
306+
expected: 0,
307+
},
308+
{
309+
name: "container still running",
310+
pod: &corev1.Pod{
311+
Status: corev1.PodStatus{
312+
ContainerStatuses: []corev1.ContainerStatus{
313+
{
314+
Name: "debug",
315+
State: corev1.ContainerState{Running: &corev1.ContainerStateRunning{}},
316+
},
317+
},
318+
},
319+
},
320+
containerName: "debug",
321+
expected: -1,
322+
},
323+
{
324+
name: "container not found",
325+
pod: &corev1.Pod{
326+
Status: corev1.PodStatus{
327+
ContainerStatuses: []corev1.ContainerStatus{
328+
{
329+
Name: "other",
330+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 1}},
331+
},
332+
},
333+
},
334+
},
335+
containerName: "debug",
336+
expected: -1,
337+
},
338+
{
339+
name: "init container terminated with non-zero exit code",
340+
pod: &corev1.Pod{
341+
Status: corev1.PodStatus{
342+
InitContainerStatuses: []corev1.ContainerStatus{
343+
{
344+
Name: "init",
345+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 137}},
346+
},
347+
},
348+
},
349+
},
350+
containerName: "init",
351+
expected: 137,
352+
},
353+
{
354+
name: "empty pod status",
355+
pod: &corev1.Pod{},
356+
containerName: "debug",
357+
expected: -1,
358+
},
359+
}
360+
361+
for _, tt := range tests {
362+
t.Run(tt.name, func(t *testing.T) {
363+
got := containerExitCode(tt.pod, tt.containerName)
364+
if got != tt.expected {
365+
t.Errorf("containerExitCode() = %d, want %d", got, tt.expected)
366+
}
367+
})
368+
}
369+
}
370+
371+
func TestExitCodeError(t *testing.T) {
372+
tests := []struct {
373+
name string
374+
pod *corev1.Pod
375+
containerName string
376+
expectError bool
377+
expectedExitCode int
378+
}{
379+
{
380+
name: "non-zero exit code returns CodeExitError",
381+
pod: &corev1.Pod{
382+
Status: corev1.PodStatus{
383+
ContainerStatuses: []corev1.ContainerStatus{
384+
{
385+
Name: "debug",
386+
State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 42}},
387+
},
388+
},
389+
},
390+
},
391+
containerName: "debug",
392+
expectError: true,
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+
expectError: false,
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+
expectError: false,
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+
expectError: false,
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+
expectError: true,
454+
expectedExitCode: 127,
455+
},
456+
}
457+
458+
for _, tt := range tests {
459+
t.Run(tt.name, func(t *testing.T) {
460+
err := exitCodeError(tt.pod, tt.containerName)
461+
if tt.expectError {
462+
if err == nil {
463+
t.Fatalf("expected error, got nil")
464+
}
465+
exitErr, ok := err.(utilexec.ExitError)
466+
if !ok {
467+
t.Fatalf("expected utilexec.ExitError, got %T", err)
468+
}
469+
if exitErr.ExitStatus() != tt.expectedExitCode {
470+
t.Errorf("ExitStatus() = %d, want %d", exitErr.ExitStatus(), tt.expectedExitCode)
471+
}
472+
} else {
473+
if err != nil {
474+
t.Errorf("expected nil, got %v", err)
475+
}
476+
}
477+
})
478+
}
479+
}

0 commit comments

Comments
 (0)