Merge pull request #62139 from juanvallejo/jvallejo/begin-wiring-print-flags

Automatic merge from submit-queue (batch tested with PRs 62063, 62169, 62155, 62139, 61445). 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>.

begin wiring printflags through set cmds

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

Begin wiring PrintFlags struct through commands

cc @deads2k @soltysh

pkg/kubectl/cmd/set/BUILD

@@ -26,6 +26,7 @@ go_library(
         "//pkg/kubectl/cmd/util/env:go_default_library",
         "//pkg/kubectl/resource:go_default_library",
         "//pkg/kubectl/util/i18n:go_default_library",
+        "//pkg/printers:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -66,6 +67,7 @@ go_test(
         "//pkg/kubectl/cmd/util:go_default_library",
         "//pkg/kubectl/resource:go_default_library",
         "//pkg/kubectl/scheme:go_default_library",
+        "//pkg/printers:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/apps/v1:go_default_library",

pkg/kubectl/cmd/set/set_env.go

@@ -33,6 +33,7 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	envutil "k8s.io/kubernetes/pkg/kubectl/cmd/util/env"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
+	"k8s.io/kubernetes/pkg/printers"
 
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 )
@@ -91,6 +92,8 @@ var (
 )
 
 type EnvOptions struct {
+	PrintFlags *printers.PrintFlags
+
 	Out io.Writer
 	Err io.Writer
 	In  io.Reader
@@ -100,13 +103,12 @@ type EnvOptions struct {
 	EnvArgs   []string
 	Resources []string
 
-	All         bool
+	All       bool
-	Resolve     bool
+	Resolve   bool
-	List        bool
+	List      bool
-	ShortOutput bool
+	Local     bool
-	Local       bool
+	Overwrite bool
-	Overwrite   bool
+	DryRun    bool
-	DryRun      bool
 
 	ResourceVersion   string
 	ContainerSelector string
@@ -115,6 +117,8 @@ type EnvOptions struct {
 	From              string
 	Prefix            string
 
+	PrintObj func(runtime.Object) error
+
 	Builder *resource.Builder
 	Infos   []*resource.Info
 
@@ -127,6 +131,8 @@ type EnvOptions struct {
 // pod templates are selected by default and allowing environment to be overwritten
 func NewEnvOptions(in io.Reader, out, errout io.Writer) *EnvOptions {
 	return &EnvOptions{
+		PrintFlags: printers.NewPrintFlags("env updated"),
+
 		Out:               out,
 		Err:               errout,
 		In:                in,
@@ -162,9 +168,9 @@ func NewCmdEnv(f cmdutil.Factory, in io.Reader, out, errout io.Writer) *cobra.Co
 	cmd.Flags().BoolVar(&options.All, "all", options.All, "If true, select all resources in the namespace of the specified resource types")
 	cmd.Flags().BoolVar(&options.Overwrite, "overwrite", options.Overwrite, "If true, allow environment to be overwritten, otherwise reject updates that overwrite existing environment.")
 
-	cmdutil.AddDryRunFlag(cmd)
+	options.PrintFlags.AddFlags(cmd)
-	cmdutil.AddPrinterFlags(cmd)
 
+	cmdutil.AddDryRunFlag(cmd)
 	return cmd
 }
 
@@ -209,7 +215,16 @@ func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 	o.Resources = resources
 	o.Cmd = cmd
 
-	o.ShortOutput = cmdutil.GetFlagString(cmd, "output") == "name"
+	o.PrintFlags.Complete(o.DryRun)
+
+	printer, err := o.PrintFlags.ToPrinter()
+	if err != nil {
+		return err
+	}
+
+	o.PrintObj = func(obj runtime.Object) error {
+		return printer.PrintObj(obj, o.Out)
+	}
 
 	if o.List && len(o.Output) > 0 {
 		return cmdutil.UsageErrorf(o.Cmd, "--list and --output may not be specified together")
@@ -418,7 +433,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 		}
 
 		if o.Local || o.DryRun {
-			if err := cmdutil.PrintObject(o.Cmd, patch.Info.AsVersioned(), o.Out); err != nil {
+			if err := o.PrintObj(patch.Info.AsVersioned()); err != nil {
 				return err
 			}
 			continue
@@ -437,14 +452,9 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 			return fmt.Errorf("at least one environment variable must be provided")
 		}
 
-		if len(o.Output) > 0 {
+		if err := o.PrintObj(info.AsVersioned()); err != nil {
-			if err := cmdutil.PrintObject(o.Cmd, info.AsVersioned(), o.Out); err != nil {
+			return err
-				return err
-			}
-			continue
 		}
-
-		cmdutil.PrintSuccess(o.ShortOutput, o.Out, info.Object, false, "env updated")
 	}
 	return utilerrors.NewAggregate(allErrs)
 }

pkg/kubectl/cmd/set/set_env_test.go

@@ -26,6 +26,8 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/kubernetes/pkg/printers"
+
 	"github.com/stretchr/testify/assert"
 	appsv1 "k8s.io/api/apps/v1"
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
@@ -61,16 +63,26 @@ func TestSetEnvLocal(t *testing.T) {
 	tf.Namespace = "test"
 	tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}}
 
+	outputFormat := "name"
+
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdEnv(tf, os.Stdin, buf, buf)
 	cmd.SetOutput(buf)
-	cmd.Flags().Set("output", "name")
+	cmd.Flags().Set("output", outputFormat)
 	cmd.Flags().Set("local", "true")
 
-	opts := EnvOptions{FilenameOptions: resource.FilenameOptions{
+	opts := EnvOptions{
-		Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}},
+		PrintFlags: &printers.PrintFlags{
+			JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
+			NamePrintFlags:     printers.NewNamePrintFlags("", false),
+
+			OutputFormat: &outputFormat,
+		},
+		FilenameOptions: resource.FilenameOptions{
+			Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}},
 		Out:   buf,
-		Local: true}
+		Local: true,
+	}
 	err := opts.Complete(tf, cmd, []string{"env=prod"})
 	if err == nil {
 		err = opts.RunEnv(tf)
@@ -100,16 +112,26 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) {
 	tf.Namespace = "test"
 	tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}}
 
+	outputFormat := "name"
+
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdEnv(tf, os.Stdin, buf, buf)
 	cmd.SetOutput(buf)
-	cmd.Flags().Set("output", "name")
+	cmd.Flags().Set("output", outputFormat)
 	cmd.Flags().Set("local", "true")
 
-	opts := EnvOptions{FilenameOptions: resource.FilenameOptions{
+	opts := EnvOptions{
-		Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}},
+		PrintFlags: &printers.PrintFlags{
+			JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
+			NamePrintFlags:     printers.NewNamePrintFlags("", false),
+
+			OutputFormat: &outputFormat,
+		},
+		FilenameOptions: resource.FilenameOptions{
+			Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}},
 		Out:   buf,
-		Local: true}
+		Local: true,
+	}
 	err := opts.Complete(tf, cmd, []string{"env=prod"})
 	if err == nil {
 		err = opts.RunEnv(tf)
@@ -482,11 +504,20 @@ func TestSetEnvRemote(t *testing.T) {
 				}),
 				VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()),
 			}
