-
Notifications
You must be signed in to change notification settings - Fork 423
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
[Test][Autoscaler][1/n] Add Ray Autoscaler e2e tests #2168
Conversation
cc @rueian @MortalHappiness would you mind reviewing this PR? Thanks! |
cc @ryanaoleary would you mind reviewing this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Rueian <rueiancsie@gmail.com> Signed-off-by: Kai-Hsun Chen <kaihsun@apache.org>
. "github.com/ray-project/kuberay/ray-operator/test/support" | ||
) | ||
|
||
func TestRayClusterAutoscaler(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A high level question: should we have all the testing logic inside python? like you can have a python script:
ray.init()
actor1 = Actor.remote()
assert ray.nodes() == 2
actor2 = Actor.remote()
assert ray.nodes() == 3
instead of doing the assertion in go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ray.init()
actor1 = Actor.remote()
assert ray.nodes() == 2
actor2 = Actor.remote()
assert ray.nodes() == 3
This is something that the Ray Autoscaler needs to test. For the KubeRay node provider, we care more about whether the Pod is actually created. For Pods, Golang is much better than Python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it makes sense.
…ay-project#2168) Signed-off-by: Rueian <rueiancsie@gmail.com>
Why are these changes needed?
See #2173 for more details.
Related issue number
Checks