diff --git a/pkg/kubectl/cmd/set/BUILD b/pkg/kubectl/cmd/set/BUILD index 023d91e9910..ba61c44cb99 100644 --- a/pkg/kubectl/cmd/set/BUILD +++ b/pkg/kubectl/cmd/set/BUILD @@ -68,6 +68,7 @@ go_test( "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", "//pkg/kubectl/genericclioptions:go_default_library", + "//pkg/kubectl/genericclioptions/printers:go_default_library", "//pkg/kubectl/genericclioptions/resource:go_default_library", "//pkg/kubectl/scheme:go_default_library", "//vendor/github.com/spf13/cobra:go_default_library", diff --git a/pkg/kubectl/cmd/set/set_selector.go b/pkg/kubectl/cmd/set/set_selector.go index 3f29dd07c05..70199c37679 100644 --- a/pkg/kubectl/cmd/set/set_selector.go +++ b/pkg/kubectl/cmd/set/set_selector.go @@ -23,7 +23,6 @@ import ( "github.com/spf13/cobra" "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -40,26 +39,23 @@ import ( // SelectorOptions is the start of the data required to perform the operation. As new fields are added, add them here instead of // referencing the cmd.Flags() type SetSelectorOptions struct { - fileOptions resource.FilenameOptions - - PrintFlags *genericclioptions.PrintFlags - RecordFlags *genericclioptions.RecordFlags - - local bool - dryrun bool - all bool - output string + // Bound + ResourceBuilderFlags *genericclioptions.ResourceBuilderFlags + PrintFlags *genericclioptions.PrintFlags + RecordFlags *genericclioptions.RecordFlags + dryrun bool + // set by args resources []string selector *metav1.LabelSelector - ClientForMapping func(mapping *meta.RESTMapping) (resource.RESTClient, error) - - PrintObj printers.ResourcePrinterFunc - Recorder genericclioptions.Recorder - - builder *resource.Builder + // computed + WriteToServer bool + PrintObj printers.ResourcePrinterFunc + Recorder genericclioptions.Recorder + ResourceFinder genericclioptions.ResourceFinder + // set at initialization genericclioptions.IOStreams } @@ -79,6 +75,12 @@ var ( func NewSelectorOptions(streams genericclioptions.IOStreams) *SetSelectorOptions { return &SetSelectorOptions{ + ResourceBuilderFlags: genericclioptions.NewResourceBuilderFlags(). + WithScheme(scheme.Scheme). + WithAll(false). + WithLocal(false). + WithUninitialized(false). + WithLatest(), PrintFlags: genericclioptions.NewPrintFlags("selector updated").WithTypeSetter(scheme.Scheme), RecordFlags: genericclioptions.NewRecordFlags(), @@ -105,16 +107,12 @@ func NewCmdSelector(f cmdutil.Factory, streams genericclioptions.IOStreams) *cob }, } + o.ResourceBuilderFlags.AddFlags(cmd.Flags()) o.PrintFlags.AddFlags(cmd) o.RecordFlags.AddFlags(cmd) - 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(&o.local, "local", o.local, "If true, set selector will NOT contact api-server but run locally.") cmd.Flags().String("resource-version", "", "If non-empty, the selectors update will only succeed if this is the current resource-version for the object. Only valid when specifying a single resource.") - usage := "the resource to update the selectors" - cmdutil.AddFilenameOptionFlags(cmd, &o.fileOptions, usage) cmdutil.AddDryRunFlag(cmd) - cmdutil.AddIncludeUninitializedFlag(cmd) return cmd } @@ -130,40 +128,14 @@ func (o *SetSelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg } o.dryrun = cmdutil.GetDryRunFlag(cmd) - o.output = cmdutil.GetFlagString(cmd, "output") - - cmdNamespace, enforceNamespace, err := f.ToRawKubeConfigLoader().Namespace() - if err != nil { - return err - } o.resources, o.selector, err = getResourcesAndSelector(args) if err != nil { return err } - includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - o.builder = f.NewBuilder(). - WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). - LocalParam(o.local). - ContinueOnError(). - NamespaceParam(cmdNamespace).DefaultNamespace(). - FilenameParam(enforceNamespace, &o.fileOptions). - IncludeUninitialized(includeUninitialized). - Flatten() - - if !o.local { - o.builder. - ResourceTypeOrNameArgs(o.all, o.resources...). - Latest() - } else { - // if a --local flag was provided, and a resource was specified in the form - // /, fail immediately as --local cannot query the api server - // for the specified resource. - if len(o.resources) > 0 { - return resource.LocalResourceError - } - } + o.ResourceFinder = o.ResourceBuilderFlags.ToBuilder(f, o.resources) + o.WriteToServer = !(*o.ResourceBuilderFlags.Local || o.dryrun) if o.dryrun { o.PrintFlags.Complete("%s (dry run)") @@ -174,17 +146,11 @@ func (o *SetSelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg } o.PrintObj = printer.PrintObj - o.ClientForMapping = func(mapping *meta.RESTMapping) (resource.RESTClient, error) { - return f.ClientForMapping(mapping) - } return err } // Validate basic inputs func (o *SetSelectorOptions) Validate() error { - if len(o.resources) < 1 && cmdutil.IsFilenameSliceEmpty(o.fileOptions.Filenames) { - return fmt.Errorf("one or more resources must be specified as or /") - } if o.selector == nil { return fmt.Errorf("one selector is required") } @@ -193,11 +159,7 @@ func (o *SetSelectorOptions) Validate() error { // RunSelector executes the command. func (o *SetSelectorOptions) RunSelector() error { - r := o.builder.Do() - err := r.Err() - if err != nil { - return err - } + r := o.ResourceFinder.Do() return r.Visit(func(info *resource.Info, err error) error { patch := &Patch{Info: info} @@ -218,7 +180,7 @@ func (o *SetSelectorOptions) RunSelector() error { if patch.Err != nil { return patch.Err } - if o.local || o.dryrun { + if !o.WriteToServer { return o.PrintObj(info.Object, o.Out) } diff --git a/pkg/kubectl/cmd/set/set_selector_test.go b/pkg/kubectl/cmd/set/set_selector_test.go index 351e6d98d35..e1924f32c56 100644 --- a/pkg/kubectl/cmd/set/set_selector_test.go +++ b/pkg/kubectl/cmd/set/set_selector_test.go @@ -17,7 +17,6 @@ limitations under the License. package set import ( - "net/http" "reflect" "strings" "testing" @@ -28,13 +27,9 @@ import ( extensionsv1beta1 "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" - restclient "k8s.io/client-go/rest" - "k8s.io/client-go/rest/fake" - cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" - "k8s.io/kubernetes/pkg/kubectl/scheme" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions/printers" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" ) func TestUpdateSelectorForObjectTypes(t *testing.T) { @@ -317,27 +312,30 @@ func TestGetResourcesAndSelector(t *testing.T) { } func TestSelectorTest(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace("test") - defer tf.Cleanup() - - tf.Client = &fake.RESTClient{ - GroupVersion: schema.GroupVersion{Version: ""}, - NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - t.Fatalf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req) - return nil, nil - }), + info := &resource.Info{ + Object: &v1.Service{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Service"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "some-ns", Name: "cassandra"}, + }, } - tf.ClientConfigVal = &restclient.Config{ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}}} - streams, _, buf, _ := genericclioptions.NewTestIOStreams() - cmd := NewCmdSelector(tf, streams) - cmd.Flags().Set("output", "name") - cmd.Flags().Set("local", "true") - cmd.Flags().Set("filename", "../../../../test/e2e/testing-manifests/statefulset/cassandra/service.yaml") + labelToSet, err := metav1.ParseToLabelSelector("environment=qa") + if err != nil { + t.Fatal(err) + } - cmd.Run(cmd, []string{"environment=qa"}) + iostreams, _, buf, _ := genericclioptions.NewTestIOStreams() + o := &SetSelectorOptions{ + selector: labelToSet, + ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(info), + Recorder: genericclioptions.NoopRecorder{}, + PrintObj: (&printers.NamePrinter{}).PrintObj, + IOStreams: iostreams, + } + if err := o.RunSelector(); err != nil { + t.Fatal(err) + } if !strings.Contains(buf.String(), "service/cassandra") { t.Errorf("did not set selector: %s", buf.String()) } diff --git a/pkg/kubectl/cmd/wait/wait.go b/pkg/kubectl/cmd/wait/wait.go index 9ccf3cb735e..37b5a4b66d1 100644 --- a/pkg/kubectl/cmd/wait/wait.go +++ b/pkg/kubectl/cmd/wait/wait.go @@ -53,9 +53,12 @@ type WaitFlags struct { // NewWaitFlags returns a default WaitFlags func NewWaitFlags(restClientGetter genericclioptions.RESTClientGetter, streams genericclioptions.IOStreams) *WaitFlags { return &WaitFlags{ - RESTClientGetter: restClientGetter, - PrintFlags: genericclioptions.NewPrintFlags("condition met"), - ResourceBuilderFlags: genericclioptions.NewResourceBuilderFlags(), + RESTClientGetter: restClientGetter, + PrintFlags: genericclioptions.NewPrintFlags("condition met"), + ResourceBuilderFlags: genericclioptions.NewResourceBuilderFlags(). + WithLabelSelector(""). + WithAllNamespaces(false). + WithLatest(), Timeout: 30 * time.Second, diff --git a/pkg/kubectl/cmd/wait/wait_test.go b/pkg/kubectl/cmd/wait/wait_test.go index 77d98e8d459..27446e840f6 100644 --- a/pkg/kubectl/cmd/wait/wait_test.go +++ b/pkg/kubectl/cmd/wait/wait_test.go @@ -219,7 +219,7 @@ func TestWaitForDeletion(t *testing.T) { t.Run(test.name, func(t *testing.T) { fakeClient := test.fakeClient() o := &WaitOptions{ - ResourceFinder: genericclioptions.NewSimpleResourceFinder(test.info), + ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info), DynamicClient: fakeClient, Timeout: test.timeout, @@ -451,7 +451,7 @@ func TestWaitForCondition(t *testing.T) { t.Run(test.name, func(t *testing.T) { fakeClient := test.fakeClient() o := &WaitOptions{ - ResourceFinder: genericclioptions.NewSimpleResourceFinder(test.info), + ResourceFinder: genericclioptions.NewSimpleFakeResourceFinder(test.info), DynamicClient: fakeClient, Timeout: test.timeout, diff --git a/pkg/kubectl/genericclioptions/builder_flags.go b/pkg/kubectl/genericclioptions/builder_flags.go index 4648751c315..43ca43b3b6e 100644 --- a/pkg/kubectl/genericclioptions/builder_flags.go +++ b/pkg/kubectl/genericclioptions/builder_flags.go @@ -18,6 +18,7 @@ package genericclioptions import ( "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" ) @@ -26,11 +27,16 @@ import ( type ResourceBuilderFlags struct { FileNameFlags *FileNameFlags - LabelSelector *string - FieldSelector *string - AllNamespaces *bool + LabelSelector *string + FieldSelector *string + AllNamespaces *bool + All *bool + Local *bool + IncludeUninitialized *bool - All bool + Scheme *runtime.Scheme + Latest bool + StopOnFirstError bool } // NewResourceBuilderFlags returns a default ResourceBuilderFlags @@ -43,17 +49,65 @@ func NewResourceBuilderFlags() *ResourceBuilderFlags { Filenames: &filenames, Recursive: boolPtr(true), }, - - LabelSelector: strPtr(""), - AllNamespaces: boolPtr(false), } } +func (o *ResourceBuilderFlags) WithFile(recurse bool, files ...string) *ResourceBuilderFlags { + o.FileNameFlags = &FileNameFlags{ + Usage: "identifying the resource.", + Filenames: &files, + Recursive: boolPtr(recurse), + } + + return o +} + +func (o *ResourceBuilderFlags) WithLabelSelector(selector string) *ResourceBuilderFlags { + o.LabelSelector = &selector + return o +} + func (o *ResourceBuilderFlags) WithFieldSelector(selector string) *ResourceBuilderFlags { o.FieldSelector = &selector return o } +func (o *ResourceBuilderFlags) WithAllNamespaces(defaultVal bool) *ResourceBuilderFlags { + o.AllNamespaces = &defaultVal + return o +} + +func (o *ResourceBuilderFlags) WithAll(defaultVal bool) *ResourceBuilderFlags { + o.All = &defaultVal + return o +} + +func (o *ResourceBuilderFlags) WithLocal(defaultVal bool) *ResourceBuilderFlags { + o.Local = &defaultVal + return o +} + +// WithUninitialized is using an alpha feature and may be dropped +func (o *ResourceBuilderFlags) WithUninitialized(defaultVal bool) *ResourceBuilderFlags { + o.IncludeUninitialized = &defaultVal + return o +} + +func (o *ResourceBuilderFlags) WithScheme(scheme *runtime.Scheme) *ResourceBuilderFlags { + o.Scheme = scheme + return o +} + +func (o *ResourceBuilderFlags) WithLatest() *ResourceBuilderFlags { + o.Latest = true + return o +} + +func (o *ResourceBuilderFlags) StopOnError() *ResourceBuilderFlags { + o.StopOnFirstError = true + return o +} + // AddFlags registers flags for finding resources func (o *ResourceBuilderFlags) AddFlags(flagset *pflag.FlagSet) { o.FileNameFlags.AddFlags(flagset) @@ -67,6 +121,15 @@ func (o *ResourceBuilderFlags) AddFlags(flagset *pflag.FlagSet) { if o.AllNamespaces != nil { flagset.BoolVar(o.AllNamespaces, "all-namespaces", *o.AllNamespaces, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.") } + if o.All != nil { + flagset.BoolVar(o.All, "all", *o.All, "Select all resources in the namespace of the specified resource types") + } + if o.Local != nil { + flagset.BoolVar(o.Local, "local", *o.Local, "If true, annotation will NOT contact api-server but run locally.") + } + if o.IncludeUninitialized != nil { + flagset.BoolVar(o.IncludeUninitialized, "include-uninitialized", *o.IncludeUninitialized, `If true, the kubectl command applies to uninitialized objects. If explicitly set to false, this flag overrides other flags that make the kubectl commands apply to uninitialized objects, e.g., "--all". Objects with empty metadata.initializers are regarded as initialized.`) + } } // ToBuilder gives you back a resource finder to visit resources that are located @@ -74,24 +137,58 @@ func (o *ResourceBuilderFlags) ToBuilder(restClientGetter RESTClientGetter, reso namespace, enforceNamespace, namespaceErr := restClientGetter.ToRawKubeConfigLoader().Namespace() builder := resource.NewBuilder(restClientGetter). - Unstructured(). - NamespaceParam(namespace).DefaultNamespace(). - ResourceTypeOrNameArgs(o.All, resources...) + NamespaceParam(namespace).DefaultNamespace() + + if o.Scheme != nil { + builder.WithScheme(o.Scheme, o.Scheme.PrioritizedVersionsAllGroups()...) + } else { + builder.Unstructured() + } + if o.FileNameFlags != nil { opts := o.FileNameFlags.ToOptions() - builder = builder.FilenameParam(enforceNamespace, &opts) + builder.FilenameParam(enforceNamespace, &opts) } - if o.LabelSelector != nil { - builder = builder.LabelSelectorParam(*o.LabelSelector) + + if o.Local == nil || !*o.Local { + // resource type/name tuples only work non-local + if o.All != nil { + builder.ResourceTypeOrNameArgs(*o.All, resources...) + } else { + builder.ResourceTypeOrNameArgs(false, resources...) + } + // label selectors only work non-local (for now) + if o.LabelSelector != nil { + builder.LabelSelectorParam(*o.LabelSelector) + } + // field selectors only work non-local (forever) + if o.FieldSelector != nil { + builder.FieldSelectorParam(*o.FieldSelector) + } + // latest only works non-local (forever) + if o.Latest { + builder.Latest() + } + + } else { + builder.Local() + + if len(resources) > 0 { + builder.AddError(resource.LocalResourceError) + } } - if o.FieldSelector != nil { - builder = builder.FieldSelectorParam(*o.FieldSelector) + + if o.IncludeUninitialized != nil { + builder.IncludeUninitialized(*o.IncludeUninitialized) + } + + if !o.StopOnFirstError { + builder.ContinueOnError() } return &ResourceFindBuilderWrapper{ builder: builder. - Latest(). - Flatten(). + Flatten(). // I think we're going to recommend this everywhere AddError(namespaceErr), } } diff --git a/pkg/kubectl/genericclioptions/builder_flags_fake.go b/pkg/kubectl/genericclioptions/builder_flags_fake.go index 15137c9e797..de968d8e5d7 100644 --- a/pkg/kubectl/genericclioptions/builder_flags_fake.go +++ b/pkg/kubectl/genericclioptions/builder_flags_fake.go @@ -21,7 +21,7 @@ import ( ) // NewSimpleResourceFinder builds a super simple ResourceFinder that just iterates over the objects you provided -func NewSimpleResourceFinder(infos ...*resource.Info) ResourceFinder { +func NewSimpleFakeResourceFinder(infos ...*resource.Info) ResourceFinder { return &fakeResourceFinder{ Infos: infos, }