Fix debug command auto-completion #2009
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sudomakeinstall2 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
1 similar comment
|
/retest |
| // debugCompletionFunc Returns a completion function that completes: | ||
| // 1- pod names that match the toComplete prefix | ||
| // 2- resource types containing pods and nodes which match the toComplete prefix | ||
| func debugCompletionFunc(f kcmdutil.Factory) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { |
There was a problem hiding this comment.
There are many built-in completion functions in kubectl and it is always better to use them. For example, upstream debug uses https://github.com/kubernetes/kubectl/blob/72b3a7e9b0e99ba874ffe7dd85f9dfd756239dd1/pkg/cmd/cmd.go#L393 and some of the oc commands use
Line 83 in beeaf09
There was a problem hiding this comment.
I tried this first but it doesn't really fit here. It fits more with oc ACTION resource name rather than oc ACTION resource/name.
For example
$ oc debug [tab]
nodes pods deployments ...
$ oc debug nod[tab] -> oc debug nodes [cursor is placed after space]
but you can't use $ oc debug nodes my-node, oc debug only accepts $ oc debug resource/name
kubectl defines doPodResourceCompletion for this reason.
I don't think upstream debug have auto completion.
| // Standard case, complete pod names | ||
| comps = completion.CompGetResource(f, "pod", toComplete) | ||
|
|
||
| validResources := []string{ |
There was a problem hiding this comment.
This list is not fully complete. oc debug supports resources
Line 1021 in beeaf09
/hold
There was a problem hiding this comment.
Added imagestreamimages, imagestreamtags and cronjobs.
I didn't add deploymentConfig since it is deprecated since 4.14.
I can add it if you think it should be here.
openshift-ci
bot
added
the
do-not-merge/hold
Currently `oc debug` auto-completes to files. This commit adds auto-completion for this command. Auto-completion suggests pods and resources that are valid for the debug command. Implemenation is based on `completions.doPodResourceCompletion` examples: ``` $ oc get pods NAME READY STATUS RESTARTS AGE openshift-adp-controller-manager-68d8974985-gsn5b 1/1 Running 0 19d $ oc debug [TAB] cronjobs/ nodes/ daemonsets/ openshift-adp-controller-manager-68d8974985-gsn5b deployments/ pods/ imagestreamimages/ replicasets/ imagestreamtags/ replicationcontrollers/ jobs/ statefulsets/ $ oc debug nodes/vcl01-hv9-ctlplane-[TAB] nodes/vcl01-hv9-ctlplane-0.redhat.com nodes/vcl01-hv9-ctlplane-2.redhat.com nodes/vcl01-hv9-ctlplane-1.redhat.com ``` Signed-off-by: Saeid Askari <[email protected]>
sudomakeinstall2
force-pushed
the
debug-autocomplete
branch
from
6526b83 to
73ca38e
Compare
|
@sudomakeinstall2: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
bot
added
lifecycle/rotten
WalkthroughA new completion helper is imported and wired into the debug command. A Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/cli/debug/debug.go (1)
1365-1365: Redundant directive assignment.The directive is assigned the same value on line 1360 and nothing modifies it before line 1365, making this assignment redundant.
Apply this diff to remove the redundancy:
- directive = cobra.ShellCompDirectiveNoFileComp slashIdx := strings.Index(toComplete, "/")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
pkg/cli/debug/debug.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
pkg/cli/debug/debug.go
🔇 Additional comments (4)
pkg/cli/debug/debug.go (4)
210-214: LGTM!The integration of
ValidArgsFunctionwith the cobra command is correct and follows standard patterns for adding shell completion support.
1354-1363: LGTM!The function signature and early return logic are correct. Properly handles the case where arguments are already provided.
1367-1395: LGTM!The completion logic for pod names and resource types is well-structured. The NoSpace directive is correctly applied when no pods exist to prevent unwanted spaces after resource type completions like "pods/". The resource list aligns with supported types in
approximatePodTemplateForObject.
1396-1404: LGTM!The handling of
type/namecompletion format correctly extracts the resource type and completes resource names. The implementation properly formats results as"type/name"pairs.
| kcmdutil "k8s.io/kubectl/pkg/cmd/util" | ||
| "k8s.io/kubectl/pkg/polymorphichelpers" | ||
| "k8s.io/kubectl/pkg/scheme" | ||
| "k8s.io/kubectl/pkg/util/completion" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify kubectl version compatibility for completion utilities.
The import of k8s.io/kubectl/pkg/util/completion is appropriate for this use case. However, please verify that the completion utilities used (specifically CompGetResource) are available and stable in kubectl version 0.32.1.
🌐 Web query:
Is CompGetResource available in kubectl v0.32.1 and what is its function signature?
💡 Result:
I can’t find any reference to a symbol named "CompGetResource" in kubectl or Kubernetes repos. Do you have a link, file path, or the context where you saw that name (plugin, code snippet, or error)? With that I’ll look up whether it exists in kubectl v0.32.1 and its signature.
I need to examine the actual code to see what functions from the completion package are being used.
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for actual uses of the completion package in debug.go
rg -n 'completion\.' pkg/cli/debug/debug.go -A 2 -B 2
Length of output: 457
🏁 Script executed:
#!/bin/bash
# Also check the full debugCompletionFunc implementation around lines 1354-1407
sed -n '1354,1407p' pkg/cli/debug/debug.goLength of output: 1762
🏁 Script executed:
#!/bin/bash
# Check for any direct imports or usage patterns of completion functions
rg -n 'CompGet|completion\.(Get|Suggest|Complete)' pkg/cli/debug/debug.go -C 3Length of output: 618
🏁 Script executed:
#!/bin/bash
# Look at the command modification around line 214
sed -n '210,220p' pkg/cli/debug/debug.goLength of output: 451
🌐 Web query:
kubectl k8s.io/kubectl/pkg/util/completion package functions v0.32.1
💡 Result:
For kubectl v0.32.1, the k8s.io/kubectl/pkg/util/completion package exposes (not exhaustive but main) functions:
- ClusterCompletionFunc. [1]
- ContextCompletionFunc. [1]
- ResourceTypeAndNameCompletionFunc. [1]
- SpecifiedResourceTypeAndNameCompletionFunc. [1]
- SetFactoryForCompletion. [1]
- CreateApplyAnnotation. [2]
- CreateOrUpdateAnnotation. [2]
- GetModifiedConfiguration / GetOriginalConfiguration (related helpers used with annotations). [2]
- Umask. [1]
Sources: GitHub pkg tree (v0.32.1) and Go package docs. [3][1][2]
Fix function call: completion.CompGetResource does not exist in kubectl v0.32.1.
The import at line 42 is valid, but the code calls completion.CompGetResource() at lines 1369 and 1400, which is not available in kubectl v0.32.1's k8s.io/kubectl/pkg/util/completion package. The available functions are ResourceTypeAndNameCompletionFunc and SpecifiedResourceTypeAndNameCompletionFunc. Update the completion function to use the correct API from kubectl v0.32.1.
🤖 Prompt for AI Agents
In pkg/cli/debug/debug.go (import line ~42 and usage at lines ~1369 and ~1400),
the code calls the non-existent completion.CompGetResource; replace those calls
with the kubectl v0.32.1 compatible completion functions: use
completion.ResourceTypeAndNameCompletionFunc(...) for generic resource type+name
completion, or completion.SpecifiedResourceTypeAndNameCompletionFunc(...) when
your command already fixes the resource type; adjust the call signature to match
the chosen function (pass the command/resource type arguments expected by the
function) and remove references to CompGetResource.
3 participants
Currently
oc debugauto-completes to files. This commit addsauto-completion for this command. Auto-completion suggests pods and
resources that are valid for the debug command.
Implemenation is based on
completions.doPodResourceCompletionexamples: