Merge pull request #47013 from smarterclayton/fix_printer

Automatic merge from submit-queue (batch tested with PRs 47024, 47050, 47086, 47081, 47013) Wrap HumanReadablePrinter in tab output unless explicitly asked not to `kubectl get` was not properly aligning its output due to #40848 Fixes an accidental regression. In general, we should not accept an incoming tabwriter and instead manage at a higher level. Fix the bug and add a comment re: future refactoring.
kubernetes · Jun 7, 2017 · 49866b8 · 49866b8
2 parents 7e0c9e7 + b1abedb
commit 49866b8
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 19 deletions.
diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go
@@ -197,7 +197,7 @@ func Example_printReplicationControllerWithNamespace() {
 		},
 	}
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, ctrl, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, ctrl, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -252,7 +252,7 @@ func Example_printMultiContainersReplicationControllerWithWide() {
 		},
 	}
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, ctrl, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, ctrl, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -306,7 +306,7 @@ func Example_printReplicationController() {
 		},
 	}
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, ctrl, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, ctrl, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -349,7 +349,7 @@ func Example_printPodWithWideFormat() {
 		},
 	}
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, pod, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, pod, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -395,7 +395,7 @@ func Example_printPodWithShowLabels() {
 		},
 	}
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, pod, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, pod, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -519,7 +519,7 @@ func Example_printPodHideTerminated() {
 	}
 	for _, pod := range filteredPodList {
 		mapper, _ := f.Object()
-		err := f.PrintObject(cmd, mapper, pod, printers.GetNewTabWriter(os.Stdout))
+		err := f.PrintObject(cmd, mapper, pod, os.Stdout)
 		if err != nil {
 			fmt.Printf("Unexpected error: %v", err)
 		}
@@ -547,7 +547,7 @@ func Example_printPodShowAll() {
 	cmd := NewCmdRun(f, os.Stdin, os.Stdout, os.Stderr)
 	podList := newAllPhasePodList()
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, podList, printers.GetNewTabWriter(os.Stdout))
+	err := f.PrintObject(cmd, mapper, podList, os.Stdout)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}
@@ -621,10 +621,8 @@ func Example_printServiceWithNamespacesAndLabels() {
 	}
 	ld := strings.NewLineDelimiter(os.Stdout, "|")
 	defer ld.Flush()
-	out := printers.GetNewTabWriter(ld)
-	defer out.Flush()
 	mapper, _ := f.Object()
-	err := f.PrintObject(cmd, mapper, svc, out)
+	err := f.PrintObject(cmd, mapper, svc, ld)
 	if err != nil {
 		fmt.Printf("Unexpected error: %v", err)
 	}

diff --git a/pkg/printers/humanreadable.go b/pkg/printers/humanreadable.go
@@ -60,12 +60,12 @@ type handlerEntry struct {
 // will only be printed if the object type changes. This makes it useful for printing items
 // received from watches.
 type HumanReadablePrinter struct {
-	handlerMap   map[reflect.Type]*handlerEntry
-	options      PrintOptions
-	lastType     reflect.Type
-	hiddenObjNum int
-	encoder      runtime.Encoder
-	decoder      runtime.Decoder
+	handlerMap    map[reflect.Type]*handlerEntry
+	options       PrintOptions
+	lastType      reflect.Type
+	skipTabWriter bool
+	encoder       runtime.Encoder
+	decoder       runtime.Decoder
 }
 
 var _ PrintHandler = &HumanReadablePrinter{}
@@ -89,6 +89,13 @@ func NewTablePrinter() *HumanReadablePrinter {
 	}
 }
 
+// AddTabWriter sets whether the PrintObj function will format with tabwriter (true
+// by default).
+func (a *HumanReadablePrinter) AddTabWriter(t bool) *HumanReadablePrinter {
+	a.skipTabWriter = !t
+	return a
+}
+
 func (a *HumanReadablePrinter) With(fns ...func(PrintHandler)) *HumanReadablePrinter {
 	for _, fn := range fns {
 		fn(a)
@@ -267,9 +274,14 @@ func (h *HumanReadablePrinter) printHeader(columnNames []string, w io.Writer) er
 }
 
 // PrintObj prints the obj in a human-friendly format according to the type of the obj.
+// TODO: unify the behavior of PrintObj, which often expects single items and tracks
+// headers and filtering, with other printers, that expect list objects. The tracking
+// behavior should probably be a higher level wrapper (MultiObjectTablePrinter) that
+// calls into the PrintTable method and then displays consistent output.
 func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) error {
-	// if output is a tabwriter (when it's called by kubectl get), we use it; create a new tabwriter otherwise
-	if w, found := output.(*tabwriter.Writer); found {
+	if w, found := output.(*tabwriter.Writer); !found && !h.skipTabWriter {
+		w = GetNewTabWriter(output)
+		output = w
 		defer w.Flush()
 	}
 

diff --git a/pkg/printers/internalversion/printers_test.go b/pkg/printers/internalversion/printers_test.go
@@ -1341,7 +1341,7 @@ func TestPrintPodTable(t *testing.T) {
 			t.Fatal(err)
 		}
 		buf := &bytes.Buffer{}
-		p := printers.NewHumanReadablePrinter(nil, nil, test.opts).With(AddHandlers)
+		p := printers.NewHumanReadablePrinter(nil, nil, test.opts).With(AddHandlers).AddTabWriter(false)
 		if err := p.PrintObj(table, buf); err != nil {
 			t.Fatal(err)
 		}