+
+			outputFormat := "yaml"
+
 			out := new(bytes.Buffer)
 			cmd := NewCmdEnv(tf, out, out, out)
 			cmd.SetOutput(out)
-			cmd.Flags().Set("output", "yaml")
+			cmd.Flags().Set("output", outputFormat)
 			opts := EnvOptions{
+				PrintFlags: &printers.PrintFlags{
+					JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
+					NamePrintFlags:     printers.NewNamePrintFlags("", false),
+
+					OutputFormat: &outputFormat,
+				},
 				Out:   out,
 				Local: false}
 			err := opts.Complete(tf, cmd, input.args)

pkg/printers/BUILD

@@ -11,6 +11,7 @@ go_library(
     srcs = [
         "customcolumn.go",
         "customcolumn_flags.go",
+        "flags.go",
         "humanreadable.go",
         "humanreadable_flags.go",
         "interface.go",

pkg/printers/flags.go

@@ -0,0 +1,85 @@
+/*
+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 (
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+type NoCompatiblePrinterError struct {
+	options interface{}
+}
+
+func (e NoCompatiblePrinterError) Error() string {
+	return fmt.Sprintf("unable to match a printer suitable for the options specified: %#v", e.options)
+}
+
+func IsNoCompatiblePrinterError(err error) bool {
+	_, ok := err.(NoCompatiblePrinterError)
+	return ok
+}
+
+// PrintFlags composes common printer flag structs
+// used across all commands, and provides a method
+// of retrieving a known printer based on flag values provided.
+type PrintFlags struct {
+	JSONYamlPrintFlags *JSONYamlPrintFlags
+	NamePrintFlags     *NamePrintFlags
+
+	OutputFormat *string
+}
+
+func (f *PrintFlags) Complete(dryRun bool) error {
+	f.NamePrintFlags.DryRun = dryRun
+	return nil
+}
+
+func (f *PrintFlags) ToPrinter() (ResourcePrinter, error) {
+	outputFormat := ""
+	if f.OutputFormat != nil {
+		outputFormat = *f.OutputFormat
+	}
+
+	p, err := f.JSONYamlPrintFlags.ToPrinter(outputFormat)
+	if err == nil {
+		return p, nil
+	}
+
+	return f.NamePrintFlags.ToPrinter(outputFormat)
+}
+
+func (f *PrintFlags) AddFlags(cmd *cobra.Command) {
+	f.JSONYamlPrintFlags.AddFlags(cmd)
+	f.NamePrintFlags.AddFlags(cmd)
+
+	if f.OutputFormat != nil {
+		cmd.Flags().StringVarP(f.OutputFormat, "output", "o", *f.OutputFormat, "Output format. One of: json|yaml|wide|name|custom-columns=...|custom-columns-file=...|go-template=...|go-template-file=...|jsonpath=...|jsonpath-file=... See custom columns [http://kubernetes.io/docs/user-guide/kubectl-overview/#custom-columns], golang template [http://golang.org/pkg/text/template/#pkg-overview] and jsonpath template [http://kubernetes.io/docs/user-guide/jsonpath].")
+	}
+}
+
+func NewPrintFlags(operation string) *PrintFlags {
+	outputFormat := ""
+
+	return &PrintFlags{
+		OutputFormat: &outputFormat,
+
+		JSONYamlPrintFlags: NewJSONYamlPrintFlags(),
+		NamePrintFlags:     NewNamePrintFlags(operation, false),
+	}
+}

pkg/printers/json_yaml_flags.go

@@ -17,7 +17,6 @@ limitations under the License.
 package printers
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -32,17 +31,15 @@ type JSONYamlPrintFlags struct{}
 // handling --output=(yaml|json) printing.
 // Returns false if the specified outputFormat does not match a supported format.
 // Supported Format types can be found in pkg/printers/printers.go
-func (f *JSONYamlPrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, bool, error) {
+func (f *JSONYamlPrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, error) {
 	outputFormat = strings.ToLower(outputFormat)
 	switch outputFormat {
 	case "json":
-		return &JSONPrinter{}, true, nil
+		return &JSONPrinter{}, nil
 	case "yaml":
-		return &YAMLPrinter{}, true, nil
+		return &YAMLPrinter{}, nil
-	case "":
-		return nil, false, fmt.Errorf("missing output format")
 	default:
-		return nil, false, nil
+		return nil, NoCompatiblePrinterError{f}
 	}
 }
 

pkg/printers/json_yaml_flags_test.go

@@ -61,14 +61,14 @@ func TestPrinterSupportsExpectedJSONYamlFormats(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			printFlags := printers.JSONYamlPrintFlags{}
 
-			p, matched, err := printFlags.ToPrinter(tc.outputFormat)
+			p, err := printFlags.ToPrinter(tc.outputFormat)
 			if tc.expectNoMatch {
-				if matched {
+				if !printers.IsNoCompatiblePrinterError(err) {
 					t.Fatalf("expected no printer matches for output format %q", tc.outputFormat)
 				}
 				return
 			}
-			if !matched {
+			if printers.IsNoCompatiblePrinterError(err) {
 				t.Fatalf("expected to match template printer for output format %q", tc.outputFormat)
 			}
 			if err != nil {

pkg/printers/name.go

@@ -30,10 +30,9 @@ import (
 
 // NamePrinter is an implementation of ResourcePrinter which outputs "resource/name" pair of an object.
 type NamePrinter struct {
-	// DryRun indicates whether the "(dry run)" message
+	// ShortOutput indicates whether an operation should be
-	// should be appended to the finalized "successful"
+	// printed along side the "resource/name" pair for an object.
-	// message printed about an action on an object.
+	ShortOutput bool
-	DryRun bool
 	// Operation describes the name of the action that
 	// took place on an object, to be included in the
 	// finalized "successful" message.
@@ -73,7 +72,7 @@ func (p *NamePrinter) PrintObj(obj runtime.Object, w io.Writer) error {
 		}
 	}
 
-	return printObj(w, name, p.Operation, p.DryRun, GetObjectGroupKind(obj, p.Typer))
+	return printObj(w, name, p.Operation, p.ShortOutput, GetObjectGroupKind(obj, p.Typer))
 }
 
 func GetObjectGroupKind(obj runtime.Object, typer runtime.ObjectTyper) schema.GroupKind {
@@ -103,25 +102,25 @@ func GetObjectGroupKind(obj runtime.Object, typer runtime.ObjectTyper) schema.Gr
 	return schema.GroupKind{Kind: "<unknown>"}
 }
 
-func printObj(w io.Writer, name string, operation string, dryRun bool, groupKind schema.GroupKind) error {
+func printObj(w io.Writer, name string, operation string, shortOutput bool, groupKind schema.GroupKind) error {
 	if len(groupKind.Kind) == 0 {
 		return fmt.Errorf("missing kind for resource with name %v", name)
 	}
 
-	dryRunMsg := ""
-	if dryRun {
-		dryRunMsg = " (dry run)"
-	}
 	if len(operation) > 0 {
 		operation = " " + operation
 	}
 
+	if shortOutput {
+		operation = ""
+	}
+
 	if len(groupKind.Group) == 0 {
-		fmt.Fprintf(w, "%s/%s%s%s\n", strings.ToLower(groupKind.Kind), name, operation, dryRunMsg)
+		fmt.Fprintf(w, "%s/%s%s\n", strings.ToLower(groupKind.Kind), name, operation)
 		return nil
 	}
 
-	fmt.Fprintf(w, "%s.%s/%s%s%s\n", strings.ToLower(groupKind.Kind), groupKind.Group, name, operation, dryRunMsg)
+	fmt.Fprintf(w, "%s.%s/%s%s\n", strings.ToLower(groupKind.Kind), groupKind.Group, name, operation)
 	return nil
 }
 

pkg/printers/name_flags.go

@@ -17,7 +17,6 @@ limitations under the License.
 package printers
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -46,23 +45,27 @@ type NamePrintFlags struct {
 // handling --output=name printing.
 // Returns false if the specified outputFormat does not match a supported format.
 // Supported format types can be found in pkg/printers/printers.go
-func (f *NamePrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, bool, error) {
+func (f *NamePrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, error) {
 	decoders := []runtime.Decoder{scheme.Codecs.UniversalDecoder(), unstructured.UnstructuredJSONScheme}
 	namePrinter := &NamePrinter{
 		Operation: f.Operation,
-		DryRun:    f.DryRun,
 		Typer:     scheme.Scheme,
 		Decoders:  decoders,
 	}
 
+	if f.DryRun {
+		namePrinter.Operation = namePrinter.Operation + " (dry run)"
+	}
+
 	outputFormat = strings.ToLower(outputFormat)
 	switch outputFormat {
 	case "name":
-		return namePrinter, true, nil
+		namePrinter.ShortOutput = true
+		fallthrough
 	case "":
-		return nil, false, fmt.Errorf("missing output format")
+		return namePrinter, nil
 	default:
-		return nil, false, nil
+		return nil, NoCompatiblePrinterError{f}
 	}
 }
 

pkg/printers/name_flags_test.go

@@ -44,14 +44,13 @@ func TestNamePrinterSupportsExpectedFormats(t *testing.T) {
 			expectedOutput: "pod/foo",
 		},
 		{
-			name:           "valid \"name\" output format and an operation prints success message",
+			name:           "valid \"name\" output format and an operation results in a short-output (non success printer) message",
 			outputFormat:   "name",
 			operation:      "patched",
-			expectedOutput: "pod/foo patched",
+			expectedOutput: "pod/foo",
 		},
 		{
-			name:           "valid \"name\" output format and an operation prints success message with dry run",
+			name:           "empty output format and an operation prints success message with dry run",
-			outputFormat:   "name",
 			operation:      "patched",
 			dryRun:         true,
 			expectedOutput: "pod/foo patched (dry run)",
@@ -59,9 +58,15 @@ func TestNamePrinterSupportsExpectedFormats(t *testing.T) {
 		{
 			name:          "operation and no valid \"name\" output does not match a printer",
 			operation:     "patched",
+			outputFormat:  "invalid",
 			dryRun:        true,
 			expectNoMatch: true,
 		},
+		{
+			name:           "operation and empty output still matches name printer",
+			expectedOutput: "pod/foo patched",
+			operation:      "patched",
+		},
 		{
 			name:          "no printer is matched on an invalid outputFormat",
 			outputFormat:  "invalid",
@@ -81,15 +86,15 @@ func TestNamePrinterSupportsExpectedFormats(t *testing.T) {
 				DryRun:    tc.dryRun,
 			}
 
-			p, matched, err := printFlags.ToPrinter(tc.outputFormat)
+			p, err := printFlags.ToPrinter(tc.outputFormat)
 			if tc.expectNoMatch {
-				if matched {
+				if !printers.IsNoCompatiblePrinterError(err) {
 					t.Fatalf("expected no printer matches for output format %q", tc.outputFormat)
 				}
 				return
 			}
-			if !matched {
+			if printers.IsNoCompatiblePrinterError(err) {
-				t.Fatalf("expected to match template printer for output format %q", tc.outputFormat)
+				t.Fatalf("expected to match name printer for output format %q", tc.outputFormat)
 			}
 
 			if len(tc.expectedError) > 0 {

pkg/printers/printers.go

@@ -34,10 +34,7 @@ func GetStandardPrinter(typer runtime.ObjectTyper, encoder runtime.Encoder, deco
 
 	case "json", "yaml":
 		jsonYamlFlags := NewJSONYamlPrintFlags()
-		p, matched, err := jsonYamlFlags.ToPrinter(format)
+		p, err := jsonYamlFlags.ToPrinter(format)
-		if !matched {
-			return nil, fmt.Errorf("unable to match a printer to handle current print options")
-		}
 		if err != nil {
 			return nil, err
 		}
@@ -46,10 +43,7 @@ func GetStandardPrinter(typer runtime.ObjectTyper, encoder runtime.Encoder, deco
 
 	case "name":
 		nameFlags := NewNamePrintFlags("", false)
-		namePrinter, matched, err := nameFlags.ToPrinter(format)
+		namePrinter, err := nameFlags.ToPrinter(format)
-		if !matched {
-			return nil, fmt.Errorf("unable to match a name printer to handle current print options")
-		}
 		if err != nil {
 			return nil, err
 		}