-
Notifications
You must be signed in to change notification settings - Fork 102
API: Adds default status condition to InferencePool #830
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: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
/cc @robscott Tested in my dev cluster: $ kubectl get inferencepool/test -o yaml
...
status:
parent:
- conditions:
- lastTransitionTime: "1970-01-01T00:00:00Z"
message: Waiting for controller
reason: Pending
status: Unknown
type: Accepted
parentRef: {} |
Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
/approve Will leave final stamp to @robscott |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danehans, kfswain The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Since the default parent status contains an empty $ kubectl get inferencepool/vllm-llama3-8b-instruct -o yaml
...
status:
parent:
- conditions:
- lastTransitionTime: "1970-01-01T00:00:00Z"
message: Waiting for controller
reason: Pending
status: Unknown
type: Accepted
parentRef: {}
- conditions:
- lastTransitionTime: "2025-05-15T00:04:05Z"
message: InferencePool is accepted by controller kgateway.dev/kgateway
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: "2025-05-15T00:04:05Z"
message: All InferencePool references are resolved
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
parentRef:
kind: Gateway
name: inference-gateway We should update the InferencePool API to define the expected behavior of the default PoolStatus. For example:
Thoughts @robscott |
Thanks for doing this @danehans! We should update the InferencePool API to define the expected behavior of the default PoolStatus. For example:
Completely agree. I think 1) is a MUST, while 2) is a SHOULD. If you can add this to API Spec, I'll LGTM. |
|
||
// Status defines the observed state of InferencePool. | ||
// | ||
// +kubebuilder:default={parent: {{parentRef: {}, conditions: {{type: "Accepted", status: "Unknown", reason: "Pending", message: "Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}}}}} |
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.
What if instead of an empty parentRef, we filled it with something like {kind: Status, name: default}
, or some kind of similar distinction so it's easier for controllers to know that they can safely remove this.
cc @SinaChavoshi related to conformance tests |
/test pull-gateway-api-inference-extension-test-e2e-main |
Bump on this PR, the conversation makes sense to me. I think implementation of that will put this PR in a good state. |
Adds default status condition to the InferencePool API.
Fixes #825