[kubectl-plugin] Support node selectors for kubectl ray job submit #3562

CheyuWu · 2025-05-07T18:05:58Z

Why are these changes needed?

Support setting node selectors for kubectl ray job submit

Manual Testing

Here, I demonstrate the use of --worker-node-selectors and --head-node-selectors on GKE.

The worker pod is Pending because my_script.py doesn’t require GPU execution.
The head pod is also pending because I mistakenly assigned the GPU node to the head pod.

Use both `--head-node-selectors` and `--worker-node-selectors`

$ kubectl ray job submit --name rayjob-sample --working-dir . --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4 --worker-node-selectors cloud.google.com/gke-accelerator=nvidia-l4 -- python my_script.py

Use only `--head-node-selectors`

$ kubectl ray job submit --name rayjob-sample --working-dir .  --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4 -- python my_script.py

Use only `--worker-node-selectors`

$ kubectl ray job submit --name rayjob-sample --working-dir .  --worker-node-selectors cloud.google.com/gke-accelerator=nvidia-l4 -- python my_script.py

Related issue number

Closes #3550

Checks

I've made sure the tests are passing.
Testing Strategy
- Unit tests
- Manual tests
- This PR is not tested :(

CheyuWu · 2025-05-08T18:06:35Z

Hi @kevin85421 PTAL

kevin85421

should we add any tests?

kevin85421 · 2025-05-09T21:06:45Z

also cc @andrewsykim for review.

CheyuWu · 2025-05-11T19:10:20Z

Hi @kevin85421
I have added test cases for the RayJob submit. If any additional test cases are needed, please let me know.

kevin85421 · 2025-05-11T20:28:46Z

kubectl-plugin/pkg/cmd/job/job_submit.go

@@ -96,6 +98,9 @@ var (
 		# Submit generated Ray job with default values and with runtime Env file and working directory
 		kubectl ray job submit --name rayjob-sample --working-dir /path/to/working-dir/ --runtime-env /runtimeEnv.yaml -- python my_script.py

+		# Submit Ray job on GCP with GPU node


This feature isn't just for GCP, right?

You're right — this feature isn't limited to GCP. My mistake on that part.

kevin85421 · 2025-05-11T20:35:57Z

kubectl-plugin/pkg/cmd/job/job_submit.go

@@ -162,6 +167,8 @@ func NewJobSubmitCommand(cmdFactory cmdutil.Factory, streams genericclioptions.I
 	cmd.Flags().StringVar(&options.workerGPU, "worker-gpu", "0", "number of GPUs in each worker group replica")
 	cmd.Flags().BoolVar(&options.dryRun, "dry-run", false, "print the generated YAML instead of creating the cluster. Only works when filename is not provided")
 	cmd.Flags().BoolVarP(&options.verbose, "verbose", "v", false, "Passing the '--verbose' flag to the 'ray job submit' command")
+	cmd.Flags().StringToStringVar(&options.headNodeSelectors, "head-node-selectors", nil, "Node selectors to apply to all head pods in the cluster (e.g. --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4,cloud.google.com/gke-nodepool=my-node-pool)")


Suggested change

cmd.Flags().StringToStringVar(&options.headNodeSelectors, "head-node-selectors", nil, "Node selectors to apply to all head pods in the cluster (e.g. --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4,cloud.google.com/gke-nodepool=my-node-pool)")

cmd.Flags().StringToStringVar(&options.headNodeSelectors, "head-node-selectors", nil, "Node selectors to apply to the head pod in the cluster (e.g. --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4,cloud.google.com/gke-nodepool=my-node-pool)")

kevin85421 · 2025-05-11T20:37:45Z

kubectl-plugin/pkg/cmd/job/job_submit.go

@@ -557,6 +566,22 @@ func (options *SubmitJobOptions) raySubmitCmd() ([]string, error) {
 		raySubmitCmd = append(raySubmitCmd, "--verbose")
 	}

+	if len(options.headNodeSelectors) > 0 {


Hmm, does ray job submit support these two options?

cc @MortalHappiness Would you mind shepherding this PR and making sure it's well tested? Thanks!

I think it is already well tested because this PR reuses the generation.WorkerGroup object, which already has tests for NodeSelectors.

@MortalHappiness L569 - L583 adds --head-node-selectors and --worker-node-selectors to ray job submit command if I understand correctly. Does ray job submit support these two options?

No, ray job submit does not have those two flags. But I think it's worth adding them, because it's common for data scientists to select GPU nodes. We don't need to make all the flags identical to ray job submit.

Hmm, if it doesn't exist, the ray job submit command will fail, right?

Require to add more tests.

andrewsykim · 2025-05-12T16:58:06Z

kubectl-plugin/pkg/cmd/job/job_submit.go

-	noWait             bool
-	dryRun             bool
-	verbose            bool
+	cmdFactory          cmdutil.Factory


@chiayi @MortalHappiness @davidxia we should probably figure out a way to share the same struct we use for kubectl ray create cluster so we don't have to duplicate the same arguments each time

sgtm, can either be done in this PR or follow-up

we can do it in a follow up PR.

I have created a follow up issue #3582

CheyuWu · 2025-05-13T16:11:55Z

Hi @kevin85421, I have addressed the issues regarding this #3562 (comment). Need to wait for @MortalHappiness to respond.

MortalHappiness · 2025-05-13T16:28:15Z

Hi @kevin85421, I have addressed the issues regarding this #3562 (comment). Need to wait for @MortalHappiness to respond.

What did you change? The comment you mentioned looks like a question and not a request for change.

CheyuWu · 2025-05-13T18:39:03Z

Hi @kevin85421, I have addressed the issues regarding this #3562 (comment). Need to wait for @MortalHappiness to respond.

What did you change? The comment you mentioned looks like a question and not a request for change.

Sorry, this is my fault. I type the wrong sentence

MortalHappiness · 2025-05-13T23:40:57Z

@CheyuWu After offline discussion with @kevin85421, please add an e2e test to kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go to cover this case. Thanks.

CheyuWu · 2025-05-15T18:40:29Z

Hi @kevin85421, @MortalHappiness, I have addressed the issue. PTAL

Add e2e test for node selectors
Revert the wrong rayjob submit with node selectors
Manually testing the node selectors command with

$ cd kubectl-plugin/test/e2e
$ kubectl ray job submit --name rayjob-sample --runtime-env testdata/rayjob-submit-working-dir/runtime-env-sample.yaml --head-cpu 1 --head-memory 2Gi --worker-cpu 1 --worker-memory 2Gi --head-node-selectors kubernetes.io/os=linux --worker-node-selectors kubernetes.io/os=linux -- python entrypoint-python-sample.py

kevin85421 · 2025-05-15T20:20:21Z

cc @MortalHappiness

MortalHappiness · 2025-05-16T13:27:03Z

kubectl-plugin/pkg/cmd/job/job_submit.go

@@ -105,6 +107,9 @@ var (
 		# Submit generated Ray job with default values and with runtime Env file and working directory
 		kubectl ray job submit --name rayjob-sample --working-dir /path/to/working-dir/ --runtime-env /runtimeEnv.yaml -- python my_script.py

+		# Submit a Ray job with specific head-node-selectors and worker-node-selectors using kubectl ray job submit
+		kubectl ray job submit --name rayjob-sample --working-dir /path/to/working-dir/ --head-node-selectors kubernetes.io/os=linux --worker-node-selectors kubernetes.io/os=linux -- python my_script.py


I think for this example, we can still show the user how to select GPU nodes instead of selecting Linux nodes.

kevin85421 · 2025-05-17T18:08:01Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

 		output, err = cmd.CombinedOutput()
 		Expect(err).ToNot(HaveOccurred())

 		Expect(string(output)).To(Equal("Complete"))
 	})
+
+	It("succeed in submitting RayJob with headNodeSelectors and workerNodeSelectors", func() {
+		killKubectlCmd := exec.Command("pkill", "-9", "kubectl")


What’s this for? This seems like a risky anti-pattern to me, as it might kill processes not created by these tests. Do we really need this?

cc @MortalHappiness

I think this is necessary, because if we remove the code and run the test, the error will occur

Waiting for portforwarding...Port Forwarding service rayjob-sample-sm8j9-head-svc Unable to listen on port 8265: Listeners failed to create with the following errors: [unable to create listener: Error listen tcp4 127.0.0.1:8265: bind: address already in use unable to create listener: Error listen tcp6 [::1]:8265: bind: address already in use] error: unable to listen on any of the requested ports: [{8265 8265}]

Perhaps, we just only need to kill the port-forwarding

Related to #3124. We want to make sure no port-forwarding processes alive.

kevin85421 · 2025-05-17T18:12:09Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

+			"python",
+			entrypointSampleFileName,
+		)
+		output, err := cmd.CombinedOutput()


is this a blocking operation?

kevin85421 · 2025-05-17T18:14:37Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

+		// Retrieve the Job ID from the output
+		regexExp := regexp.MustCompile(`'([^']*raysubmit[^']*)'`)
+		matches := regexExp.FindStringSubmatch(string(output))
+		Expect(len(matches)).To(BeNumerically(">=", 2))


Why >= 2? Can you add a utility function for it?

This will retrieve the Job ID from the output and check whether it exists.

Perhaps, I can use >=1 instead

OK, no problem

I don't think it's necessary to use >=2, I will change it to >=1

kevin85421 · 2025-05-17T18:18:41Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

+		matches := regexExp.FindStringSubmatch(string(output))
+		Expect(len(matches)).To(BeNumerically(">=", 2))
+		cmdOutputJobID := matches[1]
+		// Use kubectl to check status of the rayjob


Can you only execute the kubectl command once and get the whole CR instead of executing it multiple times?

I also open a follow up issue to avoid using kubectl: [kubectl-plugin] Use a more Golang-native approach to retrieve the CR status for testing #3626

Signed-off-by: Cheyu Wu <[email protected]>

This reverts commit 9bd1d7c.

CheyuWu · 2025-05-18T18:47:10Z

Hi @kevin85421, I have addressed this comment. PTAL

change killKubectlCmd := exec.Command("pkill", "-9", "kubectl") to KillPortForwardOn8265()
Handle the blocking operation
Use >= 1 instead when getting the rayjob ID, and add a utility function for it. (comment)[https://github.com/[kubectl-plugin] Support node selectors for kubectl ray job submit #3562#discussion_r2094578737]
Execute the kubectl command once and get the whole CR instead of executing it multiple times

kevin85421 · 2025-05-19T05:09:12Z

@CheyuWu can you fix the CI error? It seems to be related to your PR.

kevin85421 · 2025-05-19T05:12:46Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

-		killKubectlCmd := exec.Command("pkill", "-9", "kubectl")
-		_ = killKubectlCmd.Run()
-		cmd := exec.Command("kubectl", "ray", "job", "submit", "--namespace", namespace, "-f", rayJobFilePath, "--working-dir", kubectlRayJobWorkingDir, "--", "python", entrypointSampleFileName)
+		// To avoid port conflict, kill any existing port-forward process on 8265


Can you help me understand why a port conflict is possible? Could it be a side effect of other tests, or are other users running other workloads? If it's the latter, maybe we shouldn't kill the process.

Can you help me understand why a port conflict is possible? Could it be a side effect of other tests, or are other users running other workloads? If it's the latter, maybe we shouldn't kill the process.

The reason for the port conflict is that the test case — succeed in forwarding RayCluster and should be able to cancel in kubectl-plugin/test/e2e/kubectl_ray_session_test.go — does not fully clean up its child processes with os.Interrupt. It only terminates the main process.

As a result, when I run the command lsof -i :8265 after the test case finishes, there are still child processes remain:

COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME kubectl 322356 user 8u IPv4 2180543 0t0 TCP localhost:8265 (LISTEN) kubectl 322356 user 9u IPv6 2180544 0t0 TCP ip6-localhost:8265 (LISTEN)

OK, in that case, we need to fix the tests which leak the processes. Could you openan issue to track the progress?

I have already fixed this in this PR. Do I need to revert it?

Is it required to make this PR's CI passes? If yes, we don't need to split them into different PRs.

If we decide not to address the port-forwarding issue in this PR, using killKubectlCmd := exec.Command("pkill", "-9", "kubectl") will be necessary to ensure the ci passes.

If we want to remove killKubectlCmd := exec.Command("pkill", "-9", "kubectl"), we cannot split them into different PRs

OK, I am fine to separate them into different PRs or in a single PR. It depends on you.

I think we can handle them in this PR

kevin85421 · 2025-05-19T05:13:40Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

 	})
 })
+
+// extractRayJobID extracts the Ray Job ID from the kubectl ray job submit output.


Suggested change

// extractRayJobID extracts the Ray Job ID from the kubectl ray job submit output.

// `extractRayJobID` extracts the Ray Job ID from the output of `kubectl ray job submit`.

kevin85421 · 2025-05-19T05:22:13Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

+	return ""
+}
+
+func assertRayJobSucceeded(ctx context.Context, namespace, jobName, cmdOutputJobID string) (rayjob rayv1.RayJob) {


we should not limit the function to specific status only.

Suggested change

func assertRayJobSucceeded(ctx context.Context, namespace, jobName, cmdOutputJobID string) (rayjob rayv1.RayJob) {

func getAndCheckRayJob(ctx context.Context, namespace, name, expectedJobID, expectedJobStatus, expectedJobDeploymentStatus string) (rayjob rayv1.RayJob) {

kevin85421 · 2025-05-19T05:22:36Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

-		cmd := exec.Command("kubectl", "ray", "job", "submit", "--namespace", namespace, "-f", rayJobFilePath, "--working-dir", kubectlRayJobWorkingDir, "--", "python", entrypointSampleFileName)
+		// To avoid port conflict, kill any existing port-forward process on 8265
+		KillPortForwardOn8265()
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)


what's the motivation to add this?

I want to ensure the operations are not stuck in some unexpected state.

I don’t think it’s necessary to add a timeout in these tests. The execution time of kubectl ray job submit can vary significantly across different environments. For example, if some developers don’t have high-speed internet, it might take more than 3 minutes to execute the command.

In addition, our CI has out-of-box timeout go test -timeout.

+1. Don't add timeout.

… change the comment, and remove KillPortForwardOn8265 which is not necessary anymore

CheyuWu · 2025-05-19T17:24:46Z

@CheyuWu can you fix the CI error? It seems to be related to your PR.

Sorry about this. This won't happened again

CheyuWu · 2025-05-19T17:32:37Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go

 		}
+		// Check if the port-forwarding process is still running
+		_, err = exec.CommandContext(ctx, "lsof", "-i", ":8265").CombinedOutput()
+		Expect(err).To(HaveOccurred())


If an error occurs, it means the port-forwarding does not exist.

Should these lines be put in the defer above?

Yes, you're right

kevin85421

@MortalHappiness would you mind reviewing this PR again especially the ray session part? Thanks!

kevin85421 · 2025-05-19T16:53:48Z

kubectl-plugin/test/e2e/support.go

+	err = json.Unmarshal(output, &rayJob)
+	Expect(err).ToNot(HaveOccurred())
+
+	// Retrieve Job ID


These comments don't provide any additional information.

Suggested change

// Retrieve Job ID

kevin85421 · 2025-05-19T16:53:58Z

kubectl-plugin/test/e2e/support.go

+
+	// Retrieve Job ID
+	Expect(rayJob.Status.JobId).To(Equal(expectedJobID))
+	// Retrieve Job Status


Suggested change

// Retrieve Job Status

kevin85421 · 2025-05-19T16:54:06Z

kubectl-plugin/test/e2e/support.go

+	Expect(rayJob.Status.JobId).To(Equal(expectedJobID))
+	// Retrieve Job Status
+	Expect(string(rayJob.Status.JobStatus)).To(Equal(expectedJobStatus))
+	// Retrieve Job Deployment Status


Suggested change

// Retrieve Job Deployment Status

kevin85421 · 2025-05-19T17:01:28Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

-		cmd := exec.Command("kubectl", "ray", "job", "submit", "--namespace", namespace, "-f", rayJobFilePath, "--working-dir", kubectlRayJobWorkingDir, "--", "python", entrypointSampleFileName)
+		// To avoid port conflict, kill any existing port-forward process on 8265
+		KillPortForwardOn8265()
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)


