tableprinter: simplifies default printer handler

pkg/printers/BUILD

@@ -22,6 +22,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/duration:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//vendor/github.com/liggitt/tabwriter:go_default_library",
     ],

pkg/printers/internalversion/BUILD

@@ -95,7 +95,6 @@ go_library(
         "//staging/src/k8s.io/api/rbac/v1beta1:go_default_library",
         "//staging/src/k8s.io/api/scheduling/v1:go_default_library",
         "//staging/src/k8s.io/api/storage/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",

pkg/printers/internalversion/printers.go

@@ -37,7 +37,6 @@ import (
 	rbacv1beta1 "k8s.io/api/rbac/v1beta1"
 	schedulingv1 "k8s.io/api/scheduling/v1"
 	storagev1 "k8s.io/api/storage/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -468,48 +467,6 @@ func AddHandlers(h printers.PrintHandler) {
 	}
 	h.TableHandler(volumeAttachmentColumnDefinitions, printVolumeAttachment)
 	h.TableHandler(volumeAttachmentColumnDefinitions, printVolumeAttachmentList)
-
-	AddDefaultHandlers(h)
-}
-
-// AddDefaultHandlers adds handlers that can work with most Kubernetes objects.
-func AddDefaultHandlers(h printers.PrintHandler) {
-	// types without defined columns
-	objectMetaColumnDefinitions := []metav1beta1.TableColumnDefinition{
-		{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
-		{Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]},
-	}
-	h.DefaultTableHandler(objectMetaColumnDefinitions, printObjectMeta)
-}
-
-func printObjectMeta(obj runtime.Object, options printers.PrintOptions) ([]metav1beta1.TableRow, error) {
-	if meta.IsListType(obj) {
-		rows := make([]metav1beta1.TableRow, 0, 16)
-		err := meta.EachListItem(obj, func(obj runtime.Object) error {
-			nestedRows, err := printObjectMeta(obj, options)
-			if err != nil {
-				return err
-			}
-			rows = append(rows, nestedRows...)
-			return nil
-		})
-		if err != nil {
-			return nil, err
-		}
-		return rows, nil
-	}
-
-	rows := make([]metav1beta1.TableRow, 0, 1)
-	m, err := meta.Accessor(obj)
-	if err != nil {
-		return nil, err
-	}
-	row := metav1beta1.TableRow{
-		Object: runtime.RawExtension{Object: obj},
-	}
-	row.Cells = append(row.Cells, m.GetName(), translateTimestampSince(m.GetCreationTimestamp()))
-	rows = append(rows, row)
-	return rows, nil
 }
 
 // Pass ports=nil for all ports.

pkg/printers/internalversion/printers_test.go

@@ -171,7 +171,7 @@ func TestPrintUnstructuredObject(t *testing.T) {
 
 	for _, test := range tests {
 		out.Reset()
-		printer := printers.NewTablePrinter(test.options).With(AddDefaultHandlers)
+		printer := printers.NewTablePrinter(test.options)
 		printer.PrintObj(test.object, out)
 
 		matches, err := regexp.MatchString(test.expected, out.String())
@@ -1339,7 +1339,8 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) {
 					Addresses: []api.EndpointAddress{{IP: "127.0.0.1"}, {IP: "localhost"}},
 					Ports:     []api.EndpointPort{{Port: 8080}},
 				},
-				}},
+				},
+			},
 			isNamespaced: true,
 		},
 		{
@@ -1370,7 +1371,7 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) {
 		},
 		{
 			obj: &api.PersistentVolume{
-				ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespaceName},
+				ObjectMeta: metav1.ObjectMeta{Name: name},
 				Spec:       api.PersistentVolumeSpec{},
 			},
 			isNamespaced: false,
@@ -1414,33 +1415,46 @@ func TestPrintHumanReadableWithNamespace(t *testing.T) {
 			},
 			isNamespaced: false,
 		},
