[Autoscaler] Match autoscaler image to Ray head image for Ray >= 2.0.0 #423
Conversation
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
|
so |
|
/cc @akanso to take a look, ali use autoscaler a lot |
The "official image is out" after the last commit is made to the release branch and we announce the release. Maybe not the best way of doing things but that's the way it is at the moment. |
| @@ -92,6 +92,9 @@ const ( | |||
| // Ray health check related configurations | |||
| RayAgentRayletHealthPath = "api/local_raylet_healthz" | |||
| RayDashboardGCSHealthPath = "api/gcs_healthz" | |||
|
|
|||
| // Default autoscaler image when running Ray at versions older than 2.0.0 | |||
| FallbackDefaultAutoscalerImage = "rayproject/ray:2.0.0" | |||
There was a problem hiding this comment.
This image exists but is unofficial at the moment.
Its contents may change slightly over the course of the Ray 2.0.0 release process.
| // Example inputs that return false: "1.13.0", "1.12", "1". | ||
| func autoscalerSupportIsStable(rayVersion string) bool { | ||
| // Try to determine major version by extracting everything that comes before the first "." | ||
| firstDotIndex := strings.Index(rayVersion, ".") |
There was a problem hiding this comment.
minor: In our downstream code, we have similar version specific configuration. for those cases, we use hashicorp/go-version to directly compare versions if the version follow semantic versioning.
import "github.com/hashicorp/go-version"
// LessThanVersion tests if version1 is less than version2.
func LessThanVersion(version1 string, version2 string) (bool, error) {
v1, err := version.NewVersion(version1)
if err != nil {
return false, err
}
v2, err := version.NewVersion(version2)
if err != nil {
return false, err
}
return v1.LessThan(v2), nil
}There was a problem hiding this comment.
Thanks for the tip! I'll keep it in mind to use that library for version comparisons in the future.
For now, I prefer to implement the logic directly, as I'd like to be super explicit with what we're doing with ill-formatted version strings.
|
#424 is merged to help improve test stability. You can rebase the change to see if nightly version pass |
|
Let's merge this one and it's time to cut rc.0 release. If other reviewers have further feedback, feel free to leave it here. |
ray-project#423) * Implement the logic. Signed-off-by: Dmitri Gekhtman <[email protected]> * Fix function call. Signed-off-by: Dmitri Gekhtman <[email protected]> * Test. Signed-off-by: Dmitri Gekhtman <[email protected]> * Update example. Signed-off-by: Dmitri Gekhtman <[email protected]> * lowercase Signed-off-by: Dmitri Gekhtman <[email protected]> * lint Signed-off-by: Dmitri Gekhtman <[email protected]> * wording Signed-off-by: Dmitri Gekhtman <[email protected]>
Why are these changes needed?
This PR updates the logic that selects a default autoscaler image.
For Ray versions at least 2.0.0, use the same image for the autoscaler as for the Ray container.
This eliminates the possibility of autoscaler/Ray incompatibility and reduces docker pull time.
For earlier Ray versions, use
rayproject/ray:2.0.0to guarantee up-to-date autoscaler functionality.As of the Ray 2.0.0 branch cut earlier today, an image tagged
rayproject/ray:2.0.0exists on Dockerhub.Until the official Ray release in two weeks, this image is unofficial and its actual contents are a moving target -- but I think we can live with the inconsistency in the short term. (I'm open to other opinions on this choice.)
Related issue number
Closes #360
Checks