-
Notifications
You must be signed in to change notification settings - Fork 538
[apiserver] Start apiserver v2 in apiserver/cmd/main.go #3603
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Troy Chiu <[email protected]>
@rueian could you take a look? Thank you |
Signed-off-by: Troy Chiu <[email protected]>
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
@kevin85421 PTAL thank you! |
Signed-off-by: Troy Chiu <[email protected]>
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 chatted with @rueian yesterday. Our conclusions are:
- v1.4.0 is the last release where we fix bugs and add new features for v1.
- We want to launch both v1 and v2 at the same time so that users can incrementally migrate from v1 to v2, and don't need to be 100% one time.
- We don't want to create a new quay repository such as
kuberay/apiserverv2
. This may increase the complexity of migration. - For v1.5.0, the image ``kuberay/apiserver` will be v2 only.
apiserver/cmd/main.go
Outdated
@@ -144,8 +146,19 @@ func startHttpProxy() { | |||
registerHttpHandlerFromEndpoint(ctx, api.RegisterRayServeServiceHandlerFromEndpoint, "ServeService", runtimeMux) | |||
registerHttpHandlerFromEndpoint(ctx, api.RegisterRayJobSubmissionServiceHandlerFromEndpoint, "RayJobSubmissionService", runtimeMux) | |||
|
|||
kubernetesConfig, err := config.GetConfig() |
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.
Can you also add a feature gate (maybe a env var) for users to disable v2?
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 added a flag instead, since that has been a pattern in api server
Signed-off-by: Troy Chiu <[email protected]>
Signed-off-by: Troy Chiu <[email protected]>
Signed-off-by: Troy Chiu <[email protected]>
@@ -28,6 +25,8 @@ require ( | |||
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 | |||
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 | |||
github.com/onsi/gomega v1.37.0 | |||
github.com/ray-project/kuberay/apiserversdk v0.0.0-20250519054718-f3ebea713a2f |
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.
since apiserversdk hasn't been released. I use the master commit instead.
@@ -28,6 +25,8 @@ require ( | |||
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 | |||
github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 | |||
github.com/onsi/gomega v1.37.0 | |||
github.com/ray-project/kuberay/apiserversdk v0.0.0-20250519054718-f3ebea713a2f | |||
github.com/ray-project/kuberay/ray-operator v1.3.1 |
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 did some survey and this should be a better practice. That is, we should use a version instead of a local placeholder (v0.0.0-00010101000000-000000000000)
Eventually I think we should either use go workspace or only keep one root go.mod
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.
created #3640
Signed-off-by: Troy Chiu <[email protected]>
Signed-off-by: Troy Chiu <[email protected]>
Why are these changes needed?
This PR start apiserver v2 in
apiserver/cmd/main.go
to ship the apiserver v2.Related issue number
Closes #3596
Checks