Merge pull request #61506 from juanvallejo/jvallejo/add-humanreadable-flags

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

wire through humanreadable print flags

**Release note**:
```release-note
NONE
```

~~Work in progress... Opening PR now to gather feedback as this is implemented.~~

Begin implementing pieces needed to retrieve humanreadable printers from a set of flags.
Proposal: https://docs.google.com/document/d/19ZZFVe9oD1KQmk5uExggRWtRl_hKGfYnBXvHZJlgEro/edit#heading=h.pnvbfi14v4zz

cc @soltysh @deads2k @pwittrock

pkg/kubectl/BUILD

@@ -33,7 +33,7 @@ go_test(
         "service_basic_test.go",
         "service_test.go",
         "serviceaccount_test.go",
-        "sorting_printer_test.go",
+        "sorter_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
@@ -115,7 +115,7 @@ go_library(
         "service.go",
         "service_basic.go",
         "serviceaccount.go",
-        "sorting_printer.go",
+        "sorter.go",
     ],
     importpath = "k8s.io/kubernetes/pkg/kubectl",
     deps = [

pkg/printers/BUILD

@@ -12,6 +12,7 @@ go_library(
         "customcolumn.go",
         "customcolumn_flags.go",
         "humanreadable.go",
+        "humanreadable_flags.go",
         "interface.go",
         "json.go",
         "json_yaml_flags.go",
@@ -50,6 +51,7 @@ go_test(
     srcs = [
         "customcolumn_flags_test.go",
         "customcolumn_test.go",
+        "humanreadable_flags_test.go",
         "json_yaml_flags_test.go",
         "jsonpath_flags_test.go",
         "name_flags_test.go",
@@ -59,9 +61,11 @@ go_test(
         ":go_default_library",
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/core:go_default_library",
+        "//pkg/printers/internalversion:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
     ],
 )
 

pkg/printers/humanreadable.go

@@ -278,12 +278,9 @@ func printHeader(columnNames []string, w io.Writer) error {
 }
 
 // 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 w, found := output.(*tabwriter.Writer); !found && !h.skipTabWriter {
+	w, found := output.(*tabwriter.Writer)
+	if !found && !h.skipTabWriter {
 		w = GetNewTabWriter(output)
 		output = w
 		defer w.Flush()
@@ -307,6 +304,11 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er
 	t := reflect.TypeOf(obj)
 	if handler := h.handlerMap[t]; handler != nil {
 		includeHeaders := h.lastType != t && !h.options.NoHeaders
+
+		if h.lastType != nil && h.lastType != t && !h.options.NoHeaders {
+			fmt.Fprintln(output)
+		}
+
 		if err := printRowsForHandlerEntry(output, handler, obj, h.options, includeHeaders); err != nil {
 			return err
 		}
@@ -317,6 +319,11 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er
 	// print with the default handler if set, and use the columns from the last time
 	if h.defaultHandler != nil {
 		includeHeaders := h.lastType != h.defaultHandler && !h.options.NoHeaders
+
+		if h.lastType != nil && h.lastType != h.defaultHandler && !h.options.NoHeaders {
+			fmt.Fprintln(output)
+		}
+
 		if err := printRowsForHandlerEntry(output, h.defaultHandler, obj, h.options, includeHeaders); err != nil {
 			return err
 		}

pkg/printers/humanreadable_flags.go

@@ -0,0 +1,123 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+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 printers
+
+import (
+	"github.com/spf13/cobra"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/kubernetes/pkg/kubectl/scheme"
+)
+
+// HumanPrintFlags provides default flags necessary for printing.
+// Given the following flag values, a printer can be requested that knows
+// how to handle printing based on these values.
+type HumanPrintFlags struct {
+	ShowKind     *bool
+	ShowLabels   *bool
+	SortBy       *string
+	ColumnLabels *[]string
+
+	// get.go-specific values
+	NoHeaders bool
+
+	Kind               schema.GroupKind
+	AbsoluteTimestamps bool
+	WithNamespace      bool
+}
+
+// ToPrinter receives an outputFormat and returns a printer capable of
+// handling human-readable output.
+func (f *HumanPrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, bool, error) {
+	if len(outputFormat) > 0 && outputFormat != "wide" {
+		return nil, false, nil
+	}
+
+	encoder := scheme.Codecs.LegacyCodec(scheme.Registry.EnabledVersions()...)
+	decoder := scheme.Codecs.UniversalDecoder()
+
+	showKind := false
+	if f.ShowKind != nil {
+		showKind = *f.ShowKind
+	}
+
+	showLabels := false
+	if f.ShowLabels != nil {
+		showLabels = *f.ShowLabels
+	}
+
+	columnLabels := []string{}
+	if f.ColumnLabels != nil {
+		columnLabels = *f.ColumnLabels
+	}
+
+	p := NewHumanReadablePrinter(encoder, decoder, PrintOptions{
+		Kind:          f.Kind,
+		WithKind:      showKind,
+		NoHeaders:     f.NoHeaders,
+		Wide:          outputFormat == "wide",
+		WithNamespace: f.WithNamespace,
+		ColumnLabels:  columnLabels,
+		ShowLabels:    showLabels,
+	})
+
+	// TODO(juanvallejo): enable this here once we wire commands to instantiate PrintFlags directly.
+	// PrintHandlers are currently added through cmd/util/printing.go#PrinterForOptions
+	//printersinternal.AddHandlers(p)
+
+	// TODO(juanvallejo): handle sorting here
+
+	return p, true, nil
+}
+
+// AddFlags receives a *cobra.Command reference and binds
+// flags related to human-readable printing to it
+func (f *HumanPrintFlags) AddFlags(c *cobra.Command) {
+	if f.ShowLabels != nil {
+		c.Flags().BoolVar(f.ShowLabels, "show-labels", *f.ShowLabels, "When printing, show all labels as the last column (default hide labels column)")
+	}
+	if f.SortBy != nil {
+		c.Flags().StringVar(f.SortBy, "sort-by", *f.SortBy, "If non-empty, sort list types using this field specification.  The field specification is expressed as a JSONPath expression (e.g. '{.metadata.name}'). The field in the API resource specified by this JSONPath expression must be an integer or a string.")
+	}
+	if f.ColumnLabels != nil {
+		c.Flags().StringSliceVarP(f.ColumnLabels, "label-columns", "L", *f.ColumnLabels, "Accepts a comma separated list of labels that are going to be presented as columns. Names are case-sensitive. You can also use multiple flag options like -L label1 -L label2...")
+	}
+	if f.ShowKind != nil {
+		c.Flags().BoolVar(f.ShowKind, "show-kind", *f.ShowKind, "If present, list the resource type for the requested object(s).")
+	}
+}
+
+// NewHumanPrintFlags returns flags associated with
+// human-readable printing, with default values set.
+func NewHumanPrintFlags(kind schema.GroupKind, noHeaders, withNamespace, absoluteTimestamps bool) *HumanPrintFlags {
+	showLabels := false
+	sortBy := ""
+	showKind := false
+	columnLabels := []string{}
+
+	return &HumanPrintFlags{
+		NoHeaders:          noHeaders,
+		WithNamespace:      withNamespace,
+		AbsoluteTimestamps: absoluteTimestamps,
+		ColumnLabels:       &columnLabels,
+
+		Kind:       kind,
+		ShowLabels: &showLabels,
+		SortBy:     &sortBy,
+		ShowKind:   &showKind,
+	}
+}

pkg/printers/humanreadable_flags_test.go

@@ -0,0 +1,151 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+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 printers_test
+
+import (
+	"bytes"
+	"regexp"
+	"strings"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/printers"
+	printersinternal "k8s.io/kubernetes/pkg/printers/internalversion"
+)
+
+func TestHumanReadablePrinterSupportsExpectedOptions(t *testing.T) {
+	testObject := &api.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}
+
+	testCases := []struct {
+		name       string
+		showKind   bool
+		showLabels bool
+
+		// TODO(juanvallejo): test sorting once it's moved to the HumanReadablePrinter
+		sortBy       string
+		columnLabels []string
+
+		noHeaders     bool
+		withNamespace bool
+
+		outputFormat string
+
+		expectedError  string
+		expectedOutput string
+		expectNoMatch  bool
+	}{
+		{
+			name:           "empty output format matches a humanreadable printer",
+			expectedOutput: "NAME\\ +READY\\ +STATUS\\ +RESTARTS\\ +AGE\nfoo\\ +0/0\\ +0\\ +<unknown>\n",
+		},
+		{
+			name:           "\"wide\" output format prints",
+			outputFormat:   "wide",
+			expectedOutput: "NAME\\ +READY\\ +STATUS\\ +RESTARTS\\ +AGE\\ +IP\\ +NODE\nfoo\\ +0/0\\ +0\\ +<unknown>\\ +<none>\\ +<none>\n",
+		},
+		{
+			name:           "no-headers prints output with no headers",
+			noHeaders:      true,
+			expectedOutput: "foo\\ +0/0\\ +0\\ +<unknown>\n",
+		},
+		{
+			name:           "no-headers and a \"wide\" output format prints output with no headers and additional columns",
+			outputFormat:   "wide",
+			noHeaders:      true,
+			expectedOutput: "foo\\ +0/0\\ +0\\ +<unknown>\\ +<none>\\ +<none>\n",
+		},
+		{
+			name:           "show-kind displays the resource's kind, even when printing a single type of resource",
+			showKind:       true,
+			expectedOutput: "NAME\\ +READY\\ +STATUS\\ +RESTARTS\\ +AGE\npod/foo\\ +0/0\\ +0\\ +<unknown>\n",
+		},
+		{
+			name:           "withNamespace displays an additional NAMESPACE column",
+			withNamespace:  true,
+			expectedOutput: "NAMESPACE\\ +NAME\\ +READY\\ +STATUS\\ +RESTARTS\\ +AGE\n\\ +foo\\ +0/0\\ +0\\ +<unknown>\n",
+		},
+		{
+			name:          "no printer is matched on an invalid outputFormat",
+			outputFormat:  "invalid",
+			expectNoMatch: true,
+		},
+		{
+			name:          "printer should not match on any other format supported by another printer",
+			outputFormat:  "go-template",
+			expectNoMatch: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			printFlags := printers.HumanPrintFlags{
+				ShowKind:     &tc.showKind,
+				ShowLabels:   &tc.showLabels,
+				SortBy:       &tc.sortBy,
+				ColumnLabels: &tc.columnLabels,
+
+				NoHeaders:     tc.noHeaders,
+				WithNamespace: tc.withNamespace,
+			}
+
+			if tc.showKind {
+				printFlags.Kind = schema.GroupKind{Kind: "pod"}
+			}
+
+			p, matched, err := printFlags.ToPrinter(tc.outputFormat)
+			if tc.expectNoMatch {
+				if matched {
+					t.Fatalf("expected no printer matches for output format %q", tc.outputFormat)
+				}
+				return
+			}
+			if !matched {
+				t.Fatalf("expected to match template printer for output format %q", tc.outputFormat)
+			}
+
+			if len(tc.expectedError) > 0 {
+				if err == nil || !strings.Contains(err.Error(), tc.expectedError) {
+					t.Errorf("expecting error %q, got %v", tc.expectedError, err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			// TODO(juanvallejo): remove this once we wire PrintFlags at the command level.
+			// handlers should be attached to the printer inside of the ToPrinter method.
+			printersinternal.AddHandlers(p.(*printers.HumanReadablePrinter))
+
+			out := bytes.NewBuffer([]byte{})
+			err = p.PrintObj(testObject, out)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+
+			match, err := regexp.Match(tc.expectedOutput, out.Bytes())
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+			if !match {
+				t.Errorf("unexpected output: expecting %q, got %q", tc.expectedOutput, out.String())
+			}
+		})
+	}
+}

pkg/printers/name_flags_test.go

@@ -18,9 +18,6 @@ package printers_test
 
 import (
 	"bytes"
-	"fmt"
-	"io/ioutil"
-	"os"
 	"strings"
 	"testing"
 
@@ -32,17 +29,6 @@ import (
 func TestNamePrinterSupportsExpectedFormats(t *testing.T) {
 	testObject := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}
 
-	customColumnsFile, err := ioutil.TempFile("", "printers_jsonpath_flags")
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-	defer func(tempFile *os.File) {
-		tempFile.Close()
-		os.Remove(tempFile.Name())
-	}(customColumnsFile)
-
-	fmt.Fprintf(customColumnsFile, "NAME\n.metadata.name")
-
 	testCases := []struct {
 		name           string
 		outputFormat   string

pkg/printers/printers.go

@@ -97,8 +97,24 @@ func GetStandardPrinter(typer runtime.ObjectTyper, encoder runtime.Encoder, deco
 	case "wide":
 		fallthrough
 	case "":
+		humanPrintFlags := NewHumanPrintFlags(options.Kind, options.NoHeaders, options.WithNamespace, options.AbsoluteTimestamps)
+
+		// TODO: these should be bound through a call to humanPrintFlags#AddFlags(cmd) once we instantiate PrintFlags at the command level
+		humanPrintFlags.ShowKind = &options.WithKind
+		humanPrintFlags.ShowLabels = &options.ShowLabels
+		humanPrintFlags.ColumnLabels = &options.ColumnLabels
+		humanPrintFlags.SortBy = &options.SortBy
+
+		humanPrinter, matches, err := humanPrintFlags.ToPrinter(format)
+		if !matches {
+			return nil, fmt.Errorf("unable to match a printer to handle current print options")
+		}
+		if err != nil {
+			return nil, err
+		}
+
+		printer = humanPrinter
 
-		printer = NewHumanReadablePrinter(encoder, decoders[0], options)
 	default:
 		return nil, fmt.Errorf("output format %q not recognized", format)
 	}