add-state-machine-and-exposing-port #319
Conversation
proto/cluster.proto
Outdated
| @@ -152,6 +152,12 @@ message Cluster { | |||
|
|
|||
| // Output. The time that the cluster deleted. | |||
| google.protobuf.Timestamp deleted_at = 8; | |||
|
|
|||
| // Output. The service endpoint of the cluster | |||
| map<string, string> service_endpoint = 9; | |||
There was a problem hiding this comment.
just curious why make the port a string instead of int32?
There was a problem hiding this comment.
Sound good, I remember at first we do not have any intend to use string or int32, and if we would like to use it to represent nodeIP:port as a endpoints, I think string would be better. WDYT @Jeffwan
There was a problem hiding this comment.
This is our internal representation. Honestly, I am not sure whether community like it or not? Any suggestions? Do we need the port or endpoint here? /cc @davidxia
There was a problem hiding this comment.
It's a little confusing from an outsider's perspective.
Would we also consider exposing other access endpoint? e.g. the head pod ip?
apiserver/pkg/model/converter.go
Outdated
| } | ||
|
|
||
| // loop container and find the resource | ||
| pbCluster.ClusterSpec = &api.ClusterSpec{} | ||
| pbCluster.ClusterSpec.HeadGroupSpec = PopulateHeadNodeSpec(cluster.Spec.HeadGroupSpec) | ||
| pbCluster.ClusterSpec.WorkerGroupSepc = PopulateWorkerNodeSpec(cluster.Spec.WorkerGroupSpecs) | ||
|
|
||
| pbCluster.ServiceEndpoint = map[string]string{} | ||
| for name, port := range cluster.Status.Endpoints { | ||
| pbCluster.ServiceEndpoint[name] = strconv.Itoa(int(port)) |
There was a problem hiding this comment.
just wondering why convert port from string to int back to string?
|
#223 just need |
Yes, we can put a new PR to add the service endpoints, since as a part in the status, they are also needed by that story as well as the state. |
|
Feel free to leave comments and file follow up PRs. Current implementation should be good as the start. |
|
@scarlet25151 let me know if you need any help on the endpoint code that was removed from this PR. When you have time can you open another PR for that for further discussion? Thanks! |
* add-state-machine-and-exposing-port * remove code conflict Co-authored-by: chenyu.jiang <chenyu.jiang@bytedance.com>
Why are these changes needed?
Expose the state and service endpoints port in status.
Related issue number
Related to #223
Checks