kubectl: enhance podtemplate describer

pkg/kubectl/cmd/rollout/rollout_history.go

@@ -101,7 +101,6 @@ func RunHistory(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []st
 			continue
 		}
 
-		formattedOutput := ""
 		if revisionDetail > 0 {
 			// Print details of a specific revision
 			template, ok := historyInfo.RevisionToTemplate[revisionDetail]
@@ -109,16 +108,16 @@ func RunHistory(f *cmdutil.Factory, cmd *cobra.Command, out io.Writer, args []st
 				return fmt.Errorf("unable to find revision %d of %s %q", revisionDetail, mapping.Resource, info.Name)
 			}
 			fmt.Fprintf(out, "%s %q revision %d\n", mapping.Resource, info.Name, revisionDetail)
-			formattedOutput, err = kubectl.DescribePodTemplate(template)
+			kubectl.DescribePodTemplate(template, out)
 		} else {
 			// Print all revisions
-			formattedOutput, err = kubectl.PrintRolloutHistory(historyInfo, mapping.Resource, info.Name)
-		}
-		if err != nil {
-			errs = append(errs, err)
-			continue
+			formattedOutput, printErr := kubectl.PrintRolloutHistory(historyInfo, mapping.Resource, info.Name)
+			if printErr != nil {
+				errs = append(errs, printErr)
+				continue
+			}
+			fmt.Fprintf(out, "%s\n", formattedOutput)
 		}
-		fmt.Fprintf(out, "%s\n", formattedOutput)
 	}
 
 	return errors.NewAggregate(errs)

pkg/kubectl/describe.go

@@ -512,8 +512,7 @@ func describePod(pod *api.Pod, events *api.EventList) (string, error) {
 		}
 		fmt.Fprintf(out, "IP:\t%s\n", pod.Status.PodIP)
 		fmt.Fprintf(out, "Controllers:\t%s\n", printControllers(pod.Annotations))
-		fmt.Fprintf(out, "Containers:\n")
-		DescribeContainers(pod.Spec.Containers, pod.Status.ContainerStatuses, EnvValueRetriever(pod), out)
+		describeContainers(pod.Spec.Containers, pod.Status.ContainerStatuses, EnvValueRetriever(pod), out, "")
 		if len(pod.Status.Conditions) > 0 {
 			fmt.Fprint(out, "Conditions:\n  Type\tStatus\n")
 			for _, c := range pod.Status.Conditions {
@@ -522,7 +521,7 @@ func describePod(pod *api.Pod, events *api.EventList) (string, error) {
 					c.Status)
 			}
 		}
-		describeVolumes(pod.Spec.Volumes, out)
+		describeVolumes(pod.Spec.Volumes, out, "")
 		if events != nil {
 			DescribeEvents(events, out)
 		}
@@ -542,14 +541,19 @@ func printControllers(annotation map[string]string) string {
 	return "<none>"
 }
 