I don’t think it’s necessary to add a timeout in these tests. The execution time of kubectl ray job submit can vary significantly across different environments. For example, if some developers don’t have high-speed internet, it might take more than 3 minutes to execute the command.

In addition, our CI has out-of-box timeout go test -timeout.

kevin85421 · 2025-05-19T17:05:44Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

-		killKubectlCmd := exec.Command("pkill", "-9", "kubectl")
-		_ = killKubectlCmd.Run()
-		cmd := exec.Command("kubectl", "ray", "job", "submit", "--namespace", namespace, "-f", rayJobFilePath, "--working-dir", kubectlRayJobWorkingDir, "--", "python", entrypointSampleFileName)
+		// To avoid port conflict, kill any existing port-forward process on 8265


OK, in that case, we need to fix the tests which leak the processes. Could you openan issue to track the progress?

kevin85421 · 2025-05-19T17:20:30Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go


 		// Use kubectl to check status of the rayjob
-		// Retrieve Job ID


Nice! The code becomes much cleaner!

CheyuWu · 2025-05-19T17:33:29Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go

 			}
+		case <-time.After(3 * time.Second):
+			// force kill the process if it doesn't exit within the timeout
+			_ = syscall.Kill(-pgid, syscall.SIGKILL)


-pgid means all process group

Is it possible to delete this line and rely on the defer above?

