Adding support for multiple inferencePools API - #659
Conversation
There was a problem hiding this comment.
Pull request overview
Adds runtime support for reconciling InferencePool resources from either the stable (inference.networking.k8s.io) or alpha (inference.networking.x-k8s.io) API group.
Changes:
- Introduces constants for the supported InferencePool API groups and validates selection at startup.
- Replaces the previous default-only Pool GKNN helper with a group-aware
GetPoolGKNN(poolGroup)that returns an error on invalid input. - Plumbs a new
POOL_GROUPenvironment variable intocmd/main.goto select which API group to watch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
internal/utils/pool/pool.go |
Adds supported group constants and a validated group-aware GKNN constructor. |
cmd/main.go |
Uses POOL_GROUP to build the Pool GKNN and fails fast on invalid configuration. |
f716e35 to
161d6e3
Compare
|
|
||
| // Create InferencePool reconciler | ||
| poolGroupEnv := os.Getenv("POOL_GROUP") | ||
| poolGKNN, err := poolutil.GetPoolGKNN(os.Getenv(poolGroupEnv)) |
There was a problem hiding this comment.
why the indirection? POOL_GROUP is the name of the envar? Why?
Also does that mean a WVA instance can only support one group at a time?
There was a problem hiding this comment.
@lionelvillard, this is to allow to configure the inferencepool API group via an envar. In the EPP pod the API group for the inferencepool is configured via a container flag.
The POOL_GROUP env var does not need to be set - the default value is inference.networking.x-k8s.io
Each EPP instance only supports one group at a time and we used similar pattern for the WVA implementation of scaleFromZero. Otherwise, we would need to have two reconcile, one for each group.
There was a problem hiding this comment.
sorry I still don't get it. Why poolutil.GetPoolGKNN(os.Getenv(poolGroupEnv)) and not poolutil.GetPoolGKNN(poolGroupEnv)
There was a problem hiding this comment.
@lionelvillard, I pushed a commit last night updating the code. See lines https://github.com/dumb0002/workload-variant-autoscaler/blob/inferencepool-multiple-apis/cmd/main.go#L426-L430.
Does it answer your questions?
161d6e3 to
67cabf4
Compare
|
/ok-to-test |
|
🚀 E2E tests triggered by /ok-to-test |
c113972 to
ba9efab
Compare
274d5d6 to
fb8fabb
Compare
Signed-off-by: Braulio Dumba <Braulio.Dumba@ibm.com>
c8f18e1 to
4864310
Compare
|
/ok-to-test |
|
🚀 E2E tests triggered by /ok-to-test |
The PR adds support for both
inference.networking.k8s.ioandinference.networking.x-k8s.ioInferencePool API groups.