Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] REP 54: Refactor usages of the inconsistentRayClusterStatus function #2235

Closed
2 tasks done
rueian opened this issue Jul 11, 2024 · 2 comments · Fixed by #2249
Closed
2 tasks done

[Refactor] REP 54: Refactor usages of the inconsistentRayClusterStatus function #2235

rueian opened this issue Jul 11, 2024 · 2 comments · Fixed by #2249
Assignees
Labels
crd observability enhancement New feature or request

Comments

@rueian
Copy link
Contributor

rueian commented Jul 11, 2024

Search before asking

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

Description

This is one of the tracking items of the REP: RayCluster status improvement, the step 3 in the design doc: https://docs.google.com/document/d/1bRL0cZa87eCX6SI7gqthN68CgmHaB6l3-vJuIse-BrY

As @kevin85421 stated in the design doc, the current status update process in the RayCluster controller is quite random. For example, if some errors are thrown, the function rayClusterReconcile returns immediately and updates some parts of the RayCluster CR status randomly. Sometimes it doesn’t update anything; sometimes it updates Status.State and/or Status.Reason.

We should make sure every reconciliation that has a status change goes through the inconsistentRayClusterStatus function, and we only call r.Status().Update(...) when it returns true.

By doing so, we can then move forward with confidence that future works in the REP: RayCluster status improvement will be on track.

Use case

No response

Related issues

ray-project/enhancements#54

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!
@rueian rueian added enhancement New feature or request triage labels Jul 11, 2024
@rueian
Copy link
Contributor Author

rueian commented Jul 11, 2024

Hi @MortalHappiness, would you like to take this one?

@MortalHappiness
Copy link
Member

MortalHappiness commented Jul 11, 2024

Hi @rueian. Yes. I am glad to take this issue. Can you assign it to me? Thanks.

MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 16, 2024
MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 16, 2024
MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 16, 2024
MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 17, 2024
MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 17, 2024
MortalHappiness added a commit to MortalHappiness/kuberay that referenced this issue Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crd observability enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants