[RayService] auth token mode e2e test #4225
Open
+68
−0
Conversation
This file contains hidden or 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
Contributor
|
e2e test ci failed, plz fix it. |
Author
|
Please don't review for now. It's still wip. Will be investigate it further. |
ryankert01
force-pushed
the
e2e/ray-service-runtime-env-update
branch
from
5c05505 to
b4cd1e2
Compare
Signed-off-by: Ryan <ryan980053@gmail.com>
Signed-off-by: Ryan <ryan980053@gmail.com>
ryankert01
force-pushed
the
e2e/ray-service-runtime-env-update
branch
2 times, most recently
from
95d58a8 to
3145953
Compare
Signed-off-by: Ryan <ryan980053@gmail.com>
ryankert01
changed the title
Signed-off-by: Ryan Huang <ryankert01@gmail.com>
Removed worker group spec and related verification for auth token propagation in RayService tests. Signed-off-by: Ryan Huang <ryankert01@gmail.com>
Author
|
@400Ping PTAL |
Signed-off-by: Ryan Huang <ryankert01@gmail.com>
Refactor test for RayService authentication to improve clarity and maintainability. Signed-off-by: Ryan Huang <ryankert01@gmail.com>
rueian
approved these changes
Dec 1, 2025
Future-Outlier
changed the title
Future-Outlier
requested changes
Dec 6, 2025
Member
Future-Outlier
left a comment
Future-Outlier
left a comment
There was a problem hiding this comment.
Can we do something like
g.Expect(rayService.Status.NumServeEndpoints).To(BeNumerically(">", 0),
"RayService should have at least one serve endpoint")to test RayService auth token mode, or this PR is just doing the same thing as the raycluster one.
cc @machichima @JiangJiaWei1103 to review together, thank you
Member
Future-Outlier
left a comment
Future-Outlier
left a comment
There was a problem hiding this comment.
cc @AndySung320 @justinyeh1995 @400Ping for review if you are interested
Co-authored-by: Han-Ju Chen (Future-Outlier) <eric901201@gmail.com> Signed-off-by: Ryan Huang <ryankert01@gmail.com>
400Ping
approved these changes
Dec 7, 2025
Contributor
|
@ryankert01 overall LG, and please fix the CI error. |
win5923
reviewed
Dec 7, 2025
Co-authored-by: Jun-Hao Wan <ken89@kimo.com> Signed-off-by: Ryan Huang <ryankert01@gmail.com>
Signed-off-by: ryankert01 <ryan980053@gmail.com>
Signed-off-by: ryankert01 <ryan980053@gmail.com>
Author
win5923
approved these changes
Dec 7, 2025
Signed-off-by: Future-Outlier <eric901201@gmail.com>
400Ping
approved these changes
Dec 8, 2025
AndySung320
approved these changes
Dec 8, 2025
Signed-off-by: Future-Outlier <eric901201@gmail.com>
| g.Expect(headPod).NotTo(BeNil()) | ||
| LogWithTimestamp(test.T(), "Found head pod %s/%s", headPod.Namespace, headPod.Name) | ||
|
|
||
| // Verify Ray container has auth token env vars |
Contributor
There was a problem hiding this comment.
Suggested change
| // Verify Ray container has auth token env vars | |
| // Verify Ray container has auth token env vars | |
| g.Expect(len(headPod.Spec.Containers)).To(BeNumerically(">", utils.RayContainerIndex), | |
| "Head pod should have at least %d containers", utils.RayContainerIndex+1) |
How about adding a container bounds checking?
Member
There was a problem hiding this comment.
I think current implementation is good enough, thank you for the review!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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?
AuthModeTokenenabledRAY_AUTH_MODEandRAY_AUTH_TOKENenvironment variables are correctly set in head podSecretKeyRefwith the correct keyKey implementation details:
RayServiceSampleYamlApplyConfiguration()and adds auth options and a worker group programmatically (no separate YAML file needed)VerifyContainerAuthTokenEnvVars()helper fromtest/support/ray.goRelated issue number
Related to #4203
Checks