Merge pull request #9032 from andronat/fix_7843

Decrease columns and refactor get pods layout

pkg/kubectl/resource_printer.go

@@ -35,7 +35,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/volume"
-	"github.com/docker/docker/pkg/units"
 	"github.com/ghodss/yaml"
 	"github.com/golang/glog"
 )
@@ -247,7 +246,7 @@ func (h *HumanReadablePrinter) HandledResources() []string {
 	return keys
 }
 
-var podColumns = []string{"POD", "IP", "CONTAINER(S)", "IMAGE(S)", "HOST", "LABELS", "STATUS", "CREATED", "MESSAGE"}
+var podColumns = []string{"NAME", "READY", "REASON", "RESTARTS", "AGE"}
 var podTemplateColumns = []string{"TEMPLATE", "CONTAINER(S)", "IMAGE(S)", "PODLABELS"}
 var replicationControllerColumns = []string{"CONTROLLER", "CONTAINER(S)", "IMAGE(S)", "SELECTOR", "REPLICAS"}
 var serviceColumns = []string{"NAME", "LABELS", "SELECTOR", "IP(S)", "PORT(S)"}
@@ -350,107 +349,65 @@ func podHostString(host, ip string) string {
 	return host + "/" + ip
 }
 
+func shortHumanDuration(d time.Duration) string {
+	if seconds := int(d.Seconds()); seconds < 60 {
+		return fmt.Sprintf("%ds", seconds)
+	} else if minutes := int(d.Minutes()); minutes < 60 {
+		return fmt.Sprintf("%dm", minutes)
+	} else if hours := int(d.Hours()); hours < 24 {
+		return fmt.Sprintf("%dh", hours)
+	} else if hours < 24*364 {
+		return fmt.Sprintf("%dd", hours/24)
+	}
+	return fmt.Sprintf("%dy", int(d.Hours()/24/365))
+}
+
 // translateTimestamp returns the elapsed time since timestamp in
 // human-readable approximation.
 func translateTimestamp(timestamp util.Time) string {
-	return units.HumanDuration(time.Now().Sub(timestamp.Time))
+	return shortHumanDuration(time.Now().Sub(timestamp.Time))
-}
-
-// interpretContainerStatus interprets the container status and returns strings
-// associated with columns "STATUS", "CREATED", and "MESSAGE".
-// The meaning of MESSAGE varies based on the context of STATUS:
-//     STATUS: Waiting; MESSAGE: reason for waiting
-//     STATUS: Running; MESSAGE: reason for the last termination
-//     STATUS: Terminated; MESSAGE: reason for this termination
-func interpretContainerStatus(status *api.ContainerStatus) (string, string, string, error) {
-	// Helper function to compose a meaning message from terminate state.
-	getTermMsg := func(state *api.ContainerStateTerminated) string {
-		var message string
-		if state != nil {
-			message = fmt.Sprintf("exit code %d", state.ExitCode)
-			if state.Reason != "" {
-				message = fmt.Sprintf("%s, reason: %s", message, state.Reason)
-			}
-		}
-		return message
-	}
-
-	state := &status.State
-	if state.Waiting != nil {
-		return "Waiting", "", state.Waiting.Reason, nil
-	} else if state.Running != nil {
-		// Get the information of the last termination state. This is useful if
-		// a container is stuck in a crash loop.
-		message := getTermMsg(status.LastTerminationState.Terminated)
-		if message != "" {
-			message = "last termination: " + message
-		}
-		stateMsg := "Running"
-		if !status.Ready {
-			stateMsg = stateMsg + " *not ready*"
-		}
-		return stateMsg, translateTimestamp(state.Running.StartedAt), message, nil
-	} else if state.Terminated != nil {
-		return "Terminated", translateTimestamp(state.Terminated.StartedAt), getTermMsg(state.Terminated), nil
-	}
-	return "", "", "", fmt.Errorf("unknown container state %#v", *state)
 }
 
 func printPod(pod *api.Pod, w io.Writer, withNamespace bool) error {
-	var name string
+	name := pod.Name
 	if withNamespace {
-		name = types.NamespacedName{pod.Namespace, pod.Name}.String()
+		name = types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}.String()
-	} else {
-		name = pod.Name
 	}
 
