diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index e55a3fa276..4a00c074a2 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/pkg/kubectl/cmd/set/set_env.go @@ -215,13 +215,17 @@ func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri o.Resources = resources o.Cmd = cmd - o.PrintFlags.Complete(o.DryRun) - + if o.DryRun { + // TODO(juanvallejo): This can be cleaned up even further by creating + // a PrintFlags struct that binds the --dry-run flag, and whose + // ToPrinter method returns a printer that understands how to print + // this success message. + o.PrintFlags.Complete("%s (dry run)") + } printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } - o.PrintObj = func(obj runtime.Object) error { return printer.PrintObj(obj, o.Out) } diff --git a/pkg/kubectl/cmd/set/set_env_test.go b/pkg/kubectl/cmd/set/set_env_test.go index b05bf7562d..2495f6394f 100644 --- a/pkg/kubectl/cmd/set/set_env_test.go +++ b/pkg/kubectl/cmd/set/set_env_test.go @@ -74,7 +74,7 @@ func TestSetEnvLocal(t *testing.T) { opts := EnvOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -123,7 +123,7 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) { opts := EnvOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -514,7 +514,7 @@ func TestSetEnvRemote(t *testing.T) { opts := EnvOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, diff --git a/pkg/kubectl/cmd/set/set_image.go b/pkg/kubectl/cmd/set/set_image.go index 21ce08c1da..c6fe5a2e5a 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/pkg/kubectl/cmd/set/set_image.go @@ -20,6 +20,8 @@ import ( "fmt" "io" + "k8s.io/kubernetes/pkg/printers" + "github.com/spf13/cobra" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -36,12 +38,13 @@ import ( type ImageOptions struct { resource.FilenameOptions + PrintFlags *printers.PrintFlags + Infos []*resource.Info Selector string Out io.Writer Err io.Writer DryRun bool - ShortOutput bool All bool Record bool Output string @@ -50,6 +53,8 @@ type ImageOptions struct { Cmd *cobra.Command ResolveImage func(in string) (string, error) + PrintObj func(runtime.Object) error + UpdatePodSpecForObject func(obj runtime.Object, fn func(*v1.PodSpec) error) (bool, error) Resources []string ContainerImages map[string]string @@ -81,6 +86,8 @@ var ( func NewCmdImage(f cmdutil.Factory, out, err io.Writer) *cobra.Command { options := &ImageOptions{ + PrintFlags: printers.NewPrintFlags("image updated"), + Out: out, Err: err, } @@ -98,7 +105,8 @@ func NewCmdImage(f cmdutil.Factory, out, err io.Writer) *cobra.Command { }, } - cmdutil.AddPrinterFlags(cmd) + options.PrintFlags.AddFlags(cmd) + usage := "identifying the resource to get from a server." cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmd.Flags().BoolVar(&options.All, "all", options.All, "Select all resources, including uninitialized ones, in the namespace of the specified resource types") @@ -112,7 +120,6 @@ func NewCmdImage(f cmdutil.Factory, out, err io.Writer) *cobra.Command { func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { o.UpdatePodSpecForObject = f.UpdatePodSpecForObject - o.ShortOutput = cmdutil.GetFlagString(cmd, "output") == "name" o.Record = cmdutil.GetRecordFlag(cmd) o.ChangeCause = f.Command(cmd, false) o.DryRun = cmdutil.GetDryRunFlag(cmd) @@ -120,6 +127,19 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st o.ResolveImage = f.ResolveImage o.Cmd = cmd + if o.DryRun { + o.PrintFlags.Complete("%s (dry run)") + } + + printer, err := o.PrintFlags.ToPrinter() + if err != nil { + return err + } + + o.PrintObj = func(obj runtime.Object) error { + return printer.PrintObj(obj, o.Out) + } + cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err @@ -238,7 +258,7 @@ func (o *ImageOptions) Run() 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 @@ -263,13 +283,9 @@ func (o *ImageOptions) Run() error { info.Refresh(obj, true) - 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, o.DryRun, "image updated") } return utilerrors.NewAggregate(allErrs) } diff --git a/pkg/kubectl/cmd/set/set_image_test.go b/pkg/kubectl/cmd/set/set_image_test.go index 5c5cf35885..45764a41b3 100644 --- a/pkg/kubectl/cmd/set/set_image_test.go +++ b/pkg/kubectl/cmd/set/set_image_test.go @@ -25,6 +25,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,14 +63,23 @@ func TestImageLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + outputFormat := "name" + buf := bytes.NewBuffer([]byte{}) cmd := NewCmdImage(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "name") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") - opts := ImageOptions{FilenameOptions: resource.FilenameOptions{ - Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}}, + opts := ImageOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}}, Out: buf, Local: true} err := opts.Complete(tf, cmd, []string{"cassandra=thingy"}) @@ -87,6 +98,11 @@ func TestImageLocal(t *testing.T) { } func TestSetImageValidation(t *testing.T) { + printFlags := &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + } + testCases := []struct { name string imageOptions *ImageOptions @@ -94,13 +110,14 @@ func TestSetImageValidation(t *testing.T) { }{ { name: "test resource < 1 and filenames empty", - imageOptions: &ImageOptions{}, + imageOptions: &ImageOptions{PrintFlags: printFlags}, expectErr: "[one or more resources must be specified as or /, at least one image update is required]", }, { name: "test containerImages < 1", imageOptions: &ImageOptions{ - Resources: []string{"a", "b", "c"}, + PrintFlags: printFlags, + Resources: []string{"a", "b", "c"}, FilenameOptions: resource.FilenameOptions{ Filenames: []string{"testFile"}, @@ -111,7 +128,8 @@ func TestSetImageValidation(t *testing.T) { { name: "test containerImages > 1 and all containers are already specified by *", imageOptions: &ImageOptions{ - Resources: []string{"a", "b", "c"}, + PrintFlags: printFlags, + Resources: []string{"a", "b", "c"}, FilenameOptions: resource.FilenameOptions{ Filenames: []string{"testFile"}, }, @@ -125,7 +143,8 @@ func TestSetImageValidation(t *testing.T) { { name: "success case", imageOptions: &ImageOptions{ - Resources: []string{"a", "b", "c"}, + PrintFlags: printFlags, + Resources: []string{"a", "b", "c"}, FilenameOptions: resource.FilenameOptions{ Filenames: []string{"testFile"}, }, @@ -166,14 +185,23 @@ func TestSetMultiResourcesImageLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + outputFormat := "name" + buf := bytes.NewBuffer([]byte{}) cmd := NewCmdImage(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "name") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") - opts := ImageOptions{FilenameOptions: resource.FilenameOptions{ - Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, + opts := ImageOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, Out: buf, Local: true} err := opts.Complete(tf, cmd, []string{"*=thingy"}) @@ -552,11 +580,21 @@ func TestSetImageRemote(t *testing.T) { }), VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()), } + + outputFormat := "yaml" + out := new(bytes.Buffer) cmd := NewCmdImage(tf, out, out) cmd.SetOutput(out) - cmd.Flags().Set("output", "yaml") + cmd.Flags().Set("output", outputFormat) opts := ImageOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + Out: out, Local: false} err := opts.Complete(tf, cmd, input.args) diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index 155dbaa62b..812740ad69 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -21,6 +21,8 @@ import ( "io" "strings" + "k8s.io/kubernetes/pkg/printers" + "github.com/spf13/cobra" "k8s.io/api/core/v1" @@ -61,6 +63,8 @@ var ( type ResourcesOptions struct { resource.FilenameOptions + PrintFlags *printers.PrintFlags + Infos []*resource.Info Out io.Writer Err io.Writer @@ -73,6 +77,10 @@ type ResourcesOptions struct { Local bool Cmd *cobra.Command + DryRun bool + + PrintObj func(runtime.Object) error + Limits string Requests string ResourceRequirements v1.ResourceRequirements @@ -85,6 +93,8 @@ type ResourcesOptions struct { // pod templates are selected by default. func NewResourcesOptions(out io.Writer, errOut io.Writer) *ResourcesOptions { return &ResourcesOptions{ + PrintFlags: printers.NewPrintFlags("resource requirements updated"), + Out: out, Err: errOut, ContainerSelector: "*", @@ -112,7 +122,8 @@ func NewCmdResources(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra. }, } - cmdutil.AddPrinterFlags(cmd) + options.PrintFlags.AddFlags(cmd) + //usage := "Filename, directory, or URL to a file identifying the resource to get from the server" //kubectl.AddJsonFilenameFlag(cmd, &options.Filenames, usage) usage := "identifying the resource to get from a server." @@ -135,6 +146,18 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args o.Record = cmdutil.GetRecordFlag(cmd) o.ChangeCause = f.Command(cmd, false) o.Cmd = cmd + o.DryRun = cmdutil.GetDryRunFlag(o.Cmd) + + if o.DryRun { + o.PrintFlags.Complete("%s (dry run)") + } + printer, err := o.PrintFlags.ToPrinter() + if err != nil { + return err + } + o.PrintObj = func(obj runtime.Object) error { + return printer.PrintObj(obj, o.Out) + } cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { @@ -238,8 +261,8 @@ func (o *ResourcesOptions) Run() error { continue } - if o.Local || cmdutil.GetDryRunFlag(o.Cmd) { - if err := cmdutil.PrintObject(o.Cmd, patch.Info.AsVersioned(), o.Out); err != nil { + if o.Local || o.DryRun { + if err := o.PrintObj(patch.Info.AsVersioned()); err != nil { return err } continue @@ -262,14 +285,9 @@ func (o *ResourcesOptions) Run() error { } info.Refresh(obj, true) - shortOutput := o.Output == "name" - if len(o.Output) > 0 && !shortOutput { - 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(shortOutput, o.Out, info.Object, false, "resource requirements updated") } return utilerrors.NewAggregate(allErrs) } diff --git a/pkg/kubectl/cmd/set/set_resources_test.go b/pkg/kubectl/cmd/set/set_resources_test.go index 2685fb0a3a..457f9c6873 100644 --- a/pkg/kubectl/cmd/set/set_resources_test.go +++ b/pkg/kubectl/cmd/set/set_resources_test.go @@ -25,6 +25,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" @@ -60,14 +62,23 @@ func TestResourcesLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + outputFormat := "name" + buf := bytes.NewBuffer([]byte{}) cmd := NewCmdResources(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "name") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") - opts := ResourcesOptions{FilenameOptions: resource.FilenameOptions{ - Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}}, + opts := ResourcesOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}}, Out: buf, Local: true, Limits: "cpu=200m,memory=512Mi", @@ -105,14 +116,23 @@ func TestSetMultiResourcesLimitsLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + outputFormat := "name" + buf := bytes.NewBuffer([]byte{}) cmd := NewCmdResources(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "name") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") - opts := ResourcesOptions{FilenameOptions: resource.FilenameOptions{ - Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, + opts := ResourcesOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, Out: buf, Local: true, Limits: "cpu=200m,memory=512Mi", @@ -478,11 +498,21 @@ func TestSetResourcesRemote(t *testing.T) { }), VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()), } + + outputFormat := "yaml" + buf := new(bytes.Buffer) cmd := NewCmdResources(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "yaml") + cmd.Flags().Set("output", outputFormat) opts := ResourcesOptions{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + Out: buf, Limits: "cpu=200m,memory=512Mi", ContainerSelector: "*"} diff --git a/pkg/kubectl/cmd/set/set_selector.go b/pkg/kubectl/cmd/set/set_selector.go index 8209162232..367269d6da 100644 --- a/pkg/kubectl/cmd/set/set_selector.go +++ b/pkg/kubectl/cmd/set/set_selector.go @@ -20,6 +20,8 @@ import ( "fmt" "io" + "k8s.io/kubernetes/pkg/printers" + "github.com/spf13/cobra" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -38,6 +40,8 @@ import ( type SelectorOptions struct { fileOptions resource.FilenameOptions + PrintFlags *printers.PrintFlags + local bool dryrun bool all bool @@ -49,9 +53,10 @@ type SelectorOptions struct { selector *metav1.LabelSelector out io.Writer - PrintObject func(obj runtime.Object) error ClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error) + PrintObj func(runtime.Object) error + builder *resource.Builder mapper meta.RESTMapper } @@ -73,6 +78,8 @@ var ( // NewCmdSelector is the "set selector" command. func NewCmdSelector(f cmdutil.Factory, out io.Writer) *cobra.Command { options := &SelectorOptions{ + PrintFlags: printers.NewPrintFlags("selector updated"), + out: out, } @@ -88,7 +95,9 @@ func NewCmdSelector(f cmdutil.Factory, out io.Writer) *cobra.Command { cmdutil.CheckErr(options.RunSelector()) }, } - cmdutil.AddPrinterFlags(cmd) + + options.PrintFlags.AddFlags(cmd) + cmd.Flags().Bool("all", false, "Select all resources, including uninitialized ones, in the namespace of the specified resource types") cmd.Flags().Bool("local", false, "If true, set selector will NOT contact api-server but run locally.") cmd.Flags().String("resource-version", "", "If non-empty, the selectors update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.") @@ -146,9 +155,17 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ } } - o.PrintObject = func(obj runtime.Object) error { - return cmdutil.PrintObject(cmd, obj, o.out) + if o.dryrun { + o.PrintFlags.Complete("%s (dry run)") } + printer, err := o.PrintFlags.ToPrinter() + if err != nil { + return err + } + o.PrintObj = func(obj runtime.Object) error { + return printer.PrintObj(obj, o.out) + } + o.ClientForMapping = func(mapping *meta.RESTMapping) (resource.RESTClient, error) { return f.ClientForMapping(mapping) } @@ -191,7 +208,7 @@ func (o *SelectorOptions) RunSelector() error { return patch.Err } if o.local || o.dryrun { - return o.PrintObject(info.Object) + return o.PrintObj(info.Object) } patched, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch.Patch) @@ -208,13 +225,7 @@ func (o *SelectorOptions) RunSelector() error { } info.Refresh(patched, true) - - shortOutput := o.output == "name" - if len(o.output) > 0 && !shortOutput { - return o.PrintObject(patched) - } - cmdutil.PrintSuccess(shortOutput, o.out, info.Object, o.dryrun, "selector updated") - return nil + return o.PrintObj(patch.Info.AsVersioned()) }) } diff --git a/pkg/kubectl/cmd/set/set_serviceaccount.go b/pkg/kubectl/cmd/set/set_serviceaccount.go index e374267bfd..954b6b6c17 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount.go @@ -21,6 +21,8 @@ import ( "fmt" "io" + "k8s.io/kubernetes/pkg/printers" + "github.com/spf13/cobra" "k8s.io/api/core/v1" @@ -54,6 +56,8 @@ var ( // serviceAccountConfig encapsulates the data required to perform the operation. type serviceAccountConfig struct { + PrintFlags *printers.PrintFlags + fileNameOptions resource.FilenameOptions out io.Writer err io.Writer @@ -68,11 +72,15 @@ type serviceAccountConfig struct { updatePodSpecForObject func(runtime.Object, func(*v1.PodSpec) error) (bool, error) infos []*resource.Info serviceAccountName string + + PrintObj func(runtime.Object) error } // NewCmdServiceAccount returns the "set serviceaccount" command. func NewCmdServiceAccount(f cmdutil.Factory, out, err io.Writer) *cobra.Command { saConfig := &serviceAccountConfig{ + PrintFlags: printers.NewPrintFlags("serviceaccount updated"), + out: out, err: err, } @@ -89,7 +97,8 @@ func NewCmdServiceAccount(f cmdutil.Factory, out, err io.Writer) *cobra.Command cmdutil.CheckErr(saConfig.Run()) }, } - cmdutil.AddPrinterFlags(cmd) + + saConfig.PrintFlags.AddFlags(cmd) usage := "identifying the resource to get from a server." cmdutil.AddFilenameOptionFlags(cmd, &saConfig.fileNameOptions, usage) @@ -111,6 +120,17 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com saConfig.updatePodSpecForObject = f.UpdatePodSpecForObject saConfig.cmd = cmd + if saConfig.dryRun { + saConfig.PrintFlags.Complete("%s (dry run)") + } + printer, err := saConfig.PrintFlags.ToPrinter() + if err != nil { + return err + } + saConfig.PrintObj = func(obj runtime.Object) error { + return printer.PrintObj(obj, saConfig.out) + } + cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err @@ -151,6 +171,7 @@ func (saConfig *serviceAccountConfig) Run() error { }) return runtime.Encode(cmdutil.InternalVersionJSONEncoder(), info.Object) } + patches := CalculatePatches(saConfig.infos, cmdutil.InternalVersionJSONEncoder(), patchFn) for _, patch := range patches { info := patch.Info @@ -159,7 +180,7 @@ func (saConfig *serviceAccountConfig) Run() error { continue } if saConfig.local || saConfig.dryRun { - if err := cmdutil.PrintObject(saConfig.cmd, patch.Info.AsVersioned(), saConfig.out); err != nil { + if err := saConfig.PrintObj(patch.Info.AsVersioned()); err != nil { return err } continue @@ -177,13 +198,10 @@ func (saConfig *serviceAccountConfig) Run() error { } } } - if len(saConfig.output) > 0 { - if err := cmdutil.PrintObject(saConfig.cmd, info.AsVersioned(), saConfig.out); err != nil { - return err - } - continue + + if err := saConfig.PrintObj(info.AsVersioned()); err != nil { + return err } - cmdutil.PrintSuccess(saConfig.shortOutput, saConfig.out, info.Object, saConfig.dryRun, "serviceaccount updated") } return utilerrors.NewAggregate(patchErrs) } diff --git a/pkg/kubectl/cmd/set/set_serviceaccount_test.go b/pkg/kubectl/cmd/set/set_serviceaccount_test.go index 90bf81b87f..a4d3ffa074 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount_test.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount_test.go @@ -25,6 +25,8 @@ import ( "path" "testing" + "k8s.io/kubernetes/pkg/printers" + "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" appsv1beta1 "k8s.io/api/apps/v1beta1" @@ -77,15 +79,25 @@ func TestSetServiceAccountLocal(t *testing.T) { return nil, nil }), } + + outputFormat := "yaml" + tf.Namespace = "test" out := new(bytes.Buffer) cmd := NewCmdServiceAccount(tf, out, out) cmd.SetOutput(out) - cmd.Flags().Set("output", "yaml") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") testapi.Default = testapi.Groups[input.apiGroup] - saConfig := serviceAccountConfig{fileNameOptions: resource.FilenameOptions{ - Filenames: []string{input.yaml}}, + saConfig := serviceAccountConfig{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + fileNameOptions: resource.FilenameOptions{ + Filenames: []string{input.yaml}}, out: out, local: true} err := saConfig.Complete(tf, cmd, []string{serviceAccount}) @@ -114,13 +126,22 @@ func TestSetServiceAccountMultiLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} + outputFormat := "name" + buf := bytes.NewBuffer([]byte{}) cmd := NewCmdServiceAccount(tf, buf, buf) cmd.SetOutput(buf) - cmd.Flags().Set("output", "name") + cmd.Flags().Set("output", outputFormat) cmd.Flags().Set("local", "true") - opts := serviceAccountConfig{fileNameOptions: resource.FilenameOptions{ - Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, + opts := serviceAccountConfig{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + fileNameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"}}, out: buf, local: true} @@ -349,11 +370,21 @@ func TestSetServiceAccountRemote(t *testing.T) { }), VersionedAPIPath: path.Join(input.apiPrefix, testapi.Default.GroupVersion().String()), } + + outputFormat := "yaml" + out := new(bytes.Buffer) cmd := NewCmdServiceAccount(tf, out, out) cmd.SetOutput(out) - cmd.Flags().Set("output", "yaml") + cmd.Flags().Set("output", outputFormat) saConfig := serviceAccountConfig{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + out: out, local: false} err := saConfig.Complete(tf, cmd, input.args) @@ -385,12 +416,22 @@ func TestServiceAccountValidation(t *testing.T) { return nil, nil }), } + + outputFormat := "" + tf.Namespace = "test" out := bytes.NewBuffer([]byte{}) cmd := NewCmdServiceAccount(tf, out, out) cmd.SetOutput(out) - saConfig := &serviceAccountConfig{} + saConfig := &serviceAccountConfig{ + PrintFlags: &printers.PrintFlags{ + JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), + NamePrintFlags: printers.NewNamePrintFlags(""), + + OutputFormat: &outputFormat, + }, + } err := saConfig.Complete(tf, cmd, input.args) assert.EqualError(t, err, input.errorString) }) diff --git a/pkg/kubectl/cmd/set/set_subject.go b/pkg/kubectl/cmd/set/set_subject.go index 056aac9094..ea0b139269 100644 --- a/pkg/kubectl/cmd/set/set_subject.go +++ b/pkg/kubectl/cmd/set/set_subject.go @@ -21,6 +21,8 @@ import ( "io" "strings" + "k8s.io/kubernetes/pkg/printers" + "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/runtime" @@ -54,6 +56,8 @@ type updateSubjects func(existings []rbac.Subject, targets []rbac.Subject) (bool // SubjectOptions is the start of the data required to perform the operation. As new fields are added, add them here instead of // referencing the cmd.Flags type SubjectOptions struct { + PrintFlags *printers.PrintFlags + resource.FilenameOptions Infos []*resource.Info @@ -70,11 +74,13 @@ type SubjectOptions struct { Groups []string ServiceAccounts []string - PrintObject func(obj runtime.Object, out io.Writer) error + PrintObj func(obj runtime.Object) error } func NewCmdSubject(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra.Command { options := &SubjectOptions{ + PrintFlags: printers.NewPrintFlags("subjects updated"), + Out: out, Err: errOut, } @@ -92,7 +98,8 @@ func NewCmdSubject(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra.Co }, } - cmdutil.AddPrinterFlags(cmd) + options.PrintFlags.AddFlags(cmd) + usage := "the resource to update the subjects" cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) cmd.Flags().BoolVar(&options.All, "all", options.All, "Select all resources, including uninitialized ones, in the namespace of the specified resource types") @@ -109,8 +116,16 @@ func NewCmdSubject(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra.Co func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { o.Output = cmdutil.GetFlagString(cmd, "output") o.DryRun = cmdutil.GetDryRunFlag(cmd) - o.PrintObject = func(obj runtime.Object, out io.Writer) error { - return cmdutil.PrintObject(cmd, obj, out) + + if o.DryRun { + o.PrintFlags.Complete("%s (dry run)") + } + printer, err := o.PrintFlags.ToPrinter() + if err != nil { + return err + } + o.PrintObj = func(obj runtime.Object) error { + return printer.PrintObj(obj, o.Out) } cmdNamespace, enforceNamespace, err := f.DefaultNamespace() @@ -236,7 +251,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { } if o.Local || o.DryRun { - if err := o.PrintObject(info.Object, o.Out); err != nil { + if err := o.PrintObj(info.Object); err != nil { return err } continue @@ -249,11 +264,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { } info.Refresh(obj, true) - shortOutput := o.Output == "name" - if len(o.Output) > 0 && !shortOutput { - return o.PrintObject(info.AsVersioned(), o.Out) - } - cmdutil.PrintSuccess(shortOutput, o.Out, info.Object, false, "subjects updated") + return o.PrintObj(info.AsVersioned()) } return utilerrors.NewAggregate(allErrs) } diff --git a/pkg/printers/flags.go b/pkg/printers/flags.go index e0eea99141..8519b85bfa 100644 --- a/pkg/printers/flags.go +++ b/pkg/printers/flags.go @@ -45,8 +45,8 @@ type PrintFlags struct { OutputFormat *string } -func (f *PrintFlags) Complete(dryRun bool) error { - f.NamePrintFlags.DryRun = dryRun +func (f *PrintFlags) Complete(messageTemplate string) error { + f.NamePrintFlags.Operation = fmt.Sprintf(messageTemplate, f.NamePrintFlags.Operation) return nil } @@ -80,6 +80,6 @@ func NewPrintFlags(operation string) *PrintFlags { OutputFormat: &outputFormat, JSONYamlPrintFlags: NewJSONYamlPrintFlags(), - NamePrintFlags: NewNamePrintFlags(operation, false), + NamePrintFlags: NewNamePrintFlags(operation), } } diff --git a/pkg/printers/name_flags.go b/pkg/printers/name_flags.go index 24b0f4aefe..63553c3d0d 100644 --- a/pkg/printers/name_flags.go +++ b/pkg/printers/name_flags.go @@ -31,10 +31,6 @@ import ( // a resource's fully-qualified Kind.group/name, or a successful // message about that resource if an Operation is provided. type NamePrintFlags 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 // Operation describes the name of the action that // took place on an object, to be included in the // finalized "successful" message. @@ -53,10 +49,6 @@ func (f *NamePrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, error) Decoders: decoders, } - if f.DryRun { - namePrinter.Operation = namePrinter.Operation + " (dry run)" - } - outputFormat = strings.ToLower(outputFormat) switch outputFormat { case "name": @@ -75,9 +67,8 @@ func (f *NamePrintFlags) AddFlags(c *cobra.Command) {} // NewNamePrintFlags returns flags associated with // --name printing, with default values set. -func NewNamePrintFlags(operation string, dryRun bool) *NamePrintFlags { +func NewNamePrintFlags(operation string) *NamePrintFlags { return &NamePrintFlags{ Operation: operation, - DryRun: dryRun, } } diff --git a/pkg/printers/name_flags_test.go b/pkg/printers/name_flags_test.go index 6c06d0c035..ed8d7529f6 100644 --- a/pkg/printers/name_flags_test.go +++ b/pkg/printers/name_flags_test.go @@ -49,12 +49,6 @@ func TestNamePrinterSupportsExpectedFormats(t *testing.T) { operation: "patched", expectedOutput: "pod/foo", }, - { - name: "empty output format and an operation prints success message with dry run", - operation: "patched", - dryRun: true, - expectedOutput: "pod/foo patched (dry run)", - }, { name: "operation and no valid \"name\" output does not match a printer", operation: "patched", @@ -83,7 +77,6 @@ func TestNamePrinterSupportsExpectedFormats(t *testing.T) { t.Run(tc.name, func(t *testing.T) { printFlags := printers.NamePrintFlags{ Operation: tc.operation, - DryRun: tc.dryRun, } p, err := printFlags.ToPrinter(tc.outputFormat) diff --git a/pkg/printers/printers.go b/pkg/printers/printers.go index e6791124e7..86f5236185 100644 --- a/pkg/printers/printers.go +++ b/pkg/printers/printers.go @@ -42,7 +42,7 @@ func GetStandardPrinter(typer runtime.ObjectTyper, encoder runtime.Encoder, deco printer = p case "name": - nameFlags := NewNamePrintFlags("", false) + nameFlags := NewNamePrintFlags("") namePrinter, err := nameFlags.ToPrinter(format) if err != nil { return nil, err