Fix up alignment of columns w/ namespaces.

Fixes #10842

All issues for types that use "extra lines" for extended information.  Two issues fixed: (1) When namespaces are listed an extra column isn't inserted for extra lines and (2) trailing tabs aren't inserted when label columns are specified.

This code should probably move to a more explicit model of putting data into "cells".

The test for this hits https://github.com/golang/go/issues/6416 and so I introduced a "LineDelimiter" writer filter to make white space more visible.

pkg/kubectl/cmd/cmd_test.go

@@ -319,6 +319,72 @@ func ExamplePrintPodWithWideFormat() {
 	// test1     1/2       podPhase   6          10y       kubernetes-minion-abcd
 }
 
+func ExamplePrintServiceWithNamespacesAndLabels() {
+	f, tf, codec := NewAPIFactory()
+	tf.Printer = kubectl.NewHumanReadablePrinter(false, true, false, []string{"l1"})
+	tf.Client = &client.FakeRESTClient{
+		Codec:  codec,
+		Client: nil,
+	}
+	cmd := NewCmdRun(f, os.Stdout)
+	svc := &api.ServiceList{
+		Items: []api.Service{
+			{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "svc1",
+					Namespace: "ns1",
+					Labels: map[string]string{
+						"l1": "value",
+					},
+				},
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{Protocol: "UDP", Port: 53},
+						{Protocol: "TCP", Port: 53},
+					},
+					Selector: map[string]string{
+						"s": "magic",
+					},
+					ClusterIP: "10.1.1.1",
+				},
+				Status: api.ServiceStatus{},
+			},
+			{
+				ObjectMeta: api.ObjectMeta{
+					Name:      "svc2",
+					Namespace: "ns2",
+					Labels: map[string]string{
+						"l1": "dolla-bill-yall",
+					},
+				},
+				Spec: api.ServiceSpec{
+					Ports: []api.ServicePort{
+						{Protocol: "TCP", Port: 80},
+						{Protocol: "TCP", Port: 8080},
+					},
+					Selector: map[string]string{
+						"s": "kazam",
+					},
+					ClusterIP: "10.1.1.2",
+				},
+				Status: api.ServiceStatus{},
+			}},
+	}
+	ld := util.NewLineDelimiter(os.Stdout, "|")
+	defer ld.Flush()
+	err := f.PrintObject(cmd, svc, ld)
+	if err != nil {
+		fmt.Printf("Unexpected error: %v", err)
+	}
+	// Output:
+	// |NAMESPACE   NAME      LABELS               SELECTOR   IP(S)      PORT(S)    L1|
+	// |ns1         svc1      l1=value             s=magic    10.1.1.1   53/UDP     value|
+	// |                                                                 53/TCP     |
+	// |ns2         svc2      l1=dolla-bill-yall   s=kazam    10.1.1.2   80/TCP     dolla-bill-yall|
+	// |                                                                 8080/TCP   |
+	// ||
+}
+
 func TestNormalizationFuncGlobalExistance(t *testing.T) {
 	// This test can be safely deleted when we will not support multiple flag formats
 	root := NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr)

pkg/kubectl/resource_printer.go

@@ -475,11 +475,18 @@ func printPodTemplate(pod *api.PodTemplate, w io.Writer, withNamespace bool, wid
 	}
 
 	// Lay out all the other containers on separate lines.
+	extraLinePrefix := "\t"
+	if withNamespace {
+		extraLinePrefix = "\t\t"
+	}
 	for _, container := range containers {
-		_, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", "", container.Name, container.Image, "")
+		_, err := fmt.Fprintf(w, "%s%s\t%s\t%s", extraLinePrefix, container.Name, container.Image, "")
 		if err != nil {
 			return err
 		}
+		if _, err := fmt.Fprint(w, appendLabelTabs(columnLabels)); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -522,11 +529,18 @@ func printReplicationController(controller *api.ReplicationController, w io.Writ
 	}
 
 	// Lay out all the other containers on separate lines.
+	extraLinePrefix := "\t"
+	if withNamespace {
+		extraLinePrefix = "\t\t"
+	}
 	for _, container := range containers {
-		_, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", "", container.Name, container.Image, "", "")
+		_, err := fmt.Fprintf(w, "%s%s\t%s\t%s\t%s", extraLinePrefix, container.Name, container.Image, "", "")
 		if err != nil {
 			return err
 		}
+		if _, err := fmt.Fprint(w, appendLabelTabs(columnLabels)); err != nil {
+			return err
+		}
 	}
 	return nil
 }
