Bug 1771553: oc debug node return container exit code

Bug 1771549: oc debug pod logs to stderr if container exit code != 0
Bug 1798549: oc debug fail quickly on errimagepull

pkg/cli/debug/debug.go

@@ -444,7 +444,7 @@ func (o *DebugOptions) RunDebug() error {
 
 		ctx, cancel := context.WithTimeout(context.Background(), o.Timeout)
 		defer cancel()
-		switch containerRunningEvent, err := watchtools.UntilWithoutRetry(ctx, w, conditions.PodContainerRunning(o.Attach.ContainerName)); {
+		switch containerRunningEvent, err := watchtools.UntilWithoutRetry(ctx, w, conditions.PodContainerRunning(o.Attach.ContainerName, o.CoreClient)); {
 		// api didn't error right away but the pod wasn't even created
 		case kapierrors.IsNotFound(err):
 			msg := fmt.Sprintf("unable to create the debug pod %q", pod.Name)
@@ -453,20 +453,14 @@ func (o *DebugOptions) RunDebug() error {
 			}
 			return fmt.Errorf(msg)
 			// switch to logging output
-		case err == krun.ErrPodCompleted, err == conditions.ErrContainerTerminated, !o.Attach.Stdin:
-			return logs.LogsOptions{
-				Object: pod,
-				Options: &corev1.PodLogOptions{
-					Container: o.Attach.ContainerName,
-					Follow:    true,
-				},
-				RESTClientGetter: o.RESTClientGetter,
-				ConsumeRequestFn: logs.DefaultConsumeRequest,
-				IOStreams:        o.IOStreams,
-				LogsForObject:    o.LogsForObject,
-			}.RunLogs()
+		case err == krun.ErrPodCompleted, err == conditions.ErrContainerTerminated:
+			o.logsOptions(pod)
+		case err == conditions.ErrNonZeroExitCode:
+			return o.getPodLogsForError(pod)
 		case err != nil:
 			return err
+		case !o.Attach.Stdin:
+			o.logsOptions(pod)
 		default:
 			// TODO this doesn't do us much good for remote debugging sessions, but until we get a local port
 			// set up to proxy, this is what we've got.
@@ -477,6 +471,7 @@ func (o *DebugOptions) RunDebug() error {
 			// TODO: attach can race with pod completion, allow attach to switch to logs
 			return o.Attach.Run()
 		}
+		return nil
 	})
 }
 
@@ -962,6 +957,40 @@ func (o *DebugOptions) approximatePodTemplateForObject(object runtime.Object) (*
 	return nil, fmt.Errorf("unable to extract pod template from type %v", reflect.TypeOf(object))
 }
 
+func (o *DebugOptions) logsOptions(pod *corev1.Pod) error {
+	return logs.LogsOptions{
+		Object: pod,
+		Options: &corev1.PodLogOptions{
+			Container: o.Attach.ContainerName,
+			Follow:    true,
+		},
+		RESTClientGetter: o.RESTClientGetter,
+		ConsumeRequestFn: logs.DefaultConsumeRequest,
+		IOStreams:        o.IOStreams,
+		LogsForObject:    o.LogsForObject,
+	}.RunLogs()
+}
+
+func (o *DebugOptions) getPodLogsForError(pod *corev1.Pod) error {
+	opts := &corev1.PodLogOptions{
+		Container:  o.Attach.ContainerName,
+		Follow:     true,
+		Timestamps: false,
+	}
+	logsResult := o.CoreClient.Pods(pod.Namespace).GetLogs(pod.Name, opts).Do()
+	logs, err := logsResult.Raw()
+	if err != nil {
+		return err
+	}
+	// ex: oc debug node/nodename -- false
+	if len(logs) == 0 {
+		return conditions.ErrNonZeroExitCode
+	// ex: oc debug node/nodename -- ls /foo
+	} else {
+		return fmt.Errorf(string(logs))
+	}
+}
+
 func setNodeName(template *corev1.PodTemplateSpec, nodeName string) *corev1.PodTemplateSpec {
 	template.Spec.NodeName = nodeName
 	return template

pkg/helpers/conditions/conditions.go

@@ -7,18 +7,19 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/watch"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	watchtools "k8s.io/client-go/tools/watch"
-	"k8s.io/klog"
 	krun "k8s.io/kubectl/pkg/cmd/run"
 )
 
 // ErrContainerTerminated is returned by PodContainerRunning in the intermediate
 // state where the pod indicates it's still running, but its container is already terminated
 var ErrContainerTerminated = fmt.Errorf("container terminated")
+var ErrNonZeroExitCode = fmt.Errorf("non-zero exit code from debug container")
 
 // PodContainerRunning returns false until the named container has ContainerStatus running (at least once),
 // and will return an error if the pod is deleted, runs to completion, or the container pod is not available.
-func PodContainerRunning(containerName string) watchtools.ConditionFunc {
+func PodContainerRunning(containerName string, coreClient corev1client.CoreV1Interface) watchtools.ConditionFunc {
 	return func(event watch.Event) (bool, error) {
 		switch event.Type {
 		case watch.Deleted:
@@ -28,13 +29,28 @@ func PodContainerRunning(containerName string) watchtools.ConditionFunc {
 		case *corev1.Pod:
 			switch t.Status.Phase {
 			case corev1.PodRunning, corev1.PodPending:
-				if t.Status.ContainerStatuses[0].State.Waiting != nil {
-					if t.Status.ContainerStatuses[0].State.Waiting.Reason == "CreateContainerError" {
-						klog.V(0).Info(t.Status.ContainerStatuses[0].State.Waiting.Message)
-						return false, fmt.Errorf(t.Status.ContainerStatuses[0].State.Waiting.Message)
+				for _, s := range t.Status.ContainerStatuses {
+					if s.State.Waiting != nil {
+						// Return error here if pod is pending and container status indicates a failure
+						// otherwise, user would have to wait the timeout period (15 min)
+						// for pod to exit.
+						if s.State.Waiting.Reason == "CreateContainerError" || s.State.Waiting.Reason == "ImagePullBackOff" {
+							return false, fmt.Errorf(s.State.Waiting.Message)
+						}
 					}
 				}
 			case corev1.PodFailed, corev1.PodSucceeded:
+				for _, s := range t.Status.ContainerStatuses {
+					if s.State.Terminated != nil {
+						exitCode := s.State.Terminated.ExitCode
+						if exitCode != 0 {
+							// User will get more information about non-zero exit code from pod logs retrieval
+							// in debug.go.  Here we mark the non-zero exit to separate success logs
+							// from failed container logs.
+							return false, ErrNonZeroExitCode
+						}
+					}
+				}
 				return false, krun.ErrPodCompleted
 			default:
 				return false, nil