[RayJob] Validate RayJob spec

[kevin85421]

Why are these changes needed?

#1783 (review)

Kueue does not admit RayJob with shutDownAfterFinishes = false, so at KubeRay level, we may want to consider this an invalid configuration for suspended RayJobs.

Related issue number

Checks

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

[kevin85421]

cc @astefanutti @andrewsykim

[architkulkarni]

The code looks good to me, but why do we want this to be invalid?

[kevin85421]

The code looks good to me, but why do we want this to be invalid?

#1813 (comment)?

[architkulkarni]

I read that but I still don't understand, I think I'm missing some context. Why doesn't Kueue accept jobs with shutDownAfterFinishes = false, and secondly, why should this be enforced by KubeRay and not just by Kueue?

[kevin85421]

Got it.

Why doesn't Kueue accept jobs with shutDownAfterFinishes = false

In my understanding, if users set shutDownAfterFinishes to false, it means they do not want to delete the cluster for some reason. Hence, we should not delete the RayCluster when suspend is set to true.

secondly, why should this be enforced by KubeRay and not just by Kueue?

This is a good question. Kueue can prevent the suspension of a RayJob with shutDownAfterFinishes: false. However, users might still be able to suspend the RayJob manually on their own. It might be better to align with Kueue's behavior. Therefore, adding the validation in KubeRay makes sense to me.

cc @andrewsykim @astefanutti for more context about Kueue

[andrewsykim]

Why doesn't Kueue accept jobs with shutDownAfterFinishes = false

This is mainly because we release the quota for the job when the job finishes, but leaving the cluster would mean those resources are still used.

secondly, why should this be enforced by KubeRay and not just by Kueue?

Kueue does have a validating webhook for RayJob and does enforce this: https://github.com/kubernetes-sigs/kueue/blob/main/pkg/controller/jobs/rayjob/rayjob_webhook.go#L89-L92

[kevin85421]

@architkulkarni Does the explanation make sense to you? Thanks!

[kevin85421]

test-rayservice-sample-yamls-nightly-operator is flaky due to #1808 and has no relationship with this PR.