From e9ca80c9ba2f431950221bca963f3affaa4623dc Mon Sep 17 00:00:00 2001 From: yassin Date: Sat, 12 Sep 2026 16:02:27 +0000 Subject: [PATCH] fix(infra): default ALB idle timeout to 600s and gateway keepalive to 630s Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../litellm/templates/gateway/deployment.yaml | 4 +++ helm/litellm/templates/ingress.yaml | 8 +++-- helm/litellm/tests/gateway_env_tests.yaml | 33 ++++++++++++++++++ .../tests/ingress_controller_tests.yaml | 34 +++++++++++++++++++ helm/litellm/values.yaml | 9 +++++ terraform/litellm/aws/README.md | 11 ++++++ terraform/litellm/aws/alb.tf | 2 +- terraform/litellm/aws/ecs.tf | 4 +++ terraform/litellm/aws/variables.tf | 11 ++++++ 9 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 helm/litellm/tests/gateway_env_tests.yaml diff --git a/helm/litellm/templates/gateway/deployment.yaml b/helm/litellm/templates/gateway/deployment.yaml index c06cc9583a0..51442eed688 100644 --- a/helm/litellm/templates/gateway/deployment.yaml +++ b/helm/litellm/templates/gateway/deployment.yaml @@ -61,6 +61,10 @@ spec: - name: NUM_WORKERS value: {{ .Values.gateway.numWorkers | quote }} {{- end }} + {{- if .Values.gateway.keepaliveTimeoutSeconds }} + - name: KEEPALIVE_TIMEOUT + value: {{ .Values.gateway.keepaliveTimeoutSeconds | quote }} + {{- end }} {{- if .Values.database.connectionPool.enabled }} {{- include "litellm.connectionPoolEnv" $ | nindent 12 }} {{- end }} diff --git a/helm/litellm/templates/ingress.yaml b/helm/litellm/templates/ingress.yaml index d42558b9396..8298c0436f6 100644 --- a/helm/litellm/templates/ingress.yaml +++ b/helm/litellm/templates/ingress.yaml @@ -6,6 +6,10 @@ {{- $backendPort := .Values.backend.service.port -}} {{- $uiPort := .Values.ui.service.port -}} {{- $controller := .Values.ingress.controller | default "alb" -}} +{{- $annotations := deepCopy (.Values.ingress.annotations | default (dict)) -}} +{{- if and (eq $controller "alb") .Values.ingress.albIdleTimeoutSeconds (not (hasKey $annotations "alb.ingress.kubernetes.io/load-balancer-attributes")) -}} +{{- $_ := set $annotations "alb.ingress.kubernetes.io/load-balancer-attributes" (printf "idle_timeout.timeout_seconds=%v" .Values.ingress.albIdleTimeoutSeconds) -}} +{{- end }} {{- if not (has $controller (list "alb" "nginx")) }} {{- fail (printf "ingress.controller: unknown controller %q, expected one of alb, nginx" $controller) }} {{- end }} @@ -96,9 +100,9 @@ metadata: name: {{ include "litellm.fullname" . }} labels: {{- include "litellm.commonLabels" . | nindent 4 }} - {{- with .Values.ingress.annotations }} + {{- if $annotations }} annotations: - {{- toYaml . | nindent 4 }} + {{- toYaml $annotations | nindent 4 }} {{- end }} spec: {{- with .Values.ingress.className }} diff --git a/helm/litellm/tests/gateway_env_tests.yaml b/helm/litellm/tests/gateway_env_tests.yaml new file mode 100644 index 00000000000..85620e00dbc --- /dev/null +++ b/helm/litellm/tests/gateway_env_tests.yaml @@ -0,0 +1,33 @@ +suite: test gateway environment defaults +templates: + - gateway/deployment.yaml + - gateway/configmap.yaml +values: + - ./values/required.yaml +tests: + - it: sets KEEPALIVE_TIMEOUT only on the gateway container + template: gateway/deployment.yaml + set: + gateway.collector.enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: KEEPALIVE_TIMEOUT + value: "630" + - notContains: + path: spec.template.spec.containers[1].env + content: + name: KEEPALIVE_TIMEOUT + any: true + + - it: omits KEEPALIVE_TIMEOUT when configured to zero + template: gateway/deployment.yaml + set: + gateway.keepaliveTimeoutSeconds: 0 + asserts: + - notContains: + path: spec.template.spec.containers[0].env + content: + name: KEEPALIVE_TIMEOUT + any: true diff --git a/helm/litellm/tests/ingress_controller_tests.yaml b/helm/litellm/tests/ingress_controller_tests.yaml index aa30db3c9c1..663ca26d9ea 100644 --- a/helm/litellm/tests/ingress_controller_tests.yaml +++ b/helm/litellm/tests/ingress_controller_tests.yaml @@ -4,6 +4,40 @@ templates: values: - ./values/required.yaml tests: + - it: adds the default ALB idle timeout annotation + set: + ingress.enabled: true + asserts: + - equal: + path: metadata.annotations["alb.ingress.kubernetes.io/load-balancer-attributes"] + value: idle_timeout.timeout_seconds=600 + + - it: preserves a user-supplied ALB load balancer attributes annotation + set: + ingress.enabled: true + ingress.annotations: + alb.ingress.kubernetes.io/load-balancer-attributes: "idle_timeout.timeout_seconds=120,routing.http2.enabled=true" + asserts: + - equal: + path: metadata.annotations["alb.ingress.kubernetes.io/load-balancer-attributes"] + value: idle_timeout.timeout_seconds=120,routing.http2.enabled=true + + - it: does not add the ALB idle timeout annotation for ingress-nginx + set: + ingress.enabled: true + ingress.controller: nginx + asserts: + - notExists: + path: metadata.annotations["alb.ingress.kubernetes.io/load-balancer-attributes"] + + - it: does not add the ALB idle timeout annotation when disabled + set: + ingress.enabled: true + ingress.albIdleTimeoutSeconds: 0 + asserts: + - notExists: + path: metadata.annotations["alb.ingress.kubernetes.io/load-balancer-attributes"] + - it: keeps the AWS Load Balancer Controller path types by default set: ingress.enabled: true diff --git a/helm/litellm/values.yaml b/helm/litellm/values.yaml index 4ca54131d6a..766c6a32d4b 100644 --- a/helm/litellm/values.yaml +++ b/helm/litellm/values.yaml @@ -23,6 +23,11 @@ ingress: # wildcard pathType, so that rule could never match there. controller: alb annotations: {} + # ALB idle timeout in seconds, rendered as the + # alb.ingress.kubernetes.io/load-balancer-attributes annotation when + # controller is alb and ingress.annotations does not set that key itself. + # Streams silent longer than this (slow first token) are cut with a 504. + albIdleTimeoutSeconds: 600 host: "" # optional; if set, becomes the rule's host tls: [] # Extra HTTP paths appended to the ingress rule. Additive: every built-in @@ -282,6 +287,10 @@ gateway: # Number of uvicorn worker processes per gateway pod. Sets NUM_WORKERS, # consumed by the gateway image entrypoint. Default is 1. numWorkers: 1 + # Uvicorn keep-alive timeout in seconds (KEEPALIVE_TIMEOUT). Must exceed the + # load balancer idle timeout in front of the gateway, otherwise the LB can + # hand a request to a connection uvicorn is closing. + keepaliveTimeoutSeconds: 630 extraEnv: [] # Add extra environment variables to the gateway envConfigMaps: [] # Add extra environment variables to the gateway from config maps envSecrets: [] # Add extra environment variables to the gateway from secrets diff --git a/terraform/litellm/aws/README.md b/terraform/litellm/aws/README.md index 6fcbdc2f500..66305b11c05 100644 --- a/terraform/litellm/aws/README.md +++ b/terraform/litellm/aws/README.md @@ -335,6 +335,17 @@ gateway_tokens_metric = { } ``` +### Load balancer and gateway timeouts + +The ALB idle timeout defaults to 600 seconds through `alb_idle_timeout_seconds`. +The gateway receives `KEEPALIVE_TIMEOUT` set to 30 seconds above that value, so +uvicorn keeps connections open longer than the load balancer. Override +`gateway_extra_env.KEEPALIVE_TIMEOUT` when a different gateway timeout is needed. + +```hcl +alb_idle_timeout_seconds = 600 +``` + Worked example for the request policy: 1,000 rps across 10 tasks is 100 rps per task (the ALB reports it as 6,000 per minute per target) against a target of 90 (5,400), so target tracking sizes the service to diff --git a/terraform/litellm/aws/alb.tf b/terraform/litellm/aws/alb.tf index bb07a83caa7..b17ff89a25c 100644 --- a/terraform/litellm/aws/alb.tf +++ b/terraform/litellm/aws/alb.tf @@ -5,7 +5,7 @@ resource "aws_lb" "this" { security_groups = [aws_security_group.alb.id] subnets = local.public_subnet_ids - idle_timeout = 120 + idle_timeout = var.alb_idle_timeout_seconds lifecycle { precondition { diff --git a/terraform/litellm/aws/ecs.tf b/terraform/litellm/aws/ecs.tf index 2b235c2bad5..a26bb97dd23 100644 --- a/terraform/litellm/aws/ecs.tf +++ b/terraform/litellm/aws/ecs.tf @@ -176,6 +176,9 @@ locals { backend_extra_env_list = [ for k, v in var.backend_extra_env : { name = k, value = v } ] + gateway_timeout_env = contains(keys(var.gateway_extra_env), "KEEPALIVE_TIMEOUT") ? [] : [ + { name = "KEEPALIVE_TIMEOUT", value = tostring(var.alb_idle_timeout_seconds + 30) }, + ] # Storing models in the DB needs a DB. Without one the backend reads its # model list from proxy_config only. @@ -292,6 +295,7 @@ locals { local.shared_env, local.gateway_otel_env, local.billing_metrics_env, + local.gateway_timeout_env, local.gateway_extra_env_list, local.proxy_config_env, local.metrics_env, diff --git a/terraform/litellm/aws/variables.tf b/terraform/litellm/aws/variables.tf index 580a0cc657a..b4851b8ca46 100644 --- a/terraform/litellm/aws/variables.tf +++ b/terraform/litellm/aws/variables.tf @@ -878,3 +878,14 @@ variable "collector_drain_timeout_seconds" { error_message = "collector_drain_timeout_seconds must be > 0." } } + +variable "alb_idle_timeout_seconds" { + description = "ALB idle timeout in seconds. Streaming responses that stay silent longer than this (e.g. a slow first token) are cut by the ALB with a 504, so keep it at or above the proxy's request_timeout." + type = number + default = 600 + + validation { + condition = var.alb_idle_timeout_seconds >= 1 && var.alb_idle_timeout_seconds <= 4000 + error_message = "alb_idle_timeout_seconds must be between 1 and 4000." + } +}