Remove Factory from more Run commands

hack/make-rules/test-cmd-util.sh

@@ -3132,7 +3132,7 @@ run_deployment_tests() {
   # Set env of deployments for all container
   kubectl set env deployment nginx-deployment env=prod "${kube_flags[@]}"
   # Set env of deployments for specific container
-  kubectl set env deployment nginx-deployment env=prod -c=nginx "${kube_flags[@]}"
+  kubectl set env deployment nginx-deployment superenv=superprod -c=nginx "${kube_flags[@]}"
   # Set env of deployments by configmap
   kubectl set env deployment nginx-deployment --from=configmap/test-set-env-config "${kube_flags[@]}"
   # Set env of deployments by secret

pkg/kubectl/cmd/annotate.go

@@ -59,10 +59,15 @@ type AnnotateOptions struct {
 	outputFormat    string
 
 	// results of arg parsing
-	resources         []string
+	resources                    []string
-	newAnnotations    map[string]string
+	newAnnotations               map[string]string
-	removeAnnotations []string
+	removeAnnotations            []string
-	Recorder          genericclioptions.Recorder
+	Recorder                     genericclioptions.Recorder
+	namespace                    string
+	enforceNamespace             bool
+	builder                      *resource.Builder
+	unstructuredClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
+	includeUninitialized         bool
 
 	genericclioptions.IOStreams
 }
@@ -123,13 +128,9 @@ func NewCmdAnnotate(parent string, f cmdutil.Factory, ioStreams genericclioption
 		Long:    annotateLong + "\n\n" + cmdutil.SuggestApiResources(parent),
 		Example: annotateExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			if err := o.Complete(f, cmd, args); err != nil {
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-				cmdutil.CheckErr(cmdutil.UsageErrorf(cmd, "%v", err))
+			cmdutil.CheckErr(o.Validate())
-			}
+			cmdutil.CheckErr(o.RunAnnotate())
-			if err := o.Validate(); err != nil {
-				cmdutil.CheckErr(cmdutil.UsageErrorf(cmd, "%v", err))
-			}
-			cmdutil.CheckErr(o.RunAnnotate(f, cmd))
 		},
 		ValidArgs:  validArgs,
 		ArgAliases: kubectl.ResourceAliases(validArgs),
@@ -176,6 +177,14 @@ func (o *AnnotateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [
 		return printer.PrintObj(obj, out)
 	}
 
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+	o.includeUninitialized = cmdutil.ShouldIncludeUninitialized(cmd, false)
+	o.builder = f.NewBuilder()
+	o.unstructuredClientForMapping = f.UnstructuredClientForMapping
+
 	// retrieves resource and annotation args from args
 	// also checks args to verify that all resources are specified before annotations
 	resources, annotationArgs, err := cmdutil.GetResourcesAndPairs(args, "annotation")
@@ -184,7 +193,11 @@ func (o *AnnotateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [
 	}
 	o.resources = resources
 	o.newAnnotations, o.removeAnnotations, err = parseAnnotations(annotationArgs)
-	return err
+	if err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // Validate checks to the AnnotateOptions to see if there is sufficient information run the command.
@@ -202,20 +215,14 @@ func (o AnnotateOptions) Validate() error {
 }
 
 // RunAnnotate does the work
-func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o AnnotateOptions) RunAnnotate() error {
-	namespace, enforceNamespace, err := f.DefaultNamespace()
+	b := o.builder.
-	if err != nil {
-		return err
-	}
-
-	includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
-	b := f.NewBuilder().
 		Unstructured().
 		LocalParam(o.local).
 		ContinueOnError().
-		NamespaceParam(namespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
-		IncludeUninitialized(includeUninitialized).
+		IncludeUninitialized(o.includeUninitialized).
 		Flatten()
 
 	if !o.local {
@@ -276,7 +283,7 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro
 			}
 
 			mapping := info.ResourceMapping()
-			client, err := f.UnstructuredClientForMapping(mapping)
+			client, err := o.unstructuredClientForMapping(mapping)
 			if err != nil {
 				return err
 			}

pkg/kubectl/cmd/annotate_test.go

@@ -499,7 +499,7 @@ func TestAnnotateObject(t *testing.T) {
 	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(tf, cmd); err != nil {
+	if err := options.RunAnnotate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -554,7 +554,7 @@ func TestAnnotateObjectFromFile(t *testing.T) {
 	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(tf, cmd); err != nil {
+	if err := options.RunAnnotate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -586,7 +586,7 @@ func TestAnnotateLocal(t *testing.T) {
 	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(tf, cmd); err != nil {
+	if err := options.RunAnnotate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }
@@ -642,7 +642,7 @@ func TestAnnotateMultipleObjects(t *testing.T) {
 	if err := options.Validate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if err := options.RunAnnotate(tf, cmd); err != nil {
+	if err := options.RunAnnotate(); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 }

pkg/kubectl/cmd/apiversions.go

@@ -18,14 +18,15 @@ package cmd
 
 import (
 	"fmt"
-	"io"
 	"sort"
 
 	"github.com/spf13/cobra"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/discovery"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/util/i18n"
 )
 
@@ -35,37 +36,54 @@ var (
 		kubectl api-versions`))
 )
 
-func NewCmdApiVersions(f cmdutil.Factory, out io.Writer) *cobra.Command {
+type ApiVersionsOptions struct {
+	discoveryClient discovery.CachedDiscoveryInterface
+
+	genericclioptions.IOStreams
+}
+
+func NewApiVersionsOptions(ioStreams genericclioptions.IOStreams) *ApiVersionsOptions {
+	return &ApiVersionsOptions{
+		IOStreams: ioStreams,
+	}
+}
+
+func NewCmdApiVersions(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
+	o := NewApiVersionsOptions(ioStreams)
 	cmd := &cobra.Command{
 		Use:     "api-versions",
 		Short:   "Print the supported API versions on the server, in the form of \"group/version\"",
 		Long:    "Print the supported API versions on the server, in the form of \"group/version\"",
 		Example: apiversionsExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			err := RunApiVersions(f, out)
+			cmdutil.CheckErr(o.Complete(f))
-			cmdutil.CheckErr(err)
+			cmdutil.CheckErr(o.RunApiVersions())
 		},
 	}
 	return cmd
 }
 
-func RunApiVersions(f cmdutil.Factory, w io.Writer) error {
+func (o *ApiVersionsOptions) Complete(f cmdutil.Factory) error {
-	discoveryclient, err := f.DiscoveryClient()
+	var err error
+	o.discoveryClient, err = f.DiscoveryClient()
 	if err != nil {
 		return err
 	}
+	return nil
+}
 
+func (o *ApiVersionsOptions) RunApiVersions() error {
 	// Always request fresh data from the server
-	discoveryclient.Invalidate()
+	o.discoveryClient.Invalidate()
 
-	groupList, err := discoveryclient.ServerGroups()
+	groupList, err := o.discoveryClient.ServerGroups()
 	if err != nil {
 		return fmt.Errorf("Couldn't get available api versions from server: %v\n", err)
 	}
 	apiVersions := metav1.ExtractGroupVersions(groupList)
 	sort.Strings(apiVersions)
 	for _, v := range apiVersions {
-		fmt.Fprintln(w, v)
+		fmt.Fprintln(o.Out, v)
 	}
 	return nil
 }

pkg/kubectl/cmd/apply_set_last_applied.go

@@ -18,19 +18,14 @@ package cmd
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
-	"io"
 
-	"github.com/ghodss/yaml"
 	"github.com/spf13/cobra"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
-	apijson "k8s.io/apimachinery/pkg/util/json"
+	"k8s.io/kubernetes/pkg/api/legacyscheme"
-	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
@@ -39,21 +34,27 @@ import (
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/kubectl/scheme"
 	"k8s.io/kubernetes/pkg/kubectl/util/i18n"
+	"k8s.io/kubernetes/pkg/printers"
 )
 
 type SetLastAppliedOptions struct {
-	FilenameOptions  resource.FilenameOptions
-	Selector         string
-	InfoList         []*resource.Info
-	Mapper           meta.RESTMapper
-	Namespace        string
-	EnforceNamespace bool
-	DryRun           bool
-	ShortOutput      bool
 	CreateAnnotation bool
-	Output           string
+
-	PatchBufferList  []PatchBuffer
+	PrintFlags *printers.PrintFlags
-	Factory          cmdutil.Factory
+	PrintObj   printers.ResourcePrinterFunc
+
+	FilenameOptions resource.FilenameOptions
+
+	infoList                     []*resource.Info
+	mapper                       meta.RESTMapper
+	namespace                    string
+	enforceNamespace             bool
+	dryRun                       bool
+	shortOutput                  bool
+	output                       string
+	patchBufferList              []PatchBuffer
+	builder                      *resource.Builder
+	unstructuredClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
 
 	genericclioptions.IOStreams
 }
@@ -83,12 +84,13 @@ var (
 
 func NewSetLastAppliedOptions(ioStreams genericclioptions.IOStreams) *SetLastAppliedOptions {
 	return &SetLastAppliedOptions{
-		IOStreams: ioStreams,
+		PrintFlags: printers.NewPrintFlags("configured"),
+		IOStreams:  ioStreams,
 	}
 }
 
 func NewCmdApplySetLastApplied(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	options := NewSetLastAppliedOptions(ioStreams)
+	o := NewSetLastAppliedOptions(ioStreams)
 	cmd := &cobra.Command{
 		Use: "set-last-applied -f FILENAME",
 		DisableFlagsInUseLine: true,
@@ -96,38 +98,55 @@ func NewCmdApplySetLastApplied(f cmdutil.Factory, ioStreams genericclioptions.IO
 		Long:    applySetLastAppliedLong,
 		Example: applySetLastAppliedExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.Complete(f, cmd))
+			cmdutil.CheckErr(o.Complete(f, cmd))
-			cmdutil.CheckErr(options.Validate(f, cmd))
+			cmdutil.CheckErr(o.Validate())
-			cmdutil.CheckErr(options.RunSetLastApplied(f, cmd))
+			cmdutil.CheckErr(o.RunSetLastApplied())
 		},
 	}
 
 	cmdutil.AddDryRunFlag(cmd)
 	cmdutil.AddPrinterFlags(cmd)
-	cmd.Flags().BoolVar(&options.CreateAnnotation, "create-annotation", options.CreateAnnotation, "Will create 'last-applied-configuration' annotations if current objects doesn't have one")
+	cmd.Flags().BoolVar(&o.CreateAnnotation, "create-annotation", o.CreateAnnotation, "Will create 'last-applied-configuration' annotations if current objects doesn't have one")
-	usage := "that contains the last-applied-configuration annotations"
+	kubectl.AddJsonFilenameFlag(cmd, &o.FilenameOptions.Filenames, "Filename, directory, or URL to files that contains the last-applied-configuration annotations")
-	kubectl.AddJsonFilenameFlag(cmd, &options.FilenameOptions.Filenames, "Filename, directory, or URL to files "+usage)
 
 	return cmd
 }
 
 func (o *SetLastAppliedOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
-	o.DryRun = cmdutil.GetDryRunFlag(cmd)
+	o.dryRun = cmdutil.GetDryRunFlag(cmd)
-	o.Output = cmdutil.GetFlagString(cmd, "output")
+	o.output = cmdutil.GetFlagString(cmd, "output")
-	o.ShortOutput = o.Output == "name"
+	o.shortOutput = o.output == "name"
-
-	o.Mapper = f.RESTMapper()
 
+	o.mapper = f.RESTMapper()
 	var err error
-	o.Namespace, o.EnforceNamespace, err = f.DefaultNamespace()
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
-	return err
+	if err != nil {
+		return err
+	}
+	o.builder = f.NewBuilder()
+	o.unstructuredClientForMapping = f.UnstructuredClientForMapping
+
+	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 = printer.PrintObj
+
+	return nil
 }
 
-func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o *SetLastAppliedOptions) Validate() error {
-	r := f.NewBuilder().
+	r := o.builder.
 		Unstructured().
-		NamespaceParam(o.Namespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(o.EnforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
 		Flatten().
 		Do()
 
@@ -153,14 +172,14 @@ func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command)
 			return cmdutil.AddSourceToErr(fmt.Sprintf("retrieving current configuration of:\n%s\nfrom server for:", info.String()), info.Source, err)
 		}
 		if originalBuf == nil && !o.CreateAnnotation {
-			return cmdutil.UsageErrorf(cmd, "no last-applied-configuration annotation found on resource: %s, to create the annotation, run the command with --create-annotation", info.Name)
+			return fmt.Errorf("no last-applied-configuration annotation found on resource: %s, to create the annotation, run the command with --create-annotation", info.Name)
 		}
 
 		//only add to PatchBufferList when changed
 		if !bytes.Equal(cmdutil.StripComments(originalBuf), cmdutil.StripComments(diffBuf)) {
 			p := PatchBuffer{Patch: patchBuf, PatchType: patchType}
-			o.PatchBufferList = append(o.PatchBufferList, p)
+			o.patchBufferList = append(o.patchBufferList, p)
-			o.InfoList = append(o.InfoList, info)
+			o.infoList = append(o.infoList, info)
 		} else {
 			fmt.Fprintf(o.Out, "set-last-applied %s: no changes required.\n", info.Name)
 		}
@@ -170,68 +189,25 @@ func (o *SetLastAppliedOptions) Validate(f cmdutil.Factory, cmd *cobra.Command)
 	return err
 }
 
-func (o *SetLastAppliedOptions) RunSetLastApplied(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o *SetLastAppliedOptions) RunSetLastApplied() error {
-	for i, patch := range o.PatchBufferList {
+	for i, patch := range o.patchBufferList {
-		info := o.InfoList[i]
+		info := o.infoList[i]
-		if !o.DryRun {
+		if !o.dryRun {
 			mapping := info.ResourceMapping()
-			client, err := f.UnstructuredClientForMapping(mapping)
+			client, err := o.unstructuredClientForMapping(mapping)
 			if err != nil {
 				return err
 			}
 			helper := resource.NewHelper(client, mapping)
-			patchedObj, err := helper.Patch(o.Namespace, info.Name, patch.PatchType, patch.Patch)
+			patchedObj, err := helper.Patch(o.namespace, info.Name, patch.PatchType, patch.Patch)
 			if err != nil {
 				return err
 			}
-
+			info.Refresh(patchedObj, false)
-			if len(o.Output) > 0 && !o.ShortOutput {
-				info.Refresh(patchedObj, false)
-				return cmdutil.PrintObject(cmd, info.Object, o.Out)
-			}
-			cmdutil.PrintSuccess(o.ShortOutput, o.Out, info.Object, o.DryRun, "configured")
-
-		} else {
-			err := o.formatPrinter(o.Output, patch.Patch, o.Out)
-			if err != nil {
-				return err
-			}
-			cmdutil.PrintSuccess(o.ShortOutput, o.Out, info.Object, o.DryRun, "configured")
 		}
-	}
+		if err := o.PrintObj(info.AsVersioned(legacyscheme.Scheme), o.Out); err != nil {
-	return nil
-}
-
-func (o *SetLastAppliedOptions) formatPrinter(output string, buf []byte, w io.Writer) error {
-	yamlOutput, err := yaml.JSONToYAML(buf)
-	if err != nil {
-		return err
-	}
-	switch output {
-	case "json":
-		jsonBuffer := &bytes.Buffer{}
-		err = json.Indent(jsonBuffer, buf, "", "  ")
-		if err != nil {
 			return err
 		}
-		fmt.Fprintf(w, "%s\n", jsonBuffer.String())
-	case "yaml":
-		fmt.Fprintf(w, "%s\n", string(yamlOutput))
 	}
 	return nil
 }
-
-func (o *SetLastAppliedOptions) getPatch(info *resource.Info) ([]byte, []byte, error) {
-	objMap := map[string]map[string]map[string]string{}
-	metadataMap := map[string]map[string]string{}
-	annotationsMap := map[string]string{}
-	localFile, err := runtime.Encode(scheme.DefaultJSONEncoder(), info.Object)
-	if err != nil {
-		return nil, localFile, err
-	}
-	annotationsMap[api.LastAppliedConfigAnnotation] = string(localFile)
-	metadataMap["annotations"] = annotationsMap
-	objMap["metadata"] = metadataMap
-	jsonString, err := apijson.Marshal(objMap)
-	return jsonString, localFile, err
-}

pkg/kubectl/cmd/apply_test.go

@@ -1121,7 +1121,7 @@ func TestRunApplySetLastApplied(t *testing.T) {
 			name:        "set with exist object",
 			filePath:    filenameRC,
 			expectedErr: "",
-			expectedOut: "replicationcontroller/test-rc\n",
+			expectedOut: "replicationcontroller/test-rc configured\n",
 			output:      "name",
 		},
 		{
@@ -1134,7 +1134,7 @@ func TestRunApplySetLastApplied(t *testing.T) {
 		{
 			name:        "set for the annotation does not exist on the live object",
 			filePath:    filenameRCNoAnnotation,
-			expectedErr: "error: no last-applied-configuration annotation found on resource: no-annotation, to create the annotation, run the command with --create-annotation\nSee 'set-last-applied -h' for help and examples.",
+			expectedErr: "error: no last-applied-configuration annotation found on resource: no-annotation, to create the annotation, run the command with --create-annotation",
 			expectedOut: "",
 			output:      "name",
 		},
@@ -1142,14 +1142,14 @@ func TestRunApplySetLastApplied(t *testing.T) {
 			name:        "set with exist object output json",
 			filePath:    filenameRCJSON,
 			expectedErr: "",
-			expectedOut: "replicationcontroller/test-rc\n",
+			expectedOut: "replicationcontroller/test-rc configured\n",
 			output:      "name",
 		},
 		{
 			name:        "set test for a directory of files",
 			filePath:    dirName,
 			expectedErr: "",
-			expectedOut: "replicationcontroller/test-rc\nreplicationcontroller/test-rc\n",
+			expectedOut: "replicationcontroller/test-rc configured\nreplicationcontroller/test-rc configured\n",
 			output:      "name",
 		},
 	}

pkg/kubectl/cmd/autoscale.go

@@ -24,7 +24,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/kubectl"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
@@ -59,20 +58,22 @@ type AutoscaleOptions struct {
 	PrintFlags *printers.PrintFlags
 	ToPrinter  func(string) (printers.ResourcePrinterFunc, error)
 
-	Builder         *resource.Builder
+	Name       string
-	CanBeAutoscaled func(kind schema.GroupKind) error
+	Generator  string
+	Min        int32
+	Max        int32
+	CpuPercent int32
 
-	CreateAnnotation bool
+	createAnnotation bool
-	DryRun           bool
+	args             []string
-	EnforceNamespace bool
+	enforceNamespace bool
-
+	namespace        string
-	Mapper           meta.RESTMapper
+	dryRun           bool
-	ClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
+	builder          *resource.Builder
-
+	mapper           meta.RESTMapper
-	GeneratorFunc func(string, *meta.RESTMapping) (kubectl.StructuredGenerator, error)
+	canBeAutoscaled  func(kind schema.GroupKind) error
-
+	clientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
-	Namespace   string
+	generatorFunc    func(string, *meta.RESTMapping) (kubectl.StructuredGenerator, error)
-	BuilderArgs []string
 
 	genericclioptions.IOStreams
 }
@@ -102,7 +103,7 @@ func NewCmdAutoscale(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *
 		Example: autoscaleExample,
 		Run: func(cmd *cobra.Command, args []string) {
 			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(o.Validate(cmd))
+			cmdutil.CheckErr(o.Validate())
 			cmdutil.CheckErr(o.Run())
 		},
 		ValidArgs:  validArgs,
@@ -113,27 +114,26 @@ func NewCmdAutoscale(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *
 	o.RecordFlags.AddFlags(cmd)
 	o.PrintFlags.AddFlags(cmd)
 
-	cmd.Flags().String("generator", cmdutil.HorizontalPodAutoscalerV1GeneratorName, i18n.T("The name of the API generator to use. Currently there is only 1 generator."))
+	cmd.Flags().StringVar(&o.Generator, "generator", cmdutil.HorizontalPodAutoscalerV1GeneratorName, i18n.T("The name of the API generator to use. Currently there is only 1 generator."))
-	cmd.Flags().Int32("min", -1, "The lower limit for the number of pods that can be set by the autoscaler. If it's not specified or negative, the server will apply a default value.")
+	cmd.Flags().Int32Var(&o.Min, "min", -1, "The lower limit for the number of pods that can be set by the autoscaler. If it's not specified or negative, the server will apply a default value.")
-	cmd.Flags().Int32("max", -1, "The upper limit for the number of pods that can be set by the autoscaler. Required.")
+	cmd.Flags().Int32Var(&o.Max, "max", -1, "The upper limit for the number of pods that can be set by the autoscaler. Required.")
 	cmd.MarkFlagRequired("max")
-	cmd.Flags().Int32("cpu-percent", -1, fmt.Sprintf("The target average CPU utilization (represented as a percent of requested CPU) over all the pods. If it's not specified or negative, a default autoscaling policy will be used."))
+	cmd.Flags().Int32Var(&o.CpuPercent, "cpu-percent", -1, fmt.Sprintf("The target average CPU utilization (represented as a percent of requested CPU) over all the pods. If it's not specified or negative, a default autoscaling policy will be used."))
-	cmd.Flags().String("name", "", i18n.T("The name for the newly created object. If not specified, the name of the input resource will be used."))
+	cmd.Flags().StringVar(&o.Name, "name", "", i18n.T("The name for the newly created object. If not specified, the name of the input resource will be used."))
 	cmdutil.AddDryRunFlag(cmd)
-	usage := "identifying the resource to autoscale."
+	cmdutil.AddFilenameOptionFlags(cmd, o.FilenameOptions, "identifying the resource to autoscale.")
-	cmdutil.AddFilenameOptionFlags(cmd, o.FilenameOptions, usage)
 	cmdutil.AddApplyAnnotationFlags(cmd)
 	return cmd
 }
 
 func (o *AutoscaleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
-	o.DryRun = cmdutil.GetFlagBool(cmd, "dry-run")
+	o.dryRun = cmdutil.GetFlagBool(cmd, "dry-run")
-	o.CreateAnnotation = cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag)
+	o.createAnnotation = cmdutil.GetFlagBool(cmd, cmdutil.ApplyAnnotationsFlag)
-	o.Builder = f.NewBuilder()
+	o.builder = f.NewBuilder()
-	o.CanBeAutoscaled = f.CanBeAutoscaled
+	o.canBeAutoscaled = f.CanBeAutoscaled
-	o.Mapper = f.RESTMapper()
+	o.mapper = f.RESTMapper()
-	o.ClientForMapping = f.ClientForMapping
+	o.clientForMapping = f.ClientForMapping
-	o.BuilderArgs = args
+	o.args = args
 	o.RecordFlags.Complete(f.Command(cmd, false))
 
 	var err error
@@ -143,34 +143,31 @@ func (o *AutoscaleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args
 	}
 
 	// get the generator
-	o.GeneratorFunc = func(name string, mapping *meta.RESTMapping) (kubectl.StructuredGenerator, error) {
+	o.generatorFunc = func(name string, mapping *meta.RESTMapping) (kubectl.StructuredGenerator, error) {
-		var generator kubectl.StructuredGenerator
+		switch o.Generator {
-		switch generatorName := cmdutil.GetFlagString(cmd, "generator"); generatorName {
 		case cmdutil.HorizontalPodAutoscalerV1GeneratorName:
-			generator = &kubectl.HorizontalPodAutoscalerGeneratorV1{
+			return &kubectl.HorizontalPodAutoscalerGeneratorV1{
 				Name:               name,
-				MinReplicas:        cmdutil.GetFlagInt32(cmd, "min"),
+				MinReplicas:        o.Min,
-				MaxReplicas:        cmdutil.GetFlagInt32(cmd, "max"),
+				MaxReplicas:        o.Max,
-				CPUPercent:         cmdutil.GetFlagInt32(cmd, "cpu-percent"),
+				CPUPercent:         o.CpuPercent,
 				ScaleRefName:       name,
 				ScaleRefKind:       mapping.GroupVersionKind.Kind,
 				ScaleRefApiVersion: mapping.GroupVersionKind.GroupVersion().String(),
-			}
+			}, nil
 		default:
-			return nil, cmdutil.UsageErrorf(cmd, "Generator %s not supported. ", generatorName)
+			return nil, cmdutil.UsageErrorf(cmd, "Generator %s not supported. ", o.Generator)
 		}
-
-		return generator, nil
 	}
 
-	o.Namespace, o.EnforceNamespace, err = f.DefaultNamespace()
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
 	if err != nil {
 		return err
 	}
 
 	o.ToPrinter = func(operation string) (printers.ResourcePrinterFunc, error) {
 		o.PrintFlags.NamePrintFlags.Operation = operation
-		if o.DryRun {
+		if o.dryRun {
 			o.PrintFlags.Complete("%s (dry run)")
 		}
 
@@ -185,21 +182,24 @@ func (o *AutoscaleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args
 	return nil
 }
 
-func (o *AutoscaleOptions) Validate(cmd *cobra.Command) error {
+func (o *AutoscaleOptions) Validate() error {
-	if err := validateFlags(cmd); err != nil {
+	if o.Max < 1 {
-		return err
+		return fmt.Errorf("--max=MAXPODS is required and must be at least 1, max: %d", o.Max)
+	}
+	if o.Max < o.Min {
+		return fmt.Errorf("--max=MAXPODS must be larger or equal to --min=MINPODS, max: %d, min: %d", o.Max, o.Min)
 	}
 
 	return nil
 }
 
 func (o *AutoscaleOptions) Run() error {
-	r := o.Builder.
+	r := o.builder.
 		Internal(legacyscheme.Scheme).
 		ContinueOnError().
-		NamespaceParam(o.Namespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(o.EnforceNamespace, o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, o.FilenameOptions).
-		ResourceTypeOrNameArgs(false, o.BuilderArgs...).
+		ResourceTypeOrNameArgs(false, o.args...).
 		Flatten().
 		Do()
 	if err := r.Err(); err != nil {
@@ -213,11 +213,11 @@ func (o *AutoscaleOptions) Run() error {
 		}
 
 		mapping := info.ResourceMapping()
-		if err := o.CanBeAutoscaled(mapping.GroupVersionKind.GroupKind()); err != nil {
+		if err := o.canBeAutoscaled(mapping.GroupVersionKind.GroupKind()); err != nil {
 			return err
 		}
 
-		generator, err := o.GeneratorFunc(info.Name, mapping)
+		generator, err := o.generatorFunc(info.Name, mapping)
 		if err != nil {
 			return err
 		}
@@ -229,8 +229,8 @@ func (o *AutoscaleOptions) Run() error {
 		}
 
 		resourceMapper := &resource.Mapper{
-			RESTMapper:   o.Mapper,
+			RESTMapper:   o.mapper,
-			ClientMapper: resource.ClientMapperFunc(o.ClientForMapping),
+			ClientMapper: resource.ClientMapperFunc(o.clientForMapping),
 			Decoder:      cmdutil.InternalVersionDecoder(),
 		}
 		hpa, err := resourceMapper.InfoForObject(object, legacyscheme.Scheme, nil)
@@ -242,7 +242,7 @@ func (o *AutoscaleOptions) Run() error {
 		}
 		object = hpa.Object
 
-		if o.DryRun {
+		if o.dryRun {
 			count++
 
 			printer, err := o.ToPrinter("created")
@@ -252,11 +252,11 @@ func (o *AutoscaleOptions) Run() error {
 			return printer.PrintObj(hpa.AsVersioned(legacyscheme.Scheme), o.Out)
 		}
 
-		if err := kubectl.CreateOrUpdateAnnotation(o.CreateAnnotation, hpa.Object, cmdutil.InternalVersionJSONEncoder()); err != nil {
+		if err := kubectl.CreateOrUpdateAnnotation(o.createAnnotation, hpa.Object, cmdutil.InternalVersionJSONEncoder()); err != nil {
 			return err
 		}
 
-		_, err = resource.NewHelper(hpa.Client, hpa.Mapping).Create(o.Namespace, false, object)
+		_, err = resource.NewHelper(hpa.Client, hpa.Mapping).Create(o.namespace, false, object)
 		if err != nil {
 			return err
 		}
@@ -276,15 +276,3 @@ func (o *AutoscaleOptions) Run() error {
 	}
 	return nil
 }
-
-func validateFlags(cmd *cobra.Command) error {
-	errs := []error{}
-	max, min := cmdutil.GetFlagInt32(cmd, "max"), cmdutil.GetFlagInt32(cmd, "min")
-	if max < 1 {
-		errs = append(errs, fmt.Errorf("--max=MAXPODS is required and must be at least 1, max: %d", max))
-	}
-	if max < min {
-		errs = append(errs, fmt.Errorf("--max=MAXPODS must be larger or equal to --min=MINPODS, max: %d, min: %d", max, min))
-	}
-	return utilerrors.NewAggregate(errs)
-}

pkg/kubectl/cmd/clusterinfo_dump.go

@@ -21,11 +21,15 @@ import (
 	"io"
 	"os"
 	"path"
+	"time"
 
 	"github.com/spf13/cobra"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	restclient "k8s.io/client-go/rest"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
@@ -37,6 +41,15 @@ type ClusterInfoDumpOptions struct {
 	PrintFlags *printers.PrintFlags
 	PrintObj   printers.ResourcePrinterFunc
 
+	OutputDir     string
+	AllNamespaces bool
+	Namespaces    []string
+
+	timeout       time.Duration
+	clientset     internalclientset.Interface
+	namespace     string
+	logsForObject func(object, options runtime.Object, timeout time.Duration) (*restclient.Request, error)
+
 	genericclioptions.IOStreams
 }
 
@@ -54,13 +67,13 @@ func NewCmdClusterInfoDump(f cmdutil.Factory, ioStreams genericclioptions.IOStre
 		Long:    dumpLong,
 		Example: dumpExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(o.Complete())
+			cmdutil.CheckErr(o.Complete(f, cmd))
-			cmdutil.CheckErr(o.Run(f, cmd))
+			cmdutil.CheckErr(o.Run())
 		},
 	}
-	cmd.Flags().String("output-directory", "", i18n.T("Where to output the files.  If empty or '-' uses stdout, otherwise creates a directory hierarchy in that directory"))
+	cmd.Flags().StringVar(&o.OutputDir, "output-directory", "", i18n.T("Where to output the files.  If empty or '-' uses stdout, otherwise creates a directory hierarchy in that directory"))
-	cmd.Flags().StringSlice("namespaces", []string{}, "A comma separated list of namespaces to dump.")
+	cmd.Flags().StringSliceVar(&o.Namespaces, "namespaces", []string{}, "A comma separated list of namespaces to dump.")
-	cmd.Flags().Bool("all-namespaces", false, "If true, dump all namespaces.  If true, --namespaces is ignored.")
+	cmd.Flags().BoolVar(&o.AllNamespaces, "all-namespaces", false, "If true, dump all namespaces.  If true, --namespaces is ignored.")
 	cmdutil.AddPodRunningTimeoutFlag(cmd, defaultPodLogsTimeout)
 	return cmd
 }
@@ -89,8 +102,7 @@ var (
     kubectl cluster-info dump --namespaces default,kube-system --output-directory=/path/to/cluster-state`))
 )
 
-func setupOutputWriter(cmd *cobra.Command, defaultWriter io.Writer, filename string) io.Writer {
+func setupOutputWriter(dir string, defaultWriter io.Writer, filename string) io.Writer {
-	dir := cmdutil.GetFlagString(cmd, "output-directory")
 	if len(dir) == 0 || dir == "-" {
 		return defaultWriter
 	}
@@ -103,7 +115,7 @@ func setupOutputWriter(cmd *cobra.Command, defaultWriter io.Writer, filename str
 	return file
 }
 
-func (o *ClusterInfoDumpOptions) Complete() error {
+func (o *ClusterInfoDumpOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 	printer, err := o.PrintFlags.ToPrinter()
 	if err != nil {
 		return err
@@ -113,32 +125,36 @@ func (o *ClusterInfoDumpOptions) Complete() error {
 	o.PrintFlags.OutputFormat = &jsonOutputFmt
 	o.PrintObj = printer.PrintObj
 
+	o.timeout, err = cmdutil.GetPodRunningTimeoutFlag(cmd)
+	if err != nil {
+		return err
+	}
+	o.clientset, err = f.ClientSet()
+	if err != nil {
+		return err
+	}
+	o.namespace, _, err = f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+	o.logsForObject = f.LogsForObject
+
 	return nil
 }
 
-func (o *ClusterInfoDumpOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o *ClusterInfoDumpOptions) Run() error {
-	timeout, err := cmdutil.GetPodRunningTimeoutFlag(cmd)
+	nodes, err := o.clientset.Core().Nodes().List(metav1.ListOptions{})
-	if err != nil {
-		return cmdutil.UsageErrorf(cmd, err.Error())
-	}
-
-	clientset, err := f.ClientSet()
 	if err != nil {
 		return err
 	}
 
-	nodes, err := clientset.Core().Nodes().List(metav1.ListOptions{})
+	if err := o.PrintObj(nodes, setupOutputWriter(o.OutputDir, o.Out, "nodes.json")); err != nil {
-	if err != nil {
-		return err
-	}
-
-	if err := o.PrintObj(nodes, setupOutputWriter(cmd, o.Out, "nodes.json")); err != nil {
 		return err
 	}
 
 	var namespaces []string
-	if cmdutil.GetFlagBool(cmd, "all-namespaces") {
+	if o.AllNamespaces {
-		namespaceList, err := clientset.Core().Namespaces().List(metav1.ListOptions{})
+		namespaceList, err := o.clientset.Core().Namespaces().List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
@@ -146,75 +162,70 @@ func (o *ClusterInfoDumpOptions) Run(f cmdutil.Factory, cmd *cobra.Command) erro
 			namespaces = append(namespaces, namespaceList.Items[ix].Name)
 		}
 	} else {
-		namespaces = cmdutil.GetFlagStringSlice(cmd, "namespaces")
+		if len(o.Namespaces) == 0 {
-		if len(namespaces) == 0 {
-			cmdNamespace, _, err := f.DefaultNamespace()
-			if err != nil {
-				return err
-			}
 			namespaces = []string{
 				metav1.NamespaceSystem,
-				cmdNamespace,
+				o.namespace,
 			}
 		}
 	}
 	for _, namespace := range namespaces {
 		// TODO: this is repetitive in the extreme.  Use reflection or
 		// something to make this a for loop.
-		events, err := clientset.Core().Events(namespace).List(metav1.ListOptions{})
+		events, err := o.clientset.Core().Events(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(events, setupOutputWriter(cmd, o.Out, path.Join(namespace, "events.json"))); err != nil {
+		if err := o.PrintObj(events, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "events.json"))); err != nil {
 			return err
 		}
 
-		rcs, err := clientset.Core().ReplicationControllers(namespace).List(metav1.ListOptions{})
+		rcs, err := o.clientset.Core().ReplicationControllers(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(rcs, setupOutputWriter(cmd, o.Out, path.Join(namespace, "replication-controllers.json"))); err != nil {
+		if err := o.PrintObj(rcs, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "replication-controllers.json"))); err != nil {
 			return err
 		}
 
-		svcs, err := clientset.Core().Services(namespace).List(metav1.ListOptions{})
+		svcs, err := o.clientset.Core().Services(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(svcs, setupOutputWriter(cmd, o.Out, path.Join(namespace, "services.json"))); err != nil {
+		if err := o.PrintObj(svcs, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "services.json"))); err != nil {
 			return err
 		}
 
-		sets, err := clientset.Extensions().DaemonSets(namespace).List(metav1.ListOptions{})
+		sets, err := o.clientset.Extensions().DaemonSets(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(sets, setupOutputWriter(cmd, o.Out, path.Join(namespace, "daemonsets.json"))); err != nil {
+		if err := o.PrintObj(sets, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "daemonsets.json"))); err != nil {
 			return err
 		}
 
-		deps, err := clientset.Extensions().Deployments(namespace).List(metav1.ListOptions{})
+		deps, err := o.clientset.Extensions().Deployments(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(deps, setupOutputWriter(cmd, o.Out, path.Join(namespace, "deployments.json"))); err != nil {
+		if err := o.PrintObj(deps, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "deployments.json"))); err != nil {
 			return err
 		}
 
-		rps, err := clientset.Extensions().ReplicaSets(namespace).List(metav1.ListOptions{})
+		rps, err := o.clientset.Extensions().ReplicaSets(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
-		if err := o.PrintObj(rps, setupOutputWriter(cmd, o.Out, path.Join(namespace, "replicasets.json"))); err != nil {
+		if err := o.PrintObj(rps, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "replicasets.json"))); err != nil {
 			return err
 		}
 
-		pods, err := clientset.Core().Pods(namespace).List(metav1.ListOptions{})
+		pods, err := o.clientset.Core().Pods(namespace).List(metav1.ListOptions{})
 		if err != nil {
 			return err
 		}
 
-		if err := o.PrintObj(pods, setupOutputWriter(cmd, o.Out, path.Join(namespace, "pods.json"))); err != nil {
+		if err := o.PrintObj(pods, setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, "pods.json"))); err != nil {
 			return err
 		}
 
@@ -222,7 +233,7 @@ func (o *ClusterInfoDumpOptions) Run(f cmdutil.Factory, cmd *cobra.Command) erro
 			writer.Write([]byte(fmt.Sprintf("==== START logs for container %s of pod %s/%s ====\n", container.Name, pod.Namespace, pod.Name)))
 			defer writer.Write([]byte(fmt.Sprintf("==== END logs for container %s of pod %s/%s ====\n", container.Name, pod.Namespace, pod.Name)))
 
-			request, err := f.LogsForObject(pod, &api.PodLogOptions{Container: container.Name}, timeout)
+			request, err := o.logsForObject(pod, &api.PodLogOptions{Container: container.Name}, timeout)
 			if err != nil {
 				// Print error and return.
 				writer.Write([]byte(fmt.Sprintf("Create log request error: %s\n", err.Error())))
@@ -241,19 +252,15 @@ func (o *ClusterInfoDumpOptions) Run(f cmdutil.Factory, cmd *cobra.Command) erro
 		for ix := range pods.Items {
 			pod := &pods.Items[ix]
 			containers := pod.Spec.Containers
-			writer := setupOutputWriter(cmd, o.Out, path.Join(namespace, pod.Name, "logs.txt"))
+			writer := setupOutputWriter(o.OutputDir, o.Out, path.Join(namespace, pod.Name, "logs.txt"))
 
 			for i := range containers {
 				printContainer(writer, containers[i], pod)
 			}
 		}
 	}
-	dir := cmdutil.GetFlagString(cmd, "output-directory")
+	if o.OutputDir != "-" {
-	if len(dir) == 0 {
+		fmt.Fprintf(o.Out, "Cluster info dumped to %s\n", o.OutputDir)
-		dir = "standard output"
-	}
-	if dir != "-" {
-		fmt.Fprintf(o.Out, "Cluster info dumped to %s\n", dir)
 	}
 	return nil
 }

pkg/kubectl/cmd/clusterinfo_dump_test.go

@@ -33,9 +33,7 @@ func TestSetupOutputWriterNoOp(t *testing.T) {
 		f := cmdtesting.NewTestFactory()
 		defer f.Cleanup()
 
-		cmd := NewCmdClusterInfoDump(f, genericclioptions.NewTestIOStreamsDiscard())
+		writer := setupOutputWriter(test, buf, "/some/file/that/should/be/ignored")
-		cmd.Flag("output-directory").Value.Set(test)
-		writer := setupOutputWriter(cmd, buf, "/some/file/that/should/be/ignored")
 		if writer != buf {
 			t.Errorf("expected: %v, saw: %v", buf, writer)
 		}
@@ -55,9 +53,7 @@ func TestSetupOutputWriterFile(t *testing.T) {
 	f := cmdtesting.NewTestFactory()
 	defer f.Cleanup()
 
-	cmd := NewCmdClusterInfoDump(f, genericclioptions.NewTestIOStreamsDiscard())
+	writer := setupOutputWriter(dir, buf, file)
-	cmd.Flag("output-directory").Value.Set(dir)
-	writer := setupOutputWriter(cmd, buf, file)
 	if writer == buf {
 		t.Errorf("expected: %v, saw: %v", buf, writer)
 	}

pkg/kubectl/cmd/cmd.go

@@ -308,7 +308,7 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 			Message: "Advanced Commands:",
 			Commands: []*cobra.Command{
 				NewCmdApply("kubectl", f, ioStreams),
-				NewCmdPatch(f, out),
+				NewCmdPatch(f, ioStreams),
 				NewCmdReplace(f, out, err),
 				NewCmdConvert(f, ioStreams),
 			},
@@ -349,8 +349,8 @@ func NewKubectlCommand(f cmdutil.Factory, in io.Reader, out, err io.Writer) *cob
 	cmds.AddCommand(alpha)
 	cmds.AddCommand(cmdconfig.NewCmdConfig(f, clientcmd.NewDefaultPathOptions(), out, err))
 	cmds.AddCommand(NewCmdPlugin(f, in, out, err))
-	cmds.AddCommand(NewCmdVersion(f, out))
+	cmds.AddCommand(NewCmdVersion(f, ioStreams))
-	cmds.AddCommand(NewCmdApiVersions(f, out))
+	cmds.AddCommand(NewCmdApiVersions(f, ioStreams))
 	cmds.AddCommand(NewCmdApiResources(f, ioStreams))
 	cmds.AddCommand(NewCmdOptions(out))
 

pkg/kubectl/cmd/label.go

@@ -68,6 +68,12 @@ type LabelOptions struct {
 
 	Recorder genericclioptions.Recorder
 
+	namespace                    string
+	enforceNamespace             bool
+	includeUninitialized         bool
+	builder                      *resource.Builder
+	unstructuredClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
+
 	// Common shared fields
 	genericclioptions.IOStreams
 }
@@ -125,13 +131,9 @@ func NewCmdLabel(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobr
 		Long:    fmt.Sprintf(labelLong, validation.LabelValueMaxLength),
 		Example: labelExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			if err := o.Complete(f, cmd, args); err != nil {
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-				cmdutil.CheckErr(cmdutil.UsageErrorf(cmd, err.Error()))
+			cmdutil.CheckErr(o.Validate())
-			}
+			cmdutil.CheckErr(o.RunLabel())
-			if err := o.Validate(); err != nil {
-				cmdutil.CheckErr(cmdutil.UsageErrorf(cmd, err.Error()))
-			}
-			cmdutil.CheckErr(o.RunLabel(f, cmd))
 		},
 		ValidArgs:  validArgs,
 		ArgAliases: kubectl.ResourceAliases(validArgs),
@@ -188,10 +190,18 @@ func (o *LabelOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st
 	o.newLabels, o.removeLabels, err = parseLabels(labelArgs)
 
 	if o.list && len(o.outputFormat) > 0 {
-		return cmdutil.UsageErrorf(cmd, "--list and --output may not be specified together")
+		return fmt.Errorf("--list and --output may not be specified together")
 	}
 
-	return err
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+	o.includeUninitialized = cmdutil.ShouldIncludeUninitialized(cmd, false)
+	o.builder = f.NewBuilder()
+	o.unstructuredClientForMapping = f.UnstructuredClientForMapping
+
+	return nil
 }
 
 // Validate checks to the LabelOptions to see if there is sufficient information run the command.
@@ -209,20 +219,14 @@ func (o *LabelOptions) Validate() error {
 }
 
 // RunLabel does the work
-func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o *LabelOptions) RunLabel() error {
-	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
+	b := o.builder.
-	if err != nil {
-		return err
-	}
-
-	includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false)
-	b := f.NewBuilder().
 		Unstructured().
 		LocalParam(o.local).
 		ContinueOnError().
-		NamespaceParam(cmdNamespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
-		IncludeUninitialized(includeUninitialized).
+		IncludeUninitialized(o.includeUninitialized).
 		Flatten()
 
 	if !o.local {
@@ -294,7 +298,7 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error {
 			}
 
 			mapping := info.ResourceMapping()
-			client, err := f.UnstructuredClientForMapping(mapping)
+			client, err := o.unstructuredClientForMapping(mapping)
 			if err != nil {
 				return err
 			}

pkg/kubectl/cmd/label_test.go

@@ -342,7 +342,7 @@ func TestLabelErrors(t *testing.T) {
 				err = opts.Validate()
 			}
 			if err == nil {
-				err = opts.RunLabel(tf, cmd)
+				err = opts.RunLabel()
 			}
 			if !testCase.errFn(err) {
 				t.Errorf("%s: unexpected error: %v", k, err)
@@ -400,7 +400,7 @@ func TestLabelForResourceFromFile(t *testing.T) {
 		err = opts.Validate()
 	}
 	if err == nil {
-		err = opts.RunLabel(tf, cmd)
+		err = opts.RunLabel()
 	}
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -434,7 +434,7 @@ func TestLabelLocal(t *testing.T) {
 		err = opts.Validate()
 	}
 	if err == nil {
-		err = opts.RunLabel(tf, cmd)
+		err = opts.RunLabel()
 	}
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -491,7 +491,7 @@ func TestLabelMultipleObjects(t *testing.T) {
 		err = opts.Validate()
 	}
 	if err == nil {
-		err = opts.RunLabel(tf, cmd)
+		err = opts.RunLabel()
 	}
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)

pkg/kubectl/cmd/patch.go

@@ -18,7 +18,6 @@ package cmd
 
 import (
 	"fmt"
-	"io"
 	"reflect"
 	"strings"
 
@@ -26,6 +25,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -49,16 +49,25 @@ var patchTypes = map[string]types.PatchType{"json": types.JSONPatchType, "merge"
 // referencing the cmd.Flags()
 type PatchOptions struct {
 	resource.FilenameOptions
+
 	RecordFlags *genericclioptions.RecordFlags
 	PrintFlags  *printers.PrintFlags
 	ToPrinter   func(string) (printers.ResourcePrinterFunc, error)
+	Recorder    genericclioptions.Recorder
 
-	Local  bool
+	Local     bool
-	DryRun bool
+	PatchType string
+	Patch     string
 
-	Recorder genericclioptions.Recorder
+	namespace                    string
+	enforceNamespace             bool
+	dryRun                       bool
+	outputFormat                 string
+	args                         []string
+	builder                      *resource.Builder
+	unstructuredClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
 
-	OutputFormat string
+	genericclioptions.IOStreams
 }
 
 var (
@@ -86,16 +95,17 @@ var (
 		kubectl patch pod valid-pod --type='json' -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"new image"}]'`))
 )
 
-func NewPatchOptions() *PatchOptions {
+func NewPatchOptions(ioStreams genericclioptions.IOStreams) *PatchOptions {
 	return &PatchOptions{
 		RecordFlags: genericclioptions.NewRecordFlags(),
 		Recorder:    genericclioptions.NoopRecorder{},
 		PrintFlags:  printers.NewPrintFlags("patched"),
+		IOStreams:   ioStreams,
 	}
 }
 
-func NewCmdPatch(f cmdutil.Factory, out io.Writer) *cobra.Command {
+func NewCmdPatch(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	o := NewPatchOptions()
+	o := NewPatchOptions(ioStreams)
 	validArgs := cmdutil.ValidArgList(f)
 
 	cmd := &cobra.Command{
@@ -105,8 +115,9 @@ func NewCmdPatch(f cmdutil.Factory, out io.Writer) *cobra.Command {
 		Long:    patchLong,
 		Example: patchExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(o.Complete(f, cmd))
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(o.RunPatch(f, out, cmd, args))
+			cmdutil.CheckErr(o.Validate())
+			cmdutil.CheckErr(o.RunPatch())
 		},
 		ValidArgs:  validArgs,
 		ArgAliases: kubectl.ResourceAliases(validArgs),
@@ -115,34 +126,30 @@ func NewCmdPatch(f cmdutil.Factory, out io.Writer) *cobra.Command {
 	o.RecordFlags.AddFlags(cmd)
 	o.PrintFlags.AddFlags(cmd)
 
-	cmd.Flags().StringP("patch", "p", "", "The patch to be applied to the resource JSON file.")
+	cmd.Flags().StringVarP(&o.Patch, "patch", "p", "", "The patch to be applied to the resource JSON file.")
 	cmd.MarkFlagRequired("patch")
-	cmd.Flags().String("type", "strategic", fmt.Sprintf("The type of patch being provided; one of %v", sets.StringKeySet(patchTypes).List()))
+	cmd.Flags().StringVar(&o.PatchType, "type", "strategic", fmt.Sprintf("The type of patch being provided; one of %v", sets.StringKeySet(patchTypes).List()))
 	cmdutil.AddDryRunFlag(cmd)
-
+	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, "identifying the resource to update")
-	usage := "identifying the resource to update"
-	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, usage)
-
 	cmd.Flags().BoolVar(&o.Local, "local", o.Local, "If true, patch will operate on the content of the file, not the server-side resource.")
 
 	return cmd
 }
 
-func (o *PatchOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
+func (o *PatchOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
 	var err error
-
 	o.RecordFlags.Complete(f.Command(cmd, false))
 	o.Recorder, err = o.RecordFlags.ToRecorder()
 	if err != nil {
 		return err
 	}
 
-	o.OutputFormat = cmdutil.GetFlagString(cmd, "output")
+	o.outputFormat = cmdutil.GetFlagString(cmd, "output")
-	o.DryRun = cmdutil.GetFlagBool(cmd, "dry-run")
+	o.dryRun = cmdutil.GetFlagBool(cmd, "dry-run")
 
 	o.ToPrinter = func(operation string) (printers.ResourcePrinterFunc, error) {
 		o.PrintFlags.NamePrintFlags.Operation = operation
-		if o.DryRun {
+		if o.dryRun {
 			o.PrintFlags.Complete("%s (dry run)")
 		}
 
@@ -153,46 +160,50 @@ func (o *PatchOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 		return printer.PrintObj, nil
 	}
 
-	return err
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
-}
-
-func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string) error {
-	switch {
-	case o.Local && len(args) != 0:
-		return fmt.Errorf("cannot specify --local and server resources")
-	}
-
-	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
 	if err != nil {
 		return err
 	}
+	o.args = args
+	o.builder = f.NewBuilder()
+	o.unstructuredClientForMapping = f.UnstructuredClientForMapping
 
-	patchType := types.StrategicMergePatchType
+	return nil
-	patchTypeString := strings.ToLower(cmdutil.GetFlagString(cmd, "type"))
+}
-	if len(patchTypeString) != 0 {
+
-		ok := false
+func (o *PatchOptions) Validate() error {
-		patchType, ok = patchTypes[patchTypeString]
+	if o.Local && len(o.args) != 0 {
-		if !ok {
+		return fmt.Errorf("cannot specify --local and server resources")
-			return cmdutil.UsageErrorf(cmd, "--type must be one of %v, not %q",
+	}
-				sets.StringKeySet(patchTypes).List(), patchTypeString)
+	if len(o.Patch) == 0 {
+		return fmt.Errorf("must specify -p to patch")
+	}
+	if len(o.PatchType) != 0 {
+		if _, ok := patchTypes[strings.ToLower(o.PatchType)]; !ok {
+			return fmt.Errorf("--type must be one of %v, not %q", sets.StringKeySet(patchTypes).List(), o.PatchType)
 		}
 	}
 
-	patch := cmdutil.GetFlagString(cmd, "patch")
+	return nil
-	if len(patch) == 0 {
+}
-		return cmdutil.UsageErrorf(cmd, "Must specify -p to patch")
+
-	}
+func (o *PatchOptions) RunPatch() error {
-	patchBytes, err := yaml.ToJSON([]byte(patch))
+	patchType := types.StrategicMergePatchType
-	if err != nil {
+	if len(o.PatchType) != 0 {
-		return fmt.Errorf("unable to parse %q: %v", patch, err)
+		patchType = patchTypes[strings.ToLower(o.PatchType)]
 	}
 
-	r := f.NewBuilder().
+	patchBytes, err := yaml.ToJSON([]byte(o.Patch))
+	if err != nil {
+		return fmt.Errorf("unable to parse %q: %v", o.Patch, err)
+	}
+
+	r := o.builder.
 		Unstructured().
 		ContinueOnError().
-		NamespaceParam(cmdNamespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
-		ResourceTypeOrNameArgs(false, args...).
+		ResourceTypeOrNameArgs(false, o.args...).
 		Flatten().
 		Do()
 	err = r.Err()
@@ -207,12 +218,12 @@ func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Com
 		}
 		name, namespace := info.Name, info.Namespace
 		mapping := info.ResourceMapping()
-		client, err := f.UnstructuredClientForMapping(mapping)
+		client, err := o.unstructuredClientForMapping(mapping)
 		if err != nil {
 			return err
 		}
 
-		if !o.Local && !o.DryRun {
+		if !o.Local && !o.dryRun {
 			helper := resource.NewHelper(client, mapping)
 			patchedObj, err := helper.Patch(namespace, name, patchType, patchBytes)
 			if err != nil {
@@ -242,7 +253,7 @@ func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Com
 			if err != nil {
 				return err
 			}
-			printer.PrintObj(info.Object, out)
+			printer.PrintObj(info.Object, o.Out)
 
 			// if object was not successfully patched, exit with error code 1
 			if !didPatch {
@@ -285,7 +296,7 @@ func (o *PatchOptions) RunPatch(f cmdutil.Factory, out io.Writer, cmd *cobra.Com
 		if err != nil {
 			return err
 		}
-		return printer.PrintObj(info.Object, out)
+		return printer.PrintObj(info.Object, o.Out)
 	})
 	if err != nil {
 		return err

pkg/kubectl/cmd/patch_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cmd
 
 import (
-	"bytes"
 	"net/http"
 	"strings"
 	"testing"
@@ -25,6 +24,7 @@ import (
 	"k8s.io/client-go/rest/fake"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/scheme"
 )
 
@@ -56,9 +56,9 @@ func TestPatchObject(t *testing.T) {
 		}),
 	}
 	tf.Namespace = "test"
-	buf := bytes.NewBuffer([]byte{})
+	stream, _, buf, _ := genericclioptions.NewTestIOStreams()
 
-	cmd := NewCmdPatch(tf, buf)
+	cmd := NewCmdPatch(tf, stream)
 	cmd.Flags().Set("namespace", "test")
 	cmd.Flags().Set("patch", `{"spec":{"type":"NodePort"}}`)
 	cmd.Flags().Set("output", "name")
@@ -91,9 +91,9 @@ func TestPatchObjectFromFile(t *testing.T) {
 		}),
 	}
 	tf.Namespace = "test"
-	buf := bytes.NewBuffer([]byte{})
+	stream, _, buf, _ := genericclioptions.NewTestIOStreams()
 
-	cmd := NewCmdPatch(tf, buf)
+	cmd := NewCmdPatch(tf, stream)
 	cmd.Flags().Set("namespace", "test")
 	cmd.Flags().Set("patch", `{"spec":{"type":"NodePort"}}`)
 	cmd.Flags().Set("output", "name")
@@ -139,8 +139,8 @@ func TestPatchNoop(t *testing.T) {
 			patchObject.Annotations = map[string]string{}
 		}
 		patchObject.Annotations["foo"] = "bar"
-		buf := bytes.NewBuffer([]byte{})
+		stream, _, buf, _ := genericclioptions.NewTestIOStreams()
-		cmd := NewCmdPatch(tf, buf)
+		cmd := NewCmdPatch(tf, stream)
 		cmd.Flags().Set("namespace", "test")
 		cmd.Flags().Set("patch", `{"metadata":{"annotations":{"foo":"bar"}}}`)
 		cmd.Run(cmd, []string{"services", "frontend"})
@@ -179,9 +179,9 @@ func TestPatchObjectFromFileOutput(t *testing.T) {
 		}),
 	}
 	tf.Namespace = "test"
-	buf := bytes.NewBuffer([]byte{})
+	stream, _, buf, _ := genericclioptions.NewTestIOStreams()
 
-	cmd := NewCmdPatch(tf, buf)
+	cmd := NewCmdPatch(tf, stream)
 	cmd.Flags().Set("namespace", "test")
 	cmd.Flags().Set("patch", `{"spec":{"type":"NodePort"}}`)
 	cmd.Flags().Set("output", "yaml")

pkg/kubectl/cmd/scale.go

@@ -70,46 +70,40 @@ type ScaleOptions struct {
 	PrintFlags      *printers.PrintFlags
 	PrintObj        printers.ResourcePrinterFunc
 
-	BuilderArgs      []string
+	Selector        string
-	Namespace        string
+	All             bool
-	EnforceNamespace bool
+	Replicas        int
-
-	Builder   *resource.Builder
-	ClientSet internalclientset.Interface
-	Scaler    kubectl.Scaler
-
-	All      bool
-	Selector string
-
-	CmdParent string
-
 	ResourceVersion string
 	CurrentReplicas int
-	Replicas        int
+	Timeout         time.Duration
-	Duration        time.Duration
 
-	ClientForMapping func(*meta.RESTMapping) (resource.RESTClient, error)
+	Recorder                     genericclioptions.Recorder
-
+	builder                      *resource.Builder
-	Recorder genericclioptions.Recorder
+	namespace                    string
+	enforceNamespace             bool
+	args                         []string
+	shortOutput                  bool
+	clientSet                    internalclientset.Interface
+	scaler                       kubectl.Scaler
+	unstructuredClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error)
+	parent                       string
 
 	genericclioptions.IOStreams
 }
 
 func NewScaleOptions(ioStreams genericclioptions.IOStreams) *ScaleOptions {
 	return &ScaleOptions{
-		RecordFlags: genericclioptions.NewRecordFlags(),
+		RecordFlags:     genericclioptions.NewRecordFlags(),
-		PrintFlags:  printers.NewPrintFlags("scaled"),
+		PrintFlags:      printers.NewPrintFlags("scaled"),
-
 		CurrentReplicas: -1,
-
+		Recorder:        genericclioptions.NoopRecorder{},
-		Recorder:  genericclioptions.NoopRecorder{},
+		IOStreams:       ioStreams,
-		IOStreams: ioStreams,
 	}
 }
 
 // NewCmdScale returns a cobra command with the appropriate configuration and flags to run scale
-func NewCmdScale(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command {
+func NewCmdScale(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	o := NewScaleOptions(streams)
+	o := NewScaleOptions(ioStreams)
 
 	validArgs := []string{"deployment", "replicaset", "replicationcontroller", "statefulset"}
 	argAliases := kubectl.ResourceAliases(validArgs)
@@ -122,7 +116,7 @@ func NewCmdScale(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.
 		Example: scaleExample,
 		Run: func(cmd *cobra.Command, args []string) {
 			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd))
+			cmdutil.CheckErr(o.Validate(cmd))
 			cmdutil.CheckErr(o.RunScale())
 		},
 		ValidArgs:  validArgs,
@@ -138,71 +132,68 @@ func NewCmdScale(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.
 	cmd.Flags().IntVar(&o.CurrentReplicas, "current-replicas", o.CurrentReplicas, "Precondition for current size. Requires that the current size of the resource match this value in order to scale.")
 	cmd.Flags().IntVar(&o.Replicas, "replicas", o.Replicas, "The new desired number of replicas. Required.")
 	cmd.MarkFlagRequired("replicas")
-	cmd.Flags().DurationVar(&o.Duration, "timeout", o.Duration, "The length of time to wait before giving up on a scale operation, zero means don't wait. Any other values should contain a corresponding time unit (e.g. 1s, 2m, 3h).")
+	cmd.Flags().DurationVar(&o.Timeout, "timeout", 0, "The length of time to wait before giving up on a scale operation, zero means don't wait. Any other values should contain a corresponding time unit (e.g. 1s, 2m, 3h).")
-
+	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, "identifying the resource to set a new size")
-	usage := "identifying the resource to set a new size"
-	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, usage)
 	return cmd
 }
 
 func (o *ScaleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error {
 	var err error
-	o.Namespace, o.EnforceNamespace, err = f.DefaultNamespace()
+	o.RecordFlags.Complete(f.Command(cmd, false))
+	o.Recorder, err = o.RecordFlags.ToRecorder()
 	if err != nil {
 		return err
 	}
-
-	o.CmdParent = cmd.Parent().Name()
-	o.Builder = f.NewBuilder()
-
-	o.ClientSet, err = f.ClientSet()
-	if err != nil {
-		return err
-	}
-
-	o.Scaler, err = f.Scaler()
-	if err != nil {
-		return err
-	}
-
-	o.BuilderArgs = args
-	o.ClientForMapping = f.UnstructuredClientForMapping
-
 	printer, err := o.PrintFlags.ToPrinter()
 	if err != nil {
 		return err
 	}
 	o.PrintObj = printer.PrintObj
 
-	o.RecordFlags.Complete(f.Command(cmd, false))
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
-	o.Recorder, err = o.RecordFlags.ToRecorder()
 	if err != nil {
 		return err
 	}
+	o.builder = f.NewBuilder()
+	o.args = args
+	o.shortOutput = cmdutil.GetFlagString(cmd, "output") == "name"
+	o.clientSet, err = f.ClientSet()
+	if err != nil {
+		return err
+	}
+	o.scaler, err = f.Scaler()
+	if err != nil {
+		return err
+	}
+	o.unstructuredClientForMapping = f.UnstructuredClientForMapping
+	o.parent = cmd.Parent().Name()
+
+	return nil
+}
+
+func (o *ScaleOptions) Validate(cmd *cobra.Command) error {
+	if err := cmdutil.ValidateOutputArgs(cmd); err != nil {
+		return err
+	}
+	if o.Replicas < 0 {
+		return fmt.Errorf("The --replicas=COUNT flag is required, and COUNT must be greater than or equal to 0")
+	}
 
 	return nil
 }
 
 // RunScale executes the scaling
 func (o *ScaleOptions) RunScale() error {
-
+	r := o.builder.
-	if o.Replicas < 0 {
-		return fmt.Errorf("The --replicas=COUNT flag is required, and COUNT must be greater than or equal to 0")
-	}
-
-	r := o.Builder.
 		Unstructured().
 		ContinueOnError().
-		NamespaceParam(o.Namespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(o.EnforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
-		ResourceTypeOrNameArgs(o.All, o.BuilderArgs...).
+		ResourceTypeOrNameArgs(o.All, o.args...).
 		Flatten().
 		LabelSelectorParam(o.Selector).
 		Do()
 	err := r.Err()
-	if resource.IsUsageError(err) {
-		return fmt.Errorf("%v", err)
-	}
 	if err != nil {
 		return err
 	}
@@ -219,12 +210,11 @@ func (o *ScaleOptions) RunScale() error {
 		return fmt.Errorf("cannot use --resource-version with multiple resources")
 	}
 
-	currentSize := o.CurrentReplicas
+	precondition := &kubectl.ScalePrecondition{Size: o.CurrentReplicas, ResourceVersion: o.ResourceVersion}
-	precondition := &kubectl.ScalePrecondition{Size: currentSize, ResourceVersion: o.ResourceVersion}
 	retry := kubectl.NewRetryParams(kubectl.Interval, kubectl.Timeout)
 
 	var waitForReplicas *kubectl.RetryParams
-	if timeout := o.Duration; timeout != 0 {
+	if o.Timeout != 0 {
 		waitForReplicas = kubectl.NewRetryParams(kubectl.Interval, timeout)
 	}
 
@@ -237,15 +227,15 @@ func (o *ScaleOptions) RunScale() error {
 		mapping := info.ResourceMapping()
 		if mapping.Resource == "jobs" {
 			// go down the legacy jobs path.  This can be removed in 3.14  For now, contain it.
-			fmt.Fprintf(o.ErrOut, "%s scale job is DEPRECATED and will be removed in a future version.\n", o.CmdParent)
+			fmt.Fprintf(o.ErrOut, "%s scale job is DEPRECATED and will be removed in a future version.\n", o.parent)
 
-			if err := ScaleJob(info, o.ClientSet.Batch(), uint(o.Replicas), precondition, retry, waitForReplicas); err != nil {
+			if err := ScaleJob(info, o.clientSet.Batch(), uint(o.Replicas), precondition, retry, waitForReplicas); err != nil {
 				return err
 			}
 
 		} else {
 			gvk := mapping.GroupVersionKind.GroupVersion().WithResource(mapping.Resource)
-			if err := o.Scaler.Scale(info.Namespace, info.Name, uint(o.Replicas), precondition, retry, waitForReplicas, gvk.GroupResource()); err != nil {
+			if err := o.scaler.Scale(info.Namespace, info.Name, uint(o.Replicas), precondition, retry, waitForReplicas, gvk.GroupResource()); err != nil {
 				return err
 			}
 		}
@@ -254,7 +244,7 @@ func (o *ScaleOptions) RunScale() error {
 		if mergePatch, err := o.Recorder.MakeRecordMergePatch(info.Object); err != nil {
 			glog.V(4).Infof("error recording current command: %v", err)
 		} else if len(mergePatch) > 0 {
-			client, err := o.ClientForMapping(mapping)
+			client, err := o.unstructuredClientForMapping(mapping)
 			if err != nil {
 				return err
 			}

pkg/kubectl/cmd/set/set_env.go

@@ -24,19 +24,19 @@ import (
 	"strings"
 
 	"github.com/spf13/cobra"
+
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	envutil "k8s.io/kubernetes/pkg/kubectl/cmd/util/env"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/printers"
-
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/kubernetes/pkg/api/legacyscheme"
-	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 )
 
 var (
@@ -94,32 +94,30 @@ var (
 
 type EnvOptions struct {
 	PrintFlags *printers.PrintFlags
-
 	resource.FilenameOptions
-	EnvParams []string
-	EnvArgs   []string
-	Resources []string
 
-	All       bool
+	EnvParams         []string
-	Resolve   bool
+	All               bool
-	List      bool
+	Resolve           bool
-	Local     bool
+	List              bool
-	Overwrite bool
+	Local             bool
-	DryRun    bool
+	Overwrite         bool
-
-	ResourceVersion   string
 	ContainerSelector string
 	Selector          string
-	Output            string
 	From              string
 	Prefix            string
 
 	PrintObj printers.ResourcePrinterFunc
 
-	Builder *resource.Builder
+	envArgs                []string
-	Infos   []*resource.Info
+	resources              []string
-
+	output                 string
-	UpdatePodSpecForObject func(obj runtime.Object, fn func(*v1.PodSpec) error) (bool, error)
+	dryRun                 bool
+	builder                func() *resource.Builder
+	updatePodSpecForObject func(obj runtime.Object, fn func(*v1.PodSpec) error) (bool, error)
+	namespace              string
+	enforceNamespace       bool
+	clientset              *kubernetes.Clientset
 
 	genericclioptions.IOStreams
 }
@@ -139,7 +137,7 @@ func NewEnvOptions(streams genericclioptions.IOStreams) *EnvOptions {
 
 // NewCmdEnv implements the OpenShift cli env command
 func NewCmdEnv(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command {
-	options := NewEnvOptions(streams)
+	o := NewEnvOptions(streams)
 	cmd := &cobra.Command{
 		Use: "env RESOURCE/NAME KEY_1=VAL_1 ... KEY_N=VAL_N",
 		DisableFlagsInUseLine: true,
@@ -147,24 +145,25 @@ func NewCmdEnv(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Co
 		Long:    envLong,
 		Example: fmt.Sprintf(envExample),
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.Complete(f, cmd, args))
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(options.RunEnv(f))
+			cmdutil.CheckErr(o.Validate())
+			cmdutil.CheckErr(o.RunEnv())
 		},
 	}
 	usage := "the resource to update the env"
-	cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
+	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, usage)
-	cmd.Flags().StringVarP(&options.ContainerSelector, "containers", "c", options.ContainerSelector, "The names of containers in the selected pod templates to change - may use wildcards")
+	cmd.Flags().StringVarP(&o.ContainerSelector, "containers", "c", o.ContainerSelector, "The names of containers in the selected pod templates to change - may use wildcards")
-	cmd.Flags().StringP("from", "", "", "The name of a resource from which to inject environment variables")
+	cmd.Flags().StringVarP(&o.From, "from", "", "", "The name of a resource from which to inject environment variables")
-	cmd.Flags().StringP("prefix", "", "", "Prefix to append to variable names")
+	cmd.Flags().StringVarP(&o.Prefix, "prefix", "", "", "Prefix to append to variable names")
-	cmd.Flags().StringArrayVarP(&options.EnvParams, "env", "e", options.EnvParams, "Specify a key-value pair for an environment variable to set into each container.")
+	cmd.Flags().StringArrayVarP(&o.EnvParams, "env", "e", o.EnvParams, "Specify a key-value pair for an environment variable to set into each container.")
-	cmd.Flags().BoolVar(&options.List, "list", options.List, "If true, display the environment and any changes in the standard format. this flag will removed when we have kubectl view env.")
+	cmd.Flags().BoolVar(&o.List, "list", o.List, "If true, display the environment and any changes in the standard format. this flag will removed when we have kubectl view env.")
-	cmd.Flags().BoolVar(&options.Resolve, "resolve", options.Resolve, "If true, show secret or configmap references when listing variables")
+	cmd.Flags().BoolVar(&o.Resolve, "resolve", o.Resolve, "If true, show secret or configmap references when listing variables")
-	cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on")
+	cmd.Flags().StringVarP(&o.Selector, "selector", "l", o.Selector, "Selector (label query) to filter on")
-	cmd.Flags().BoolVar(&options.Local, "local", options.Local, "If true, set env will NOT contact api-server but run locally.")
+	cmd.Flags().BoolVar(&o.Local, "local", o.Local, "If true, set env will NOT contact api-server but run locally.")
-	cmd.Flags().BoolVar(&options.All, "all", options.All, "If true, select all resources in the namespace of the specified resource types")
+	cmd.Flags().BoolVar(&o.All, "all", o.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.")
+	cmd.Flags().BoolVar(&o.Overwrite, "overwrite", o.Overwrite, "If true, allow environment to be overwritten, otherwise reject updates that overwrite existing environment.")
 
-	options.PrintFlags.AddFlags(cmd)
+	o.PrintFlags.AddFlags(cmd)
 
 	cmdutil.AddDryRunFlag(cmd)
 	return cmd
@@ -187,30 +186,17 @@ func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 	if o.All && len(o.Selector) > 0 {
 		return fmt.Errorf("cannot set --all and --selector at the same time")
 	}
-	resources, envArgs, ok := envutil.SplitEnvironmentFromResources(args)
+	ok := false
+	o.resources, o.envArgs, ok = envutil.SplitEnvironmentFromResources(args)
 	if !ok {
-		return cmdutil.UsageErrorf(cmd, "all resources must be specified before environment changes: %s", strings.Join(args, " "))
+		return fmt.Errorf("all resources must be specified before environment changes: %s", strings.Join(args, " "))
-	}
-	if len(o.Filenames) == 0 && len(resources) < 1 {
-		return cmdutil.UsageErrorf(cmd, "one or more resources must be specified as <resource> <name> or <resource>/<name>")
 	}
 
-	o.UpdatePodSpecForObject = f.UpdatePodSpecForObject
+	o.updatePodSpecForObject = f.UpdatePodSpecForObject
-	o.ContainerSelector = cmdutil.GetFlagString(cmd, "containers")
+	o.output = cmdutil.GetFlagString(cmd, "output")
-	o.List = cmdutil.GetFlagBool(cmd, "list")
+	o.dryRun = cmdutil.GetDryRunFlag(cmd)
-	o.Resolve = cmdutil.GetFlagBool(cmd, "resolve")
-	o.Selector = cmdutil.GetFlagString(cmd, "selector")
-	o.All = cmdutil.GetFlagBool(cmd, "all")
-	o.Overwrite = cmdutil.GetFlagBool(cmd, "overwrite")
-	o.Output = cmdutil.GetFlagString(cmd, "output")
-	o.From = cmdutil.GetFlagString(cmd, "from")
-	o.Prefix = cmdutil.GetFlagString(cmd, "prefix")
-	o.DryRun = cmdutil.GetDryRunFlag(cmd)
 
-	o.EnvArgs = envArgs
+	if o.dryRun {
-	o.Resources = resources
-
-	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
@@ -223,41 +209,43 @@ func (o *EnvOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri
 	}
 	o.PrintObj = printer.PrintObj
 
-	if o.List && len(o.Output) > 0 {
+	o.clientset, err = f.KubernetesClientSet()
-		return cmdutil.UsageErrorf(cmd, "--list and --output may not be specified together")
+	if err != nil {
+		return err
 	}
+	o.namespace, o.enforceNamespace, err = f.DefaultNamespace()
+	if err != nil {
+		return err
+	}
+	o.builder = f.NewBuilder
 
 	return nil
 }
 
+func (o *EnvOptions) Validate() error {
+	if len(o.Filenames) == 0 && len(o.resources) < 1 {
+		return fmt.Errorf("one or more resources must be specified as <resource> <name> or <resource>/<name>")
+	}
+	if o.List && len(o.output) > 0 {
+		return fmt.Errorf("--list and --output may not be specified together")
+	}
+	return nil
+}
+
 // RunEnv contains all the necessary functionality for the OpenShift cli env command
-func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
+func (o *EnvOptions) RunEnv() error {
-	var kubeClient *kubernetes.Clientset
+	env, remove, err := envutil.ParseEnv(append(o.EnvParams, o.envArgs...), o.In)
-	if o.List {
-		client, err := f.KubernetesClientSet()
-		if err != nil {
-			return err
-		}
-		kubeClient = client
-	}
-
-	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
-	if err != nil {
-		return err
-	}
-
-	env, remove, err := envutil.ParseEnv(append(o.EnvParams, o.EnvArgs...), o.In)
 	if err != nil {
 		return err
 	}
 
 	if len(o.From) != 0 {
-		b := f.NewBuilder().
+		b := o.builder().
 			Internal(legacyscheme.Scheme).
 			LocalParam(o.Local).
 			ContinueOnError().
-			NamespaceParam(cmdNamespace).DefaultNamespace().
+			NamespaceParam(o.namespace).DefaultNamespace().
-			FilenameParam(enforceNamespace, &o.FilenameOptions).
+			FilenameParam(o.enforceNamespace, &o.FilenameOptions).
 			Flatten()
 
 		if !o.Local {
@@ -320,27 +308,27 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 		}
 	}
 
-	b := f.NewBuilder().
+	b := o.builder().
 		Internal(legacyscheme.Scheme).
 		LocalParam(o.Local).
 		ContinueOnError().
-		NamespaceParam(cmdNamespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
-		FilenameParam(enforceNamespace, &o.FilenameOptions).
+		FilenameParam(o.enforceNamespace, &o.FilenameOptions).
 		Flatten()
 
 	if !o.Local {
 		b.LabelSelectorParam(o.Selector).
-			ResourceTypeOrNameArgs(o.All, o.Resources...).
+			ResourceTypeOrNameArgs(o.All, o.resources...).
 			Latest()
 	}
 
-	o.Infos, err = b.Do().Infos()
+	infos, err := b.Do().Infos()
 	if err != nil {
 		return err
 	}
-	patches := CalculatePatches(o.Infos, cmdutil.InternalVersionJSONEncoder(), func(info *resource.Info) ([]byte, error) {
+	patches := CalculatePatches(infos, cmdutil.InternalVersionJSONEncoder(), func(info *resource.Info) ([]byte, error) {
 		info.Object = info.AsVersioned(legacyscheme.Scheme)
-		_, err := o.UpdatePodSpecForObject(info.Object, func(spec *v1.PodSpec) error {
+		_, err := o.updatePodSpecForObject(info.Object, func(spec *v1.PodSpec) error {
 			resolutionErrorsEncountered := false
 			containers, _ := selectContainers(spec.Containers, o.ContainerSelector)
 			if len(containers) == 0 {
@@ -373,7 +361,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 							continue
 						}
 
-						value, err := envutil.GetEnvVarRefValue(kubeClient, cmdNamespace, store, env.ValueFrom, info.Object, c)
+						value, err := envutil.GetEnvVarRefValue(o.clientset, o.namespace, store, env.ValueFrom, info.Object, c)
 						// Print the resolved value
 						if err == nil {
 							fmt.Fprintf(o.Out, "%s=%s\n", env.Name, value)
@@ -429,7 +417,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 			continue
 		}
 
-		if o.Local || o.DryRun {
+		if o.Local || o.dryRun {
 			if err := o.PrintObj(patch.Info.AsVersioned(legacyscheme.Scheme), o.Out); err != nil {
 				return err
 			}
@@ -445,7 +433,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error {
 
 		// make sure arguments to set or replace environment variables are set
 		// before returning a successful message
-		if len(env) == 0 && len(o.EnvArgs) == 0 {
+		if len(env) == 0 && len(o.envArgs) == 0 {
 			return fmt.Errorf("at least one environment variable must be provided")
 		}
 

pkg/kubectl/cmd/set/set_env_test.go

@@ -61,33 +61,28 @@ func TestSetEnvLocal(t *testing.T) {
 	}
 	tf.Namespace = "test"
 	tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}}
-
 	outputFormat := "name"
 
-	streams, _, buf, _ := genericclioptions.NewTestIOStreams()
+	streams, _, buf, bufErr := genericclioptions.NewTestIOStreams()
-	cmd := NewCmdEnv(tf, streams)
+	opts := NewEnvOptions(streams)
-	cmd.SetOutput(buf)
+	opts.PrintFlags = &printers.PrintFlags{
-	cmd.Flags().Set("output", outputFormat)
+		JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
-	cmd.Flags().Set("local", "true")
+		NamePrintFlags:     printers.NewNamePrintFlags(""),
-
+		OutputFormat:       &outputFormat,
-	opts := EnvOptions{
-		PrintFlags: &printers.PrintFlags{
-			JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
-			NamePrintFlags:     printers.NewNamePrintFlags(""),
-
-			OutputFormat: &outputFormat,
-		},
-		FilenameOptions: resource.FilenameOptions{
-			Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"}},
-		Local:     true,
-		IOStreams: streams,
 	}
-	err := opts.Complete(tf, cmd, []string{"env=prod"})
+	opts.FilenameOptions = resource.FilenameOptions{
-	if err == nil {
+		Filenames: []string{"../../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"},
-		err = opts.RunEnv(tf)
 	}
-	if err != nil {
+	opts.Local = true
-		t.Fatalf("unexpected error: %v", err)
+
+	err := opts.Complete(tf, NewCmdEnv(tf, streams), []string{"env=prod"})
+	assert.NoError(t, err)
+	err = opts.Validate()
+	assert.NoError(t, err)
+	err = opts.RunEnv()
+	assert.NoError(t, err)
+	if bufErr.Len() > 0 {
+		t.Errorf("unexpected error: %s", string(bufErr.String()))
 	}
 	if !strings.Contains(buf.String(), "replicationcontroller/cassandra") {
 		t.Errorf("did not set env: %s", buf.String())
@@ -99,7 +94,6 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) {
 	defer tf.Cleanup()
 
 	ns := legacyscheme.Codecs
-
 	tf.Client = &fake.RESTClient{
 		GroupVersion:         schema.GroupVersion{Version: ""},
 		NegotiatedSerializer: ns,
@@ -112,33 +106,27 @@ func TestSetMultiResourcesEnvLocal(t *testing.T) {
 	tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}}
 
 	outputFormat := "name"
-
+	streams, _, buf, bufErr := genericclioptions.NewTestIOStreams()
-	streams, _, buf, _ := genericclioptions.NewTestIOStreams()
+	opts := NewEnvOptions(streams)
-	cmd := NewCmdEnv(tf, streams)
+	opts.PrintFlags = &printers.PrintFlags{
-	cmd.SetOutput(buf)
+		JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
-	cmd.Flags().Set("output", outputFormat)
+		NamePrintFlags:     printers.NewNamePrintFlags(""),
-	cmd.Flags().Set("local", "true")
+		OutputFormat:       &outputFormat,
-
-	opts := EnvOptions{
-		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"}},
-		Local:     true,
-		IOStreams: streams,
 	}
-	err := opts.Complete(tf, cmd, []string{"env=prod"})
+	opts.FilenameOptions = resource.FilenameOptions{
-	if err == nil {
+		Filenames: []string{"../../../../test/fixtures/pkg/kubectl/cmd/set/multi-resource-yaml.yaml"},
-		err = opts.RunEnv(tf)
-	}
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
 	}
+	opts.Local = true
 
+	err := opts.Complete(tf, NewCmdEnv(tf, streams), []string{"env=prod"})
+	assert.NoError(t, err)
+	err = opts.Validate()
+	assert.NoError(t, err)
+	err = opts.RunEnv()
+	assert.NoError(t, err)
+	if bufErr.Len() > 0 {
+		t.Errorf("unexpected error: %s", string(bufErr.String()))
+	}
 	expectedOut := "replicationcontroller/first-rc\nreplicationcontroller/second-rc\n"
 	if buf.String() != expectedOut {
 		t.Errorf("expected out:\n%s\nbut got:\n%s", expectedOut, buf.String())
@@ -472,6 +460,7 @@ func TestSetEnvRemote(t *testing.T) {
 			groupVersion := schema.GroupVersion{Group: input.apiGroup, Version: input.apiVersion}
 			testapi.Default = testapi.Groups[input.testAPIGroup]
 			tf := cmdtesting.NewTestFactory()
+			tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}}
 			defer tf.Cleanup()
 
 			codec := scheme.Codecs.CodecForVersions(scheme.Codecs.LegacyCodec(groupVersion), scheme.Codecs.UniversalDecoder(groupVersion), groupVersion, groupVersion)
@@ -505,23 +494,18 @@ func TestSetEnvRemote(t *testing.T) {
 			}
 
 			outputFormat := "yaml"
-
 			streams := genericclioptions.NewTestIOStreamsDiscard()
-			cmd := NewCmdEnv(tf, streams)
+			opts := NewEnvOptions(streams)
-			cmd.Flags().Set("output", outputFormat)
+			opts.PrintFlags = &printers.PrintFlags{
-			opts := EnvOptions{
+				JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
-				PrintFlags: &printers.PrintFlags{
+				NamePrintFlags:     printers.NewNamePrintFlags(""),
-					JSONYamlPrintFlags: printers.NewJSONYamlPrintFlags(),
+				OutputFormat:       &outputFormat,
-					NamePrintFlags:     printers.NewNamePrintFlags(""),
-
-					OutputFormat: &outputFormat,
-				},
-				Local:     false,
-				IOStreams: streams,
 			}
-			err := opts.Complete(tf, cmd, input.args)
+			opts.Local = false
+			opts.IOStreams = streams
+			err := opts.Complete(tf, NewCmdEnv(tf, streams), input.args)
 			assert.NoError(t, err)
-			err = opts.RunEnv(tf)
+			err = opts.RunEnv()
 			assert.NoError(t, err)
 		})
 	}

pkg/kubectl/cmd/set/set_subject.go

@@ -73,6 +73,8 @@ type SubjectOptions struct {
 	Groups          []string
 	ServiceAccounts []string
 
+	namespace string
+
 	PrintObj printers.ResourcePrinterFunc
 
 	genericclioptions.IOStreams
@@ -87,8 +89,7 @@ func NewSubjectOptions(streams genericclioptions.IOStreams) *SubjectOptions {
 }
 
 func NewCmdSubject(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command {
-	options := NewSubjectOptions(streams)
+	o := NewSubjectOptions(streams)
-
 	cmd := &cobra.Command{
 		Use: "subject (-f FILENAME | TYPE NAME) [--user=username] [--group=groupname] [--serviceaccount=namespace:serviceaccountname] [--dry-run]",
 		DisableFlagsInUseLine: true,
@@ -96,23 +97,22 @@ func NewCmdSubject(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobr
 		Long:    subject_long,
 		Example: subject_example,
 		Run: func(cmd *cobra.Command, args []string) {
-			cmdutil.CheckErr(options.Complete(f, cmd, args))
+			cmdutil.CheckErr(o.Complete(f, cmd, args))
-			cmdutil.CheckErr(options.Validate())
+			cmdutil.CheckErr(o.Validate())
-			cmdutil.CheckErr(options.Run(f, addSubjects))
+			cmdutil.CheckErr(o.Run(addSubjects))
 		},
 	}
 
-	options.PrintFlags.AddFlags(cmd)
+	o.PrintFlags.AddFlags(cmd)
 
-	usage := "the resource to update the subjects"
+	cmdutil.AddFilenameOptionFlags(cmd, &o.FilenameOptions, "the resource to update the subjects")
-	cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage)
+	cmd.Flags().BoolVar(&o.All, "all", o.All, "Select all resources, including uninitialized ones, in the namespace of the specified resource types")
-	cmd.Flags().BoolVar(&options.All, "all", options.All, "Select all resources, including uninitialized ones, in the namespace of the specified resource types")
+	cmd.Flags().StringVarP(&o.Selector, "selector", "l", o.Selector, "Selector (label query) to filter on, not including uninitialized ones, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
-	cmd.Flags().StringVarP(&options.Selector, "selector", "l", options.Selector, "Selector (label query) to filter on, not including uninitialized ones, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
+	cmd.Flags().BoolVar(&o.Local, "local", o.Local, "If true, set subject will NOT contact api-server but run locally.")
-	cmd.Flags().BoolVar(&options.Local, "local", options.Local, "If true, set subject will NOT contact api-server but run locally.")
 	cmdutil.AddDryRunFlag(cmd)
-	cmd.Flags().StringArrayVar(&options.Users, "user", options.Users, "Usernames to bind to the role")
+	cmd.Flags().StringArrayVar(&o.Users, "user", o.Users, "Usernames to bind to the role")
-	cmd.Flags().StringArrayVar(&options.Groups, "group", options.Groups, "Groups to bind to the role")
+	cmd.Flags().StringArrayVar(&o.Groups, "group", o.Groups, "Groups to bind to the role")
-	cmd.Flags().StringArrayVar(&options.ServiceAccounts, "serviceaccount", options.ServiceAccounts, "Service accounts to bind to the role")
+	cmd.Flags().StringArrayVar(&o.ServiceAccounts, "serviceaccount", o.ServiceAccounts, "Service accounts to bind to the role")
 	cmdutil.AddIncludeUninitializedFlag(cmd)
 	return cmd
 }
@@ -130,7 +130,8 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []
 	}
 	o.PrintObj = printer.PrintObj
 
-	cmdNamespace, enforceNamespace, err := f.DefaultNamespace()
+	var enforceNamespace bool
+	o.namespace, enforceNamespace, err = f.DefaultNamespace()
 	if err != nil {
 		return err
 	}
@@ -140,7 +141,7 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []
 		Internal(legacyscheme.Scheme).
 		LocalParam(o.Local).
 		ContinueOnError().
-		NamespaceParam(cmdNamespace).DefaultNamespace().
+		NamespaceParam(o.namespace).DefaultNamespace().
 		FilenameParam(enforceNamespace, &o.FilenameOptions).
 		IncludeUninitialized(includeUninitialized).
 		Flatten()
@@ -192,8 +193,7 @@ func (o *SubjectOptions) Validate() error {
 	return nil
 }
 
-func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error {
+func (o *SubjectOptions) Run(fn updateSubjects) error {
-	var err error
 	patches := CalculatePatches(o.Infos, cmdutil.InternalVersionJSONEncoder(), func(info *resource.Info) ([]byte, error) {
 		subjects := []rbac.Subject{}
 		for _, user := range sets.NewString(o.Users...).List() {
@@ -217,10 +217,7 @@ func (o *SubjectOptions) Run(f cmdutil.Factory, fn updateSubjects) error {
 			namespace := tokens[0]
 			name := tokens[1]
 			if len(namespace) == 0 {
-				namespace, _, err = f.DefaultNamespace()
+				namespace = o.namespace
-				if err != nil {
-					return nil, err
-				}
 			}
 			subject := rbac.Subject{
 				Kind:      rbac.ServiceAccountKind,

pkg/kubectl/cmd/version.go

@@ -20,14 +20,15 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 
 	"github.com/ghodss/yaml"
 	"github.com/spf13/cobra"
 
 	apimachineryversion "k8s.io/apimachinery/pkg/version"
+	"k8s.io/client-go/discovery"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	"k8s.io/kubernetes/pkg/kubectl/genericclioptions"
 	"k8s.io/kubernetes/pkg/kubectl/util/i18n"
 	"k8s.io/kubernetes/pkg/version"
 )
@@ -37,52 +38,67 @@ type Version struct {
 	ServerVersion *apimachineryversion.Info `json:"serverVersion,omitempty" yaml:"serverVersion,omitempty"`
 }
 
-// VersionOptions: describe the options available to users of the "kubectl
-// version" command.
-type VersionOptions struct {
-	clientOnly bool
-	short      bool
-	output     string
-}
-
 var (
 	versionExample = templates.Examples(i18n.T(`
 		# Print the client and server versions for the current context
 		kubectl version`))
 )
 
-func NewCmdVersion(f cmdutil.Factory, out io.Writer) *cobra.Command {
+type VersionOptions struct {
+	ClientOnly bool
+	Short      bool
+	Output     string
+
+	discoveryClient discovery.CachedDiscoveryInterface
+
+	genericclioptions.IOStreams
+}
+
+func NewVersionOptions(ioStreams genericclioptions.IOStreams) *VersionOptions {
+	return &VersionOptions{
+		IOStreams: ioStreams,
+	}
+
+}
+
+func NewCmdVersion(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
+	o := NewVersionOptions(ioStreams)
 	cmd := &cobra.Command{
 		Use:     "version",
 		Short:   i18n.T("Print the client and server version information"),
 		Long:    "Print the client and server version information for the current context",
 		Example: versionExample,
 		Run: func(cmd *cobra.Command, args []string) {
-			options := new(VersionOptions)
+			cmdutil.CheckErr(o.Complete(f, cmd))
-			cmdutil.CheckErr(options.Complete(cmd))
+			cmdutil.CheckErr(o.Validate())
-			cmdutil.CheckErr(options.Validate())
+			cmdutil.CheckErr(o.Run())
-			cmdutil.CheckErr(options.Run(f, out))
 		},
 	}
-	cmd.Flags().BoolP("client", "c", false, "Client version only (no server required).")
+	cmd.Flags().BoolVarP(&o.ClientOnly, "client", "c", o.ClientOnly, "Client version only (no server required).")
-	cmd.Flags().BoolP("short", "", false, "Print just the version number.")
+	cmd.Flags().BoolVarP(&o.Short, "short", "", o.Short, "Print just the version number.")
-	cmd.Flags().StringP("output", "o", "", "One of 'yaml' or 'json'.")
+	cmd.Flags().StringVarP(&o.Output, "output", "o", o.Output, "One of 'yaml' or 'json'.")
 	cmd.Flags().MarkShorthandDeprecated("client", "please use --client instead.")
 	return cmd
 }
 
-func retrieveServerVersion(f cmdutil.Factory) (*apimachineryversion.Info, error) {
+func (o *VersionOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
-	discoveryClient, err := f.DiscoveryClient()
+	var err error
+	o.discoveryClient, err = f.DiscoveryClient()
 	if err != nil {
-		return nil, err
+		return err
 	}
-
+	return nil
-	// Always request fresh data from the server
-	discoveryClient.Invalidate()
-	return discoveryClient.ServerVersion()
 }
 
-func (o *VersionOptions) Run(f cmdutil.Factory, out io.Writer) error {
+func (o *VersionOptions) Validate() error {
+	if o.Output != "" && o.Output != "yaml" && o.Output != "json" {
+		return errors.New(`--output must be 'yaml' or 'json'`)
+	}
+
+	return nil
+}
+
+func (o *VersionOptions) Run() error {
 	var (
 		serverVersion *apimachineryversion.Info
 		serverErr     error
@@ -92,22 +108,24 @@ func (o *VersionOptions) Run(f cmdutil.Factory, out io.Writer) error {
 	clientVersion := version.Get()
 	versionInfo.ClientVersion = &clientVersion
 
-	if !o.clientOnly {
+	if !o.ClientOnly {
-		serverVersion, serverErr = retrieveServerVersion(f)
+		// Always request fresh data from the server
+		o.discoveryClient.Invalidate()
+		serverVersion, serverErr = o.discoveryClient.ServerVersion()
 		versionInfo.ServerVersion = serverVersion
 	}
 
-	switch o.output {
+	switch o.Output {
 	case "":
-		if o.short {
+		if o.Short {
-			fmt.Fprintf(out, "Client Version: %s\n", clientVersion.GitVersion)
+			fmt.Fprintf(o.Out, "Client Version: %s\n", clientVersion.GitVersion)
 			if serverVersion != nil {
-				fmt.Fprintf(out, "Server Version: %s\n", serverVersion.GitVersion)
+				fmt.Fprintf(o.Out, "Server Version: %s\n", serverVersion.GitVersion)
 			}
 		} else {
-			fmt.Fprintf(out, "Client Version: %s\n", fmt.Sprintf("%#v", clientVersion))
+			fmt.Fprintf(o.Out, "Client Version: %s\n", fmt.Sprintf("%#v", clientVersion))
 			if serverVersion != nil {
-				fmt.Fprintf(out, "Server Version: %s\n", fmt.Sprintf("%#v", *serverVersion))
+				fmt.Fprintf(o.Out, "Server Version: %s\n", fmt.Sprintf("%#v", *serverVersion))
 			}
 		}
 	case "yaml":
@@ -115,33 +133,18 @@ func (o *VersionOptions) Run(f cmdutil.Factory, out io.Writer) error {
 		if err != nil {
 			return err
 		}
-		fmt.Fprintln(out, string(marshalled))
+		fmt.Fprintln(o.Out, string(marshalled))
 	case "json":
 		marshalled, err := json.MarshalIndent(&versionInfo, "", "  ")
 		if err != nil {
 			return err
 		}
-		fmt.Fprintln(out, string(marshalled))
+		fmt.Fprintln(o.Out, string(marshalled))
 	default:
 		// There is a bug in the program if we hit this case.
 		// However, we follow a policy of never panicking.
-		return fmt.Errorf("VersionOptions were not validated: --output=%q should have been rejected", o.output)
+		return fmt.Errorf("VersionOptions were not validated: --output=%q should have been rejected", o.Output)
 	}
 
 	return serverErr
 }
-
-func (o *VersionOptions) Complete(cmd *cobra.Command) error {
-	o.clientOnly = cmdutil.GetFlagBool(cmd, "client")
-	o.short = cmdutil.GetFlagBool(cmd, "short")
-	o.output = cmdutil.GetFlagString(cmd, "output")
-	return nil
-}
-
-func (o *VersionOptions) Validate() error {
-	if o.output != "" && o.output != "yaml" && o.output != "json" {
-		return errors.New(`--output must be 'yaml' or 'json'`)
-	}
-
-	return nil
-}