Yes, you are right. This is not necessary.

CheyuWu · 2025-05-19T17:34:00Z

Hi @kevin85421, PTAL

I have addressed the side effect in kubectl-plugin/test/e2e/kubectl_ray_session_test.go`, which could cause a port-forwarding conflict. comment

Use process groups to ensure that both the main process and child processes receive the interrupt signal.

Add lsof -i :8265 to verify whether port-forwarding is active or not.

Replaced some exec.Command calls with exec.CommandContext to prevent commands from getting stuck in unexpected states.
Move func getAndCheckRayJob to support.go

MortalHappiness · 2025-05-20T02:48:52Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go

@@ -39,58 +43,73 @@ var _ = Describe("Calling ray plugin `session` command", Ordered, func() {

 		// Send a request to localhost:8265, it should succeed
 		Eventually(func() error {
-			_, err := exec.Command("curl", "http://localhost:8265").CombinedOutput()
+			_, err := exec.CommandContext(ctx, "curl", "http://localhost:8265").CombinedOutput()
 			return err
 		}, 3*time.Second, 500*time.Millisecond).ShouldNot(HaveOccurred())

 		// Send a signal to cancel the command


Suggested change

// Send a signal to cancel the command

MortalHappiness · 2025-05-20T02:52:15Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go

 		}
+		// Check if the port-forwarding process is still running
+		_, err = exec.CommandContext(ctx, "lsof", "-i", ":8265").CombinedOutput()
+		Expect(err).To(HaveOccurred())


Should these lines be put in the defer above?

MortalHappiness · 2025-05-20T02:52:43Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go

 			}
+		case <-time.After(3 * time.Second):
+			// force kill the process if it doesn't exit within the timeout
+			_ = syscall.Kill(-pgid, syscall.SIGKILL)


Is it possible to delete this line and rely on the defer above?

MortalHappiness · 2025-05-20T03:00:51Z

kubectl-plugin/test/e2e/kubectl_ray_session_test.go


 		err := sessionCmd.Start()
 		Expect(err).NotTo(HaveOccurred())

 		// Send a request to localhost:8265, it should succeed
 		Eventually(func() error {
-			_, err := exec.Command("curl", "http://localhost:8265").CombinedOutput()
+			_, err := exec.CommandContext(ctx, "curl", "http://localhost:8265").CombinedOutput()


This is incorrect. If you pass ctx to multiple commands, they'll share the same context and have a total 1 minute timeout. So if you pass ctx to 7 commands and each command takes 9 seconds, then the last command will timeout.

Ok, I will fix this.

MortalHappiness · 2025-05-20T03:14:11Z

kubectl-plugin/test/e2e/support.go

+func getAndCheckRayJob(
+	ctx context.Context,
+	namespace,
+	name,
+	expectedJobID,
+	expectedJobStatus,
+	expectedJobDeploymentStatus string,
+) (rayjob rayv1.RayJob) {


Since this is a helper function, call GinkgoHelper() inside this function.

MortalHappiness · 2025-05-20T03:15:21Z

kubectl-plugin/test/e2e/kubectl_ray_job_submit_test.go

-		cmd := exec.Command("kubectl", "ray", "job", "submit", "--namespace", namespace, "-f", rayJobFilePath, "--working-dir", kubectlRayJobWorkingDir, "--", "python", entrypointSampleFileName)
+		// To avoid port conflict, kill any existing port-forward process on 8265
+		KillPortForwardOn8265()
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)


+1. Don't add timeout.

…_test, and refactor cleanup func

CheyuWu force-pushed the feat/rayjob/nodeSelectors branch 2 times, most recently from a95d6ca to ac87a4d Compare May 7, 2025 18:26

CheyuWu changed the title ~~[kubectl-plugin] Support node selectors for kubectl ray job submit~~ [need to test][kubectl-plugin] Support node selectors for kubectl ray job submit May 7, 2025

CheyuWu changed the title ~~[need to test][kubectl-plugin] Support node selectors for kubectl ray job submit~~ [kubectl-plugin] Support node selectors for kubectl ray job submit May 8, 2025

kevin85421 assigned MortalHappiness May 8, 2025

MortalHappiness previously approved these changes May 8, 2025

View reviewed changes

MortalHappiness assigned kevin85421 May 8, 2025

kevin85421 mentioned this pull request May 9, 2025

[kubectl-plugin] Support node selectors for kubectl ray job submit #3265

Closed

4 tasks

kevin85421 reviewed May 9, 2025

View reviewed changes

kevin85421 requested changes May 11, 2025

View reviewed changes

MortalHappiness approved these changes May 12, 2025

View reviewed changes

andrewsykim reviewed May 12, 2025

View reviewed changes

CheyuWu mentioned this pull request May 12, 2025

[Refactor][kubectl-plugin] Share common struct for cluster-related CLI options to reduce duplication #3582

Open

2 tasks

CheyuWu force-pushed the feat/rayjob/nodeSelectors branch from 1619710 to d4a7688 Compare May 13, 2025 14:50

CheyuWu force-pushed the feat/rayjob/nodeSelectors branch 2 times, most recently from 571590a to ce12965 Compare May 15, 2025 16:49

MortalHappiness reviewed May 16, 2025

View reviewed changes

MortalHappiness approved these changes May 17, 2025

View reviewed changes

kevin85421 reviewed May 17, 2025

View reviewed changes

feat: ray job support node selectors

b05fa66

Signed-off-by: Cheyu Wu <[email protected]>

CheyuWu added 8 commits May 18, 2025 19:37

feat: add testcase for ray job summit

b035889

fix: cmd line message

91cbcc8

Revert "feat: add testcase for ray job summit"

9e35607

This reverts commit 9bd1d7c.

feat: add rayjob submit e2e test for node selectors

0589d0d

fix: linter issue

92d01fa

chore: update help comment

5af9c1a

chore: update comment

d60ad3f

refactor: rayhob submit test

cb923e4

CheyuWu force-pushed the feat/rayjob/nodeSelectors branch from 2f989d2 to cb923e4 Compare May 18, 2025 18:41

kevin85421 reviewed May 19, 2025

View reviewed changes

fix: kubectl_ray_session_test does not fully clean the child process,…

7a1d958

… change the comment, and remove KillPortForwardOn8265 which is not necessary anymore

CheyuWu commented May 19, 2025

View reviewed changes

kevin85421 reviewed May 19, 2025

View reviewed changes

CheyuWu commented May 19, 2025

View reviewed changes

MortalHappiness reviewed May 20, 2025

View reviewed changes

fix: rm unnecessary context and timeout, add process group to session…

a3903ce

…_test, and refactor cleanup func

	cmd.Flags().StringToStringVar(&options.headNodeSelectors, "head-node-selectors", nil, "Node selectors to apply to all head pods in the cluster (e.g. --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4,cloud.google.com/gke-nodepool=my-node-pool)")
	cmd.Flags().StringToStringVar(&options.headNodeSelectors, "head-node-selectors", nil, "Node selectors to apply to the head pod in the cluster (e.g. --head-node-selectors cloud.google.com/gke-accelerator=nvidia-l4,cloud.google.com/gke-nodepool=my-node-pool)")

	// extractRayJobID extracts the Ray Job ID from the kubectl ray job submit output.
	// `extractRayJobID` extracts the Ray Job ID from the output of `kubectl ray job submit`.

	func assertRayJobSucceeded(ctx context.Context, namespace, jobName, cmdOutputJobID string) (rayjob rayv1.RayJob) {
	func getAndCheckRayJob(ctx context.Context, namespace, name, expectedJobID, expectedJobStatus, expectedJobDeploymentStatus string) (rayjob rayv1.RayJob) {


		// Use kubectl to check status of the rayjob
		// Retrieve Job ID

[kubectl-plugin] Support node selectors for kubectl ray job submit #3562

Are you sure you want to change the base?

[kubectl-plugin] Support node selectors for kubectl ray job submit #3562

Conversation

CheyuWu commented May 7, 2025 • edited Loading

Why are these changes needed?

Manual Testing

Use both --head-node-selectors and --worker-node-selectors

Use only --head-node-selectors

Use only --worker-node-selectors

Related issue number

Checks

CheyuWu commented May 8, 2025

kevin85421 left a comment

Choose a reason for hiding this comment

kevin85421 commented May 9, 2025

CheyuWu commented May 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu commented May 13, 2025

MortalHappiness commented May 13, 2025

CheyuWu commented May 13, 2025 • edited Loading

MortalHappiness commented May 13, 2025

CheyuWu commented May 15, 2025 • edited Loading

kevin85421 commented May 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu May 18, 2025 • edited Loading

Choose a reason for hiding this comment

CheyuWu May 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu May 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu May 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu commented May 18, 2025 • edited Loading

kevin85421 commented May 19, 2025

Choose a reason for hiding this comment

CheyuWu May 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu May 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu commented May 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevin85421 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CheyuWu commented May 7, 2025 •

edited

Loading

Use both `--head-node-selectors` and `--worker-node-selectors`

Use only `--head-node-selectors`

Use only `--worker-node-selectors`

CheyuWu commented May 13, 2025 •

edited

Loading

CheyuWu commented May 15, 2025 •

edited

Loading

CheyuWu May 18, 2025 •

edited

Loading

CheyuWu May 18, 2025 •

edited

Loading

CheyuWu May 18, 2025 •

edited

Loading

CheyuWu May 18, 2025 •

edited

Loading

CheyuWu commented May 18, 2025 •

edited

Loading

CheyuWu May 19, 2025 •

edited

Loading

CheyuWu May 19, 2025 •

edited

Loading

CheyuWu commented May 19, 2025 •

edited

Loading

MortalHappiness May 20, 2025 •

edited

Loading