Skip to content

Commit 0dc7968

Browse files
andrewsykimrueiankevin85421davidxiaMortalHappiness
authored
Sync release-1.3 with master (#2916)
* [RayService] More envtests that follow the most common scenario in the RayService code path (#2880) Signed-off-by: Rueian <[email protected]> * [RayService] Remove outdated env tests (#2886) Signed-off-by: kaihsun <[email protected]> * [RayService] Refactor envtests (#2888) * [docs][ray-operator] fix typo in Golang version (#2893) Project uses Golang 1.22 not 1.20. Signed-off-by: David Xia <[email protected]> * [Fix][kubectl-plugin] Fix no context nil error SIGSEGV in tests (#2892) Signed-off-by: Chi-Sheng Liu <[email protected]> * [CI] Enable testifylint rule (#2896) Signed-off-by: Chi-Sheng Liu <[email protected]> * [CI] Enable `testifylint` `error-nil` rule (#2907) * [kubectl-plugin][feat] support specifying number of head GPUs (#2895) when creating a RayCluster with `kubectl ray create cluster NAME --head-gpu N`. Similar to the `--worker-gpu` switch. Signed-off-by: David Xia <[email protected]> * Use webhook.CustomValidator instead of deprecated webhook.Validator. (#2803) * Use webhook.CustomValidator instead of deprecated webhook.Validator. * Move RayClusterWebhook to pkg/webhook/v1. * [kubectl-plugin] update context error messages (#2891) to tell user they can use `--context` to set the K8s context. Add tests Signed-off-by: David Xia <[email protected]> Co-authored-by: Chi-Sheng Liu <[email protected]> * update version for v1.3.0-rc.0 (#2885) Signed-off-by: Andrew Sy Kim <[email protected]> --------- Signed-off-by: Rueian <[email protected]> Signed-off-by: kaihsun <[email protected]> Signed-off-by: David Xia <[email protected]> Signed-off-by: Chi-Sheng Liu <[email protected]> Signed-off-by: Andrew Sy Kim <[email protected]> Co-authored-by: Rueian <[email protected]> Co-authored-by: Kai-Hsun Chen <[email protected]> Co-authored-by: David Xia <[email protected]> Co-authored-by: Chi-Sheng Liu <[email protected]> Co-authored-by: David Xia <[email protected]> Co-authored-by: Mykhailo Bobrovskyi <[email protected]>
1 parent 8053aa1 commit 0dc7968

37 files changed

+989
-913
lines changed

‎.golangci.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,17 @@ linters-settings:
5555
- fieldalignment
5656
lll:
5757
line-length: 120
58+
# TODO: Enable all testifylint rules
59+
testifylint:
60+
disable:
61+
- formatter
62+
- float-compare
63+
- require-error
64+
- expected-actual
65+
- bool-compare
66+
- len
67+
- compares
68+
- empty
5869
linters:
5970
enable:
6071
- asciicheck
@@ -83,6 +94,7 @@ linters:
8394
- unparam
8495
- unused
8596
- wastedassign
97+
- testifylint
8698
disable-all: true
8799
issues:
88100
max-issues-per-linter: 0

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66

7+
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
78
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util/generation"
89
"github.com/spf13/cobra"
910
"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -22,6 +23,7 @@ type CreateClusterOptions struct {
2223
image string
2324
headCPU string
2425
headMemory string
26+
headGPU string
2527
workerCPU string
2628
workerMemory string
2729
workerGPU string
@@ -75,10 +77,11 @@ func NewCreateClusterCommand(streams genericclioptions.IOStreams) *cobra.Command
7577
cmd.Flags().StringVar(&options.image, "image", options.image, "Ray image to use in the Ray Cluster yaml")
7678
cmd.Flags().StringVar(&options.headCPU, "head-cpu", "2", "Number of CPU for the ray head. Default to 2")
7779
cmd.Flags().StringVar(&options.headMemory, "head-memory", "4Gi", "Amount of memory to use for the ray head. Default to 4Gi")
80+
cmd.Flags().StringVar(&options.headGPU, "head-gpu", "0", "Number of GPUs for the ray head. Default to 0")
7881
cmd.Flags().Int32Var(&options.workerReplicas, "worker-replicas", 1, "Number of the worker group replicas. Default of 1")
7982
cmd.Flags().StringVar(&options.workerCPU, "worker-cpu", "2", "Number of CPU for the ray worker. Default to 2")
8083
cmd.Flags().StringVar(&options.workerMemory, "worker-memory", "4Gi", "Amount of memory to use for the ray worker. Default to 4Gi")
81-
cmd.Flags().StringVar(&options.workerGPU, "worker-gpu", "0", "Number of GPU for the ray worker. Default to 0")
84+
cmd.Flags().StringVar(&options.workerGPU, "worker-gpu", "0", "Number of GPUs for the ray worker. Default to 0")
8285
cmd.Flags().BoolVar(&options.dryRun, "dry-run", false, "Will not apply the generated cluster and will print out the generated yaml")
8386

8487
options.configFlags.AddFlags(cmd.Flags())
@@ -107,8 +110,8 @@ func (options *CreateClusterOptions) Validate() error {
107110
if err != nil {
108111
return fmt.Errorf("Error retrieving raw config: %w", err)
109112
}
110-
if len(config.CurrentContext) == 0 {
111-
return fmt.Errorf("no context is currently set, use %q to select a new one", "kubectl config use-context <context>")
113+
if !util.HasKubectlContext(config, options.configFlags) {
114+
return fmt.Errorf("no context is currently set, use %q or %q to select a new one", "--context", "kubectl config use-context <context>")
112115
}
113116

114117
return nil
@@ -129,6 +132,7 @@ func (options *CreateClusterOptions) Run(ctx context.Context, factory cmdutil.Fa
129132
Image: options.image,
130133
HeadCPU: options.headCPU,
131134
HeadMemory: options.headMemory,
135+
HeadGPU: options.headGPU,
132136
WorkerReplicas: options.workerReplicas,
133137
WorkerCPU: options.workerCPU,
134138
WorkerMemory: options.workerMemory,

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

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package create
22

33
import (
4-
"os"
5-
"path/filepath"
64
"testing"
75

6+
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
87
"github.com/spf13/cobra"
98
"github.com/stretchr/testify/assert"
109
"k8s.io/cli-runtime/pkg/genericclioptions"
11-
"k8s.io/client-go/tools/clientcmd"
12-
"k8s.io/client-go/tools/clientcmd/api"
1310
)
1411

1512
func TestRayCreateClusterComplete(t *testing.T) {
@@ -19,56 +16,20 @@ func TestRayCreateClusterComplete(t *testing.T) {
1916
cmd := &cobra.Command{Use: "cluster"}
2017

2118
err := fakeCreateClusterOptions.Complete(cmd, fakeArgs)
22-
assert.Nil(t, err)
19+
assert.NoError(t, err)
2320
assert.Equal(t, "default", *fakeCreateClusterOptions.configFlags.Namespace)
2421
assert.Equal(t, "testRayClusterName", fakeCreateClusterOptions.clusterName)
2522
}
2623

2724
func TestRayCreateClusterValidate(t *testing.T) {
2825
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()
29-
3026
testNS, testContext, testBT, testImpersonate := "test-namespace", "test-context", "test-bearer-token", "test-person"
3127

32-
// Fake directory for kubeconfig
33-
fakeDir, err := os.MkdirTemp("", "fake-dir")
34-
assert.Nil(t, err)
35-
defer os.RemoveAll(fakeDir)
36-
37-
// Set up fake config for kubeconfig
38-
config := &api.Config{
39-
Clusters: map[string]*api.Cluster{
40-
"test-cluster": {
41-
Server: "https://fake-kubernetes-cluster.example.com",
42-
InsecureSkipTLSVerify: true, // For testing purposes
43-
},
44-
},
45-
Contexts: map[string]*api.Context{
46-
"my-fake-context": {
47-
Cluster: "my-fake-cluster",
48-
AuthInfo: "my-fake-user",
49-
},
50-
},
51-
CurrentContext: "my-fake-context",
52-
AuthInfos: map[string]*api.AuthInfo{
53-
"my-fake-user": {
54-
Token: "", // Empty for testing without authentication
55-
},
56-
},
57-
}
58-
59-
fakeFile := filepath.Join(fakeDir, ".kubeconfig")
60-
61-
err = clientcmd.WriteToFile(*config, fakeFile)
62-
assert.Nil(t, err)
28+
kubeConfigWithCurrentContext, err := util.CreateTempKubeConfigFile(t, testContext)
29+
assert.NoError(t, err)
6330

64-
fakeConfigFlags := &genericclioptions.ConfigFlags{
65-
Namespace: &testNS,
66-
Context: &testContext,
67-
KubeConfig: &fakeFile,
68-
BearerToken: &testBT,
69-
Impersonate: &testImpersonate,
70-
ImpersonateGroup: &[]string{"fake-group"},
71-
}
31+
kubeConfigWithoutCurrentContext, err := util.CreateTempKubeConfigFile(t, "")
32+
assert.NoError(t, err)
7233

7334
tests := []struct {
7435
name string
@@ -81,17 +42,54 @@ func TestRayCreateClusterValidate(t *testing.T) {
8142
configFlags: genericclioptions.NewConfigFlags(false),
8243
ioStreams: &testStreams,
8344
},
84-
expectError: "no context is currently set, use \"kubectl config use-context <context>\" to select a new one",
45+
expectError: "no context is currently set, use \"--context\" or \"kubectl config use-context <context>\" to select a new one",
46+
},
47+
{
48+
name: "no error when kubeconfig has current context and --context switch isn't set",
49+
opts: &CreateClusterOptions{
50+
configFlags: &genericclioptions.ConfigFlags{
51+
KubeConfig: &kubeConfigWithCurrentContext,
52+
},
53+
ioStreams: &testStreams,
54+
},
55+
},
56+
{
57+
name: "no error when kubeconfig has no current context and --context switch is set",
58+
opts: &CreateClusterOptions{
59+
configFlags: &genericclioptions.ConfigFlags{
60+
KubeConfig: &kubeConfigWithoutCurrentContext,
61+
Context: &testContext,
62+
},
63+
ioStreams: &testStreams,
64+
},
65+
},
66+
{
67+
name: "no error when kubeconfig has current context and --context switch is set",
68+
opts: &CreateClusterOptions{
69+
configFlags: &genericclioptions.ConfigFlags{
70+
KubeConfig: &kubeConfigWithCurrentContext,
71+
Context: &testContext,
72+
},
73+
ioStreams: &testStreams,
74+
},
8575
},
8676
{
8777
name: "Successful submit job validation with RayJob",
8878
opts: &CreateClusterOptions{
89-
configFlags: fakeConfigFlags,
79+
configFlags: &genericclioptions.ConfigFlags{
80+
Namespace: &testNS,
81+
Context: &testContext,
82+
KubeConfig: &kubeConfigWithCurrentContext,
83+
BearerToken: &testBT,
84+
Impersonate: &testImpersonate,
85+
ImpersonateGroup: &[]string{"fake-group"},
86+
},
9087
ioStreams: &testStreams,
9188
clusterName: "fakeclustername",
9289
rayVersion: "ray-version",
9390
image: "ray-image",
9491
headCPU: "5",
92+
headGPU: "1",
9593
headMemory: "5Gi",
9694
workerReplicas: 3,
9795
workerCPU: "4",
@@ -104,9 +102,9 @@ func TestRayCreateClusterValidate(t *testing.T) {
104102
t.Run(tc.name, func(t *testing.T) {
105103
err := tc.opts.Validate()
106104
if tc.expectError != "" {
107-
assert.Equal(t, tc.expectError, err.Error())
105+
assert.Error(t, err, tc.expectError)
108106
} else {
109-
assert.Nil(t, err)
107+
assert.NoError(t, err)
110108
}
111109
})
112110
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"k8s.io/cli-runtime/pkg/genericclioptions"
99
"k8s.io/client-go/rest"
1010

11+
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
1112
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util/client"
1213

1314
rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"
@@ -112,8 +113,8 @@ func (options *CreateWorkerGroupOptions) Validate() error {
112113
if err != nil {
113114
return fmt.Errorf("Error retrieving raw config: %w", err)
114115
}
115-
if len(config.CurrentContext) == 0 {
116-
return fmt.Errorf("no context is currently set, use %q to select a new one", "kubectl config use-context <context>")
116+
if !util.HasKubectlContext(config, options.configFlags) {
117+
return fmt.Errorf("no context is currently set, use %q or %q to select a new one", "--context", "kubectl config use-context <context>")
117118
}
118119

119120
return nil

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ func (options *DeleteOptions) Validate() error {
118118
if err != nil {
119119
return fmt.Errorf("Error retrieving raw config: %w", err)
120120
}
121-
if len(config.CurrentContext) == 0 {
122-
return fmt.Errorf("no context is currently set, use %q to select a new one", "kubectl config use-context <context>")
121+
if !util.HasKubectlContext(config, options.configFlags) {
122+
return fmt.Errorf("no context is currently set, use %q or %q to select a new one", "--context", "kubectl config use-context <context>")
123123
}
124124
return nil
125125
}

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

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,97 @@ func TestComplete(t *testing.T) {
100100
fakeDeleteOptions.configFlags.Namespace = &tc.namespace
101101
err := fakeDeleteOptions.Complete(cmd, tc.args)
102102
if tc.hasErr {
103-
assert.NotNil(t, err)
103+
assert.Error(t, err)
104104
} else {
105-
assert.Nil(t, err)
105+
assert.NoError(t, err)
106106
assert.Equal(t, tc.expectedName, fakeDeleteOptions.ResourceName)
107107
assert.Equal(t, tc.expectedNamespace, fakeDeleteOptions.Namespace)
108108
assert.Equal(t, tc.expectedResourceType, fakeDeleteOptions.ResourceType)
109109
}
110110
})
111111
}
112112
}
113+
114+
func TestRayDeleteValidate(t *testing.T) {
115+
testStreams, _, _, _ := genericclioptions.NewTestIOStreams()
116+
117+
testNS, testContext, testBT, testImpersonate := "test-namespace", "test-context", "test-bearer-token", "test-person"
118+
119+
kubeConfigWithCurrentContext, err := util.CreateTempKubeConfigFile(t, testContext)
120+
assert.NoError(t, err)
121+
122+
kubeConfigWithoutCurrentContext, err := util.CreateTempKubeConfigFile(t, "")
123+
assert.NoError(t, err)
124+
125+
tests := []struct {
126+
name string
127+
opts *DeleteOptions
128+
expectError string
129+
}{
130+
{
131+
name: "Test validation when no context is set",
132+
opts: &DeleteOptions{
133+
configFlags: genericclioptions.NewConfigFlags(false),
134+
ioStreams: &testStreams,
135+
},
136+
expectError: "no context is currently set, use \"--context\" or \"kubectl config use-context <context>\" to select a new one",
137+
},
138+
{
139+
name: "no error when kubeconfig has current context and --context switch isn't set",
140+
opts: &DeleteOptions{
141+
configFlags: &genericclioptions.ConfigFlags{
142+
KubeConfig: &kubeConfigWithCurrentContext,
143+
},
144+
ioStreams: &testStreams,
145+
},
146+
},
147+
{
148+
name: "no error when kubeconfig has no current context and --context switch is set",
149+
opts: &DeleteOptions{
150+
configFlags: &genericclioptions.ConfigFlags{
151+
KubeConfig: &kubeConfigWithoutCurrentContext,
152+
Context: &testContext,
153+
},
154+
ioStreams: &testStreams,
155+
},
156+
},
157+
{
158+
name: "no error when kubeconfig has current context and --context switch is set",
159+
opts: &DeleteOptions{
160+
configFlags: &genericclioptions.ConfigFlags{
161+
KubeConfig: &kubeConfigWithCurrentContext,
162+
Context: &testContext,
163+
},
164+
ioStreams: &testStreams,
165+
},
166+
},
167+
{
168+
name: "Successful submit job validation with RayJob",
169+
opts: &DeleteOptions{
170+
configFlags: &genericclioptions.ConfigFlags{
171+
Namespace: &testNS,
172+
Context: &testContext,
173+
KubeConfig: &kubeConfigWithCurrentContext,
174+
BearerToken: &testBT,
175+
Impersonate: &testImpersonate,
176+
ImpersonateGroup: &[]string{"fake-group"},
177+
},
178+
ioStreams: &testStreams,
179+
ResourceType: util.RayJob,
180+
ResourceName: "test-rayjob",
181+
Namespace: testNS,
182+
},
183+
},
184+
}
185+
186+
for _, tc := range tests {
187+
t.Run(tc.name, func(t *testing.T) {
188+
err := tc.opts.Validate()
189+
if tc.expectError != "" {
190+
assert.Error(t, err, tc.expectError)
191+
} else {
192+
assert.NoError(t, err)
193+
}
194+
})
195+
}
196+
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"io"
77
"time"
88

9+
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util"
910
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util/client"
1011
"github.com/ray-project/kuberay/kubectl-plugin/pkg/util/completion"
1112
"github.com/spf13/cobra"
@@ -77,8 +78,8 @@ func (options *GetClusterOptions) Validate() error {
7778
if err != nil {
7879
return fmt.Errorf("Error retrieving raw config: %w", err)
7980
}
80-
if len(config.CurrentContext) == 0 {
81-
return fmt.Errorf("no context is currently set, use %q to select a new one", "kubectl config use-context <context>")
81+
if !util.HasKubectlContext(config, options.configFlags) {
82+
return fmt.Errorf("no context is currently set, use %q or %q to select a new one", "--context", "kubectl config use-context <context>")
8283
}
8384
if len(options.args) > 1 {
8485
return fmt.Errorf("too many arguments, either one or no arguments are allowed")

0 commit comments

Comments
 (0)