diff --git a/kubectl-plugin/pkg/cmd/get/get_cluster.go b/kubectl-plugin/pkg/cmd/get/get_cluster.go index e9c23ec36de..546c22a2974 100644 --- a/kubectl-plugin/pkg/cmd/get/get_cluster.go +++ b/kubectl-plugin/pkg/cmd/get/get_cluster.go @@ -23,7 +23,7 @@ import ( type GetClusterOptions struct { configFlags *genericclioptions.ConfigFlags ioStreams *genericclioptions.IOStreams - args []string + cluster string AllNamespaces bool } @@ -44,6 +44,7 @@ func NewGetClusterCommand(streams genericclioptions.IOStreams) *cobra.Command { Short: "Get cluster information.", SilenceUsage: true, ValidArgsFunction: completion.RayClusterCompletionFunc(cmdFactory), + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if err := options.Complete(args); err != nil { return err @@ -69,7 +70,10 @@ func (options *GetClusterOptions) Complete(args []string) error { options.AllNamespaces = true } - options.args = args + if len(args) >= 1 { + options.cluster = args[0] + } + return nil } @@ -82,9 +86,6 @@ func (options *GetClusterOptions) Validate() error { if !util.HasKubectlContext(config, options.configFlags) { return fmt.Errorf("no context is currently set, use %q or %q to select a new one", "--context", "kubectl config use-context ") } - if len(options.args) > 1 { - return fmt.Errorf("too many arguments, either one or no arguments are allowed") - } return nil } @@ -102,26 +103,26 @@ func getRayClusters(ctx context.Context, options *GetClusterOptions, k8sClient c var err error listopts := v1.ListOptions{} - if len(options.args) == 1 { + if options.cluster != "" { listopts = v1.ListOptions{ - FieldSelector: fmt.Sprintf("metadata.name=%s", options.args[0]), + FieldSelector: fmt.Sprintf("metadata.name=%s", options.cluster), } } if options.AllNamespaces { rayclusterList, err = k8sClient.RayClient().RayV1().RayClusters("").List(ctx, listopts) if err != nil { - return nil, fmt.Errorf("unable to retrieve raycluster for all namespaces: %w", err) + return nil, fmt.Errorf("unable to retrieve Ray clusters for all namespaces: %w", err) } } else { rayclusterList, err = k8sClient.RayClient().RayV1().RayClusters(*options.configFlags.Namespace).List(ctx, listopts) if err != nil { - return nil, fmt.Errorf("unable to retrieve raycluster for namespace %s: %w", *options.configFlags.Namespace, err) + return nil, fmt.Errorf("unable to retrieve Ray clusters for namespace %s: %w", *options.configFlags.Namespace, err) } } - if len(options.args) == 1 && len(rayclusterList.Items) == 0 { - errMsg := fmt.Sprintf("Ray cluster %s not found", options.args[0]) + if options.cluster != "" && len(rayclusterList.Items) == 0 { + errMsg := fmt.Sprintf("Ray cluster %s not found", options.cluster) if options.AllNamespaces { errMsg += " in any namespace" } else { diff --git a/kubectl-plugin/pkg/cmd/get/get_cluster_test.go b/kubectl-plugin/pkg/cmd/get/get_cluster_test.go index 3bcf9b39480..2c33fedbeb2 100644 --- a/kubectl-plugin/pkg/cmd/get/get_cluster_test.go +++ b/kubectl-plugin/pkg/cmd/get/get_cluster_test.go @@ -24,22 +24,62 @@ import ( rayClientFake "github.com/ray-project/kuberay/ray-operator/pkg/client/clientset/versioned/fake" ) -// This is to test Complete() and ensure that it is setting the namespace and arguments correctly +// 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) { // Initialize members of the cluster get option struct and the struct itself testStreams, _, _, _ := genericclioptions.NewTestIOStreams() - fakeClusterGetOptions := NewGetClusterOptions(testStreams) - fakeArgs := []string{"Expected", "output"} - *fakeClusterGetOptions.configFlags.Namespace = "" - fakeClusterGetOptions.AllNamespaces = false + tests := []struct { + name string + namespace string + expectedCluster string + args []string + expectedAllNamespaces bool + }{ + { + name: "neither namespace nor args set", + namespace: "", + args: []string{}, + expectedAllNamespaces: true, + expectedCluster: "", + }, + { + name: "namespace set, args not set", + namespace: "foo", + args: []string{}, + expectedAllNamespaces: false, + expectedCluster: "", + }, + { + name: "namespace not set, args set", + namespace: "", + args: []string{"foo", "bar"}, + expectedAllNamespaces: true, + expectedCluster: "foo", + }, + { + name: "both namespace and args set", + namespace: "foo", + args: []string{"bar", "qux"}, + expectedAllNamespaces: false, + expectedCluster: "bar", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeClusterGetOptions := NewGetClusterOptions(testStreams) - err := fakeClusterGetOptions.Complete(fakeArgs) - require.NoError(t, err) + *fakeClusterGetOptions.configFlags.Namespace = tc.namespace + + err := fakeClusterGetOptions.Complete(tc.args) + require.NoError(t, err) - assert.True(t, fakeClusterGetOptions.AllNamespaces) - assert.Equal(t, fakeClusterGetOptions.args, fakeArgs) + assert.Equal(t, tc.expectedAllNamespaces, fakeClusterGetOptions.AllNamespaces) + assert.Equal(t, tc.expectedCluster, fakeClusterGetOptions.cluster) + }) + } } // Test the Validation() step of the command. @@ -67,7 +107,7 @@ func TestRayClusterGetValidate(t *testing.T) { KubeConfig: &kubeConfigWithoutCurrentContext, }, AllNamespaces: false, - args: []string{"random_arg"}, + cluster: "random_arg", ioStreams: &testStreams, }, expectError: "no context is currently set, use \"--context\" or \"kubectl config use-context \" to select a new one", @@ -101,24 +141,6 @@ func TestRayClusterGetValidate(t *testing.T) { ioStreams: &testStreams, }, }, - { - name: "Test validation when more than 1 arg", - opts: &GetClusterOptions{ - // Use fake config to bypass the config flag checks - configFlags: &genericclioptions.ConfigFlags{ - Namespace: &testNS, - Context: &testContext, - KubeConfig: &kubeConfigWithCurrentContext, - BearerToken: &testBT, - Impersonate: &testImpersonate, - ImpersonateGroup: &[]string{"fake-group"}, - }, - AllNamespaces: false, - args: []string{"fake", "args"}, - ioStreams: &testStreams, - }, - expectError: "too many arguments, either one or no arguments are allowed", - }, { name: "Successful validation call", opts: &GetClusterOptions{ @@ -132,7 +154,7 @@ func TestRayClusterGetValidate(t *testing.T) { ImpersonateGroup: &[]string{"fake-group"}, }, AllNamespaces: false, - args: []string{"random_arg"}, + cluster: "random_arg", ioStreams: &testStreams, }, expectError: "", @@ -236,56 +258,52 @@ func TestGetRayClusters(t *testing.T) { name string expectedError string expectedOutput string - args []string + cluster string rayClusters []runtime.Object }{ { name: "should not error if no cluster name is provided, searching all namespaces, and no clusters are found", - args: []string{}, namespace: nil, rayClusters: []runtime.Object{}, }, { name: "should error if a cluster name is provided, searching all namespaces, and no clusters are found", - args: []string{"my-cluster"}, + cluster: "my-cluster", namespace: nil, rayClusters: []runtime.Object{}, expectedError: "Ray cluster my-cluster not found", }, { name: "should not error if no cluster name is provided, searching one namespace, and no clusters are found", - args: []string{}, namespace: &namespace, rayClusters: []runtime.Object{}, }, { name: "should error if a cluster name is provided, searching one namespace, and no clusters are found", - args: []string{"my-cluster"}, + cluster: "my-cluster", namespace: &namespace, rayClusters: []runtime.Object{}, expectedError: fmt.Sprintf("Ray cluster my-cluster not found in namespace %s", namespace), }, { name: "should not error if no cluster name is provided, searching all namespaces, and clusters are found", - args: []string{}, namespace: nil, rayClusters: []runtime.Object{rayCluster}, }, { name: "should not error if a cluster name is provided, searching all namespaces, and clusters are found", - args: []string{"my-cluster"}, + cluster: "my-cluster", namespace: nil, rayClusters: []runtime.Object{rayCluster}, }, { name: "should not error if no cluster name is provided, searching one namespace, and clusters are found", - args: []string{}, namespace: &namespace, rayClusters: []runtime.Object{rayCluster}, }, { name: "should not error if a cluster name is provided, searching one namespace, and clusters are found", - args: []string{"my-cluster"}, + cluster: "my-cluster", namespace: &namespace, rayClusters: []runtime.Object{rayCluster}, }, @@ -296,7 +314,7 @@ func TestGetRayClusters(t *testing.T) { fakeClusterGetOptions := GetClusterOptions{ configFlags: genericclioptions.NewConfigFlags(true), ioStreams: &testStreams, - args: tc.args, + cluster: tc.cluster, } if tc.namespace != nil { *fakeClusterGetOptions.configFlags.Namespace = *tc.namespace