@@ -566,6 +580,10 @@ func printService(svc *api.Service, w io.Writer, withNamespace bool, wide bool,
 		return err
 	}
 
+	extraLinePrefix := "\t\t\t"
+	if withNamespace {
+		extraLinePrefix = "\t\t\t\t"
+	}
 	count := len(svc.Spec.Ports)
 	if len(ips) > count {
 		count = len(ips)
@@ -580,7 +598,10 @@ func printService(svc *api.Service, w io.Writer, withNamespace bool, wide bool,
 			port = fmt.Sprintf("%d/%s", svc.Spec.Ports[i].Port, svc.Spec.Ports[i].Protocol)
 		}
 		// Lay out additional ports.
-		if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", "", "", "", ip, port); err != nil {
+		if _, err := fmt.Fprintf(w, "%s%s\t%s", extraLinePrefix, ip, port); err != nil {
+			return err
+		}
+		if _, err := fmt.Fprint(w, appendLabelTabs(columnLabels)); err != nil {
 			return err
 		}
 	}
@@ -626,7 +647,7 @@ func printNamespace(item *api.Namespace, w io.Writer, withNamespace bool, wide b
 	if withNamespace {
 		return fmt.Errorf("namespace is not namespaced")
 	}
-	if _, err := fmt.Fprintf(w, "%s\t%s\t%s\n", item.Name, formatLabels(item.Labels), item.Status.Phase); err != nil {
+	if _, err := fmt.Fprintf(w, "%s\t%s\t%s", item.Name, formatLabels(item.Labels), item.Status.Phase); err != nil {
 		return err
 	}
 	_, err := fmt.Fprint(w, appendLabels(item.Labels, columnLabels))
@@ -945,6 +966,19 @@ func appendLabels(itemLabels map[string]string, columnLabels []string) string {
 	return buffer.String()
 }
 
+// Append a set of tabs for each label column.  We need this in the case where
+// we have extra lines so that the tabwriter will still line things up.
+func appendLabelTabs(columnLabels []string) string {
+	var buffer bytes.Buffer
+
+	for range columnLabels {
+		buffer.WriteString("\t")
+	}
+	buffer.WriteString("\n")
+
+	return buffer.String()
+}
+
 func formatLabelHeaders(columnLabels []string) []string {
 	formHead := make([]string, len(columnLabels))
 	for i, l := range columnLabels {

pkg/util/line_delimiter.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"bytes"
+	"io"
+	"strings"
+)
+
+// A Line Delimiter is a filter that will
+type LineDelimiter struct {
+	output    io.Writer
+	delimiter []byte
+	buf       bytes.Buffer
+}
+
+// NewLineDelimiter allocates a new io.Writer that will split input on lines
+// and bracket each line with the delimiter string.  This can be useful in
+// output tests where it is difficult to see and test trailing whitespace.
+func NewLineDelimiter(output io.Writer, delimiter string) *LineDelimiter {
+	return &LineDelimiter{output: output, delimiter: []byte(delimiter)}
+}
+
+// Write writes buf to the LineDelimiter ld. The only errors returned are ones
+// encountered while writing to the underlying output stream.
+func (ld *LineDelimiter) Write(buf []byte) (n int, err error) {
+	return ld.buf.Write(buf)
+}
+
+// Flush all lines up until now.  This will assume insert a linebreak at the current point of the stream.
+func (ld *LineDelimiter) Flush() (err error) {
+	lines := strings.Split(ld.buf.String(), "\n")
+	for _, line := range lines {
+		if _, err = ld.output.Write(ld.delimiter); err != nil {
+			return
+		}
+		if _, err = ld.output.Write([]byte(line)); err != nil {
+			return
+		}
+		if _, err = ld.output.Write(ld.delimiter); err != nil {
+			return
+		}
+		if _, err = ld.output.Write([]byte("\n")); err != nil {
+			return
+		}
+	}
+	return
+}

pkg/util/line_delimiter_test.go

@@ -0,0 +1,40 @@
+/*
+Copyright 2015 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"fmt"
+	"os"
+)
+
+func ExampleTrailingNewline() {
+	ld := NewLineDelimiter(os.Stdout, "|")
+	defer ld.Flush()
+	fmt.Fprint(ld, "  Hello  \n  World  \n")
+	// Output:
+	// |  Hello  |
+	// |  World  |
+	// ||
+}
+func ExampleNoTrailingNewline() {
+	ld := NewLineDelimiter(os.Stdout, "|")
+	defer ld.Flush()
+	fmt.Fprint(ld, "  Hello  \n  World  ")
+	// Output:
+	// |  Hello  |
+	// |  World  |
+}