[Feat][kubectl-plugin] Add shell completion for for kubectl ray get [workergroups|nodes] #4291

justinyeh1995 · 2025-12-19T10:27:41Z

Why are these changes needed?

Add shell completion for two specific commands, kubectl ray get workergroup & kubectl ray get node .

These expected behaviors are as followed.

$ kubectl ray get workergroup <TAB> # list out workergroups in the default namespace
$ kubectl ray get workergroup -n <namespace> <TAB> # list out workergroups in this <namespace>
$ kubectl ray get workergroup --ray-cluster <raycluster name> <TAB> # list out workergroups in this specific RayCluster
$ kubectl ray get workergroup -n <namespace> --ray-cluster <raycluster name> <TAB> # list out workergroups in the specific RayCluster in this namespace
$ kubectl ray get workergroup -A # no shell completion (falls back to file completion)

If no resource is fetched, kubectl will fall back to the default completion behavior showing directories.

The same goes for kubectl ray get node.

Demo video

Screen.Recording.2025-12-20.at.4.12.28.PM.mov

Manual Testing Steps

Build and install the local kubectl-ray binary

From the KubeRay repository root:

pushd kubectl-plugin > /dev/null
go build cmd/kubectl-ray.go
mv kubectl-ray ~/.krew/bin
popd > /dev/null

Follow this doc to enable shell completion for kubectl ray by doing

kubectl krew install --manifest-url https://raw.githubusercontent.com/marckhouzam/kubectl-plugin_completion/v0.1.0/plugin-completion.yaml

kubectl plugin-completion generate
export PATH=$PATH:$HOME/.kubectl-plugin-completion

Related issue number

Closes #3051

Checks

I've made sure the tests are passing.
Testing Strategy
- Unit tests
- Manual tests

kubectl-plugin/pkg/util/completion/completion.go

justinyeh1995 · 2025-12-21T10:34:21Z

kubectl-plugin/pkg/util/completion/completion.go

+// createRayNodeLabelSelectors creates a map of K8s label selectors for Ray nodes
+// TODO: duplicated function as in kubectl/pkg/cmd/get/get_nodes.go
+func createRayNodeLabelSelectors(cluster string) map[string]string {
+	labelSelectors := map[string]string{
+		util.RayIsRayNodeLabelKey: "yes",
+	}
+	if cluster != "" {
+		labelSelectors[util.RayClusterLabelKey] = cluster
+	}
+	return labelSelectors
+}
+


Similarly to joinLabelMap, I grab this function from kubectl-plugin/pkg/cmd/get/get_nodes.go.

For this one, I think it's ok to expose as a public function to be used in different places. Maybe we can put it under pkg/util then import by others? Would like to know how others think for this one
@seanlaii @troychiu @JiangJiaWei1103 @AndySung320

I agree that factoring this out into a shared helper makes sense.
The only open question might be what the most appropriate name / package would be, given that this is CLI-specific logic.

I noticed there are many createRayxxxLabelSelectors functions, maybe we can extract these to a shared utility, such as pkg/util/selector.go.

This can be a follow up PR

Yes, thank you for pointing this out. I agree this can be in a follow-up PR.

justinyeh1995 · 2025-12-23T07:37:35Z

cc @win5923 and @Future-Outlier to review or help redistribute review work.

Future-Outlier

cc @seanlaii @troychiu @machichima @AndySung320 for help

Copilot

Pull request overview

This PR adds shell completion support for kubectl ray get workergroup and kubectl ray get node commands. The completion functions respect namespace filtering (-n/--namespace), cluster filtering (--ray-cluster), and disable completion when --all-namespaces is set, falling back to file completion in that case.

Key changes:

Implemented WorkerGroupCompletionFunc and NodeCompletionFunc to provide intelligent shell completion
Added comprehensive unit tests covering various scenarios including namespace filtering, cluster filtering, prefix matching, and edge cases
Integrated the completion functions into the respective command definitions

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
kubectl-plugin/pkg/util/completion/completion.go	Implements worker group and node completion functions with namespace and cluster filtering support
kubectl-plugin/pkg/util/completion/completion_test.go	Adds comprehensive unit tests for both completion functions covering multiple scenarios
kubectl-plugin/pkg/cmd/get/get_workergroup.go	Integrates WorkerGroupCompletionFunc into the workergroup command
kubectl-plugin/pkg/cmd/get/get_nodes.go	Integrates NodeCompletionFunc into the node command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kubectl-plugin/pkg/util/completion/completion.go

kubectl-plugin/pkg/util/completion/completion_test.go

kubectl-plugin/pkg/util/completion/completion.go

AndySung320 · 2025-12-24T02:43:38Z

kubectl-plugin/pkg/util/completion/completion.go

+		namespace = "default"
+	}
+
+	rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(context.Background(), v1.ListOptions{})


Suggested change

rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(context.Background(), v1.ListOptions{})

ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)

defer cancel()

rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(ctx, v1.ListOptions{})

Since this API call runs during shell completion, would it be safer to guard it with a context timeout ?
If the API server is slow or unreachable, blocking here could freeze the terminal, which can be a pretty rough UX.

I think this is a solid suggestion but the common pattern in kubectl-plugin/ seems to be using cmd.Context() which is actually context.Background(). I have yet to figure out why that is. I am open to this change but would like to hear about others thoughts on this one first.

I think kubectl default to no timeout and we can set it through --request-timeout (see docs). I think we can just follow the kubectl setting here?

That's a good point, but I think shell completion is a special case here.
When a user types kubectl ray get workergroup <TAB>, the completion code runs in the background before they've finished typing the command. This means:

The user can't pass --request-timeout because they haven't completed the command yet.

Users generally expect tab completion to be instant, or to fail quickly, especially if the API server is slow or unreachable.

Because of that, it seems safer for the completion path to guard itself with a short timeout, independent of the normal kubectl --request-timeout behavior for command execution.
Happy to hear your thoughts if there's an established pattern I'm missing!

Make sense! I think the normal kubectl command have no timeout, so sometimes using auto completion will block terminal for a while. I think having a timeout here would be great. But I got a few concern here:

The timeout we set here will only be applied to querying ray cluster. There's still no timeout for auto completing namespaces (as this is provided by native kubectl), there's a bit inconsistency here

After timeout, what shall we show to the user? When using normal kubectl command, if there's nothing shown when trying to do auto completion, it can be either: 1) the auto completion plugin not installed by user; or 2) the resource not found. I'm just wondering if it will provide any confusion to the user?

Generally I think adding timeout here is a good idea, but just want to prevent any confusion users might have.

cc @seanlaii @troychiu @JiangJiaWei1103 for some ideas here

Nice, I think we can follow the kubectl-plugin pattern.

Great find! However, I think the reason why AttachablePodForObjectFn have timeout is because it's watching the pods to meet the specific condition (code path: AttachablePodForObjectFn -> attachablePodForObject -> GetFirstPod -> GetPodList here)

For command like kubectl get pods, they calls resourceTypeAndNameCompletionFunc here, which do not have timeout set.

In this case I think no timeout here aligns with the kubectl get commands behavior

Thanks for the follow-up! This is a great find, and it made me dig a little bit deeper.

I think the two scenarios are different after tracing the code path. ref file

The code path that I traced:

ResourceAndPortCompletionFunc | ContainerCompletionFunc
-> convertResourceNameToPodName (100ms timeout)
-> AttachablePodForObjectFn

Here's why,

AttachablePodForObjectFn is used because convertResourceNameToPodName needs to convert a resource reference to a single running pod name. However, after the conversion, kubectl always calls actual resource listing functions like CompGetContainers, CompGetPodContainerPorts, etc with no timeout.

For example,
ContainerCompletionFunc
->convertResourceNameToPodName (100ms timeout) finds the pod
-> CompGetContainers (no timeout) lists containers in that pod

we're directly listing all resources just asCompGetResource does so following what they do seems more reasonable to me. Also, for larger clusters with many resources, this might be slow, and we should avoid false negatives that would confuse users.

Would love to discuss further if you see it differently.

Thank you all for the deep dive and tracing the code paths. This provides great clarity on why kubectl applies timeouts selectively.
I agree that staying consistent with the native CompGetResource behavior makes sense here to avoid false negatives and maintain a predictable user experience.
I think we could proceed without the timeout for now. If we receive feedback regarding terminal hangs in high-latency clusters in the future, we can revisit more advanced completion strategies then.

Thanks again for the great discussion~

Likewise, thanks for the thorough review and discussion!

AndySung320 · 2025-12-24T02:48:49Z

kubectl-plugin/pkg/util/completion/completion.go

