From aa7a828f20b479a8a943d897224e8e76c3bb6cff Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Fri, 23 Dec 2022 14:56:59 -0500 Subject: [PATCH] cmd/get: Remove cmd argument from Run() Removes the need to pass cmd as an argument to Run(). This change required reading the --sort-by flag in Complete() in a way similar to other flags. This change allows the cobra.Command not to need to be passed throughout the completion code, which I updated as part of this commit. It also is a step in the direction of the TODO comment requesting the removal of arguments passed to Run() and watch(). --- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 2 +- staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go | 6 +-- .../cmd/create/create_clusterrolebinding.go | 2 +- staging/src/k8s.io/kubectl/pkg/cmd/get/get.go | 35 ++++++-------- .../kubectl/pkg/util/completion/completion.go | 47 ++++++++++--------- 5 files changed, 43 insertions(+), 49 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index 8313179b007..dbafd25c6f1 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -500,7 +500,7 @@ func registerCompletionFuncForGlobalFlags(cmd *cobra.Command, f cmdutil.Factory) cmdutil.CheckErr(cmd.RegisterFlagCompletionFunc( "namespace", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - return utilcomp.CompGetResource(f, cmd, "namespace", toComplete), cobra.ShellCompDirectiveNoFileComp + return utilcomp.CompGetResource(f, "namespace", toComplete), cobra.ShellCompDirectiveNoFileComp })) cmdutil.CheckErr(cmd.RegisterFlagCompletionFunc( "context", diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go index b05bd307dc3..49af3138dfe 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cp/cp.go @@ -109,14 +109,14 @@ func NewCmdCp(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.C // complete / namespace := toComplete[:idx] template := "{{ range .items }}{{ .metadata.namespace }}/{{ .metadata.name }}: {{ end }}" - comps = completion.CompGetFromTemplate(&template, f, namespace, cmd, []string{"pod"}, toComplete) + comps = completion.CompGetFromTemplate(&template, f, namespace, []string{"pod"}, toComplete) } else { // Complete namespaces followed by a / - for _, ns := range completion.CompGetResource(f, cmd, "namespace", toComplete) { + for _, ns := range completion.CompGetResource(f, "namespace", toComplete) { comps = append(comps, fmt.Sprintf("%s/", ns)) } // Complete pod names followed by a : - for _, pod := range completion.CompGetResource(f, cmd, "pod", toComplete) { + for _, pod := range completion.CompGetResource(f, "pod", toComplete) { comps = append(comps, fmt.Sprintf("%s:", pod)) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_clusterrolebinding.go b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_clusterrolebinding.go index 697cb2702cb..18e9ece6aba 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/create/create_clusterrolebinding.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/create/create_clusterrolebinding.go @@ -108,7 +108,7 @@ func NewCmdCreateClusterRoleBinding(f cmdutil.Factory, ioStreams genericclioptio cmdutil.CheckErr(cmd.RegisterFlagCompletionFunc( "clusterrole", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - return completion.CompGetResource(f, cmd, "clusterrole", toComplete), cobra.ShellCompDirectiveNoFileComp + return completion.CompGetResource(f, "clusterrole", toComplete), cobra.ShellCompDirectiveNoFileComp })) return cmd diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go b/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go index b5fd0c904d7..b90114fe853 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/get/get.go @@ -76,11 +76,11 @@ type GetOptions struct { Namespace string ExplicitNamespace bool Subresource string + SortBy string ServerPrint bool NoHeaders bool - Sort bool IgnoreNotFound bool genericclioptions.IOStreams @@ -170,7 +170,7 @@ func NewCmdGet(parent string, f cmdutil.Factory, streams genericclioptions.IOStr Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(o.Complete(f, cmd, args)) cmdutil.CheckErr(o.Validate()) - cmdutil.CheckErr(o.Run(f, cmd, args)) + cmdutil.CheckErr(o.Run(f, args)) }, SuggestFor: []string{"list", "ps"}, } @@ -211,11 +211,9 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri o.ExplicitNamespace = false } - sortBy, err := cmd.Flags().GetString("sort-by") - if err != nil { - return err + if o.PrintFlags.HumanReadableFlags.SortBy != nil { + o.SortBy = *o.PrintFlags.HumanReadableFlags.SortBy } - o.Sort = len(sortBy) > 0 o.NoHeaders = cmdutil.GetFlagBool(cmd, "no-headers") @@ -264,8 +262,8 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri return nil, err } - if o.Sort { - printer = &SortingPrinter{Delegate: printer, SortField: sortBy} + if len(o.SortBy) > 0 { + printer = &SortingPrinter{Delegate: printer, SortField: o.SortBy} } if outputObjects != nil { printer = &skipPrinter{delegate: printer, output: outputObjects} @@ -278,7 +276,7 @@ func (o *GetOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []stri switch { case o.Watch || o.WatchOnly: - if o.Sort { + if len(o.SortBy) > 0 { fmt.Fprintf(o.IOStreams.ErrOut, "warning: --watch or --watch-only requested, --sort-by will be ignored\n") } default: @@ -448,14 +446,14 @@ func (o *GetOptions) transformRequests(req *rest.Request) { }, ",")) // if sorting, ensure we receive the full object in order to introspect its fields via jsonpath - if o.Sort { + if len(o.SortBy) > 0 { req.Param("includeObject", "Object") } } // Run performs the get operation. // TODO: remove the need to pass these arguments, like other commands. -func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) error { +func (o *GetOptions) Run(f cmdutil.Factory, args []string) error { if len(o.Raw) > 0 { restClient, err := f.RESTClient() if err != nil { @@ -464,11 +462,11 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e return rawhttp.RawGet(restClient, o.IOStreams, o.Raw) } if o.Watch || o.WatchOnly { - return o.watch(f, cmd, args) + return o.watch(f, args) } chunkSize := o.ChunkSize - if o.Sort { + if len(o.SortBy) > 0 { // TODO(juanvallejo): in the future, we could have the client use chunking // to gather all results, then sort them all at the end to reduce server load. chunkSize = 0 @@ -513,14 +511,9 @@ func (o *GetOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e objs[ix] = infos[ix].Object } - sorting, err := cmd.Flags().GetString("sort-by") - if err != nil { - return err - } - var positioner OriginalPositioner - if o.Sort { - sorter := NewRuntimeSorter(objs, sorting) + if len(o.SortBy) > 0 { + sorter := NewRuntimeSorter(objs, o.SortBy) if err := sorter.Sort(); err != nil { return err } @@ -632,7 +625,7 @@ func (s *separatorWriterWrapper) SetReady(state bool) { // watch starts a client-side watch of one or more resources. // TODO: remove the need for arguments here. -func (o *GetOptions) watch(f cmdutil.Factory, cmd *cobra.Command, args []string) error { +func (o *GetOptions) watch(f cmdutil.Factory, args []string) error { r := f.NewBuilder(). Unstructured(). NamespaceParam(o.Namespace).DefaultNamespace().AllNamespaces(o.AllNamespaces). diff --git a/staging/src/k8s.io/kubectl/pkg/util/completion/completion.go b/staging/src/k8s.io/kubectl/pkg/util/completion/completion.go index 9a1b0ed99cc..a3193878ce3 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/completion/completion.go +++ b/staging/src/k8s.io/kubectl/pkg/util/completion/completion.go @@ -25,6 +25,7 @@ import ( "time" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/printers" @@ -69,10 +70,10 @@ func SpecifiedResourceTypeAndNameNoRepeatCompletionFunc(f cmdutil.Factory, allow // that don't support that form. For commands that apply to pods and that support the / // form, please use PodResourceNameCompletionFunc() func ResourceNameCompletionFunc(f cmdutil.Factory, resourceType string) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { - return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var comps []string if len(args) == 0 { - comps = CompGetResource(f, cmd, resourceType, toComplete) + comps = CompGetResource(f, resourceType, toComplete) } return comps, cobra.ShellCompDirectiveNoFileComp } @@ -82,11 +83,11 @@ func ResourceNameCompletionFunc(f cmdutil.Factory, resourceType string) func(*co // 1- pod names that match the toComplete prefix // 2- resource types containing pods which match the toComplete prefix func PodResourceNameCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { - return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var comps []string directive := cobra.ShellCompDirectiveNoFileComp if len(args) == 0 { - comps, directive = doPodResourceCompletion(f, cmd, toComplete) + comps, directive = doPodResourceCompletion(f, toComplete) } return comps, directive } @@ -97,14 +98,14 @@ func PodResourceNameCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []str // 2- resource types containing pods which match the toComplete prefix // and as a second argument the containers within the specified pod. func PodResourceNameAndContainerCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { - return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var comps []string directive := cobra.ShellCompDirectiveNoFileComp if len(args) == 0 { - comps, directive = doPodResourceCompletion(f, cmd, toComplete) + comps, directive = doPodResourceCompletion(f, toComplete) } else if len(args) == 1 { podName := convertResourceNameToPodName(f, args[0]) - comps = CompGetContainers(f, cmd, podName, toComplete) + comps = CompGetContainers(f, podName, toComplete) } return comps, directive } @@ -114,13 +115,13 @@ func PodResourceNameAndContainerCompletionFunc(f cmdutil.Factory) func(*cobra.Co // pod specified by the first argument. The resource containing the pod can be specified in // the / form. func ContainerCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { - return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return func(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var comps []string // We need the pod name to be able to complete the container names, it must be in args[0]. // That first argument can also be of the form / so we need to convert it. if len(args) > 0 { podName := convertResourceNameToPodName(f, args[0]) - comps = CompGetContainers(f, cmd, podName, toComplete) + comps = CompGetContainers(f, podName, toComplete) } return comps, cobra.ShellCompDirectiveNoFileComp } @@ -128,7 +129,7 @@ func ContainerCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, s // ContextCompletionFunc is a completion function that completes as a first argument the // context names that match the toComplete prefix -func ContextCompletionFunc(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func ContextCompletionFunc(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { return ListContextsInConfig(toComplete), cobra.ShellCompDirectiveNoFileComp } @@ -137,7 +138,7 @@ func ContextCompletionFunc(cmd *cobra.Command, args []string, toComplete string) // ClusterCompletionFunc is a completion function that completes as a first argument the // cluster names that match the toComplete prefix -func ClusterCompletionFunc(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func ClusterCompletionFunc(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { return ListClustersInConfig(toComplete), cobra.ShellCompDirectiveNoFileComp } @@ -146,7 +147,7 @@ func ClusterCompletionFunc(cmd *cobra.Command, args []string, toComplete string) // UserCompletionFunc is a completion function that completes as a first argument the // user names that match the toComplete prefix -func UserCompletionFunc(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { +func UserCompletionFunc(_ *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { return ListUsersInConfig(toComplete), cobra.ShellCompDirectiveNoFileComp } @@ -154,20 +155,20 @@ func UserCompletionFunc(cmd *cobra.Command, args []string, toComplete string) ([ } // CompGetResource gets the list of the resource specified which begin with `toComplete`. -func CompGetResource(f cmdutil.Factory, cmd *cobra.Command, resourceName string, toComplete string) []string { +func CompGetResource(f cmdutil.Factory, resourceName string, toComplete string) []string { template := "{{ range .items }}{{ .metadata.name }} {{ end }}" - return CompGetFromTemplate(&template, f, "", cmd, []string{resourceName}, toComplete) + return CompGetFromTemplate(&template, f, "", []string{resourceName}, toComplete) } // CompGetContainers gets the list of containers of the specified pod which begin with `toComplete`. -func CompGetContainers(f cmdutil.Factory, cmd *cobra.Command, podName string, toComplete string) []string { +func CompGetContainers(f cmdutil.Factory, podName string, toComplete string) []string { template := "{{ range .spec.initContainers }}{{ .name }} {{end}}{{ range .spec.containers }}{{ .name }} {{ end }}" - return CompGetFromTemplate(&template, f, "", cmd, []string{"pod", podName}, toComplete) + return CompGetFromTemplate(&template, f, "", []string{"pod", podName}, toComplete) } // CompGetFromTemplate executes a Get operation using the specified template and args and returns the results // which begin with `toComplete`. -func CompGetFromTemplate(template *string, f cmdutil.Factory, namespace string, cmd *cobra.Command, args []string, toComplete string) []string { +func CompGetFromTemplate(template *string, f cmdutil.Factory, namespace string, args []string, toComplete string) []string { buf := new(bytes.Buffer) streams := genericclioptions.IOStreams{In: os.Stdin, Out: buf, ErrOut: io.Discard} o := get.NewGetOptions("kubectl", streams) @@ -199,7 +200,7 @@ func CompGetFromTemplate(template *string, f cmdutil.Factory, namespace string, return printer.PrintObj, nil } - o.Run(f, cmd, args) + o.Run(f, args) var comps []string resources := strings.Split(buf.String(), " ") @@ -304,7 +305,7 @@ func resourceTypeAndNameCompletionFunc(f cmdutil.Factory, allowedTypes []string, // The first argument is of the form (e.g., pods) // All following arguments should be a resource name. if allowRepeat || len(args) == 1 { - comps = CompGetResource(f, cmd, args[0], toComplete) + comps = CompGetResource(f, args[0], toComplete) // Remove choices already on the command-line if len(args) > 1 { @@ -356,7 +357,7 @@ func resourceTypeAndNameCompletionFunc(f cmdutil.Factory, allowedTypes []string, if allowRepeat || len(args) == 0 { resourceType := toComplete[:slashIdx] toComplete = toComplete[slashIdx+1:] - nameComps := CompGetResource(f, cmd, resourceType, toComplete) + nameComps := CompGetResource(f, resourceType, toComplete) for _, c := range nameComps { comps = append(comps, fmt.Sprintf("%s/%s", resourceType, c)) } @@ -375,13 +376,13 @@ func resourceTypeAndNameCompletionFunc(f cmdutil.Factory, allowedTypes []string, // doPodResourceCompletion Returns completions of: // 1- pod names that match the toComplete prefix // 2- resource types containing pods which match the toComplete prefix -func doPodResourceCompletion(f cmdutil.Factory, cmd *cobra.Command, toComplete string) ([]string, cobra.ShellCompDirective) { +func doPodResourceCompletion(f cmdutil.Factory, toComplete string) ([]string, cobra.ShellCompDirective) { var comps []string directive := cobra.ShellCompDirectiveNoFileComp slashIdx := strings.Index(toComplete, "/") if slashIdx == -1 { // Standard case, complete pod names - comps = CompGetResource(f, cmd, "pod", toComplete) + comps = CompGetResource(f, "pod", toComplete) // Also include resource choices for the / form, // but only for resources that contain pods @@ -410,7 +411,7 @@ func doPodResourceCompletion(f cmdutil.Factory, cmd *cobra.Command, toComplete s // Dealing with the / form, use the specified resource type resourceType := toComplete[:slashIdx] toComplete = toComplete[slashIdx+1:] - nameComps := CompGetResource(f, cmd, resourceType, toComplete) + nameComps := CompGetResource(f, resourceType, toComplete) for _, c := range nameComps { comps = append(comps, fmt.Sprintf("%s/%s", resourceType, c)) }