Skip to content

Commit b1ff884

Browse files
kash2104kash2104
committed
refactor: Standardize namespace resolution for kubectl-plugin.
1 parent df6eff5 commit b1ff884

21 files changed

+112
-102
lines changed

‎kubectl-plugin/pkg/cmd/create/create_cluster.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func NewCreateClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptio
150150
}
151151

152152
func (options *CreateClusterOptions) Complete(cmd *cobra.Command, args []string) error {
153-
namespace, err := cmd.Flags().GetString("namespace")
153+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
154154
if err != nil {
155155
return fmt.Errorf("failed to get namespace: %w", err)
156156
}
@@ -241,7 +241,10 @@ func (options *CreateClusterOptions) resolveClusterName() (string, error) {
241241

242242
// resolveNamespace resolves the namespace from the CLI flag and the config file
243243
func (options *CreateClusterOptions) resolveNamespace() (string, error) {
244-
namespace := "default"
244+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
245+
if err != nil {
246+
return "", fmt.Errorf("failed to get current namespace: %w", err)
247+
}
245248

246249
if options.rayClusterConfig.Namespace != nil && *options.rayClusterConfig.Namespace != "" && options.namespace != "" && options.namespace != *options.rayClusterConfig.Namespace {
247250
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
274277
}
275278
options.rayClusterConfig.Namespace = &namespace
276279
} else {
277-
if options.namespace == "" {
278-
options.namespace = "default"
279-
}
280-
281280
options.rayClusterConfig = &generation.RayClusterConfig{
282281
Namespace: &options.namespace,
283282
Name: &options.clusterName,

‎kubectl-plugin/pkg/cmd/create/create_cluster_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ func TestRayCreateClusterComplete(t *testing.T) {
6161

6262
for name, tc := range tests {
6363
t.Run(name, func(t *testing.T) {
64-
cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))
64+
configFlags := genericclioptions.NewConfigFlags(true)
65+
cmdFactory := cmdutil.NewFactory(configFlags)
6566
fakeCreateClusterOptions := NewCreateClusterOptions(cmdFactory, testStreams)
6667
cmd := &cobra.Command{Use: "cluster"}
67-
cmd.Flags().StringVarP(&fakeCreateClusterOptions.namespace, "namespace", "n", "", "")
68+
configFlags.AddFlags(cmd.Flags())
6869
fakeCreateClusterOptions.rayVersion = tc.rayVersion
6970

7071
if tc.image != "" {

‎kubectl-plugin/pkg/cmd/create/create_workergroup.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,12 @@ func NewCreateWorkerGroupCommand(cmdFactory cmdutil.Factory, streams genericclio
108108
}
109109

110110
func (options *CreateWorkerGroupOptions) Complete(cmd *cobra.Command, args []string) error {
111-
namespace, err := cmd.Flags().GetString("namespace")
111+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
112112
if err != nil {
113113
return fmt.Errorf("failed to get namespace: %w", err)
114114
}
115115
options.namespace = namespace
116116

117-
if options.namespace == "" {
118-
options.namespace = "default"
119-
}
120-
121117
if options.rayStartParams == nil {
122118
options.rayStartParams = map[string]string{}
123119
}

‎kubectl-plugin/pkg/cmd/create/create_workergroup_test.go‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"github.com/stretchr/testify/require"
99
corev1 "k8s.io/api/core/v1"
1010
"k8s.io/apimachinery/pkg/api/resource"
11+
"k8s.io/cli-runtime/pkg/genericclioptions"
12+
cmdutil "k8s.io/kubectl/pkg/cmd/util"
1113
"k8s.io/utils/ptr"
1214

1315
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
@@ -104,10 +106,17 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) {
104106
},
105107
},
106108
{
107-
name: "Valid input without namespace flag",
108-
args: []string{"example-group"},
109-
flags: map[string]string{},
110-
expectedError: "failed to get namespace: flag accessed but not defined: namespace",
109+
name: "Valid input without namespace flag",
110+
args: []string{"example-group"},
111+
flags: map[string]string{
112+
"namespace": "",
113+
},
114+
expected: &CreateWorkerGroupOptions{
115+
namespace: "default",
116+
groupName: "example-group",
117+
image: "rayproject/ray:latest",
118+
rayVersion: "latest",
119+
},
111120
},
112121
{
113122
name: "mMissing group name",
@@ -181,12 +190,16 @@ func TestCreateWorkerGroupCommandComplete(t *testing.T) {
181190

182191
for _, tt := range tests {
183192
t.Run(tt.name, func(t *testing.T) {
184-
cmd := &cobra.Command{}
193+
configFlags := genericclioptions.NewConfigFlags(true)
194+
cmd := &cobra.Command{}
195+
configFlags.AddFlags(cmd.Flags())
185196
for key, value := range tt.flags {
186-
cmd.Flags().String(key, value, "")
197+
cmd.Flags().Set(key, value)
187198
}
188199

200+
factory := cmdutil.NewFactory(configFlags)
189201
options := &CreateWorkerGroupOptions{
202+
cmdFactory: factory,
190203
rayVersion: "latest",
191204
}
192205

‎kubectl-plugin/pkg/cmd/delete/delete.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,11 @@ func NewDeleteCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IOSt
7878
}
7979

8080
func (options *DeleteOptions) Complete(cmd *cobra.Command, args []string) error {
81-
namespace, err := cmd.Flags().GetString("namespace")
81+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
8282
if err != nil {
8383
return fmt.Errorf("failed to get namespace: %w", err)
8484
}
8585
options.namespace = namespace
86-
if options.namespace == "" {
87-
options.namespace = "default"
88-
}
8986

9087
if options.resources == nil {
9188
options.resources = map[util.ResourceType][]string{}

‎kubectl-plugin/pkg/cmd/delete/delete_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414

1515
func TestComplete(t *testing.T) {
1616
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()
17-
cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))
1817

1918
tests := []struct {
2019
name string
@@ -102,10 +101,16 @@ func TestComplete(t *testing.T) {
102101

103102
for _, tc := range tests {
104103
t.Run(tc.name, func(t *testing.T) {
105-
fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams)
104+
configFlags := genericclioptions.NewConfigFlags(true)
105+
if tc.namespace != "" {
106+
configFlags.Namespace = &tc.namespace
107+
}
108+
cmdFactory := cmdutil.NewFactory(configFlags)
106109

110+
fakeDeleteOptions := NewDeleteOptions(cmdFactory, testStreams)
107111
cmd := &cobra.Command{}
108-
cmd.Flags().StringVarP(&fakeDeleteOptions.namespace, "namespace", "n", tc.namespace, "")
112+
flags := cmd.Flags()
113+
configFlags.AddFlags(flags)
109114

110115
err := fakeDeleteOptions.Complete(cmd, tc.args)
111116

‎kubectl-plugin/pkg/cmd/get/get_cluster.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func NewGetClusterCommand(cmdFactory cmdutil.Factory, streams genericclioptions.
4646
ValidArgsFunction: completion.RayClusterCompletionFunc(cmdFactory),
4747
Args: cobra.MaximumNArgs(1),
4848
RunE: func(cmd *cobra.Command, args []string) error {
49-
if err := options.Complete(args, cmd); err != nil {
49+
if err := options.Complete(args); err != nil {
5050
return err
5151
}
5252
// 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.
6161
return cmd
6262
}
6363

64-
func (options *GetClusterOptions) Complete(args []string, cmd *cobra.Command) error {
65-
namespace, err := cmd.Flags().GetString("namespace")
64+
func (options *GetClusterOptions) Complete(args []string) error {
65+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
6666
if err != nil {
6767
return fmt.Errorf("failed to get namespace: %w", err)
6868
}
6969
options.namespace = namespace
70-
if options.namespace == "" {
71-
options.namespace = "default"
72-
}
7370

7471
if len(args) >= 1 {
7572
options.cluster = args[0]

‎kubectl-plugin/pkg/cmd/get/get_cluster_test.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
func TestRayClusterGetComplete(t *testing.T) {
3030
// Initialize members of the cluster get option struct and the struct itself
3131
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()
32-
cmdFactory := cmdutil.NewFactory(genericclioptions.NewConfigFlags(true))
3332

3433
tests := []struct {
3534
name string
@@ -70,11 +69,16 @@ func TestRayClusterGetComplete(t *testing.T) {
7069

7170
for _, tc := range tests {
7271
t.Run(tc.name, func(t *testing.T) {
72+
configFlags := genericclioptions.NewConfigFlags(true)
73+
cmdFactory := cmdutil.NewFactory(configFlags)
7374
fakeClusterGetOptions := NewGetClusterOptions(cmdFactory, testStreams)
7475

76+
if tc.namespace != "" {
77+
configFlags.Namespace = &tc.namespace
78+
}
7579
cmd := &cobra.Command{}
76-
cmd.Flags().StringVarP(&fakeClusterGetOptions.namespace, "namespace", "n", tc.namespace, "")
77-
err := fakeClusterGetOptions.Complete(tc.args, cmd)
80+
configFlags.AddFlags(cmd.Flags())
81+
err := fakeClusterGetOptions.Complete(tc.args)
7882
require.NoError(t, err)
7983

8084
assert.Equal(t, tc.expectedAllNamespaces, fakeClusterGetOptions.allNamespaces)

‎kubectl-plugin/pkg/cmd/get/get_nodes.go‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func NewGetNodesCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO
8383
Args: cobra.MaximumNArgs(1),
8484
ValidArgsFunction: completion.NodeCompletionFunc(cmdFactory),
8585
RunE: func(cmd *cobra.Command, args []string) error {
86-
if err := options.Complete(args, cmd); err != nil {
86+
if err := options.Complete(args); err != nil {
8787
return err
8888
}
8989
k8sClient, err := client.NewClient(cmdFactory)
@@ -105,19 +105,15 @@ func NewGetNodesCommand(cmdFactory cmdutil.Factory, streams genericclioptions.IO
105105
return cmd
106106
}
107107

108-
func (options *GetNodesOptions) Complete(args []string, cmd *cobra.Command) error {
108+
func (options *GetNodesOptions) Complete(args []string) error {
109109
if options.allNamespaces {
110110
options.namespace = ""
111111
} else {
112-
namespace, err := cmd.Flags().GetString("namespace")
112+
namespace, _, err := options.cmdFactory.ToRawKubeConfigLoader().Namespace()
113113
if err != nil {
114114
return fmt.Errorf("failed to get namespace: %w", err)
115115
}
116116
options.namespace = namespace
117-
118-
if options.namespace == "" {
119-
options.namespace = "default"
120-
}
121117
}
122118

123119
if len(args) > 0 {

‎kubectl-plugin/pkg/cmd/get/get_nodes_test.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,15 @@ func TestRayNodesGetComplete(t *testing.T) {
8383

8484
for _, tc := range tests {
8585
t.Run(tc.name, func(t *testing.T) {
86-
err := flags.Set("namespace", tc.namespace)
87-
require.NoError(t, err)
88-
err = tc.opts.Complete(tc.args, cmd)
86+
configFlags := genericclioptions.NewConfigFlags(true)
87+
if tc.namespace != "" {
88+
configFlags.Namespace = &tc.namespace
89+
}
90+
tc.opts.cmdFactory = cmdutil.NewFactory(configFlags)
91+
cmd := &cobra.Command{}
92+
flags := cmd.Flags()
93+
configFlags.AddFlags(flags)
94+
err := tc.opts.Complete(tc.args)
8995
require.NoError(t, err)
9096
assert.Equal(t, tc.expectedNamespace, tc.opts.namespace)
9197
assert.Equal(t, tc.expectedNode, tc.opts.node)

0 commit comments

Comments
 (0)