-
Notifications
You must be signed in to change notification settings - Fork 425
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
[kubectl-plugin] Add e2e test for kubectl ray job submit #2614
Conversation
192dd4d
to
732be43
Compare
ray-operator/config/samples/kubectl-ray-submit-job/runtime-env-sample.yaml
Outdated
Show resolved
Hide resolved
732be43
to
b57100a
Compare
kubectl-plugin/test/e2e/testdata/rayjob-submit-working-dir/runtime-env-sample.yaml
Outdated
Show resolved
Hide resolved
@ray.remote | ||
class Counter: | ||
def __init__(self): | ||
# Used to verify runtimeEnv | ||
self.name = os.getenv("counter_name") | ||
assert self.name == "test_counter" | ||
self.counter = 0 | ||
|
||
def inc(self): | ||
self.counter += 1 | ||
|
||
def get_counter(self): | ||
return "{} got {}".format(self.name, self.counter) | ||
|
||
counter = Counter.remote() | ||
|
||
for _ in range(5): | ||
ray.get(counter.inc.remote()) | ||
print(ray.get(counter.get_counter.remote())) |
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.
I think use task would be simpler than actor. We don't need counter. A simple task that checks environment variables and try to import package in it is enough. Something like
import ray
ray.init()
@ray.remote
def f():
# import package
# check package version
# check env vars
ray.get(f.remote())
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.
PTAL, I used 2 packages that aren't dependencies and 2 env vars.
kubectl-plugin/test/e2e/testdata/ray-job.interactive-mode-no-runtime-env.yaml
Outdated
Show resolved
Hide resolved
2405cbc
to
7b11a7c
Compare
7b11a7c
to
a8ae574
Compare
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
spec: | ||
# The current value is "InteractiveMode", meaning that it will wait for user to submit job and provide the job submission ID | ||
submissionMode: 'InteractiveMode' | ||
runtimeEnvYAML: | |
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.
@kevin85421 @MortalHappiness what's the expected behavior with runtime env when using InteractiveMode
. It should be ignored right?
Why are these changes needed?
This PR adds test for the
ray job submit
command.Related issue number
Part of #2608
Checks