move most backend processing to IR construction #12799
Conversation
Signed-off-by: Alexander Liu <[email protected]>
github-actions
bot
added
kind/fix
Signed-off-by: Alexander Liu <[email protected]>
shashankram
left a comment
shashankram
left a comment
There was a problem hiding this comment.
Looks reasonable. Do we have existing translator tests (internal/kgateway/translator/gateway/gateway_translator_test.go) for all the Backends to ensure we aren't introducing regressions?
| if c == nil && other == nil { | ||
| return true | ||
| } | ||
| if c == nil || other == nil { | ||
| return false | ||
| } |
There was a problem hiding this comment.
You can reduce the nil checks here and below to:
if a == nil || b == nil {
return a == nil && b == nil
}
| tt.pCtx = pCtx | ||
| aiIR := &IR{} | ||
| err := PreprocessAIBackend(context.Background(), tt.aiBackend, aiIR) | ||
| err := PreprocessAIBackend(context.Background(), tt.aiBackend, nil, nil, aiIR) |
There was a problem hiding this comment.
we probably should extend this test to pass secret and multiSecret, but can be a follow-up
There was a problem hiding this comment.
Lets take it as a followup
internal/kgateway/extensions2/plugins/backend/ai/ai_model_cluster.go
Outdated
Show resolved
Hide resolved
We have translator test for all backends except AI backend. For AI backend there is plugin test and setup test. Given that AI backend will soon be moved completely to agentgateway, this should be enough. |
Signed-off-by: Alexander Liu <[email protected]>
shashankram
left a comment
shashankram
left a comment
There was a problem hiding this comment.
Could you add a test for an AI backend with invalid secretRef to ensure it generates a Status error on the object?
Signed-off-by: Alexander Liu <[email protected]>
3 participants
Description
For backend, only errors happened during IR construction is reported to CRD, and any error happened during translation time are not and user won't see these errors in CRD status. A complete rework on the plugin status reporting is not ideal right now since backend CRD change is in discussion for agentgateway integration, so this PR attempts to alleviate such problem by move most processing logic into construction time from translation time and report errors there.
With this change, the only errors left for backend translation time are the ones that should never happen, such as casting backend obj, MutateHttpOptions, translation plugin not found, etc..
Change Type
/kind fix
Changelog