[Feature] Add DeleteWorkersOnFailure deletion policy for RayJob

[kevin85421]

Search before asking

• I had searched in the issues and found no similar feature requirement.

Description

DeleteWorkersOnFailure: Deletes workers only when the Ray job fails and deletes the entire RayCluster when the Ray job succeeds. This seems to be a more common pattern for users.

Should we add this policy or rename DeleteWorkers to DeleteWorkersOnFailure? Does it need to be in v1.3.0?

Use case

No response

Related issues

No response

Are you willing to submit a PR?

• Yes I am willing to submit a PR!

[kevin85421]

cc @andrewsykim any thoughts?

[andrewsykim]

Having both policies probably makes sense. I'm in favor of a new policy like DeleteWorkersOnFailure. Would it be too verbose to name it something like DeleteClusterOnSuccessOrWorkersOnFailure?

[andrewsykim]

DeleteWorkersOnFailure is probably fine as long as the deletion policy on success is well documented in the API or documentation

[kevin85421]

On second thought, I realized that users may have more combinations. For example,

1. DeleteCluster on success, DeleteNone on failure
2. DeleteSelf on success, DeleteNone on failure
3. DeleteCluster on success, DeleteWorkers on failure
4. DeleteSelf on success, DeleteWorkers on failure

There are two solutions:

1. Keep the current API, but adds new API like DeleteClusterOnSuccessOrWorkersOnFailure if needed
2. Separate deletion API into OnFailureDeletionPolicy and OnSuccessDeletionPolicy.

[kevin85421]

mark this issue as v1.3.0 because we need to make a decision about the API before the release.

[andrewsykim]

On second thought, I realized that users may have more combinations. For example,

DeleteCluster on success, DeleteNone on failure
DeleteSelf on success, DeleteNone on failure
DeleteCluster on success, DeleteWorkers on failure
DeleteSelf on success, DeleteWorkers on failure
There are two solutions:

Keep the current API, but adds new API like DeleteClusterOnSuccessOrWorkersOnFailure if needed
Separate deletion API into OnFailureDeletionPolicy and OnSuccessDeletionPolicy.

These are really good considerations, since we put the feature behind an alpha feature gate I feel fine about breaking the API in v1.4 if needed.

[andrewsykim]

We can consider an API like this as well:

spec:
  deletionPolicy:
    onSuccess: DeleteCluster
    onFailure: DeleteWorkers

[kevin85421]

OK, let's update the API in v1.4.0.