+		{
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"kind":       "Foo",
+					"apiVersion": "example.com/v1",
+					"metadata":   map[string]interface{}{"name": "test", "namespace": namespaceName},
+				},
+			},
+			isNamespaced: true,
+		},
+		{
+			obj: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"kind":       "Foo",
+					"apiVersion": "example.com/v1",
+					"metadata":   map[string]interface{}{"name": "test"},
+				},
+			},
+			isNamespaced: false,
+		},
 	}
 
 	for i, test := range table {
+		printer := printers.NewTablePrinter(printers.PrintOptions{
+			WithNamespace: true,
+			NoHeaders:     true,
+		})
+		AddHandlers(printer)
+		buffer := &bytes.Buffer{}
+		err := printer.PrintObj(test.obj, buffer)
+		if err != nil {
+			t.Fatalf("An error occurred printing object: %#v", err)
+		}
 		if test.isNamespaced {
-			// Expect output to include namespace when requested.
+			if !strings.HasPrefix(buffer.String(), namespaceName+" ") {
-			printer := printers.NewTablePrinter(printers.PrintOptions{
+				t.Errorf("%d: Expect printing object %T to contain namespace %q, got %s", i, test.obj, namespaceName, buffer.String())
-				WithNamespace: true,
-			})
-			AddHandlers(printer)
-			buffer := &bytes.Buffer{}
-			err := printer.PrintObj(test.obj, buffer)
-			if err != nil {
-				t.Fatalf("An error occurred printing object: %#v", err)
-			}
-			matched := contains(strings.Fields(buffer.String()), fmt.Sprintf("%s", namespaceName))
-			if !matched {
-				t.Errorf("%d: Expect printing object to contain namespace: %#v", i, test.obj)
 			}
 		} else {
-			// Expect error when trying to get all namespaces for un-namespaced object.
+			if !strings.HasPrefix(buffer.String(), " ") {
-			printer := printers.NewTablePrinter(printers.PrintOptions{
+				t.Errorf("%d: Expect printing object %T to not contain namespace got %s", i, test.obj, buffer.String())
-				WithNamespace: true,
-			})
-			buffer := &bytes.Buffer{}
-			err := printer.PrintObj(test.obj, buffer)
-			if err == nil {
-				t.Errorf("Expected error when printing un-namespaced type")
 			}
 		}
 	}

pkg/printers/tablegenerator.go

@@ -35,7 +35,6 @@ type TableGenerator interface {
 // PrintHandler - interface to handle printing provided an array of metav1beta1.TableColumnDefinition
 type PrintHandler interface {
 	TableHandler(columns []metav1beta1.TableColumnDefinition, printFunc interface{}) error
-	DefaultTableHandler(columns []metav1beta1.TableColumnDefinition, printFunc interface{}) error
 }
 
 type handlerEntry struct {
@@ -49,11 +48,10 @@ 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
+	handlerMap  map[reflect.Type]*handlerEntry
-	defaultHandler *handlerEntry
+	options     PrintOptions
-	options        PrintOptions
+	lastType    interface{}
-	lastType       interface{}
+	lastColumns []metav1beta1.TableColumnDefinition
-	lastColumns    []metav1beta1.TableColumnDefinition
 }
 
 var _ TableGenerator = &HumanReadablePrinter{}
@@ -149,24 +147,6 @@ func (h *HumanReadablePrinter) TableHandler(columnDefinitions []metav1beta1.Tabl
 	return nil
 }
 
-// DefaultTableHandler registers a set of columns and a print func that is given a chance to process
-// any object without an explicit handler. Only the most recently set print handler is used.
-// See ValidateRowPrintHandlerFunc for required method signature.
-func (h *HumanReadablePrinter) DefaultTableHandler(columnDefinitions []metav1beta1.TableColumnDefinition, printFunc interface{}) error {
-	printFuncValue := reflect.ValueOf(printFunc)
-	if err := ValidateRowPrintHandlerFunc(printFuncValue); err != nil {
-		utilruntime.HandleError(fmt.Errorf("unable to register print function: %v", err))
-		return err
-	}
-	entry := &handlerEntry{
-		columnDefinitions: columnDefinitions,
-		printFunc:         printFuncValue,
-	}
-
-	h.defaultHandler = entry
-	return nil
-}
-
 // ValidateRowPrintHandlerFunc validates print handler signature.
 // printFunc is the function that will be called to print an object.
 // It must be of the following type:

pkg/printers/tableprinter.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/liggitt/tabwriter"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -28,10 +29,24 @@ import (
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/duration"
 )
 
 var _ ResourcePrinter = &HumanReadablePrinter{}
-var withNamespacePrefixColumns = []string{"NAMESPACE"} // TODO(erictune): print cluster name too.
+
+var (
+	defaultHandlerEntry = &handlerEntry{
+		columnDefinitions: objectMetaColumnDefinitions,
+		printFunc:         reflect.ValueOf(printObjectMeta),
+	}
+
+	objectMetaColumnDefinitions = []metav1beta1.TableColumnDefinition{
+		{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
+		{Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]},
+	}
+
+	withNamespacePrefixColumns = []string{"NAMESPACE"} // TODO(erictune): print cluster name too.
+)
 
 // NewTablePrinter creates a printer suitable for calling PrintObj().
 // TODO(seans3): Change return type to ResourcePrinter interface once we no longer need
@@ -104,23 +119,18 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er
 	}
 
 	// Case 3: Could not find print handler for "obj"; use the default print handler.
-	// print with the default handler if set, and use the columns from the last time
+	// Print with the default handler, and use the columns from the last time
-	if h.defaultHandler != nil {
+	includeHeaders := h.lastType != defaultHandlerEntry && !h.options.NoHeaders
-		includeHeaders := h.lastType != h.defaultHandler && !h.options.NoHeaders
 
-		if h.lastType != nil && h.lastType != h.defaultHandler && !h.options.NoHeaders {
+	if h.lastType != nil && h.lastType != defaultHandlerEntry && !h.options.NoHeaders {
-			fmt.Fprintln(output)
+		fmt.Fprintln(output)
-		}
-
-		if err := printRowsForHandlerEntry(output, h.defaultHandler, obj, h.options, includeHeaders); err != nil {
-			return err
-		}
-		h.lastType = h.defaultHandler
-		return nil
 	}
 
-	// we failed all reasonable printing efforts, report failure
+	if err := printRowsForHandlerEntry(output, defaultHandlerEntry, obj, h.options, includeHeaders); err != nil {
-	return fmt.Errorf("error: unknown type %#v", obj)
+		return err
+	}
+	h.lastType = defaultHandlerEntry
+	return nil
 }
 
 // PrintTable prints a table to the provided output respecting the filtering rules for options
@@ -384,3 +394,43 @@ func appendLabelCells(values []interface{}, itemLabels map[string]string, opts P
 	}
 	return values
 }
+
+func printObjectMeta(obj runtime.Object, options PrintOptions) ([]metav1beta1.TableRow, error) {
+	if meta.IsListType(obj) {
+		rows := make([]metav1beta1.TableRow, 0, 16)
+		err := meta.EachListItem(obj, func(obj runtime.Object) error {
+			nestedRows, err := printObjectMeta(obj, options)
+			if err != nil {
+				return err
+			}
+			rows = append(rows, nestedRows...)
+			return nil
+		})
+		if err != nil {
+			return nil, err
+		}
+		return rows, nil
+	}
+
+	rows := make([]metav1beta1.TableRow, 0, 1)
+	m, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, err
+	}
+	row := metav1beta1.TableRow{
+		Object: runtime.RawExtension{Object: obj},
+	}
+	row.Cells = append(row.Cells, m.GetName(), translateTimestampSince(m.GetCreationTimestamp()))
+	rows = append(rows, row)
+	return rows, nil
+}
+
+// translateTimestampSince returns the elapsed time since timestamp in
+// human-readable approximation.
+func translateTimestampSince(timestamp metav1.Time) string {
+	if timestamp.IsZero() {
+		return "<unknown>"
+	}
+
+	return duration.HumanDuration(time.Since(timestamp.Time))
+}