Adds Health gRPC Server and Refactors Main() #148
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
/lgtm |
k8s-ci-robot
added
the
lgtm
pkg/ext-proc/backend/datastore.go
Outdated
| } | ||
| ready = true | ||
| return false | ||
| }) |
There was a problem hiding this comment.
At startup, I think we want to ensure that the extension did a sync with the api server and fetched the models, but not declare itself ready only if at least one model is defined.
There was a problem hiding this comment.
The health probe now uses a client to check the API server for the configured InferencePool and that at least one InferenceModel exists in the same namespace. Should this probe also check that at least one InferenceModel references the configured InferencePool?
There was a problem hiding this comment.
I don't think that the health check needs to block on at least one InferenceModel. On the other hand, since extension is currently 1:1 with InferencePool, I think it makes sense to ensure that the extension successfully initialized the assigned InferencePool.
k8s-ci-robot
added
the
needs-rebase
k8s-ci-robot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: danehans The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
removed
the
needs-rebase
- Introduced a health gRPC server to handle liveness and readiness probes. - Refactored main() to manage server goroutines. - Added graceful shutdown for servers and controller manager. - Improved logging consistency and ensured. - Validates CLI flags. Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
danehans
changed the title
| // Ensure at least 1 InferenceModel | ||
| if len(modelList.Items) == 0 { | ||
| return fmt.Errorf("no InferenceModels exist in namespace %s", *poolNamespace) | ||
| } |
There was a problem hiding this comment.
I am not sure this is necessary.
| *targetPodHeader, | ||
| ) | ||
|
|
||
| // Wait for first error from any goroutine |
There was a problem hiding this comment.
or the controller manager returning gracefully
| client.Client | ||
| } | ||
|
|
||
| func (s *healthServer) Check(ctx context.Context, in *healthPb.HealthCheckRequest) (*healthPb.HealthCheckResponse, error) { |
There was a problem hiding this comment.
I think this can check that the datastore populated the inference pool instead of actually doing a pull from the server?
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Adds a health gRPC Server and refactors
main()for better lifecycle management:Fixes #96
Fixes #175