diff --git a/pkg/kubectl/cmd/set/BUILD b/pkg/kubectl/cmd/set/BUILD index e3af3fd55d6..c6d7f29f235 100644 --- a/pkg/kubectl/cmd/set/BUILD +++ b/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", diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index a4b458b4098..e55a3fa2768 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/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 - Resolve bool - List bool - ShortOutput bool - Local bool - Overwrite bool - DryRun bool + All bool + Resolve bool + List bool + Local bool + Overwrite 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) - cmdutil.AddPrinterFlags(cmd) + options.PrintFlags.AddFlags(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 := cmdutil.PrintObject(o.Cmd, info.AsVersioned(), o.Out); err != nil { - return err - } - continue + if err := o.PrintObj(info.AsVersioned()); err != nil { + return err } - - cmdutil.PrintSuccess(o.ShortOutput, o.Out, info.Object, false, "env updated") } return utilerrors.NewAggregate(allErrs) } diff --git a/pkg/kubectl/cmd/set/set_env_test.go b/pkg/kubectl/cmd/set/set_env_test.go index 22c429a2734..b05bf7562dc 100644 --- a/pkg/kubectl/cmd/set/set_env_test.go +++ b/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{ - Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}}, + opts := EnvOptions{ + 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{ - Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, + opts := EnvOptions{ + 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) diff --git a/pkg/printers/BUILD b/pkg/printers/BUILD index c9df102608f..bda4421e5c1 100644 --- a/pkg/printers/BUILD +++ b/pkg/printers/BUILD @@ -11,6 +11,7 @@ go_library( srcs = [ "customcolumn.go", "customcolumn_flags.go", + "flags.go", "humanreadable.go", "humanreadable_flags.go", "interface.go", diff --git a/pkg/printers/flags.go b/pkg/printers/flags.go new file mode 100644 index 00000000000..e0eea991414 --- /dev/null +++ b/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), + } +} diff --git a/pkg/printers/json_yaml_flags.go b/pkg/printers/json_yaml_flags.go index 6ef587b6491..134e646ab1f 100644 --- a/pkg/printers/json_yaml_flags.go +++ b/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 - case "": - return nil, false, fmt.Errorf("missing output format") + return &YAMLPrinter{}, nil default: - return nil, false, nil + return nil, NoCompatiblePrinterError{f} } } diff --git a/pkg/printers/json_yaml_flags_test.go b/pkg/printers/json_yaml_flags_test.go index bd56bf3d0cb..a2de7b25471 100644 --- a/pkg/printers/json_yaml_flags_test.go +++ b/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 { diff --git a/pkg/printers/name.go b/pkg/printers/name.go index 9e0d84ae91d..58bd7058a16 100644 --- a/pkg/printers/name.go +++ b/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 - // should be appended to the finalized "successful" - // message printed about an action on an object. - DryRun bool + // ShortOutput indicates whether an operation should be + // printed along side the "resource/name" pair for an object. + ShortOutput 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: ""} } -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 } diff --git a/pkg/printers/name_flags.go b/pkg/printers/name_flags.go index ebe8f7e4d34..24b0f4aefe1 100644 --- a/pkg/printers/name_flags.go +++ b/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} } } diff --git a/pkg/printers/name_flags_test.go b/pkg/printers/name_flags_test.go index 29978913af5..6c06d0c0357 100644 --- a/pkg/printers/name_flags_test.go +++ b/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", - outputFormat: "name", + name: "empty output format and an operation prints success message with dry run", 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 { - t.Fatalf("expected to match template printer for output format %q", tc.outputFormat) + if printers.IsNoCompatiblePrinterError(err) { + t.Fatalf("expected to match name printer for output format %q", tc.outputFormat) } if len(tc.expectedError) > 0 { diff --git a/pkg/printers/printers.go b/pkg/printers/printers.go index 80b97f9f1c7..e6791124e75 100644 --- a/pkg/printers/printers.go +++ b/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) - if !matched { - return nil, fmt.Errorf("unable to match a printer to handle current print options") - } + p, err := jsonYamlFlags.ToPrinter(format) 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) - if !matched { - return nil, fmt.Errorf("unable to match a name printer to handle current print options") - } + namePrinter, err := nameFlags.ToPrinter(format) if err != nil { return nil, err }