Merge pull request #34199 from asalkeld/annotate-label

Automatic merge from submit-queue

Make kubectl label and annotate more consistent

**What this PR does / why we need it**:
This makes the label and annotate cmd files more consistent which should help with code maintenance.

Some of the main changes:
- add dryrun to annotate (can push this in a different PR if requested)
- use Complete(), Validate() and RunX()
- don't place dynamic variables in the options (only user options and args)
- call the NewBuilder() in the Run function.

**Which issue this PR fixes** 
fixes #34151

**Special notes for your reviewer**:
Note: you *can* now diff the two files and the changes make sense.

**Release note**:
```release-note
kubectl annotate now supports --dry-run
```

pkg/kubectl/cmd/annotate.go

@@ -36,25 +36,26 @@ import (
 
 // AnnotateOptions have the data required to perform the annotate operation
 type AnnotateOptions struct {
+	// Filename options
 	resource.FilenameOptions
 
+	// Common user flags
+	overwrite         bool
+	local             bool
+	dryrun            bool
+	all               bool
+	resourceVersion   string
+	selector          string
+	outputFormat      string
+	recordChangeCause bool
+
+	// results of arg parsing
 	resources         []string
 	newAnnotations    map[string]string
 	removeAnnotations []string
-	builder           *resource.Builder
-	selector          string
 
-	overwrite       bool
+	// Common shared fields
-	local           bool
-	all             bool
-	resourceVersion string
-
-	changeCause       string
-	recordChangeCause bool
-
-	f   *cmdutil.Factory
 	out io.Writer
-	cmd *cobra.Command
 }
 
 var (
@@ -93,6 +94,7 @@ var (
 func NewCmdAnnotate(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	options := &AnnotateOptions{}
 
+	// retrieve a list of handled resources from printer as valid args
 	validArgs, argAliases := []string{}, []string{}
 	p, err := f.Printer(nil, kubectl.PrintOptions{
 		ColumnLabels: []string{},
@@ -110,38 +112,42 @@ func NewCmdAnnotate(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Example: annotate_example,
 		Run: func(cmd *cobra.Command, args []string) {
 			if err := options.Complete(f, out, cmd, args); err != nil {
-				cmdutil.CheckErr(err)
-			}
-			if err := options.Validate(args); err != nil {
 				cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error()))
 			}
-			if err := options.RunAnnotate(); err != nil {
+			if err := options.Validate(); err != nil {
-				cmdutil.CheckErr(err)
+				cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error()))
 			}
+			cmdutil.CheckErr(options.RunAnnotate(f, cmd))
 		},
 		ValidArgs:  validArgs,
 		ArgAliases: argAliases,
 	}
 	cmdutil.AddPrinterFlags(cmd)
-	cmdutil.AddInclude3rdPartyFlags(cmd)
+	cmd.Flags().Bool("overwrite", false, "If true, allow annotations to be overwritten, otherwise reject annotation updates that overwrite existing annotations.")
-	cmd.Flags().StringVarP(&options.selector, "selector", "l", "", "Selector (label query) to filter on")
+	cmd.Flags().Bool("local", false, "If true, annotation will NOT contact api-server but run locally.")
-	cmd.Flags().BoolVar(&options.overwrite, "overwrite", false, "If true, allow annotations to be overwritten, otherwise reject annotation updates that overwrite existing annotations.")
+	cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on")
-	cmd.Flags().BoolVar(&options.local, "local", false, "If true, annotation will NOT contact api-server but run locally.")
+	cmd.Flags().Bool("all", false, "select all resources in the namespace of the specified resource types")
-	cmd.Flags().BoolVar(&options.all, "all", false, "select all resources in the namespace of the specified resource types")
+	cmd.Flags().String("resource-version", "", "If non-empty, the annotation update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.")
-	cmd.Flags().StringVar(&options.resourceVersion, "resource-version", "", "If non-empty, the annotation update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.")
 	usage := "identifying the resource to update the annotation"
 	cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
+	cmdutil.AddDryRunFlag(cmd)
 	cmdutil.AddRecordFlag(cmd)
+	cmdutil.AddInclude3rdPartyFlags(cmd)
+
 	return cmd
 }
 
 // Complete adapts from the command line args and factory to the data required.
 func (o *AnnotateOptions) Complete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string) (err error) {
-
+	o.out = out
-	namespace, enforceNamespace, err := f.DefaultNamespace()
+	o.local = cmdutil.GetFlagBool(cmd, "local")
-	if err != nil {
+	o.overwrite = cmdutil.GetFlagBool(cmd, "overwrite")
-		return err
+	o.all = cmdutil.GetFlagBool(cmd, "all")
-	}
+	o.resourceVersion = cmdutil.GetFlagString(cmd, "resource-version")
+	o.selector = cmdutil.GetFlagString(cmd, "selector")
+	o.outputFormat = cmdutil.GetFlagString(cmd, "output")
+	o.dryrun = cmdutil.GetDryRunFlag(cmd)
+	o.recordChangeCause = cmdutil.GetRecordFlag(cmd)
 
 	// retrieves resource and annotation args from args
 	// also checks args to verify that all resources are specified before annotations
@@ -150,47 +156,42 @@ func (o *AnnotateOptions) Complete(f *cmdutil.Factory, out io.Writer, cmd *cobra
 		return err
 	}
 	o.resources = resources
-	if len(o.resources) < 1 && cmdutil.IsFilenameEmpty(o.Filenames) {
+	o.newAnnotations, o.removeAnnotations, err = parseAnnotations(annotationArgs)
-		return fmt.Errorf("one or more resources must be specified as <resource> <name> or <resource>/<name>")
-	}
-	if len(annotationArgs) < 1 {
-		return fmt.Errorf("at least one annotation update is required")
-	}
-
-	if o.newAnnotations, o.removeAnnotations, err = parseAnnotations(annotationArgs); err != nil {
 	return err
-	}
-
-	o.recordChangeCause = cmdutil.GetRecordFlag(cmd)
-	o.changeCause = f.Command()
-
-	mapper, typer := f.Object()
-	o.builder = resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
-		ContinueOnError().
-		NamespaceParam(namespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, &o.FilenameOptions).
-		Flatten()
-	if !o.local {
-		o.builder = o.builder.SelectorParam(o.selector).
-			ResourceTypeOrNameArgs(o.all, o.resources...).
-			Latest()
-	}
-
-	o.f = f
-	o.out = out
-	o.cmd = cmd
-
-	return nil
 }
 
 // Validate checks to the AnnotateOptions to see if there is sufficient information run the command.
-func (o AnnotateOptions) Validate(args []string) error {
+func (o AnnotateOptions) Validate() error {
+	if len(o.resources) < 1 && cmdutil.IsFilenameEmpty(o.Filenames) {
+		return fmt.Errorf("one or more resources must be specified as <resource> <name> or <resource>/<name>")
+	}
+	if len(o.newAnnotations) < 1 && len(o.removeAnnotations) < 1 {
+		return fmt.Errorf("at least one annotation update is required")
+	}
 	return validateAnnotations(o.removeAnnotations, o.newAnnotations)
 }
 
 // RunAnnotate does the work
-func (o AnnotateOptions) RunAnnotate() error {
+func (o AnnotateOptions) RunAnnotate(f *cmdutil.Factory, cmd *cobra.Command) error {
-	r := o.builder.Do()
+	namespace, enforceNamespace, err := f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+
+	changeCause := f.Command()
+	mapper, typer := f.Object()
+	b := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+		ContinueOnError().
+		NamespaceParam(namespace).DefaultNamespace().
+		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		Flatten()
+
+	if !o.local {
+		b = b.SelectorParam(o.selector).
+			ResourceTypeOrNameArgs(o.all, o.resources...).
+			Latest()
+	}
+	r := b.Do()
 	if err := r.Err(); err != nil {
 		return err
 	}
@@ -216,7 +217,7 @@ func (o AnnotateOptions) RunAnnotate() error {
 		if err != nil {
 			return err
 		}
-		if o.local {
+		if o.dryrun || o.local {
 			if err := o.updateAnnotations(obj); err != nil {
 				return err
 			}
@@ -229,7 +230,7 @@ func (o AnnotateOptions) RunAnnotate() error {
 			}
 			// If we should record change-cause, add it to new annotations
 			if cmdutil.ContainsChangeCause(info) || o.recordChangeCause {
-				o.newAnnotations[kubectl.ChangeCauseAnnotation] = o.changeCause
+				o.newAnnotations[kubectl.ChangeCauseAnnotation] = changeCause
 			}
 			if err := o.updateAnnotations(obj); err != nil {
 				return err
@@ -245,7 +246,7 @@ func (o AnnotateOptions) RunAnnotate() error {
 			}
 
 			mapping := info.ResourceMapping()
-			client, err := o.f.ClientForMapping(mapping)
+			client, err := f.ClientForMapping(mapping)
 			if err != nil {
 				return err
 			}
@@ -259,15 +260,11 @@ func (o AnnotateOptions) RunAnnotate() error {
 			if err != nil {
 				return err
 			}
-
 		}
-		mapper, _ := o.f.Object()
+		if o.outputFormat != "" {
-		outputFormat := cmdutil.GetFlagString(o.cmd, "output")
+			return f.PrintObject(cmd, mapper, outputObj, o.out)
-		if outputFormat != "" {
-			return o.f.PrintObject(o.cmd, mapper, outputObj, o.out)
 		}
-
+		cmdutil.PrintSuccess(mapper, false, o.out, info.Mapping.Resource, info.Name, o.dryrun, "annotated")
-		cmdutil.PrintSuccess(mapper, false, o.out, info.Mapping.Resource, info.Name, false, "annotated")
 		return nil
 	})
 }

pkg/kubectl/cmd/annotate_test.go

@@ -404,6 +404,9 @@ func TestAnnotateErrors(t *testing.T) {
 		}
 		options := &AnnotateOptions{}
 		err := options.Complete(f, buf, cmd, testCase.args)
+		if err == nil {
+			err = options.Validate()
+		}
 		if !testCase.errFn(err) {
 			t.Errorf("%s: unexpected error: %v", k, err)
 			continue
@@ -459,10 +462,10 @@ func TestAnnotateObject(t *testing.T) {
 	if err := options.Complete(f, buf, cmd, args); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.Validate(args); err != nil {
+	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(); err != nil {
+	if err := options.RunAnnotate(f, cmd); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -510,10 +513,10 @@ func TestAnnotateObjectFromFile(t *testing.T) {
 	if err := options.Complete(f, buf, cmd, args); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.Validate(args); err != nil {
+	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(); err != nil {
+	if err := options.RunAnnotate(f, cmd); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -532,16 +535,17 @@ func TestAnnotateLocal(t *testing.T) {
 
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdAnnotate(f, buf)
-	options := &AnnotateOptions{local: true}
+	cmd.Flags().Set("local", "true")
+	options := &AnnotateOptions{}
 	options.Filenames = []string{"../../../examples/storage/cassandra/cassandra-controller.yaml"}
 	args := []string{"a=b"}
 	if err := options.Complete(f, buf, cmd, args); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.Validate(args); err != nil {
+	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(); err != nil {
+	if err := options.RunAnnotate(f, cmd); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -585,16 +589,16 @@ func TestAnnotateMultipleObjects(t *testing.T) {
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdAnnotate(f, buf)
 	cmd.SetOutput(buf)
+	cmd.Flags().Set("all", "true")
 	options := &AnnotateOptions{}
-	options.all = true
 	args := []string{"pods", "a=b", "c-"}
 	if err := options.Complete(f, buf, cmd, args); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.Validate(args); err != nil {
+	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(); err != nil {
+	if err := options.RunAnnotate(f, cmd); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }

pkg/kubectl/cmd/label.go

@@ -37,6 +37,29 @@ import (
 	"k8s.io/kubernetes/pkg/util/validation"
 )
 
+// LabelOptions have the data required to perform the label operation
+type LabelOptions struct {
+	// Filename options
+	resource.FilenameOptions
+
+	// Common user flags
+	overwrite       bool
+	local           bool
+	dryrun          bool
+	all             bool
+	resourceVersion string
+	selector        string
+	outputFormat    string
+
+	// results of arg parsing
+	resources    []string
+	newLabels    map[string]string
+	removeLabels []string
+
+	// Common shared fields
+	out io.Writer
+}
+
 var (
 	label_long = dedent.Dedent(`
 		Update the labels on a resource.
@@ -66,7 +89,7 @@ var (
 )
 
 func NewCmdLabel(f *cmdutil.Factory, out io.Writer) *cobra.Command {
-	options := &resource.FilenameOptions{}
+	options := &LabelOptions{}
 
 	// retrieve a list of handled resources from printer as valid args
 	validArgs, argAliases := []string{}, []string{}
@@ -85,8 +108,13 @@ func NewCmdLabel(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 		Long:    fmt.Sprintf(label_long, validation.LabelValueMaxLength),
 		Example: label_example,
 		Run: func(cmd *cobra.Command, args []string) {
-			err := RunLabel(f, out, cmd, args, options)
+			if err := options.Complete(f, out, cmd, args); err != nil {
-			cmdutil.CheckErr(err)
+				cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error()))
+			}
+			if err := options.Validate(); err != nil {
+				cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error()))
+			}
+			cmdutil.CheckErr(options.RunLabel(f, cmd))
 		},
 		ValidArgs:  validArgs,
 		ArgAliases: argAliases,
@@ -98,7 +126,7 @@ func NewCmdLabel(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().Bool("all", false, "select all resources in the namespace of the specified resource types")
 	cmd.Flags().String("resource-version", "", "If non-empty, the labels update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.")
 	usage := "identifying the resource to update the labels"
-	cmdutil.AddFilenameOptionFlags(cmd, options, usage)
+	cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
 	cmdutil.AddDryRunFlag(cmd)
 	cmdutil.AddRecordFlag(cmd)
 	cmdutil.AddInclude3rdPartyFlags(cmd)
@@ -106,6 +134,147 @@ func NewCmdLabel(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	return cmd
 }
 
+// Complete adapts from the command line args and factory to the data required.
+func (o *LabelOptions) Complete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string) (err error) {
+	o.out = out
+	o.local = cmdutil.GetFlagBool(cmd, "local")
+	o.overwrite = cmdutil.GetFlagBool(cmd, "overwrite")
+	o.all = cmdutil.GetFlagBool(cmd, "all")
+	o.resourceVersion = cmdutil.GetFlagString(cmd, "resource-version")
+	o.selector = cmdutil.GetFlagString(cmd, "selector")
+	o.outputFormat = cmdutil.GetFlagString(cmd, "output")
+	o.dryrun = cmdutil.GetDryRunFlag(cmd)
+
+	resources, labelArgs, err := cmdutil.GetResourcesAndPairs(args, "label")
+	if err != nil {
+		return err
+	}
+	o.resources = resources
+	o.newLabels, o.removeLabels, err = parseLabels(labelArgs)
+	return err
+}
+
+// Validate checks to the LabelOptions to see if there is sufficient information run the command.
+func (o *LabelOptions) Validate() error {
+	if len(o.resources) < 1 && cmdutil.IsFilenameEmpty(o.FilenameOptions.Filenames) {
+		return fmt.Errorf("one or more resources must be specified as <resource> <name> or <resource>/<name>")
+	}
+	if len(o.newLabels) < 1 && len(o.removeLabels) < 1 {
+		return fmt.Errorf("at least one label update is required")
+	}
+	return nil
+}
+
+// RunLabel does the work
+func (o *LabelOptions) RunLabel(f *cmdutil.Factory, cmd *cobra.Command) error {
+	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+
+	changeCause := f.Command()
+	mapper, typer := f.Object()
+	b := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+		ContinueOnError().
+		NamespaceParam(cmdNamespace).DefaultNamespace().
+		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		Flatten()
+
+	if !o.local {
+		b = b.SelectorParam(o.selector).
+			ResourceTypeOrNameArgs(o.all, o.resources...).
+			Latest()
+	}
+	one := false
+	r := b.Do().IntoSingular(&one)
+	if err := r.Err(); err != nil {
+		return err
+	}
+
+	// only apply resource version locking on a single resource
+	if !one && len(o.resourceVersion) > 0 {
+		return fmt.Errorf("--resource-version may only be used with a single resource")
+	}
+
+	// TODO: support bulk generic output a la Get
+	return r.Visit(func(info *resource.Info, err error) error {
+		if err != nil {
+			return err
+		}
+
+		var outputObj runtime.Object
+		dataChangeMsg := "not labeled"
+		if o.dryrun || o.local {
+			err = labelFunc(info.Object, o.overwrite, o.resourceVersion, o.newLabels, o.removeLabels)
+			if err != nil {
+				return err
+			}
+			outputObj = info.Object
+		} else {
+			obj, err := cmdutil.MaybeConvertObject(info.Object, info.Mapping.GroupVersionKind.GroupVersion(), info.Mapping)
+			if err != nil {
+				return err
+			}
+			name, namespace := info.Name, info.Namespace
+			oldData, err := json.Marshal(obj)
+			if err != nil {
+				return err
+			}
+			accessor, err := meta.Accessor(obj)
+			if err != nil {
+				return err
+			}
+			for _, label := range o.removeLabels {
+				if _, ok := accessor.GetLabels()[label]; !ok {
+					fmt.Fprintf(o.out, "label %q not found.\n", label)
+				}
+			}
+
+			if err := labelFunc(obj, o.overwrite, o.resourceVersion, o.newLabels, o.removeLabels); err != nil {
+				return err
+			}
+			if cmdutil.ShouldRecord(cmd, info) {
+				if err := cmdutil.RecordChangeCause(obj, changeCause); err != nil {
+					return err
+				}
+			}
+			newData, err := json.Marshal(obj)
+			if err != nil {
+				return err
+			}
+			if !reflect.DeepEqual(oldData, newData) {
+				dataChangeMsg = "labeled"
+			}
+			patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj)
+			createdPatch := err == nil
+			if err != nil {
+				glog.V(2).Infof("couldn't compute patch: %v", err)
+			}
+
+			mapping := info.ResourceMapping()
+			client, err := f.ClientForMapping(mapping)
+			if err != nil {
+				return err
+			}
+			helper := resource.NewHelper(client, mapping)
+
+			if createdPatch {
+				outputObj, err = helper.Patch(namespace, name, api.StrategicMergePatchType, patchBytes)
+			} else {
+				outputObj, err = helper.Replace(namespace, name, false, obj)
+			}
+			if err != nil {
+				return err
+			}
+		}
+		if o.outputFormat != "" {
+			return f.PrintObject(cmd, mapper, outputObj, o.out)
+		}
+		cmdutil.PrintSuccess(mapper, false, o.out, info.Mapping.Resource, info.Name, o.dryrun, dataChangeMsg)
+		return nil
+	})
+}
+
 func validateNoOverwrites(accessor meta.Object, labels map[string]string) error {
 	allErrs := []error{}
 	for key := range labels {
@@ -172,133 +341,3 @@ func labelFunc(obj runtime.Object, overwrite bool, resourceVersion string, label
 	}
 	return nil
 }
-
-func RunLabel(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions) error {
-	resources, labelArgs, err := cmdutil.GetResourcesAndPairs(args, "label")
-	if err != nil {
-		return err
-	}
-	if len(resources) < 1 && cmdutil.IsFilenameEmpty(options.Filenames) {
-		return cmdutil.UsageError(cmd, "one or more resources must be specified as <resource> <name> or <resource>/<name>")
-	}
-	if len(labelArgs) < 1 {
-		return cmdutil.UsageError(cmd, "at least one label update is required")
-	}
-
-	selector := cmdutil.GetFlagString(cmd, "selector")
-	all := cmdutil.GetFlagBool(cmd, "all")
-	overwrite := cmdutil.GetFlagBool(cmd, "overwrite")
-	resourceVersion := cmdutil.GetFlagString(cmd, "resource-version")
-	local := cmdutil.GetFlagBool(cmd, "local")
-
-	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
-	if err != nil {
-		return err
-	}
-
-	lbls, remove, err := parseLabels(labelArgs)
-	if err != nil {
-		return cmdutil.UsageError(cmd, err.Error())
-	}
-	mapper, typer := f.Object()
-	b := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
-		ContinueOnError().
-		NamespaceParam(cmdNamespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, options).
-		Flatten()
-
-	if !local {
-		b = b.SelectorParam(selector).
-			ResourceTypeOrNameArgs(all, resources...).
-			Latest()
-	}
-	one := false
-	r := b.Do().IntoSingular(&one)
-	if err := r.Err(); err != nil {
-		return err
-	}
-
-	// only apply resource version locking on a single resource
-	if !one && len(resourceVersion) > 0 {
-		return cmdutil.UsageError(cmd, "--resource-version may only be used with a single resource")
-	}
-
-	// TODO: support bulk generic output a la Get
-	return r.Visit(func(info *resource.Info, err error) error {
-		if err != nil {
-			return err
-		}
-
-		var outputObj runtime.Object
-		dataChangeMsg := "not labeled"
-		if cmdutil.GetDryRunFlag(cmd) || local {
-			err = labelFunc(info.Object, overwrite, resourceVersion, lbls, remove)
-			if err != nil {
-				return err
-			}
-			outputObj = info.Object
-		} else {
-			obj, err := cmdutil.MaybeConvertObject(info.Object, info.Mapping.GroupVersionKind.GroupVersion(), info.Mapping)
-			if err != nil {
-				return err
-			}
-			name, namespace := info.Name, info.Namespace
-			oldData, err := json.Marshal(obj)
-			if err != nil {
-				return err
-			}
-			accessor, err := meta.Accessor(obj)
-			if err != nil {
-				return err
-			}
-			for _, label := range remove {
-				if _, ok := accessor.GetLabels()[label]; !ok {
-					fmt.Fprintf(out, "label %q not found.\n", label)
-				}
-			}
-
-			if err := labelFunc(obj, overwrite, resourceVersion, lbls, remove); err != nil {
-				return err
-			}
-			if cmdutil.ShouldRecord(cmd, info) {
-				if err := cmdutil.RecordChangeCause(obj, f.Command()); err != nil {
-					return err
-				}
-			}
-			newData, err := json.Marshal(obj)
-			if err != nil {
-				return err
-			}
-			if !reflect.DeepEqual(oldData, newData) {
-				dataChangeMsg = "labeled"
-			}
-			patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, obj)
-			createdPatch := err == nil
-			if err != nil {
-				glog.V(2).Infof("couldn't compute patch: %v", err)
-			}
-
-			mapping := info.ResourceMapping()
-			client, err := f.ClientForMapping(mapping)
-			if err != nil {
-				return err
-			}
-			helper := resource.NewHelper(client, mapping)
-
-			if createdPatch {
-				outputObj, err = helper.Patch(namespace, name, api.StrategicMergePatchType, patchBytes)
-			} else {
-				outputObj, err = helper.Replace(namespace, name, false, obj)
-			}
-			if err != nil {
-				return err
-			}
-		}
-		outputFormat := cmdutil.GetFlagString(cmd, "output")
-		if outputFormat != "" {
-			return f.PrintObject(cmd, mapper, outputObj, out)
-		}
-		cmdutil.PrintSuccess(mapper, false, out, info.Mapping.Resource, info.Name, cmdutil.GetDryRunFlag(cmd), dataChangeMsg)
-		return nil
-	})
-}

pkg/kubectl/cmd/label_test.go

@@ -323,7 +323,14 @@ func TestLabelErrors(t *testing.T) {
 		for k, v := range testCase.flags {
 			cmd.Flags().Set(k, v)
 		}
-		err := RunLabel(f, buf, cmd, testCase.args, &resource.FilenameOptions{})
+		opts := LabelOptions{}
+		err := opts.Complete(f, buf, cmd, testCase.args)
+		if err == nil {
+			err = opts.Validate()
+		}
+		if err == nil {
+			err = opts.RunLabel(f, cmd)
+		}
 		if !testCase.errFn(err) {
 			t.Errorf("%s: unexpected error: %v", k, err)
 			continue
@@ -371,10 +378,15 @@ func TestLabelForResourceFromFile(t *testing.T) {
 
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdLabel(f, buf)
-	options := &resource.FilenameOptions{}
+	opts := LabelOptions{FilenameOptions: resource.FilenameOptions{
-	options.Filenames = []string{"../../../examples/storage/cassandra/cassandra-controller.yaml"}
+		Filenames: []string{"../../../examples/storage/cassandra/cassandra-controller.yaml"}}}
-
+	err := opts.Complete(f, buf, cmd, []string{"a=b"})
-	err := RunLabel(f, buf, cmd, []string{"a=b"}, options)
+	if err == nil {
+		err = opts.Validate()
+	}
+	if err == nil {
+		err = opts.RunLabel(f, cmd)
+	}
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -398,11 +410,15 @@ func TestLabelLocal(t *testing.T) {
 	buf := bytes.NewBuffer([]byte{})
 	cmd := NewCmdLabel(f, buf)
 	cmd.Flags().Set("local", "true")
-	options := &resource.FilenameOptions{
+	opts := LabelOptions{FilenameOptions: resource.FilenameOptions{
-		Filenames: []string{"../../../examples/storage/cassandra/cassandra-controller.yaml"},
+		Filenames: []string{"../../../examples/storage/cassandra/cassandra-controller.yaml"}}}
+	err := opts.Complete(f, buf, cmd, []string{"a=b"})
+	if err == nil {
+		err = opts.Validate()
+	}
+	if err == nil {
+		err = opts.RunLabel(f, cmd)
 	}
-
-	err := RunLabel(f, buf, cmd, []string{"a=b"}, options)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -449,7 +465,15 @@ func TestLabelMultipleObjects(t *testing.T) {
 	cmd := NewCmdLabel(f, buf)
 	cmd.Flags().Set("all", "true")
 
-	if err := RunLabel(f, buf, cmd, []string{"pods", "a=b"}, &resource.FilenameOptions{}); err != nil {
+	opts := LabelOptions{}
+	err := opts.Complete(f, buf, cmd, []string{"pods", "a=b"})
+	if err == nil {
+		err = opts.Validate()
+	}
+	if err == nil {
+		err = opts.RunLabel(f, cmd)
+	}
+	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 	if strings.Count(buf.String(), "labeled") != len(pods.Items) {