From 8b16b33aac85838d7288732058d63a489422b047 Mon Sep 17 00:00:00 2001 From: Adam Lewis <23342526+Adam-D-Lewis@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:09:35 -0500 Subject: [PATCH] feat(routing): make AI gateway route request timeout configurable The operator generates an AIGatewayRoute per model endpoint but set no request timeout, so the Envoy AI Gateway applied its built-in 60s default to the rendered HTTPRoute. That is too short for many LLM generations (long or reasoning outputs, large prompts, cold model loads) and cannot be raised by a BackendTrafficPolicy, since an explicit route timeout always takes precedence - the route itself is the only effective lever. Add a chart-wide defaults.routing.requestTimeout value, plumbed through the LLM_ROUTE_REQUEST_TIMEOUT env var and OperatorConfig into both the external and internal AIGatewayRoutes as spec.rules[].timeouts.request. It defaults to empty, which leaves the field unset and preserves the gateway's 60s default on upgrade; set it to a duration such as "600s" to raise the timeout. The value is validated against the Gateway API duration format at operator startup. PassthroughModel routes keep their own fixed 120s timeout and are unaffected. A per-model LLMModel.spec override is left for a follow-up. --- .../templates/operator-deployment.yaml | 2 + charts/nebari-llm-serving/values.yaml | 16 ++++ operator/internal/config/config.go | 25 +++++ operator/internal/config/config_test.go | 32 +++++++ .../controller/reconcilers/passthrough.go | 5 + .../controller/reconcilers/routing.go | 94 +++++++++++-------- .../controller/reconcilers/routing_test.go | 71 ++++++++++++++ 7 files changed, 205 insertions(+), 40 deletions(-) diff --git a/charts/nebari-llm-serving/templates/operator-deployment.yaml b/charts/nebari-llm-serving/templates/operator-deployment.yaml index e1b6266..a50f4a8 100644 --- a/charts/nebari-llm-serving/templates/operator-deployment.yaml +++ b/charts/nebari-llm-serving/templates/operator-deployment.yaml @@ -54,6 +54,8 @@ spec: value: {{ .Values.platform.tls.secretName | default "" | quote }} - name: LLM_MANAGE_SHARED_LISTENERS value: {{ .Values.platform.gateway.manageSharedListeners | quote }} + - name: LLM_ROUTE_REQUEST_TIMEOUT + value: {{ .Values.defaults.routing.requestTimeout | default "" | quote }} - name: ENABLE_WEBHOOKS value: "true" # POD_NAMESPACE feeds the validating webhook's same-namespace diff --git a/charts/nebari-llm-serving/values.yaml b/charts/nebari-llm-serving/values.yaml index cd11c44..d12d33c 100644 --- a/charts/nebari-llm-serving/values.yaml +++ b/charts/nebari-llm-serving/values.yaml @@ -143,3 +143,19 @@ defaults: storageClassName: "" monitoring: enabled: true + # routing configures the AIGatewayRoutes the operator generates for each + # LLMModel's external (llm.) and internal + # (llm-internal.) endpoints. (PassthroughModel routes use a + # fixed timeout and are not affected by these settings.) + routing: + # requestTimeout is the HTTP request timeout applied to those routes + # (spec.rules[].timeouts.request) - the maximum time Envoy waits for a + # model to return a complete response. Empty (the default) leaves the + # field unset, so the Envoy AI Gateway applies its built-in 60s default. + # Raise it when generations need longer than 60s (large prompts, long or + # reasoning outputs, cold model loads) by setting a Gateway API duration + # string such as "600s" or "10m". The route is the only effective place + # to set this: an explicit route timeout takes precedence over any + # BackendTrafficPolicy. A future LLMModel.spec field will be able to + # override this per model. + requestTimeout: "" diff --git a/operator/internal/config/config.go b/operator/internal/config/config.go index 2035f60..9a6432b 100644 --- a/operator/internal/config/config.go +++ b/operator/internal/config/config.go @@ -19,6 +19,7 @@ package config import ( "fmt" "os" + "regexp" "strings" ) @@ -73,6 +74,18 @@ type OperatorConfig struct { // false the operator still creates the shared Certificate but leaves // listener management to the cluster admin (runbook). Default true. ManageSharedListeners bool // LLM_MANAGE_SHARED_LISTENERS (default: "true") + // RouteRequestTimeout is the HTTP request timeout stamped onto every + // AIGatewayRoute the operator generates (spec.rules[].timeouts.request), + // for both the external llm. and internal + // llm-internal. endpoints. The Envoy AI Gateway propagates it + // to the HTTPRoute it renders and Envoy enforces it as the upstream + // response timeout. Empty leaves the field unset so the gateway's own 60s + // default applies - which is too short for many LLM generations and, + // because an explicit route timeout always wins, is not overridable by a + // BackendTrafficPolicy. The Helm chart leaves this empty by default. Set + // it to a Gateway API duration string such as "600s" or "10m" to raise + // the timeout; invalid values are rejected at operator startup. + RouteRequestTimeout string // LLM_ROUTE_REQUEST_TIMEOUT (optional; empty = gateway 60s default) } // getEnvOrDefault returns the value of the environment variable named by key, @@ -100,6 +113,13 @@ func getEnvBool(key string, defaultVal bool) bool { } } +// gatewayAPIDurationRE matches the Gateway API Duration format (GEP-2257) +// required by AIGatewayRoute spec.rules[].timeouts.request. time.ParseDuration +// is not a substitute: it accepts units the CRD rejects (ns, us), allows +// decimals, and treats bare numbers differently, so it would disagree with the +// API server's own validation of the value. +var gatewayAPIDurationRE = regexp.MustCompile(`^([0-9]{1,5}(h|m|s|ms)){1,4}$`) + // LoadFromEnv reads configuration from environment variables and returns an // OperatorConfig. It returns a descriptive error listing all missing required // variables if any are absent. @@ -130,11 +150,16 @@ func LoadFromEnv() (*OperatorConfig, error) { ClusterIssuerName: getEnvOrDefault("LLM_CLUSTER_ISSUER_NAME", "letsencrypt-production"), TLSSecretName: os.Getenv("LLM_TLS_SECRET_NAME"), ManageSharedListeners: getEnvBool("LLM_MANAGE_SHARED_LISTENERS", true), + RouteRequestTimeout: os.Getenv("LLM_ROUTE_REQUEST_TIMEOUT"), } if len(missing) > 0 { return nil, fmt.Errorf("missing required environment variables: %s", strings.Join(missing, ", ")) } + if cfg.RouteRequestTimeout != "" && !gatewayAPIDurationRE.MatchString(cfg.RouteRequestTimeout) { + return nil, fmt.Errorf("LLM_ROUTE_REQUEST_TIMEOUT %q is not a valid Gateway API duration string (e.g. 600s, 10m)", cfg.RouteRequestTimeout) + } + return cfg, nil } diff --git a/operator/internal/config/config_test.go b/operator/internal/config/config_test.go index 5d251c4..87adf2f 100644 --- a/operator/internal/config/config_test.go +++ b/operator/internal/config/config_test.go @@ -45,6 +45,7 @@ func TestLoadFromEnv(t *testing.T) { "POD_NAMESPACE": "llm-serving", "LLM_CLUSTER_ISSUER_NAME": "my-issuer", "LLM_MANAGE_SHARED_LISTENERS": "false", + "LLM_ROUTE_REQUEST_TIMEOUT": "300s", }, wantErr: false, wantConfig: &OperatorConfig{ @@ -61,6 +62,7 @@ func TestLoadFromEnv(t *testing.T) { OperatorNamespace: "llm-serving", ClusterIssuerName: "my-issuer", ManageSharedListeners: false, + RouteRequestTimeout: "300s", }, }, { @@ -193,6 +195,32 @@ func TestLoadFromEnv(t *testing.T) { ManageSharedListeners: true, }, }, + { + name: "invalid LLM_ROUTE_REQUEST_TIMEOUT - returns error mentioning the var", + envVars: map[string]string{ + "LLM_BASE_DOMAIN": "example.com", + "LLM_EXTERNAL_GATEWAY_NAME": "external-gw", + "LLM_INTERNAL_GATEWAY_NAME": "internal-gw", + "LLM_OIDC_ISSUER_URL": "https://auth.example.com", + "POD_NAMESPACE": "llm-serving", + "LLM_ROUTE_REQUEST_TIMEOUT": "banana", + }, + wantErr: true, + errContains: []string{"LLM_ROUTE_REQUEST_TIMEOUT"}, + }, + { + name: "unitless LLM_ROUTE_REQUEST_TIMEOUT - returns error (Gateway API needs a unit)", + envVars: map[string]string{ + "LLM_BASE_DOMAIN": "example.com", + "LLM_EXTERNAL_GATEWAY_NAME": "external-gw", + "LLM_INTERNAL_GATEWAY_NAME": "internal-gw", + "LLM_OIDC_ISSUER_URL": "https://auth.example.com", + "POD_NAMESPACE": "llm-serving", + "LLM_ROUTE_REQUEST_TIMEOUT": "600", + }, + wantErr: true, + errContains: []string{"LLM_ROUTE_REQUEST_TIMEOUT"}, + }, } for _, tt := range tests { @@ -212,6 +240,7 @@ func TestLoadFromEnv(t *testing.T) { "POD_NAMESPACE", "LLM_CLUSTER_ISSUER_NAME", "LLM_MANAGE_SHARED_LISTENERS", + "LLM_ROUTE_REQUEST_TIMEOUT", } for _, v := range allVars { t.Setenv(v, "") @@ -281,6 +310,9 @@ func TestLoadFromEnv(t *testing.T) { if got.ManageSharedListeners != tt.wantConfig.ManageSharedListeners { t.Errorf("ManageSharedListeners = %v, want %v", got.ManageSharedListeners, tt.wantConfig.ManageSharedListeners) } + if got.RouteRequestTimeout != tt.wantConfig.RouteRequestTimeout { + t.Errorf("RouteRequestTimeout = %q, want %q", got.RouteRequestTimeout, tt.wantConfig.RouteRequestTimeout) + } }) } } diff --git a/operator/internal/controller/reconcilers/passthrough.go b/operator/internal/controller/reconcilers/passthrough.go index 2176735..6a72db5 100644 --- a/operator/internal/controller/reconcilers/passthrough.go +++ b/operator/internal/controller/reconcilers/passthrough.go @@ -268,6 +268,11 @@ func buildProviderBackendSecurityPolicy(pm *llmv1alpha1.PassthroughModel, labels // "route-not-found" rule to every generated HTTPRoute, and without // sectionName that rule (and the SecurityPolicies bound to this route) // would catch traffic for unrelated listeners on the shared Gateway. +// +// The 120s request timeout on the rules below is fixed and intentionally +// independent of defaults.routing.requestTimeout (which configures LLMModel +// routes): passthrough traffic fronts external providers with a different +// latency profile. Revisit if it should become configurable. func buildPassthroughRoute( name string, pm *llmv1alpha1.PassthroughModel, diff --git a/operator/internal/controller/reconcilers/routing.go b/operator/internal/controller/reconcilers/routing.go index 80cdf61..121e530 100644 --- a/operator/internal/controller/reconcilers/routing.go +++ b/operator/internal/controller/reconcilers/routing.go @@ -55,6 +55,7 @@ func BuildRoutingResources(model *llmv1alpha1.LLMModel, cfg *config.OperatorConf model.Name, model.Spec.Model.Name, SharedExternalHostname(cfg.BaseDomain), + cfg.RouteRequestTimeout, ) } @@ -69,6 +70,7 @@ func BuildRoutingResources(model *llmv1alpha1.LLMModel, cfg *config.OperatorConf model.Name, model.Spec.Model.Name, SharedInternalHostname(cfg.BaseDomain), + cfg.RouteRequestTimeout, ) } @@ -83,7 +85,58 @@ func buildAIGatewayRoute( poolName string, modelName string, hostname string, + requestTimeout string, ) *unstructured.Unstructured { + rule := map[string]interface{}{ + // Match on BOTH the shared hostname (Host header) AND the + // `x-ai-eg-model` header. With sectionName scoping on the route's + // parentRefs (set below), the Host match is defence in depth: it + // guarantees the rule only fires for the intended FQDN even if a + // future refactor loosens parent attachment. + // + // The x-ai-eg-model match gives us per-model dispatch once a request + // lands on a shared listener. The Envoy AI Gateway extproc filter + // derives the value from the request body's `model` field before + // HTTPRoute matching runs. Both matches are ANDed (per Gateway API + // spec). + "matches": []interface{}{ + map[string]interface{}{ + "headers": []interface{}{ + map[string]interface{}{ + "type": "Exact", + "name": "Host", + "value": hostname, + }, + map[string]interface{}{ + "type": "Exact", + "name": "x-ai-eg-model", + "value": modelName, + }, + }, + }, + }, + "backendRefs": []interface{}{ + map[string]interface{}{ + "group": "inference.networking.k8s.io", + "kind": "InferencePool", + "name": poolName, + }, + }, + } + + // requestTimeout, when set, becomes spec.rules[].timeouts.request on the + // AIGatewayRoute. The Envoy AI Gateway copies it onto the HTTPRoute it + // renders; left unset, the gateway injects its own 60s default, which is + // too short for many LLM generations (long completions, reasoning models, + // large prompts, cold model loads). The route timeout is the only + // effective lever here: an explicit HTTPRoute timeout takes precedence + // over a BackendTrafficPolicy, so a policy attachment cannot raise it. + if requestTimeout != "" { + rule["timeouts"] = map[string]interface{}{ + "request": requestTimeout, + } + } + return &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "aigateway.envoyproxy.io/v1alpha1", @@ -112,46 +165,7 @@ func buildAIGatewayRoute( "sectionName": listenerSectionName, }, }, - "rules": []interface{}{ - map[string]interface{}{ - // Match on BOTH the shared hostname (Host header) AND the - // `x-ai-eg-model` header. With sectionName scoping - // on parentRefs above, the Host match is defence in - // depth: it guarantees the rule only fires for the - // intended FQDN even if a future refactor loosens - // parent attachment. - // - // The x-ai-eg-model match gives us per-model dispatch - // once a request lands on a shared listener. The - // Envoy AI Gateway extproc filter derives the value - // from the request body's `model` field before - // HTTPRoute matching runs. Both matches are ANDed - // (per Gateway API spec). - "matches": []interface{}{ - map[string]interface{}{ - "headers": []interface{}{ - map[string]interface{}{ - "type": "Exact", - "name": "Host", - "value": hostname, - }, - map[string]interface{}{ - "type": "Exact", - "name": "x-ai-eg-model", - "value": modelName, - }, - }, - }, - }, - "backendRefs": []interface{}{ - map[string]interface{}{ - "group": "inference.networking.k8s.io", - "kind": "InferencePool", - "name": poolName, - }, - }, - }, - }, + "rules": []interface{}{rule}, }, }, } diff --git a/operator/internal/controller/reconcilers/routing_test.go b/operator/internal/controller/reconcilers/routing_test.go index 8da20ef..b187557 100644 --- a/operator/internal/controller/reconcilers/routing_test.go +++ b/operator/internal/controller/reconcilers/routing_test.go @@ -94,6 +94,27 @@ func hostHeaderMatchValue(t *testing.T, route *unstructured.Unstructured) string return "" } +// ruleRequestTimeout returns the spec.rules[0].timeouts.request value of the +// given AIGatewayRoute and whether a timeouts block is present on the rule. +func ruleRequestTimeout(t *testing.T, route *unstructured.Unstructured) (string, bool) { + t.Helper() + if route == nil { + t.Fatal("expected route to be non-nil") + } + spec, _ := route.Object["spec"].(map[string]interface{}) + rules, _ := spec["rules"].([]interface{}) + if len(rules) == 0 { + t.Fatalf("expected at least one rule, got %v", spec["rules"]) + } + rule, _ := rules[0].(map[string]interface{}) + timeouts, ok := rule["timeouts"].(map[string]interface{}) + if !ok { + return "", false + } + v, _ := timeouts["request"].(string) + return v, true +} + func defaultRoutingModel() *llmv1alpha1.LLMModel { return &llmv1alpha1.LLMModel{ ObjectMeta: metav1.ObjectMeta{ @@ -506,6 +527,56 @@ func TestBuildRoutingResources(t *testing.T) { //nolint:gocyclo // table-driven } }, }, + { + name: "RouteRequestTimeout set: both routes carry timeouts.request on their rule", + model: defaultRoutingModel(), + cfg: func() *config.OperatorConfig { + c := defaultRoutingConfig() + c.RouteRequestTimeout = "600s" + return c + }(), + check: func(t *testing.T, result *RoutingResources, err error) { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + for _, route := range []struct { + name string + r *unstructured.Unstructured + }{ + {"external", result.ExternalRoute}, + {"internal", result.InternalRoute}, + } { + got, ok := ruleRequestTimeout(t, route.r) + if !ok { + t.Fatalf("%s: expected a timeouts block on the rule, got none", route.name) + } + if got != "600s" { + t.Errorf("%s: expected timeouts.request 600s, got %q", route.name, got) + } + } + }, + }, + { + name: "RouteRequestTimeout empty: routes omit timeouts (defer to gateway default)", + model: defaultRoutingModel(), + cfg: defaultRoutingConfig(), // RouteRequestTimeout is "" + check: func(t *testing.T, result *RoutingResources, err error) { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + for _, route := range []struct { + name string + r *unstructured.Unstructured + }{ + {"external", result.ExternalRoute}, + {"internal", result.InternalRoute}, + } { + if _, ok := ruleRequestTimeout(t, route.r); ok { + t.Errorf("%s: expected no timeouts block when RouteRequestTimeout is empty", route.name) + } + } + }, + }, } for _, tt := range tests {