diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index e55a3fa2768..4a00c074a29 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 b05bf7562dc..2495f6394fa 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 0ae603ad888..c6fe5a2e5aa 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/pkg/kubectl/cmd/set/set_image.go @@ -127,7 +127,9 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st o.ResolveImage = f.ResolveImage o.Cmd = cmd - o.PrintFlags.Complete(o.DryRun) + if o.DryRun { + o.PrintFlags.Complete("%s (dry run)") + } printer, err := o.PrintFlags.ToPrinter() if err != nil { @@ -250,6 +252,11 @@ func (o *ImageOptions) Run() error { continue } + // no changes + if string(patch.Patch) == "{}" || len(patch.Patch) == 0 { + continue + } + if o.Local || o.DryRun { if err := o.PrintObj(patch.Info.AsVersioned()); err != nil { return err @@ -257,11 +264,6 @@ func (o *ImageOptions) Run() error { continue } - // no changes - if string(patch.Patch) == "{}" || len(patch.Patch) == 0 { - continue - } - // patch the change obj, err := resource.NewHelper(info.Client, info.Mapping).Patch(info.Namespace, info.Name, types.StrategicMergePatchType, patch.Patch) if err != nil { diff --git a/pkg/kubectl/cmd/set/set_image_test.go b/pkg/kubectl/cmd/set/set_image_test.go index b71047493bf..45764a41b33 100644 --- a/pkg/kubectl/cmd/set/set_image_test.go +++ b/pkg/kubectl/cmd/set/set_image_test.go @@ -74,7 +74,7 @@ func TestImageLocal(t *testing.T) { opts := ImageOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -100,7 +100,7 @@ func TestImageLocal(t *testing.T) { func TestSetImageValidation(t *testing.T) { printFlags := &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), } testCases := []struct { @@ -196,7 +196,7 @@ func TestSetMultiResourcesImageLocal(t *testing.T) { opts := ImageOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -590,7 +590,7 @@ func TestSetImageRemote(t *testing.T) { opts := ImageOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index 36e8324edf8..812740ad693 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -148,13 +148,13 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args o.Cmd = cmd o.DryRun = cmdutil.GetDryRunFlag(o.Cmd) - o.PrintFlags.Complete(o.DryRun) - + 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) } diff --git a/pkg/kubectl/cmd/set/set_resources_test.go b/pkg/kubectl/cmd/set/set_resources_test.go index ad0d235ccde..457f9c68731 100644 --- a/pkg/kubectl/cmd/set/set_resources_test.go +++ b/pkg/kubectl/cmd/set/set_resources_test.go @@ -73,7 +73,7 @@ func TestResourcesLocal(t *testing.T) { opts := ResourcesOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -127,7 +127,7 @@ func TestSetMultiResourcesLimitsLocal(t *testing.T) { opts := ResourcesOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -508,7 +508,7 @@ func TestSetResourcesRemote(t *testing.T) { opts := ResourcesOptions{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, diff --git a/pkg/kubectl/cmd/set/set_selector.go b/pkg/kubectl/cmd/set/set_selector.go index 6b9dffd06a5..367269d6da1 100644 --- a/pkg/kubectl/cmd/set/set_selector.go +++ b/pkg/kubectl/cmd/set/set_selector.go @@ -53,7 +53,6 @@ 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 @@ -119,8 +118,6 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ o.dryrun = cmdutil.GetDryRunFlag(cmd) o.output = cmdutil.GetFlagString(cmd, "output") - o.PrintFlags.Complete(o.dryrun) - cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err @@ -158,14 +155,17 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ } } + if o.dryrun { + o.PrintFlags.Complete("%s (dry run)") + } printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } - - o.PrintObject = func(obj runtime.Object) error { + 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) } @@ -208,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) @@ -225,7 +225,7 @@ func (o *SelectorOptions) RunSelector() error { } info.Refresh(patched, true) - return o.PrintObject(patch.Info.AsVersioned()) + 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 9184d0494b2..954b6b6c17e 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount.go @@ -120,13 +120,13 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com saConfig.updatePodSpecForObject = f.UpdatePodSpecForObject saConfig.cmd = cmd - saConfig.PrintFlags.Complete(saConfig.dryRun) - + 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) } diff --git a/pkg/kubectl/cmd/set/set_serviceaccount_test.go b/pkg/kubectl/cmd/set/set_serviceaccount_test.go index b64d2fd6192..a4d3ffa074e 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount_test.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount_test.go @@ -92,7 +92,7 @@ func TestSetServiceAccountLocal(t *testing.T) { saConfig := serviceAccountConfig{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -136,7 +136,7 @@ func TestSetServiceAccountMultiLocal(t *testing.T) { opts := serviceAccountConfig{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -380,7 +380,7 @@ func TestSetServiceAccountRemote(t *testing.T) { saConfig := serviceAccountConfig{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, @@ -427,7 +427,7 @@ func TestServiceAccountValidation(t *testing.T) { saConfig := &serviceAccountConfig{ PrintFlags: &printers.PrintFlags{ JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(), - NamePrintFlags: printers.NewNamePrintFlags("", false), + NamePrintFlags: printers.NewNamePrintFlags(""), OutputFormat: &outputFormat, }, diff --git a/pkg/kubectl/cmd/set/set_subject.go b/pkg/kubectl/cmd/set/set_subject.go index effafb35514..ea0b1392692 100644 --- a/pkg/kubectl/cmd/set/set_subject.go +++ b/pkg/kubectl/cmd/set/set_subject.go @@ -74,7 +74,7 @@ type SubjectOptions struct { Groups []string ServiceAccounts []string - PrintObject func(obj runtime.Object) error + PrintObj func(obj runtime.Object) error } func NewCmdSubject(f cmdutil.Factory, out io.Writer, errOut io.Writer) *cobra.Command { @@ -117,14 +117,14 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] o.Output = cmdutil.GetFlagString(cmd, "output") o.DryRun = cmdutil.GetDryRunFlag(cmd) - o.PrintFlags.Complete(o.DryRun) - + if o.DryRun { + o.PrintFlags.Complete("%s (dry run)") + } printer, err := o.PrintFlags.ToPrinter() if err != nil { return err } - - o.PrintObject = func(obj runtime.Object) error { + o.PrintObj = func(obj runtime.Object) error { return printer.PrintObj(obj, o.Out) } @@ -251,7 +251,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { } if o.Local || o.DryRun { - if err := o.PrintObject(info.Object); err != nil { + if err := o.PrintObj(info.Object); err != nil { return err } continue @@ -264,7 +264,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error { } info.Refresh(obj, true) - return o.PrintObject(info.AsVersioned()) + return o.PrintObj(info.AsVersioned()) } return utilerrors.NewAggregate(allErrs) } diff --git a/pkg/printers/flags.go b/pkg/printers/flags.go index e0eea991414..8519b85bfa5 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 24b0f4aefe1..63553c3d0d9 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 6c06d0c0357..ed8d7529f63 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 e6791124e75..86f52361854 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