feat(api): Introduce InferenceModelRewrite API #1816
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XXL
|
/assign @ahg-g @kfswain @nirrozenbaum @danehans Can you help take a look? I also put the implementation in another PR #1820 |
| // +listType=map | ||
| // +listMapKey=type | ||
| // +kubebuilder:validation:MaxItems=8 | ||
| // +kubebuilder:default={{type: "Accepted", status: "Unknown", reason:"Pending", message:"Waiting for controller", lastTransitionTime: "1970-01-01T00:00:00Z"}} |
There was a problem hiding this comment.
Are we going to keep it in a "Pending" reason but still actuate on it?
There was a problem hiding this comment.
Good point, I was following the pattern we have in InferenceObejctive:
But since for phase 1, it is a config-only CRD, for now I removed the default value. wdyt?
There was a problem hiding this comment.
IIRC this was an artifact of the initial API review, we were asked to have a default status on all of our objects. This status will never be removed tho as nothing actually reconciles these APIs, EPP just reads them.
There was a problem hiding this comment.
ok, I suggest to keep it as is then, and we figure out how it is reconciled later similar to infObj
| type InferenceModelRewriteSpec struct { | ||
| // PoolRef is a reference to the inference pool. | ||
| // +kubebuilder:validation:Required | ||
| PoolRef PoolObjectReference `json:"poolRef"` |
There was a problem hiding this comment.
I recommend making PoolRef a pointer so that in the future we may relax validation here and make it optional.
One idea I have is this: if PoolRef is set, then the pool's EPP is responsible for this re-write; if not set, then that is BBR's signal that it should do it since this is a pool agnostic re-write. wdyt @kfswain @robscott @nirrozenbaum @zetxqx?
There was a problem hiding this comment.
Sounds good, I updated the PoolRef to a pointer, but I still keep the // +kubebuilder:validation:Required for now. We can always remove it if BBR also support it.
|
/lgtm We are discussing removing the default condition for this and the infObj api, but I think we should do that in separate PR that does it for both. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, zetxqx 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 |
k8s-ci-robot
added
the
approved
6 participants
What type of PR is this?
What this PR does / why we need it:
This PR introduces the new InferenceModelRewrite Custom Resource Definition (CRD) and its corresponding proposal
document.
Key changes in this PR include:
This PR is purely declarative and does not include any controller or data plane logic. The implementation of the EPP
enhancements will follow in a subsequent PR.
For full details, please see the proposal document (docs/proposals/inferenceomodelrewrite-api/README.md).
Which issue(s) this PR fixes:
Fixes partially #1811
Does this PR introduce a user-facing change?: