From bb478f489f72753c85e00984af544d83d26ac0f0 Mon Sep 17 00:00:00 2001 From: kash2104 Date: Mon, 12 Jan 2026 20:28:29 +0530 Subject: [PATCH] refactor: Standardize namespace resolution for kubectl-plugin. --- .../pkg/cmd/create/create_cluster.go | 11 +-- .../pkg/cmd/create/create_cluster_test.go | 91 ++++++++++++++----- .../pkg/cmd/create/create_workergroup.go | 6 +- .../pkg/cmd/create/create_workergroup_test.go | 31 ++++++- kubectl-plugin/pkg/cmd/delete/delete.go | 5 +- kubectl-plugin/pkg/cmd/delete/delete_test.go | 11 ++- kubectl-plugin/pkg/cmd/get/get_cluster.go | 9 +- .../pkg/cmd/get/get_cluster_test.go | 52 ++++++++++- kubectl-plugin/pkg/cmd/get/get_nodes.go | 10 +- kubectl-plugin/pkg/cmd/get/get_nodes_test.go | 12 ++- kubectl-plugin/pkg/cmd/get/get_token.go | 9 +- kubectl-plugin/pkg/cmd/get/get_token_test.go | 10 +- kubectl-plugin/pkg/cmd/get/get_workergroup.go | 10 +- .../pkg/cmd/get/get_workergroup_test.go | 12 ++- kubectl-plugin/pkg/cmd/job/job_submit.go | 9 +- kubectl-plugin/pkg/cmd/job/job_submit_test.go | 8 +- kubectl-plugin/pkg/cmd/log/log.go | 6 +- kubectl-plugin/pkg/cmd/scale/scale_cluster.go | 10 +- .../pkg/cmd/scale/scale_cluster_test.go | 11 ++- kubectl-plugin/pkg/cmd/session/session.go | 5 +- .../pkg/cmd/session/session_test.go | 20 ++-- .../pkg/util/completion/completion.go | 26 +++--- .../pkg/util/completion/completion_test.go | 21 ++++- 23 files changed, 257 insertions(+), 138 deletions(-) diff --git a/kubectl-plugin/pkg/cmd/create/create_cluster.go b/kubectl-plugin/pkg/cmd/create/create_cluster.go index 6e6c268ea0f..6d34ba19c9b 100644 --- a/kubectl-plugin/pkg/cmd/create/create_cluster.go +++ b/kubectl-plugin/pkg/cmd/create/create_cluster.go @@ -150,7 +150,7 @@ func NewCreateClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptio } func (options *CreateClusterOptions) Complete(cmd *cobra.Command, args []string) error { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } @@ -241,7 +241,10 @@ func (options *CreateClusterOptions) resolveClusterName() (string, error) { // resolveNamespace resolves the namespace from the CLI flag and the config file func (options *CreateClusterOptions) resolveNamespace() (string, error) { - namespace := "default" + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() + if err != nil { + return "", fmt.Errorf("failed to get current namespace: %w", err) + } if options.rayClusterConfig.Namespace != nil && *options.rayClusterConfig.Namespace != "" && options.namespace != "" && options.namespace != *options.rayClusterConfig.Namespace { return "", fmt.Errorf("the namespace in the config file %q does not match the namespace %q. You must pass --namespace=%s to perform this operation", *options.rayClusterConfig.Namespace, options.namespace, *options.rayClusterConfig.Namespace) @@ -274,10 +277,6 @@ func (options *CreateClusterOptions) Run(ctx context.Context, k8sClient client.C } options.rayClusterConfig.Namespace = &namespace } else { - if options.namespace == "" { - options.namespace = "default" - } - options.rayClusterConfig = &generation.RayClusterConfig{ Namespace: &options.namespace, Name: &options.clusterName, diff --git a/kubectl-plugin/pkg/cmd/create/create_cluster_test.go b/kubectl-plugin/pkg/cmd/create/create_cluster_test.go index edab357640c..96b33aae3cf 100644 --- a/kubectl-plugin/pkg/cmd/create/create_cluster_test.go +++ b/kubectl-plugin/pkg/cmd/create/create_cluster_test.go @@ -13,6 +13,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/genericclioptions" kubefake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/utils/ptr" @@ -23,48 +25,94 @@ import ( rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1" ) +func createTempKubeConfigFile(t *testing.T, currentNamespace string) (string, error) { + tmpDir := t.TempDir() + + // Set up fake config for kubeconfig + config := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://fake-kubernetes-cluster.example.com", + InsecureSkipTLSVerify: true, // For testing purposes + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "my-fake-user", + Namespace: currentNamespace, + }, + }, + CurrentContext: "test-context", + AuthInfos: map[string]*api.AuthInfo{ + "my-fake-user": { + Token: "", // Empty for testing without authentication + }, + }, + } + + fakeFile := filepath.Join(tmpDir, ".kubeconfig") + + return fakeFile, clientcmd.WriteToFile(*config, fakeFile) +} + func TestRayCreateClusterComplete(t *testing.T) { + kubeConfigWithCurrentContext, err := createTempKubeConfigFile(t, "test-namespace") + require.NoError(t, err) testStreams, _, _, _ := genericclioptions.NewTestIOStreams() tests := map[string]struct { - image string - rayVersion string - expectedError string - expectedImage string - args []string + image string + namespace string + rayVersion string + expectedError string + expectedImage string + expectedNamespace string + args []string }{ "should error when there are no args": { - args: []string{}, - expectedError: "See 'cluster -h' for help and examples", + args: []string{}, + expectedError: "See 'cluster -h' for help and examples", + expectedNamespace: "test-namespace", }, "should error when too many args": { - args: []string{"testRayClusterName", "extra-arg"}, - expectedError: "See 'cluster -h' for help and examples", + args: []string{"testRayClusterName", "extra-arg"}, + expectedError: "See 'cluster -h' for help and examples", + expectedNamespace: "test-namespace", }, "should succeed with default image when no image is specified": { - args: []string{"testRayClusterName"}, - rayVersion: util.RayVersion, - expectedImage: defaultImageWithTag, + args: []string{"testRayClusterName"}, + rayVersion: util.RayVersion, + expectedImage: defaultImageWithTag, + expectedNamespace: "test-namespace", }, "should succeed with provided image when provided": { - args: []string{"testRayClusterName"}, - image: "DEADBEEF", - expectedImage: "DEADBEEF", + args: []string{"testRayClusterName"}, + image: "DEADBEEF", + expectedImage: "DEADBEEF", + expectedNamespace: "test-namespace", }, "should set the image to the same version as the ray version when the image is the default and the ray version is not the default": { - args: []string{"testRayClusterName"}, - image: defaultImageWithTag, - rayVersion: "2.52.0", - expectedImage: fmt.Sprintf("%s:2.52.0", defaultImage), + args: []string{"testRayClusterName"}, + image: defaultImageWithTag, + namespace: "foo", + rayVersion: "2.52.0", + expectedImage: fmt.Sprintf("%s:2.52.0", defaultImage), + expectedNamespace: "foo", }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) + configFlags := &genericclioptions.ConfigFlags{KubeConfig: &kubeConfigWithCurrentContext} + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + + cmdFactory := cmdutil.NewFactory(configFlags) fakeCreateClusterOptions := NewCreateClusterOptions(cmdFactory, testStreams) cmd := &cobra.Command{Use: "cluster"} - cmd.Flags().StringVarP(&fakeCreateClusterOptions.namespace, "namespace", "n", "", "") + configFlags.AddFlags(cmd.Flags()) fakeCreateClusterOptions.rayVersion = tc.rayVersion if tc.image != "" { @@ -78,6 +126,7 @@ func TestRayCreateClusterComplete(t *testing.T) { } else { require.NoError(t, err) require.Equal(t, tc.expectedImage, fakeCreateClusterOptions.image) + require.Equal(t, tc.expectedNamespace, fakeCreateClusterOptions.namespace) } }) } diff --git a/kubectl-plugin/pkg/cmd/create/create_workergroup.go b/kubectl-plugin/pkg/cmd/create/create_workergroup.go index ae1fb4c5257..3ee5fa43c93 100644 --- a/kubectl-plugin/pkg/cmd/create/create_workergroup.go +++ b/kubectl-plugin/pkg/cmd/create/create_workergroup.go @@ -108,16 +108,12 @@ func NewCreateWorkerGroupCommand(cmdFactory cmdutil.Factory, streams genericclio } func (options *CreateWorkerGroupOptions) Complete(cmd *cobra.Command, args []string) error { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } - if options.rayStartParams == nil { options.rayStartParams = map[string]string{} } diff --git a/kubectl-plugin/pkg/cmd/create/create_workergroup_test.go b/kubectl-plugin/pkg/cmd/create/create_workergroup_test.go index 6e5545223f6..514dbab127a 100644 --- a/kubectl-plugin/pkg/cmd/create/create_workergroup_test.go +++ b/kubectl-plugin/pkg/cmd/create/create_workergroup_test.go @@ -8,6 +8,8 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/utils/ptr" "github.com/ray-project/kuberay/kubectl-plugin/pkg/util" @@ -104,10 +106,17 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) { }, }, { - name: "Valid input without namespace flag", - args: []string{"example-group"}, - flags: map[string]string{}, - expectedError: "failed to get namespace: flag accessed but not defined: namespace", + name: "Valid input without namespace flag", + args: []string{"example-group"}, + flags: map[string]string{ + "namespace": "", + }, + expected: &CreateWorkerGroupOptions{ + namespace: "default", + groupName: "example-group", + image: "rayproject/ray:latest", + rayVersion: "latest", + }, }, { name: "mMissing group name", @@ -181,12 +190,24 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + configFlags := genericclioptions.NewConfigFlags(true) cmd := &cobra.Command{} + configFlags.AddFlags(cmd.Flags()) + cmd.Flags().String( + "worker-ray-start-params", + "", + "ray start parameters for worker", + ) for key, value := range tt.flags { - cmd.Flags().String(key, value, "") + err := cmd.Flags().Set(key, value) + if err != nil { + require.NoError(t, err) + } } + factory := cmdutil.NewFactory(configFlags) options := &CreateWorkerGroupOptions{ + cmdFactory: factory, rayVersion: "latest", } diff --git a/kubectl-plugin/pkg/cmd/delete/delete.go b/kubectl-plugin/pkg/cmd/delete/delete.go index 451dc8eeadd..f2dab152bc7 100644 --- a/kubectl-plugin/pkg/cmd/delete/delete.go +++ b/kubectl-plugin/pkg/cmd/delete/delete.go @@ -78,14 +78,11 @@ func NewDeleteCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IOSt } func (options *DeleteOptions) Complete(cmd *cobra.Command, args []string) error { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } if options.resources == nil { options.resources = map[util.ResourceType][]string{} diff --git a/kubectl-plugin/pkg/cmd/delete/delete_test.go b/kubectl-plugin/pkg/cmd/delete/delete_test.go index d05926e19b0..54c962b319a 100644 --- a/kubectl-plugin/pkg/cmd/delete/delete_test.go +++ b/kubectl-plugin/pkg/cmd/delete/delete_test.go @@ -14,7 +14,6 @@ import ( func TestComplete(t *testing.T) { testStreams, _, _, _ := genericclioptions.NewTestIOStreams() - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) tests := []struct { name string @@ -102,10 +101,16 @@ func TestComplete(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams) + configFlags := genericclioptions.NewConfigFlags(true) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + cmdFactory := cmdutil.NewFactory(configFlags) + fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams) cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeDeleteOptions.namespace, "namespace", "n", tc.namespace, "") + flags := cmd.Flags() + configFlags.AddFlags(flags) err := fakeDeleteOptions.Complete(cmd, tc.args) diff --git a/kubectl-plugin/pkg/cmd/get/get_cluster.go b/kubectl-plugin/pkg/cmd/get/get_cluster.go index ac8c0704ed4..65c97a36a43 100644 --- a/kubectl-plugin/pkg/cmd/get/get_cluster.go +++ b/kubectl-plugin/pkg/cmd/get/get_cluster.go @@ -46,7 +46,7 @@ func NewGetClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptions. ValidArgsFunction: completion.RayClusterCompletionFunc(cmdFactory), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := options.Complete(args, cmd); err != nil { + if err := options.Complete(args); err != nil { return err } // running cmd.Execute or cmd.ExecuteE sets the context, which will be done by root @@ -61,15 +61,12 @@ func NewGetClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptions. return cmd } -func (options *GetClusterOptions) Complete(args []string, cmd *cobra.Command) error { - namespace, err := cmd.Flags().GetString("namespace") +func (options *GetClusterOptions) Complete(args []string) error { + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } if len(args) >= 1 { options.cluster = args[0] diff --git a/kubectl-plugin/pkg/cmd/get/get_cluster_test.go b/kubectl-plugin/pkg/cmd/get/get_cluster_test.go index 8fd9bb5a5e3..1a278998972 100644 --- a/kubectl-plugin/pkg/cmd/get/get_cluster_test.go +++ b/kubectl-plugin/pkg/cmd/get/get_cluster_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "path/filepath" "testing" "time" @@ -17,6 +18,8 @@ import ( "k8s.io/cli-runtime/pkg/genericiooptions" "k8s.io/cli-runtime/pkg/printers" kubefake "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/clientcmd" + "k8s.io/client-go/tools/clientcmd/api" cmdutil "k8s.io/kubectl/pkg/cmd/util" "github.com/ray-project/kuberay/kubectl-plugin/pkg/util/client" @@ -24,17 +27,50 @@ import ( rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1" ) +func createTempKubeConfigFile(t *testing.T, currentNamespace string) (string, error) { + tmpDir := t.TempDir() + + // Set up fake config for kubeconfig + config := &api.Config{ + Clusters: map[string]*api.Cluster{ + "test-cluster": { + Server: "https://fake-kubernetes-cluster.example.com", + InsecureSkipTLSVerify: true, // For testing purposes + }, + }, + Contexts: map[string]*api.Context{ + "test-context": { + Cluster: "test-cluster", + AuthInfo: "my-fake-user", + Namespace: currentNamespace, + }, + }, + CurrentContext: "test-context", + AuthInfos: map[string]*api.AuthInfo{ + "my-fake-user": { + Token: "", // Empty for testing without authentication + }, + }, + } + + fakeFile := filepath.Join(tmpDir, ".kubeconfig") + + return fakeFile, clientcmd.WriteToFile(*config, fakeFile) +} + // This is to test Complete() and ensure that it is setting the namespace and cluster correctly // No validation test is done here func TestRayClusterGetComplete(t *testing.T) { + kubeConfigWithCurrentContext, err := createTempKubeConfigFile(t, "test-namespace") + require.NoError(t, err) // Initialize members of the cluster get option struct and the struct itself testStreams, _, _, _ := genericclioptions.NewTestIOStreams() - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) tests := []struct { name string namespace string expectedCluster string + expectedNamespace string args []string expectedAllNamespaces bool }{ @@ -44,6 +80,7 @@ func TestRayClusterGetComplete(t *testing.T) { args: []string{}, expectedAllNamespaces: false, expectedCluster: "", + expectedNamespace: "test-namespace", }, { name: "namespace set, args not set", @@ -51,6 +88,7 @@ func TestRayClusterGetComplete(t *testing.T) { args: []string{}, expectedAllNamespaces: false, expectedCluster: "", + expectedNamespace: "foo", }, { name: "namespace not set, args set", @@ -58,6 +96,7 @@ func TestRayClusterGetComplete(t *testing.T) { args: []string{"foo", "bar"}, expectedAllNamespaces: false, expectedCluster: "foo", + expectedNamespace: "test-namespace", }, { name: "both namespace and args set", @@ -65,20 +104,27 @@ func TestRayClusterGetComplete(t *testing.T) { args: []string{"bar", "qux"}, expectedAllNamespaces: false, expectedCluster: "bar", + expectedNamespace: "foo", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + configFlags := &genericclioptions.ConfigFlags{KubeConfig: &kubeConfigWithCurrentContext} + cmdFactory := cmdutil.NewFactory(configFlags) fakeClusterGetOptions := NewGetClusterOptions(cmdFactory, testStreams) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeClusterGetOptions.namespace, "namespace", "n", tc.namespace, "") - err := fakeClusterGetOptions.Complete(tc.args, cmd) + configFlags.AddFlags(cmd.Flags()) + err := fakeClusterGetOptions.Complete(tc.args) require.NoError(t, err) assert.Equal(t, tc.expectedAllNamespaces, fakeClusterGetOptions.allNamespaces) assert.Equal(t, tc.expectedCluster, fakeClusterGetOptions.cluster) + assert.Equal(t, tc.expectedNamespace, fakeClusterGetOptions.namespace) }) } } diff --git a/kubectl-plugin/pkg/cmd/get/get_nodes.go b/kubectl-plugin/pkg/cmd/get/get_nodes.go index 5e61ca5b438..2c8f0a2d7cd 100644 --- a/kubectl-plugin/pkg/cmd/get/get_nodes.go +++ b/kubectl-plugin/pkg/cmd/get/get_nodes.go @@ -83,7 +83,7 @@ func NewGetNodesCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO Args: cobra.MaximumNArgs(1), ValidArgsFunction: completion.NodeCompletionFunc(cmdFactory), RunE: func(cmd *cobra.Command, args []string) error { - if err := options.Complete(args, cmd); err != nil { + if err := options.Complete(args); err != nil { return err } k8sClient, err := client.NewClient(cmdFactory) @@ -105,19 +105,15 @@ func NewGetNodesCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO return cmd } -func (options *GetNodesOptions) Complete(args []string, cmd *cobra.Command) error { +func (options *GetNodesOptions) Complete(args []string) error { if options.allNamespaces { options.namespace = "" } else { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - - if options.namespace == "" { - options.namespace = "default" - } } if len(args) > 0 { diff --git a/kubectl-plugin/pkg/cmd/get/get_nodes_test.go b/kubectl-plugin/pkg/cmd/get/get_nodes_test.go index 462e40ddfee..489153681b1 100644 --- a/kubectl-plugin/pkg/cmd/get/get_nodes_test.go +++ b/kubectl-plugin/pkg/cmd/get/get_nodes_test.go @@ -83,9 +83,15 @@ func TestRayNodesGetComplete(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := flags.Set("namespace", tc.namespace) - require.NoError(t, err) - err = tc.opts.Complete(tc.args, cmd) + configFlags := genericclioptions.NewConfigFlags(true) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + tc.opts.cmdFactory = cmdutil.NewFactory(configFlags) + cmd := &cobra.Command{} + flags := cmd.Flags() + configFlags.AddFlags(flags) + err := tc.opts.Complete(tc.args) require.NoError(t, err) assert.Equal(t, tc.expectedNamespace, tc.opts.namespace) assert.Equal(t, tc.expectedNode, tc.opts.node) diff --git a/kubectl-plugin/pkg/cmd/get/get_token.go b/kubectl-plugin/pkg/cmd/get/get_token.go index 7c99a3ab4a9..3de693e9ffe 100644 --- a/kubectl-plugin/pkg/cmd/get/get_token.go +++ b/kubectl-plugin/pkg/cmd/get/get_token.go @@ -40,7 +40,7 @@ func NewGetTokenCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO ValidArgsFunction: completion.RayClusterCompletionFunc(cmdFactory), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := options.Complete(args, cmd); err != nil { + if err := options.Complete(args); err != nil { return err } // running cmd.Execute or cmd.ExecuteE sets the context, which will be done by root @@ -54,15 +54,12 @@ func NewGetTokenCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO return cmd } -func (options *GetTokenOptions) Complete(args []string, cmd *cobra.Command) error { - namespace, err := cmd.Flags().GetString("namespace") +func (options *GetTokenOptions) Complete(args []string) error { + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } // guarded by cobra.ExactArgs(1) options.cluster = args[0] return nil diff --git a/kubectl-plugin/pkg/cmd/get/get_token_test.go b/kubectl-plugin/pkg/cmd/get/get_token_test.go index 27240212156..05fc735161a 100644 --- a/kubectl-plugin/pkg/cmd/get/get_token_test.go +++ b/kubectl-plugin/pkg/cmd/get/get_token_test.go @@ -19,7 +19,8 @@ import ( // Tests the Run() step of the command and ensure that the output is as expected. func TestTokenGetRun(t *testing.T) { - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) + configFlags := genericclioptions.NewConfigFlags(true) + cmdFactory := cmdutil.NewFactory(configFlags) testStreams, _, resBuf, _ := genericclioptions.NewTestIOStreams() fakeTokenGetOptions := NewGetTokenOptions(cmdFactory, testStreams) @@ -50,9 +51,12 @@ func TestTokenGetRun(t *testing.T) { rayClient := clienttesting.NewRayClientset(rayCluster) k8sClients := client.NewClientForTesting(kubeClientSet, rayClient) + if secret.Namespace != "" { + configFlags.Namespace = &secret.Namespace + } cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeTokenGetOptions.namespace, "namespace", "n", secret.Namespace, "") - err := fakeTokenGetOptions.Complete([]string{rayCluster.Name}, cmd) + configFlags.AddFlags(cmd.Flags()) + err := fakeTokenGetOptions.Complete([]string{rayCluster.Name}) require.NoError(t, err) err = fakeTokenGetOptions.Run(t.Context(), k8sClients) require.NoError(t, err) diff --git a/kubectl-plugin/pkg/cmd/get/get_workergroup.go b/kubectl-plugin/pkg/cmd/get/get_workergroup.go index e245420eba3..e03b13c17b5 100644 --- a/kubectl-plugin/pkg/cmd/get/get_workergroup.go +++ b/kubectl-plugin/pkg/cmd/get/get_workergroup.go @@ -90,7 +90,7 @@ func NewGetWorkerGroupCommand(cmdFactory cmdutil.Factory, streams genericcliopti Args: cobra.MaximumNArgs(1), ValidArgsFunction: completion.WorkerGroupCompletionFunc(cmdFactory), RunE: func(cmd *cobra.Command, args []string) error { - if err := options.Complete(args, cmd); err != nil { + if err := options.Complete(args); err != nil { return err } k8sClient, err := client.NewClient(cmdFactory) @@ -112,19 +112,15 @@ func NewGetWorkerGroupCommand(cmdFactory cmdutil.Factory, streams genericcliopti return cmd } -func (options *GetWorkerGroupsOptions) Complete(args []string, cmd *cobra.Command) error { +func (options *GetWorkerGroupsOptions) Complete(args []string) error { if options.allNamespaces { options.namespace = "" } else { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - - if options.namespace == "" { - options.namespace = "default" - } } if len(args) > 0 { diff --git a/kubectl-plugin/pkg/cmd/get/get_workergroup_test.go b/kubectl-plugin/pkg/cmd/get/get_workergroup_test.go index 03feeacf3fa..13dd08e86da 100644 --- a/kubectl-plugin/pkg/cmd/get/get_workergroup_test.go +++ b/kubectl-plugin/pkg/cmd/get/get_workergroup_test.go @@ -88,9 +88,15 @@ func TestRayWorkerGroupGetComplete(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - err := flags.Set("namespace", tc.namespace) - require.NoError(t, err) - err = tc.opts.Complete(tc.args, cmd) + configFlags := genericclioptions.NewConfigFlags(true) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + tc.opts.cmdFactory = cmdutil.NewFactory(configFlags) + cmd := &cobra.Command{} + flags := cmd.Flags() + configFlags.AddFlags(flags) + err := tc.opts.Complete(tc.args) require.NoError(t, err) assert.Equal(t, tc.expectedNamespace, tc.opts.namespace) assert.Equal(t, tc.expectedWorkerGroup, tc.opts.workerGroup) diff --git a/kubectl-plugin/pkg/cmd/job/job_submit.go b/kubectl-plugin/pkg/cmd/job/job_submit.go index b534bfb9e42..66baafc6535 100644 --- a/kubectl-plugin/pkg/cmd/job/job_submit.go +++ b/kubectl-plugin/pkg/cmd/job/job_submit.go @@ -141,7 +141,7 @@ func NewJobSubmitCommand(cmdFactory cmdutil.Factory, streams genericclioptions.I return cmdutil.UsageErrorf(cmd, "%s", cmd.Use) } options.entryPoint = strings.Join(args[entryPointStart:], " ") - if err := options.Complete(cmd); err != nil { + if err := options.Complete(); err != nil { return err } if err := options.Validate(cmd); err != nil { @@ -186,15 +186,12 @@ func NewJobSubmitCommand(cmdFactory cmdutil.Factory, streams genericclioptions.I return cmd } -func (options *SubmitJobOptions) Complete(cmd *cobra.Command) error { - namespace, err := cmd.Flags().GetString("namespace") +func (options *SubmitJobOptions) Complete() error { + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } if len(options.runtimeEnv) > 0 { options.runtimeEnv = filepath.Clean(options.runtimeEnv) diff --git a/kubectl-plugin/pkg/cmd/job/job_submit_test.go b/kubectl-plugin/pkg/cmd/job/job_submit_test.go index 2cb00491471..16524ca31cb 100644 --- a/kubectl-plugin/pkg/cmd/job/job_submit_test.go +++ b/kubectl-plugin/pkg/cmd/job/job_submit_test.go @@ -18,15 +18,15 @@ import ( func TestRayJobSubmitComplete(t *testing.T) { testStreams, _, _, _ := genericclioptions.NewTestIOStreams() - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) + configFlags := genericclioptions.NewConfigFlags(true) + cmdFactory := cmdutil.NewFactory(configFlags) fakeSubmitJobOptions := NewJobSubmitOptions(cmdFactory, testStreams) fakeSubmitJobOptions.runtimeEnv = "././fake/path/to/env/yaml" fakeSubmitJobOptions.fileName = "fake/path/to/rayjob.yaml" cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeSubmitJobOptions.namespace, "namespace", "n", "", "") - - err := fakeSubmitJobOptions.Complete(cmd) + configFlags.AddFlags(cmd.Flags()) + err := fakeSubmitJobOptions.Complete() require.NoError(t, err) assert.Equal(t, "default", fakeSubmitJobOptions.namespace) assert.Equal(t, "fake/path/to/env/yaml", fakeSubmitJobOptions.runtimeEnv) diff --git a/kubectl-plugin/pkg/cmd/log/log.go b/kubectl-plugin/pkg/cmd/log/log.go index b4c221375a7..1248620f0be 100644 --- a/kubectl-plugin/pkg/cmd/log/log.go +++ b/kubectl-plugin/pkg/cmd/log/log.go @@ -153,16 +153,12 @@ func NewClusterLogCommand(cmdFactory cmdutil.Factory, streams genericclioptions. } func (options *ClusterLogOptions) Complete(cmd *cobra.Command, args []string) error { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } - typeAndName := strings.Split(args[0], "/") if len(typeAndName) == 1 { options.ResourceType = util.RayCluster diff --git a/kubectl-plugin/pkg/cmd/scale/scale_cluster.go b/kubectl-plugin/pkg/cmd/scale/scale_cluster.go index 96c8d13a799..f3cfa7992ed 100644 --- a/kubectl-plugin/pkg/cmd/scale/scale_cluster.go +++ b/kubectl-plugin/pkg/cmd/scale/scale_cluster.go @@ -69,7 +69,7 @@ func NewScaleClusterCommand(cmdFactory cmdutil.Factory, streams genericclioption SilenceUsage: true, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - if err := options.Complete(args, cmd); err != nil { + if err := options.Complete(args); err != nil { return err } if err := options.Validate(); err != nil { @@ -91,16 +91,12 @@ func NewScaleClusterCommand(cmdFactory cmdutil.Factory, streams genericclioption return cmd } -func (options *ScaleClusterOptions) Complete(args []string, cmd *cobra.Command) error { - namespace, err := cmd.Flags().GetString("namespace") +func (options *ScaleClusterOptions) Complete(args []string) error { + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } - options.cluster = args[0] return nil diff --git a/kubectl-plugin/pkg/cmd/scale/scale_cluster_test.go b/kubectl-plugin/pkg/cmd/scale/scale_cluster_test.go index 5b23a0769b0..28c5271a40a 100644 --- a/kubectl-plugin/pkg/cmd/scale/scale_cluster_test.go +++ b/kubectl-plugin/pkg/cmd/scale/scale_cluster_test.go @@ -44,14 +44,17 @@ func TestRayScaleClusterComplete(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { testStreams, _, _, _ := genericclioptions.NewTestIOStreams() - cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true)) + configFlags := genericclioptions.NewConfigFlags(true) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + cmdFactory := cmdutil.NewFactory(configFlags) fakeScaleClusterOptions := NewScaleClusterOptions(cmdFactory, testStreams) cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeScaleClusterOptions.namespace, "namespace", "n", tc.namespace, "") - - err := fakeScaleClusterOptions.Complete(tc.args, cmd) + configFlags.AddFlags(cmd.Flags()) + err := fakeScaleClusterOptions.Complete(tc.args) require.NoError(t, err) assert.Equal(t, tc.expectedNamespace, fakeScaleClusterOptions.namespace) diff --git a/kubectl-plugin/pkg/cmd/session/session.go b/kubectl-plugin/pkg/cmd/session/session.go index 76e93ac9093..563051d33ed 100644 --- a/kubectl-plugin/pkg/cmd/session/session.go +++ b/kubectl-plugin/pkg/cmd/session/session.go @@ -108,14 +108,11 @@ func NewSessionCommand(cmdFactory cmdutil.Factory, streams genericiooptions.IOSt } func (options *SessionOptions) Complete(cmd *cobra.Command, args []string) error { - namespace, err := cmd.Flags().GetString("namespace") + namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace() if err != nil { return fmt.Errorf("failed to get namespace: %w", err) } options.namespace = namespace - if options.namespace == "" { - options.namespace = "default" - } context, err := cmd.Flags().GetString("context") if err != nil || context == "" { diff --git a/kubectl-plugin/pkg/cmd/session/session_test.go b/kubectl-plugin/pkg/cmd/session/session_test.go index 25bb76a2c29..006eebbc9fb 100644 --- a/kubectl-plugin/pkg/cmd/session/session_test.go +++ b/kubectl-plugin/pkg/cmd/session/session_test.go @@ -21,9 +21,6 @@ func TestCompleteFoo(t *testing.T) { require.NoError(t, err) testStreams, _, _, _ := genericiooptions.NewTestIOStreams() - cmdFactory := cmdutil.NewFactory(&genericclioptions.ConfigFlags{ - KubeConfig: &kubeConfigWithCurrentContext, - }) tests := []struct { name string @@ -116,12 +113,19 @@ func TestCompleteFoo(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + configFlags := &genericclioptions.ConfigFlags{KubeConfig: &kubeConfigWithCurrentContext} + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + if tc.context != "" { + configFlags.Context = &tc.context + } + cmdFactory := cmdutil.NewFactory(configFlags) fakeSessionOptions := NewSessionOptions(cmdFactory, testStreams) cmd := &cobra.Command{} - cmd.Flags().StringVarP(&fakeSessionOptions.namespace, "namespace", "n", tc.namespace, "") - cmd.Flags().StringVarP(&fakeSessionOptions.currentContext, "context", "c", tc.context, "") + configFlags.AddFlags(cmd.Flags()) - err := fakeSessionOptions.Complete(cmd, tc.args) + err = fakeSessionOptions.Complete(cmd, tc.args) if tc.hasErr { require.Error(t, err) @@ -149,8 +153,8 @@ func createTempKubeConfigFile(t *testing.T, currentContext string) (string, erro }, }, Contexts: map[string]*api.Context{ - "my-fake-context": { - Cluster: "my-fake-cluster", + currentContext: { + Cluster: "test-cluster", AuthInfo: "my-fake-user", }, }, diff --git a/kubectl-plugin/pkg/util/completion/completion.go b/kubectl-plugin/pkg/util/completion/completion.go index 585d6501888..cfc1c3aef41 100644 --- a/kubectl-plugin/pkg/util/completion/completion.go +++ b/kubectl-plugin/pkg/util/completion/completion.go @@ -68,13 +68,13 @@ func WorkerGroupCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, return []string{}, cobra.ShellCompDirectiveNoFileComp } - return workerGroupCompletionFunc(cmd, args, toComplete, k8sClient) + return workerGroupCompletionFunc(cmd, args, toComplete, k8sClient, f) } } // workerGroupCompletionFunc Returns completions of: // Workergroup names that match the toComplete prefix and respect flag values (--ray-cluster, --namespace, --all-namespaces) -func workerGroupCompletionFunc(cmd *cobra.Command, args []string, toComplete string, k8sClient client.Client) ([]string, cobra.ShellCompDirective) { +func workerGroupCompletionFunc(cmd *cobra.Command, args []string, toComplete string, k8sClient client.Client, f cmdutil.Factory) ([]string, cobra.ShellCompDirective) { var comps []string directive := cobra.ShellCompDirectiveNoFileComp @@ -84,17 +84,16 @@ func workerGroupCompletionFunc(cmd *cobra.Command, args []string, toComplete str } cluster, _ := cmd.Flags().GetString("ray-cluster") - namespace, _ := cmd.Flags().GetString("namespace") + namespace, _, err := f.ToRawKubeConfigLoader().Namespace() + if err != nil { + return comps, directive + } allNamespaces, _ := cmd.Flags().GetBool("all-namespaces") if allNamespaces { return comps, directive } - if namespace == "" { - namespace = "default" - } - listopts := v1.ListOptions{} if cluster != "" { listopts = v1.ListOptions{ @@ -127,13 +126,13 @@ func NodeCompletionFunc(f cmdutil.Factory) func(*cobra.Command, []string, string return []string{}, cobra.ShellCompDirectiveNoFileComp } - return nodeCompletionFunc(cmd, args, toComplete, k8sClient) + return nodeCompletionFunc(cmd, args, toComplete, k8sClient, f) } } // nodeCompletionFunc Returns completions of: // Node names that match the toComplete prefix and respect flag values (--ray-cluster, --namespace) -func nodeCompletionFunc(cmd *cobra.Command, args []string, toComplete string, k8sClient client.Client) ([]string, cobra.ShellCompDirective) { +func nodeCompletionFunc(cmd *cobra.Command, args []string, toComplete string, k8sClient client.Client, f cmdutil.Factory) ([]string, cobra.ShellCompDirective) { var comps []string directive := cobra.ShellCompDirectiveNoFileComp @@ -143,17 +142,16 @@ func nodeCompletionFunc(cmd *cobra.Command, args []string, toComplete string, k8 } cluster, _ := cmd.Flags().GetString("ray-cluster") - namespace, _ := cmd.Flags().GetString("namespace") + namespace, _, err := f.ToRawKubeConfigLoader().Namespace() + if err != nil { + return comps, directive + } allNamespaces, _ := cmd.Flags().GetBool("all-namespaces") if allNamespaces { return comps, directive } - if namespace == "" { - namespace = "default" - } - labelSelectors := createRayNodeLabelSelectors(cluster) pods, err := k8sClient.KubernetesClient().CoreV1().Pods(namespace).List( context.Background(), diff --git a/kubectl-plugin/pkg/util/completion/completion_test.go b/kubectl-plugin/pkg/util/completion/completion_test.go index 8efed6d4c50..2d5ff1ba3bd 100644 --- a/kubectl-plugin/pkg/util/completion/completion_test.go +++ b/kubectl-plugin/pkg/util/completion/completion_test.go @@ -9,7 +9,9 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/cli-runtime/pkg/genericclioptions" kubefake "k8s.io/client-go/kubernetes/fake" + cmdutil "k8s.io/kubectl/pkg/cmd/util" "github.com/ray-project/kuberay/kubectl-plugin/pkg/util" "github.com/ray-project/kuberay/kubectl-plugin/pkg/util/client" @@ -335,16 +337,22 @@ func TestWorkerGroupCompletionFunc(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + configFlags := genericclioptions.NewConfigFlags(true) + cmdFactory := cmdutil.NewFactory(configFlags) kubeClientSet := kubefake.NewClientset() rayClient := clienttesting.NewRayClientset(tc.rayClusters...) k8sClient := client.NewClientForTesting(kubeClientSet, rayClient) cmd := &cobra.Command{} - cmd.Flags().String("namespace", tc.namespace, "") + configFlags.AddFlags(cmd.Flags()) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } + // cmd.Flags().String("namespace", tc.namespace, "") cmd.Flags().String("ray-cluster", tc.clusterName, "") cmd.Flags().Bool("all-namespaces", tc.allNamespaces, "") - comps, directive := workerGroupCompletionFunc(cmd, tc.args, tc.toComplete, k8sClient) + comps, directive := workerGroupCompletionFunc(cmd, tc.args, tc.toComplete, k8sClient, cmdFactory) checkCompletion(t, comps, tc.expectedComps, directive, cobra.ShellCompDirectiveNoFileComp) }) } @@ -611,16 +619,21 @@ func TestNodeCompletionFunc(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + configFlags := genericclioptions.NewConfigFlags(true) + cmdFactory := cmdutil.NewFactory(configFlags) kubeClientSet := kubefake.NewClientset(tc.pods...) rayClient := rayClientFake.NewClientset() k8sClient := client.NewClientForTesting(kubeClientSet, rayClient) cmd := &cobra.Command{} - cmd.Flags().String("namespace", tc.namespace, "") + configFlags.AddFlags(cmd.Flags()) + if tc.namespace != "" { + configFlags.Namespace = &tc.namespace + } cmd.Flags().String("ray-cluster", tc.clusterName, "") cmd.Flags().Bool("all-namespaces", tc.allNamespaces, "") - comps, directive := nodeCompletionFunc(cmd, tc.args, tc.toComplete, k8sClient) + comps, directive := nodeCompletionFunc(cmd, tc.args, tc.toComplete, k8sClient, cmdFactory) checkCompletion(t, comps, tc.expectedComps, directive, cobra.ShellCompDirectiveNoFileComp) }) }