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 {