[RayJob][Status][15/n] Unify the codepath for the status transition to Suspended
#1805
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevin85421
changed the title
Suspended
kevin85421
commented
Jan 5, 2024
| @@ -588,11 +570,6 @@ func (r *RayJobReconciler) getOrCreateRayClusterInstance(ctx context.Context, ra | |||
| return nil, err | |||
| } | |||
|
|
|||
| // special case: don't create a cluster instance and don't return an error if the suspend flag of the job is true | |||
There was a problem hiding this comment.
See #1801 (comment) for more details.
gvspraveen
reviewed
Jan 5, 2024
| err = r.updateState(ctx, rayJobInstance, nil, rayJobInstance.Status.JobStatus, rayv1.JobDeploymentStatusSuspending) | ||
| return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err | ||
| } | ||
| if shouldUpdate, err := r.updateStatusToSuspendingIfNeeded(ctx, rayJobInstance); shouldUpdate { |
There was a problem hiding this comment.
this looks so much more cleaner now. thanks for making this change
gvspraveen
approved these changes
Jan 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Currently, there are two paths to transition to the
Suspendedstatus.Initializing->SuspendedRunning->Suspending->SuspendedThe case "Initializing -> Suspended" is pretty tricky:
suspendto true before creating the RayCluster.Suspendeddirectly.suspendto true after the RayCluster is created and before the K8s Job creation.Runningand then toSuspending. Additionally, the user must wait until both the RayCluster and the Kubernetes Job are ready to transition toRunning, which may take a considerable amount of time (depending on the time of pulling images).This PR unifies the codepath for the status transition to
Suspended.Initializing->Suspending->SuspendedRunning->Suspending->SuspendedNote
WithStatusSubresourceto the fake client, the functionr.Status().Update(...)will complain that the RayJob cannot be found.Related issue number
Checks