-	_, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
+	restarts := 0
+	totalContainers := len(pod.Spec.Containers)
+	readyContainers := 0
+	reason := string(pod.Status.Phase)
+
+	for i := len(pod.Status.ContainerStatuses) - 1; i >= 0; i-- {
+		container := pod.Status.ContainerStatuses[i]
+
+		restarts += container.RestartCount
+		if container.State.Waiting != nil && container.State.Waiting.Reason != "" {
+			reason = container.State.Waiting.Reason
+		} else if container.State.Terminated != nil && container.State.Terminated.Reason != "" {
+			reason = container.State.Terminated.Reason
+		} else if container.State.Terminated != nil && container.State.Terminated.Reason == "" {
+			if container.State.Terminated.Signal != 0 {
+				reason = fmt.Sprintf("Signal:%d", container.State.Terminated.Signal)
+			} else {
+				reason = fmt.Sprintf("ExitCode:%d", container.State.Terminated.ExitCode)
+			}
+		} else if container.Ready && container.State.Running != nil {
+			readyContainers++
+		}
+	}
+
+	_, err := fmt.Fprintf(w, "%s\t%d/%d\t%s\t%d\t%s\n",
 		name,
-		pod.Status.PodIP,
+		readyContainers,
-		"", "",
+		totalContainers,
-		podHostString(pod.Spec.NodeName, pod.Status.HostIP),
+		reason,
-		formatLabels(pod.Labels),
+		restarts,
-		pod.Status.Phase,
+		translateTimestamp(pod.CreationTimestamp))
-		translateTimestamp(pod.CreationTimestamp),
-		pod.Status.Message,
-	)
 	if err != nil {
 		return err
 	}
-	// Lay out all containers on separate lines.
-	statuses := pod.Status.ContainerStatuses
-	if len(statuses) == 0 {
-		// Container status has not been reported yet. Print basic information
-		// of the containers and exit the function.
-		for _, container := range pod.Spec.Containers {
-			_, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
-				"", "", container.Name, container.Image, "", "", "", "", "")
-			if err != nil {
-				return err
-			}
-		}
-		return nil
-	}
-
-	// Print the actual container statuses.
-	for _, status := range statuses {
-		state, created, message, err := interpretContainerStatus(&status)
-		if err != nil {
-			return err
-		}
-		_, err = fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
-			"", "",
-			status.Name,
-			status.Image,
-			"", "",
-			state,
-			created,
-			message,
-		)
-		if err != nil {
-			return err
-		}
-	}
 	return nil
 }
 

pkg/kubectl/resource_printer_test.go

@@ -770,107 +770,6 @@ func TestPrintHumanReadableService(t *testing.T) {
 	}
 }
 
-func TestInterpretContainerStatus(t *testing.T) {
-	tests := []struct {
-		status          *api.ContainerStatus
-		expectedState   string
-		expectedMessage string
-		expectErr       bool
-		name            string
-	}{
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Running: &api.ContainerStateRunning{},
-				},
-				Ready: true,
-			},
-			expectedState:   "Running",
-			expectedMessage: "",
-			name:            "basic",
-		},
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Running: &api.ContainerStateRunning{},
-				},
-				Ready: false,
-			},
-			expectedState:   "Running *not ready*",
-			expectedMessage: "",
-			name:            "basic not ready",
-		},
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Waiting: &api.ContainerStateWaiting{},
-				},
-				Ready: false,
-			},
-			expectedState:   "Waiting",
-			expectedMessage: "",
-			name:            "waiting not ready",
-		},
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Waiting: &api.ContainerStateWaiting{},
-				},
-				Ready: true,
-			},
-			expectedState:   "Waiting",
-			expectedMessage: "",
-			name:            "waiting",
-		},
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Terminated: &api.ContainerStateTerminated{
-						ExitCode: 3,
-					},
-				},
-				Ready: false,
-			},
-			expectedState:   "Terminated",
-			expectedMessage: "exit code 3",
-			name:            "terminated not ready",
-		},
-		{
-			status: &api.ContainerStatus{
-				State: api.ContainerState{
-					Terminated: &api.ContainerStateTerminated{
-						ExitCode: 5,
-						Reason:   "test reason",
-					},
-				},
-				Ready: true,
-			},
-			expectedState:   "Terminated",
-			expectedMessage: "exit code 5, reason: test reason",
-			name:            "terminated",
-		},
-	}
-
-	for _, test := range tests {
-		// TODO: test timestamp printing.
-		state, _, msg, err := interpretContainerStatus(test.status)
-		if test.expectErr && err == nil {
-			t.Errorf("unexpected non-error (%s)", test.name)
-			continue
-		}
-		if !test.expectErr && err != nil {
-			t.Errorf("unexpected error: %v (%s)", err, test.name)
-			continue
-		}
-		if state != test.expectedState {
-			t.Errorf("expected: %s, got: %s", test.expectedState, state)
-		}
-		if msg != test.expectedMessage {
-			t.Errorf("expected: %s, got: %s", test.expectedMessage, msg)
-		}
-	}
-}
-
 func TestPrintHumanReadableWithNamespace(t *testing.T) {
 	namespaceName := "testnamespace"
 	name := "test"
@@ -1050,3 +949,81 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) {
 		}
 	}
 }
+
+func TestPrintPod(t *testing.T) {
+	tests := []struct {
+		pod    api.Pod
+		expect string
+	}{
+		{
+			// Test name, num of containers, restarts, container ready status
+			api.Pod{
+				ObjectMeta: api.ObjectMeta{Name: "test1"},
+				Spec:       api.PodSpec{Containers: make([]api.Container, 2)},
+				Status: api.PodStatus{
+					Phase: "podPhase",
+					ContainerStatuses: []api.ContainerStatus{
+						{Ready: true, RestartCount: 3, State: api.ContainerState{Running: &api.ContainerStateRunning{}}},
+						{RestartCount: 3},
+					},
+				},
+			},
+			"test1\t1/2\tpodPhase\t6\t",
+		},
+		{
+			// Test container error overwrites pod phase
+			api.Pod{
+				ObjectMeta: api.ObjectMeta{Name: "test2"},
+				Spec:       api.PodSpec{Containers: make([]api.Container, 2)},
+				Status: api.PodStatus{
+					Phase: "podPhase",
+					ContainerStatuses: []api.ContainerStatus{
+						{Ready: true, RestartCount: 3, State: api.ContainerState{Running: &api.ContainerStateRunning{}}},
+						{State: api.ContainerState{Waiting: &api.ContainerStateWaiting{Reason: "containerWaitingReason"}}, RestartCount: 3},
+					},
+				},
+			},
+			"test2\t1/2\tcontainerWaitingReason\t6\t",
+		},
+		{
+			// Test the same as the above but with Terminated state and the first container overwrites the rest
+			api.Pod{
+				ObjectMeta: api.ObjectMeta{Name: "test3"},
+				Spec:       api.PodSpec{Containers: make([]api.Container, 2)},
+				Status: api.PodStatus{
+					Phase: "podPhase",
+					ContainerStatuses: []api.ContainerStatus{
+						{State: api.ContainerState{Waiting: &api.ContainerStateWaiting{Reason: "containerWaitingReason"}}, RestartCount: 3},
+						{State: api.ContainerState{Terminated: &api.ContainerStateTerminated{Reason: "containerTerminatedReason"}}, RestartCount: 3},
+					},
+				},
+			},
+			"test3\t0/2\tcontainerWaitingReason\t6\t",
+		},
+		{
+			// Test ready is not enough for reporting running
+			api.Pod{
+				ObjectMeta: api.ObjectMeta{Name: "test4"},
+				Spec:       api.PodSpec{Containers: make([]api.Container, 2)},
+				Status: api.PodStatus{
+					Phase: "podPhase",
+					ContainerStatuses: []api.ContainerStatus{
+						{Ready: true, RestartCount: 3, State: api.ContainerState{Running: &api.ContainerStateRunning{}}},
+						{Ready: true, RestartCount: 3},
+					},
+				},
+			},
+			"test4\t1/2\tpodPhase\t6\t",
+		},
+	}
+
+	buf := bytes.NewBuffer([]byte{})
+	for _, test := range tests {
+		printPod(&test.pod, buf, false)
+		// We ignore time
+		if !strings.HasPrefix(buf.String(), test.expect) {
+			t.Fatalf("Expected: %s, got: %s", test.expect, buf.String())
+		}
+		buf.Reset()
+	}
+}