-func describeVolumes(volumes []api.Volume, out io.Writer) {
+// TODO: Do a better job at indenting, maybe by using a prefix writer
+func describeVolumes(volumes []api.Volume, out io.Writer, space string) {
 	if volumes == nil || len(volumes) == 0 {
-		fmt.Fprint(out, "No volumes.\n")
+		fmt.Fprintf(out, "%sNo volumes.\n", space)
 		return
 	}
-	fmt.Fprint(out, "Volumes:\n")
+	fmt.Fprintf(out, "%sVolumes:\n", space)
 	for _, volume := range volumes {
-		fmt.Fprintf(out, "  %v:\n", volume.Name)
+		nameIndent := ""
+		if len(space) > 0 {
+			nameIndent = " "
+		}
+		fmt.Fprintf(out, "  %s%v:\n", nameIndent, volume.Name)
 		switch {
 		case volume.VolumeSource.HostPath != nil:
 			printHostPathVolumeSource(volume.VolumeSource.HostPath, out)
@@ -766,17 +770,20 @@ func (d *PersistentVolumeClaimDescriber) Describe(namespace, name string) (strin
 	})
 }
 
-// DescribeContainers is exported for consumers in other API groups that have container templates
-func DescribeContainers(containers []api.Container, containerStatuses []api.ContainerStatus, resolverFn EnvVarResolverFunc, out io.Writer) {
+// TODO: Do a better job at indenting, maybe by using a prefix writer
+func describeContainers(containers []api.Container, containerStatuses []api.ContainerStatus, resolverFn EnvVarResolverFunc, out io.Writer, space string) {
 	statuses := map[string]api.ContainerStatus{}
 	for _, status := range containerStatuses {
 		statuses[status.Name] = status
 	}
-
+	fmt.Fprintf(out, "%sContainers:\n", space)
 	for _, container := range containers {
 		status, ok := statuses[container.Name]
-
-		fmt.Fprintf(out, "  %v:\n", container.Name)
+		nameIndent := ""
+		if len(space) > 0 {
+			nameIndent = " "
+		}
+		fmt.Fprintf(out, "  %s%v:\n", nameIndent, container.Name)
 		if ok {
 			fmt.Fprintf(out, "    Container ID:\t%s\n", status.ContainerID)
 		}
@@ -843,7 +850,11 @@ func DescribeContainers(containers []api.Container, containerStatuses []api.Cont
 			probe := DescribeProbe(container.ReadinessProbe)
 			fmt.Fprintf(out, "    Readiness:\t%s\n", probe)
 		}
-		fmt.Fprintf(out, "    Environment Variables:\n")
+		none := ""
+		if len(container.Env) == 0 {
+			none = "\t<none>"
+		}
+		fmt.Fprintf(out, "    Environment Variables:%s\n", none)
 		for _, e := range container.Env {
 			if e.ValueFrom != nil && e.ValueFrom.FieldRef != nil {
 				var valueFrom string
@@ -983,7 +994,7 @@ func describeReplicationController(controller *api.ReplicationController, events
 		fmt.Fprintf(out, "Replicas:\t%d current / %d desired\n", controller.Status.Replicas, controller.Spec.Replicas)
 		fmt.Fprintf(out, "Pods Status:\t%d Running / %d Waiting / %d Succeeded / %d Failed\n", running, waiting, succeeded, failed)
 		if controller.Spec.Template != nil {
-			describeVolumes(controller.Spec.Template.Spec.Volumes, out)
+			describeVolumes(controller.Spec.Template.Spec.Volumes, out, "")
 		}
 		if events != nil {
 			DescribeEvents(events, out)
@@ -992,18 +1003,20 @@ func describeReplicationController(controller *api.ReplicationController, events
 	})
 }
 
-func DescribePodTemplate(template *api.PodTemplateSpec) (string, error) {
-	return tabbedString(func(out io.Writer) error {
-		if template == nil {
-			fmt.Fprintf(out, "<unset>")
-			return nil
-		}
-		fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(template.Labels))
-		fmt.Fprintf(out, "Annotations:\t%s\n", labels.FormatLabels(template.Annotations))
-		fmt.Fprintf(out, "Image(s):\t%s\n", makeImageList(&template.Spec))
-		describeVolumes(template.Spec.Volumes, out)
-		return nil
-	})
+func DescribePodTemplate(template *api.PodTemplateSpec, out io.Writer) {
+	if template == nil {
+		fmt.Fprintf(out, "  <unset>")
+		return
+	}
+	fmt.Fprintf(out, "  Labels:\t%s\n", labels.FormatLabels(template.Labels))
+	if len(template.Annotations) > 0 {
+		fmt.Fprintf(out, "  Annotations:\t%s\n", labels.FormatLabels(template.Annotations))
+	}
+	if len(template.Spec.ServiceAccountName) > 0 {
+		fmt.Fprintf(out, "  Service Account:\t%s\n", template.Spec.ServiceAccountName)
+	}
+	describeContainers(template.Spec.Containers, nil, nil, out, "  ")
+	describeVolumes(template.Spec.Volumes, out, "  ")
 }
 
 // ReplicaSetDescriber generates information about a ReplicaSet and the pods it has created.
@@ -1044,7 +1057,7 @@ func describeReplicaSet(rs *extensions.ReplicaSet, events *api.EventList, runnin
 		fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(rs.Labels))
 		fmt.Fprintf(out, "Replicas:\t%d current / %d desired\n", rs.Status.Replicas, rs.Spec.Replicas)
 		fmt.Fprintf(out, "Pods Status:\t%d Running / %d Waiting / %d Succeeded / %d Failed\n", running, waiting, succeeded, failed)
-		describeVolumes(rs.Spec.Template.Spec.Volumes, out)
+		describeVolumes(rs.Spec.Template.Spec.Volumes, out, "")
 		if events != nil {
 			DescribeEvents(events, out)
 		}
@@ -1089,7 +1102,7 @@ func describeJob(job *extensions.Job, events *api.EventList) (string, error) {
 		}
 		fmt.Fprintf(out, "Labels:\t%s\n", labels.FormatLabels(job.Labels))
 		fmt.Fprintf(out, "Pods Statuses:\t%d Running / %d Succeeded / %d Failed\n", job.Status.Active, job.Status.Succeeded, job.Status.Failed)
-		describeVolumes(job.Spec.Template.Spec.Volumes, out)
+		describeVolumes(job.Spec.Template.Spec.Volumes, out, "")
 		if events != nil {
 			DescribeEvents(events, out)
 		}

pkg/kubectl/describe_test.go

@@ -268,7 +268,7 @@ func TestDescribeContainers(t *testing.T) {
 				ContainerStatuses: []api.ContainerStatus{testCase.status},
 			},
 		}
-		DescribeContainers(pod.Spec.Containers, pod.Status.ContainerStatuses, EnvValueRetriever(&pod), out)
+		describeContainers(pod.Spec.Containers, pod.Status.ContainerStatuses, EnvValueRetriever(&pod), out, "")
 		output := out.String()
 		for _, expected := range testCase.expectedElements {
 			if !strings.Contains(output, expected) {

pkg/kubectl/history.go

@@ -86,7 +86,9 @@ func (h *DeploymentHistoryViewer) History(namespace, name string) (HistoryInfo,
 		if historyInfo.RevisionToTemplate[v].Annotations == nil {
 			historyInfo.RevisionToTemplate[v].Annotations = make(map[string]string)
 		}
-		historyInfo.RevisionToTemplate[v].Annotations[ChangeCauseAnnotation] = changeCause
+		if len(changeCause) > 0 {
+			historyInfo.RevisionToTemplate[v].Annotations[ChangeCauseAnnotation] = changeCause
+		}
 	}
 	return historyInfo, nil
 }