+	}
+
+	labelSelectors := createRayNodeLabelSelectors(cluster)
+	pods, err := k8sClient.KubernetesClient().CoreV1().Pods(namespace).List(


Same concern applies here as well regarding using a context timeout for this API call during shell completion.

Future-Outlier · 2025-12-24T12:46:49Z

cc @JiangJiaWei1103 to take a look, thank you!

kubectl-plugin/pkg/util/completion/completion.go

machichima · 2025-12-25T10:21:52Z

kubectl-plugin/pkg/util/completion/completion.go

+		namespace = "default"
+	}
+
+	rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(context.Background(), v1.ListOptions{})


When listing RayCluster, I think we can provide the cluster name in the list options? So that in the following loop we do not need to use:

if cluster != "" && rayCluster.Name != cluster { continue }

Thanks for the suggestion! I'll update it to use FieldSelector in the list option.

During the fix, I found an issue with the fake client implementation.

NewSimpleClientset and its successor NewClientset do not support FieldSelector filtering in List operations. To make the unit tests pass, the client-side rayCluster.Name check is still needed as a fallback.
Also, NewSimpleClientset is deprecated in favor of NewClientset for better server-side apply testing. A future PR could migrate all usages.

Found this related issue in kubernetes: kubernetes/kubernetes#78824. They suggest using reactors. We can do it in the follow-up PR.
Thank you!

Awesome find! Thanks for pointing this out.

I will add an issue for this follow up. Thanks!

kubectl-plugin/pkg/util/completion/completion.go

Future-Outlier

cc @win5923 to take a look, thank you!

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

…nFunc to accept client.Client parameter for testability. Add initial unit tests. Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: JustinYeh <justinyeh1995@gmail.com>

…workergroup deduplication Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

kubectl-plugin/pkg/util/completion/completion.go

Future-Outlier

Hi, @justinyeh1995
I can't reproduce your result on my zsh, can you give me a full guide so that I can just copy paste and test this PR on my laptop?

justinyeh1995 · 2025-12-30T14:20:37Z

Hi, @justinyeh1995 I can't reproduce your result on my zsh, can you give me a full guide so that I can just copy paste and test this PR on my laptop?

Sure. I'll check again and write a follow-up here. Thanks for reviewing.

justinyeh1995 · 2025-12-30T16:16:52Z

Hi @Future-Outlier, would you mind trying this out in zsh

pushd kubectl-plugin > /dev/null
go build cmd/kubectl-ray.go
mv kubectl-ray ~/.krew/bin
popd > /dev/null

kubectl krew install --manifest-url https://raw.githubusercontent.com/marckhouzam/kubectl-plugin_completion/v0.1.0/plugin-completion.yaml

kubectl plugin-completion generate
export PATH=$PATH:$HOME/.kubectl-plugin-completion

source ~/.zshrc

Should be able to verify it via:

kubectl ray get workergroup <TAB>
kubectl ray get node <TAB>

Thanks!

Future-Outlier

LGTM, cc @rueian to merge, thank you!

thank you for the guidance @justinyeh1995

machichima

LGTM! Thank you

justinyeh1995 force-pushed the feature/3051-kubectl-shell-completion-ray-get-workergroups-nodes branch from 01d8732 to b7c557f Compare December 21, 2025 09:47

justinyeh1995 commented Dec 21, 2025

View reviewed changes

kubectl-plugin/pkg/util/completion/completion.go Outdated Show resolved Hide resolved

justinyeh1995 commented Dec 21, 2025

View reviewed changes

justinyeh1995 changed the title ~~[Feat][kubectl-plugin][WIP] Add shell completion for for kubectl ray get [workergroups|nodes]~~ [Feat][kubectl-plugin] Add shell completion for for kubectl ray get [workergroups|nodes] Dec 22, 2025

justinyeh1995 force-pushed the feature/3051-kubectl-shell-completion-ray-get-workergroups-nodes branch 2 times, most recently from c202da3 to 6c671e9 Compare December 23, 2025 06:18

justinyeh1995 marked this pull request as ready for review December 23, 2025 07:26

Future-Outlier moved this to In Progress in @Future-Outlier's kuberay project Dec 23, 2025

Future-Outlier added this to @Future-Outlier's kuberay project Dec 23, 2025

Future-Outlier requested a review from Copilot December 23, 2025 09:32

Future-Outlier reviewed Dec 23, 2025

View reviewed changes

Copilot started reviewing on behalf of Future-Outlier December 23, 2025 09:33 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

AndySung320 reviewed Dec 24, 2025

View reviewed changes

JiangJiaWei1103 moved this to Work in progress in My Kuberay & Ray Dec 24, 2025

JiangJiaWei1103 added this to My Kuberay & Ray Dec 24, 2025

JiangJiaWei1103 moved this from Work in progress to In review in My Kuberay & Ray Dec 24, 2025

machichima reviewed Dec 25, 2025

View reviewed changes

kubectl-plugin/pkg/util/completion/completion.go Outdated Show resolved Hide resolved

kubectl-plugin/pkg/util/completion/completion.go Outdated Show resolved Hide resolved

kubectl-plugin/pkg/util/completion/completion.go Show resolved Hide resolved

machichima reviewed Dec 25, 2025

View reviewed changes

kubectl-plugin/pkg/util/completion/completion.go Show resolved Hide resolved

justinyeh1995 force-pushed the feature/3051-kubectl-shell-completion-ray-get-workergroups-nodes branch from 5e1e1b0 to 9a85485 Compare December 25, 2025 14:39

Future-Outlier reviewed Dec 26, 2025

View reviewed changes

justinyeh1995 added 6 commits December 26, 2025 23:35

[kubectl-plugin][WIP] Add shell completion for and

ca32b62

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Fix] Skip resource fetching for shell completion with --all-namespaces

ee192e1

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Fix] Remove redundant namespace check in WorkerGroupCompletionFunc

448522a

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Fix] Improve comments in shell completion functions

f60c44a

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Refactor][Test] Refactor WorkerGroupCompletionFunc and NodeCompletio…

a793469

…nFunc to accept client.Client parameter for testability. Add initial unit tests. Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Test] Add unit tests for completion function edge cases

4dcfc88

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

justinyeh1995 and others added 4 commits December 26, 2025 23:35

[Fix][kubectl-plugin] Remove the redundant namespace check

a40e8dd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: JustinYeh <justinyeh1995@gmail.com>

[kubectl-plugin] Improve completion with FieldSelector filtering and …

4e57e1f

…workergroup deduplication Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[kubectl-plugin][Refactor] Use labels.Set for label selector formatting

b79fca7

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

[Chore] fix typo in the help text for all-namesapces flag

9ac0ee3

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

justinyeh1995 force-pushed the feature/3051-kubectl-shell-completion-ray-get-workergroups-nodes branch from 83d6ccd to 9ac0ee3 Compare December 26, 2025 15:35

[Chore][kubectl-plugin] Fix struct field alignment in tests

a2d01b1

Signed-off-by: justinyeh1995 <justinyeh1995@gmail.com>

win5923 reviewed Dec 26, 2025

View reviewed changes

kubectl-plugin/pkg/util/completion/completion.go Show resolved Hide resolved

justinyeh1995 mentioned this pull request Dec 27, 2025

[Refactor][kubectl-plugin] Standardize namespace resolution to respect kubeconfig context #4318

Open

2 tasks

Future-Outlier reviewed Dec 30, 2025

View reviewed changes

Future-Outlier approved these changes Dec 31, 2025

View reviewed changes

Future-Outlier moved this from In Progress to can be merged in @Future-Outlier's kuberay project Dec 31, 2025

machichima approved these changes Dec 31, 2025

View reviewed changes

-	rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(context.Background(), v1.ListOptions{})
+ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond)
+defer cancel()
+rayClusterList, err := k8sClient.RayClient().RayV1().RayClusters(namespace).List(ctx, v1.ListOptions{})

[Feat][kubectl-plugin] Add shell completion for for kubectl ray get [workergroups|nodes] #4291

Are you sure you want to change the base?

[Feat][kubectl-plugin] Add shell completion for for kubectl ray get [workergroups|nodes] #4291

Conversation

justinyeh1995 commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Demo video

Manual Testing Steps

Related issue number

Checks

Uh oh!

Uh oh!

justinyeh1995 Dec 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machichima Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinyeh1995 Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinyeh1995 commented Dec 23, 2025

Uh oh!

Future-Outlier left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machichima Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinyeh1995 Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

The code path that I traced:

Here's why,

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Future-Outlier commented Dec 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinyeh1995 commented Dec 19, 2025 •

edited

Loading

justinyeh1995 Dec 21, 2025 •

edited

Loading

machichima Dec 25, 2025 •

edited

Loading

justinyeh1995 Dec 26, 2025 •

edited

Loading

machichima Dec 26, 2025 •

edited

Loading

justinyeh1995 Dec 28, 2025 •

edited

Loading

justinyeh1995 commented Dec 30, 2025 •

edited

Loading