fix(e2e): wait for every gateway before using a new model and keep the network rerun

The changed-tests workflow overrode the suite's `--reruns 1` with `--reruns 0`, so a
transport blip failed a pass that pytest.ini already scopes to network errors and
5xx responses. Pass 2 of run 33692484803 also went red 15s after a model write with
"no healthy deployments": the barrier only polled /v1/models through nginx, which
proves one gateway converged, and the next request rolled the other. The stack now
exports LITELLM_PROXY_REPLICA_URLS, the barrier polls every replica with the full
budget before settling, and up.sh refuses to boot without DD_API_KEY, since the
gateway config enables the datadog callback on every run
This commit is contained in:
mateo-berri 2026-09-05 16:10:40 -07:00
parent 5051e6d44a
commit 65d8bbb8ac
8 changed files with 206 additions and 35 deletions

View file

@ -54,6 +54,12 @@ if [[ -f "${REPO_ROOT}/tests/e2e/.env" ]]; then
set +a
fi
if [[ -z "${DD_API_KEY:-}" ]]; then
log "DD_API_KEY is empty; the gateway config enables the datadog callback, so put a Datadog API key in tests/e2e/.env"
exit 1
fi
export DD_SITE="${DD_SITE:-datadoghq.com}"
if ! port_open "${DATABASE_PORT}"; then
docker run -d --name e2e-postgres -p "${DATABASE_PORT}:5432" \
-e "POSTGRES_USER=${DATABASE_USER}" -e "POSTGRES_PASSWORD=${DATABASE_PASSWORD}" -e "POSTGRES_DB=${DATABASE_NAME}" \
@ -189,6 +195,7 @@ wait_for "load balancer" "curl -fs http://127.0.0.1:${LB_PORT}/health/liveliness
cat > "${STACK_DIR}/stack.env" <<EOF
LITELLM_PROXY_URL=http://127.0.0.1:${LB_PORT}
LITELLM_CONTROL_PLANE_URL=http://127.0.0.1:${BACKEND_PORT}
LITELLM_PROXY_REPLICA_URLS=http://127.0.0.1:${GATEWAY_PORT_1},http://127.0.0.1:${GATEWAY_PORT_2}
LITELLM_MASTER_KEY=${MASTER_KEY}
REDIS_HOST=127.0.0.1
REDIS_PORT=${REDIS_PORT}

View file

@ -161,7 +161,7 @@ jobs:
echo "::add-mask::${master_key}"
cat "${RUNNER_TEMP}/litellm-e2e-stack/stack.env" >> "${GITHUB_ENV}"
- name: Run the selected tests three times with retries off
- name: Run the selected tests three times
env:
TESTS: ${{ needs.detect.outputs.tests }}
E2E_FIXTURE_MODE: live
@ -172,7 +172,7 @@ jobs:
report="${RUNNER_TEMP}/e2e-pass-${pass}.xml"
echo "::group::pass ${pass} of 3"
set +e
uv run --no-sync pytest "${test_files[@]}" --rootdir=. --reruns 0 -v -p no:cacheprovider \
uv run --no-sync pytest "${test_files[@]}" --rootdir=. -v -p no:cacheprovider \
-o junit_family=xunit1 --junitxml="${report}" > "${RUNNER_TEMP}/e2e-pass-${pass}.log" 2>&1
status=$?
set -e

View file

@ -54,13 +54,13 @@ Some suites need extra services the bare proxy does not start. The `logging/` OT
### The pull request check
Every same-repository PR that adds, modifies, or renames a `tests/e2e/**/test_*.py` file runs those changed files three times with pytest retries off. The stage-mirror stack has a control-plane backend, two gateways behind nginx, Postgres, Jaeger, and TLS cluster-mode Valkey. Documentation, harness, configuration, deleted-file, and workflow-only changes do not start the stack or request environment approval. The `ui/`, `claude_code/`, and `load/` directories and `batches/test_managed_files_enforcement_e2e.py` remain outside this check because they use separate tooling or need a differently configured stack
Every same-repository PR that adds, modifies, or renames a `tests/e2e/**/test_*.py` file runs those changed files three times. The suite's own single rerun for network errors and 5xx responses (see `pytest.ini`) applies on every pass, so a transport blip does not fail the check while a race inside a test still does. The stage-mirror stack has a control-plane backend, two gateways behind nginx, Postgres, Jaeger, and TLS cluster-mode Valkey. The stack exports every gateway address in `LITELLM_PROXY_REPLICA_URLS`, so model registration waits until each gateway lists the new model rather than whichever one the load balancer answered from. Documentation, harness, configuration, deleted-file, and workflow-only changes do not start the stack or request environment approval. The `ui/`, `claude_code/`, and `load/` directories and `batches/test_managed_files_enforcement_e2e.py` remain outside this check because they use separate tooling or need a differently configured stack
Every selected file must execute at least one passing test in each pass, and any test failure, collection error, or entirely skipped or deselected file fails the check. A failed pass stops the run without retrying. The final `e2e-changed-tests` job succeeds only when no supported test files changed or the approved run completed all three passes. Fork PRs with selected tests fail this gate until a maintainer brings the reviewed change onto a same-repository branch
Every selected file must execute at least one passing test in each pass, and any test failure, collection error, or entirely skipped or deselected file fails the check. A failed pass stops the run. The final `e2e-changed-tests` job succeeds only when no supported test files changed or the approved run completed all three passes. Fork PRs with selected tests fail this gate until a maintainer brings the reviewed change onto a same-repository branch
Repository admins must require the `e2e-changed-tests` status check for merging and configure the `e2e-changed` environment with required reviewers, self-review disabled, and admin bypass disabled. Each push cancels the previous run; a new run that selects tests needs a fresh approval. Reviewers must inspect the entire executable PR diff, including application code, dependencies, tests, and workflow helpers, before approving the exact revision. Approved code executes with provider credentials, so environment approval is a trust decision about that code
Credentials come from the existing AWS Secrets Manager secrets in us-east-1, `litellm-e2e-changed-provider-keys` and `litellm-e2e-changed-license`. The OIDC role must trust only `repo:BerriAI/litellm:environment:e2e-changed` with audience `sts.amazonaws.com` and have read access only to these secrets. The short-lived reader credentials are scoped to the fetch step. Provider credentials must cover the selected suites, including Datadog credentials when logging or MCP tests need them; missing credentials fail the run. Keep provider credentials dedicated to this lane with only the permissions those tests need
Credentials come from the existing AWS Secrets Manager secrets in us-east-1, `litellm-e2e-changed-provider-keys` and `litellm-e2e-changed-license`. The OIDC role must trust only `repo:BerriAI/litellm:environment:e2e-changed` with audience `sts.amazonaws.com` and have read access only to these secrets. The short-lived reader credentials are scoped to the fetch step. Provider credentials must cover the selected suites, including Datadog credentials when logging or MCP tests need them; missing credentials fail the run. `up.sh` refuses to start without `DD_API_KEY`, because the stack's gateway config enables the Datadog callback for every run and a gateway booted without the key fails readiness. Keep provider credentials dedicated to this lane with only the permissions those tests need
Fetched values are masked before use, and credential files and raw output are private to the runner. Public logs contain selected file names, counts, and pass status; raw pytest output, reports, and stack logs are not uploaded or printed. The workflow removes them and the credential files during cleanup. To diagnose a failed pass, reproduce the selected files locally with the appropriate credentials and inspect the local logs

View file

@ -99,5 +99,6 @@ def require_proxy_client(
base_url=cfg.base_url,
master_key=cfg.api_key,
control_plane_base_url=cfg.base_url,
replica_urls=(cfg.base_url,),
)
return ProxyClientConfig(client=client, api_key=cfg.api_key)

View file

@ -594,6 +594,7 @@ def _build_control_plane_client(proxy_config: ProxyConfig):
base_url=proxy_config.base_url,
master_key=proxy_config.api_key,
control_plane_base_url=proxy_config.base_url,
replica_urls=(proxy_config.base_url,),
)

View file

@ -10,6 +10,7 @@ import os
import time
import uuid
from pathlib import Path
from typing import Final
from dotenv import load_dotenv
@ -32,6 +33,14 @@ CONTROL_PLANE_BASE_URL = os.environ.get(
"LITELLM_CONTROL_PLANE_URL", PROXY_BASE_URL
).rstrip("/")
def parse_replica_urls(raw: str, fallback: str) -> tuple[str, ...]:
urls: Final = tuple(url.strip().rstrip("/") for url in raw.split(",") if url.strip())
return urls or (fallback,)
PROXY_REPLICA_URLS: Final = parse_replica_urls(os.environ.get("LITELLM_PROXY_REPLICA_URLS", ""), PROXY_BASE_URL)
UI_USERNAME = os.environ.get("E2E_UI_USERNAME", "admin")
UI_PASSWORD = os.environ.get("E2E_UI_PASSWORD", MASTER_KEY)
@ -86,10 +95,11 @@ SLOW_PROVIDER_TIMEOUT_SECONDS = float(os.environ.get("E2E_SLOW_PROVIDER_TIMEOUT"
# (`proxy_config_reload_interval_seconds`, 30s by default and 7s on the e2e stack)
# plus margin.
#
# The barriers below wait this out instead of returning on first sight, because a
# single successful read only proves ONE replica converged: every request opens a
# fresh connection, so a load-balanced Service routes each one independently and
# the next call re-rolls. See ProxyClient._await_model_servable.
# The barriers below wait this out on top of polling /v1/models on every replica in
# PROXY_REPLICA_URLS: that poll proves each addressed gateway converged, but not the
# workers behind it, and behind a load balancer (PROXY_REPLICA_URLS unset) a
# successful read only proves ONE replica converged, because every request opens a
# fresh connection and the next call re-rolls. See ProxyClient._await_model_servable.
PROPAGATION_TIMEOUT = float(os.environ.get("E2E_PROPAGATION_TIMEOUT", "15"))
EXPECT_RUST = os.environ.get("E2E_EXPECT_RUST", "").strip().lower() in ("1", "true", "yes")

View file

@ -10,12 +10,15 @@ from __future__ import annotations
import time
import warnings
from collections.abc import Callable
from collections.abc import Callable, Mapping
from dataclasses import dataclass
from datetime import datetime
from types import MappingProxyType
from typing import Final
from e2e_http import (
AnthropicHeaders,
AuthHeaders,
NoBody,
ProbeResult,
Result,
@ -71,6 +74,7 @@ from e2e_config import (
POLL_INTERVAL,
POLL_TIMEOUT,
PROXY_BASE_URL,
PROXY_REPLICA_URLS,
REQUEST_TIMEOUT,
SLOW_PROVIDER_TIMEOUT_SECONDS,
settle_propagation,
@ -79,10 +83,11 @@ from transport import HttpTransport, SplitTransport, Transport
RowsPredicate = Callable[[list[SpendLogRow]], bool]
# After /model/new, poll data-plane /v1/models until the model is listed (or fail).
# Bound by MODEL_SERVABLE_TIMEOUT so a stuck reload does not burn the spend
# poll_timeout (120s). Return on first listing; settle_propagation owns the separate
# wait that lets every worker and replica reload before the caller uses the model.
# After /model/new, poll /v1/models on every replica in PROXY_REPLICA_URLS until each
# lists the model (or fail). Bound by MODEL_SERVABLE_TIMEOUT per replica so a stuck
# reload does not burn the spend poll_timeout (120s). Return on first listing;
# settle_propagation owns the separate wait that lets the workers behind each replica
# reload before the caller uses the model.
MODEL_SERVABLE_TIMEOUT = 40.0
MODEL_SERVABLE_DB_SYNC_SECONDS = 0.0
MODEL_SERVABLE_INTERVAL = 2.0
@ -108,6 +113,16 @@ class NotServable:
ServableOutcome = Servable | NotServable
type ModelsPoller = Callable[[float], Result[ModelsListResponse]]
@dataclass(frozen=True, slots=True)
class NotServableOn:
"""`NotServable` labeled with the replica whose /v1/models never listed the model."""
replica: str
last_result: Result[ModelsListResponse] | None
def await_servable(
list_models: Callable[[float], Result[ModelsListResponse]],
@ -171,9 +186,41 @@ def await_servable(
sleep(wait)
def await_servable_everywhere(
pollers: Mapping[str, ModelsPoller],
*,
model_name: str,
timeout: float,
interval: float,
request_timeout: float,
db_sync_seconds: float,
now: Callable[[], float],
sleep: Callable[[float], None],
) -> Servable | NotServableOn:
"""`await_servable` against every replica in turn, each with the full budget, so
the model is only servable once every replica has listed it."""
for replica, list_models in pollers.items():
match await_servable(
list_models,
model_name=model_name,
timeout=timeout,
interval=interval,
request_timeout=request_timeout,
db_sync_seconds=db_sync_seconds,
now=now,
sleep=sleep,
):
case NotServable(last_result=last_result):
return NotServableOn(replica=replica, last_result=last_result)
case Servable():
continue
return Servable()
def servable_timeout_message(
*,
model_name: str,
replica: str,
timeout: float,
db_sync_seconds: float,
last_result: Result[ModelsListResponse] | None,
@ -184,8 +231,8 @@ def servable_timeout_message(
else ""
)
return (
f"model {model_name!r} was created but never became servable on the data "
f"plane within {timeout}s of first listing (plus {db_sync_seconds}s continuous "
f"model {model_name!r} was created but never became servable on {replica} "
f"within {timeout}s of first listing (plus {db_sync_seconds}s continuous "
f"DB sync) after /model/new (control/data-plane propagation or "
f"STORE_MODEL_IN_DB reload issue){last_error}"
)
@ -194,6 +241,7 @@ def servable_timeout_message(
@dataclass(frozen=True, slots=True)
class ProxyClient:
transport: Transport
replicas: Mapping[str, Transport]
poll_timeout: float = 120.0
poll_interval: float = 5.0
model_servable_timeout: float = MODEL_SERVABLE_TIMEOUT
@ -310,12 +358,13 @@ class ProxyClient:
model name passed". We poll the data-plane /v1/models until the model
appears, then settle for the remainder of the propagation budget.
Both steps are needed, and the second is the one that matters at >1 replica.
The poll proves *a* replica is serving the model; it cannot prove they all
are, because every request opens a fresh connection and a load-balanced
Service routes each one independently -- so the caller's next request
re-rolls and can land on a replica that has not reloaded yet. Waiting out
PROPAGATION_TIMEOUT is what makes the model safe to use anywhere."""
Both steps are needed. The poll asks every replica in PROXY_REPLICA_URLS
directly, so behind the stack's load balancer it proves each gateway serves
the model rather than whichever one the balancer routed the poll to. It still
cannot see the workers behind a gateway, nor any replica when only the
balancer address is configured (every request opens a fresh connection, so
the caller's next request re-rolls), so waiting out PROPAGATION_TIMEOUT is
what makes the model safe to use anywhere."""
model_id = unwrap(
self.transport.post(
"/model/new",
@ -330,16 +379,10 @@ class ProxyClient:
return model_id
def _await_model_servable(self, model_name: str, listed_for: str | None = None) -> None:
"""Block until the data plane lists `model_name`, or fail at model_servable_timeout."""
headers = self.transport.master if listed_for is None else self.transport.bearer(listed_for)
outcome = await_servable(
lambda poll_timeout: self.transport.get(
"/v1/models",
headers=headers,
params=NoBody(),
response_type=ModelsListResponse,
timeout=poll_timeout,
),
"""Block until every replica lists `model_name`, or fail at model_servable_timeout."""
headers: Final = self.transport.master if listed_for is None else self.transport.bearer(listed_for)
outcome: Final = await_servable_everywhere(
{url: self._models_poller(transport, headers) for url, transport in self.replicas.items()},
model_name=model_name,
timeout=self.model_servable_timeout,
interval=self.model_servable_interval,
@ -351,16 +394,27 @@ class ProxyClient:
match outcome:
case Servable():
return
case NotServable(last_result=last_result):
case NotServableOn(replica=replica, last_result=last_result):
raise AssertionError(
servable_timeout_message(
model_name=model_name,
replica=replica,
timeout=self.model_servable_timeout,
db_sync_seconds=self.model_servable_db_sync_seconds,
last_result=last_result,
)
)
@staticmethod
def _models_poller(transport: Transport, headers: AuthHeaders) -> ModelsPoller:
return lambda poll_timeout: transport.get(
"/v1/models",
headers=headers,
params=NoBody(),
response_type=ModelsListResponse,
timeout=poll_timeout,
)
def update_model(self, model_id: str, litellm_params: LiteLLMParamsBody) -> None:
"""Merge `litellm_params` over the deployment `model_id`'s stored params via
POST /model/update. The proxy overlays only the non-null fields and clears
@ -547,16 +601,20 @@ def build_proxy_client(
base_url: str = PROXY_BASE_URL,
master_key: str = MASTER_KEY,
control_plane_base_url: str = CONTROL_PLANE_BASE_URL,
replica_urls: tuple[str, ...] = PROXY_REPLICA_URLS,
) -> ProxyClient:
"""The ProxyClient every suite's client is built from: a SplitTransport that routes
LLM calls to the data plane (PROXY_BASE_URL) and management/admin calls to the
control plane (CONTROL_PLANE_BASE_URL), with the shared poll budget. The two
base URLs are the same for a monolithic proxy, so routing is then a no-op.
``replica_urls`` (PROXY_REPLICA_URLS) names every data-plane replica the model
barrier polls directly; it is the data-plane URL itself unless the stack
exports each gateway's own address.
The endpoints are injectable for callers that resolve the proxy some other
way than ``e2e_config``'s env names (see ``claude_code/_env.py``); they must
pass all three together, since a caller that overrides only the data plane
would leave management calls pointed at the env default.
pass all four together, since a caller that overrides only the data plane
would leave management calls and the replica poll pointed at the env defaults.
Test-to-proxy traffic always goes over the wire, in every E2E_FIXTURE_MODE:
record and replay scope to the proxy's provider-bound calls via the
@ -573,8 +631,15 @@ def build_proxy_client(
request_timeout=REQUEST_TIMEOUT,
),
)
replicas: Final = MappingProxyType(
{
url: HttpTransport(base_url=url, master_key=master_key, request_timeout=REQUEST_TIMEOUT)
for url in replica_urls
}
)
return ProxyClient(
transport=split,
replicas=replicas,
poll_timeout=POLL_TIMEOUT,
poll_interval=POLL_INTERVAL,
)

View file

@ -0,0 +1,87 @@
"""Harness coverage for the model barrier that gates on every replica.
No proxy needed and no ``e2e`` marker: this pins that a model registered through
the control plane only counts as servable once every configured replica lists it
on /v1/models, which is what keeps a two-gateway stack from handing a test a
model that one gateway has not reloaded yet. The fakes are plain pollers and an
injected clock, so nothing here monkeypatches anything.
"""
from __future__ import annotations
from collections.abc import Iterable, Mapping
from dataclasses import dataclass
from itertools import chain, repeat
from typing import Final
import pytest
from e2e_config import parse_replica_urls
from e2e_http import Success
from models import ModelListEntry, ModelsListResponse
from proxy_client import ModelsPoller, NotServableOn, Servable, await_servable_everywhere
MODEL: Final = "gpt-under-test"
TIMEOUT: Final = 10.0
INTERVAL: Final = 2.0
@dataclass
class FakeClock:
elapsed: float = 0.0
def now(self) -> float:
return self.elapsed
def sleep(self, seconds: float) -> None:
self.elapsed += seconds
def _listing(*model_ids: str) -> Success[ModelsListResponse]:
entries: Final = tuple(ModelListEntry(id=model_id) for model_id in model_ids)
return Success(status_code=200, data=ModelsListResponse(data=entries))
def _poller(results: Iterable[Success[ModelsListResponse]]) -> ModelsPoller:
it: Final = iter(results)
return lambda _timeout: next(it)
def _await(pollers: Mapping[str, ModelsPoller]) -> Servable | NotServableOn:
clock: Final = FakeClock()
return await_servable_everywhere(
pollers,
model_name=MODEL,
timeout=TIMEOUT,
interval=INTERVAL,
request_timeout=5.0,
db_sync_seconds=0.0,
now=clock.now,
sleep=clock.sleep,
)
class TestAwaitServableEverywhere:
@pytest.mark.parametrize("missing", ["gateway-1", "gateway-2"])
def test_fails_on_the_replica_that_never_lists_the_model(self, missing: str) -> None:
pollers: Final = {
"gateway-1": _poller(repeat(_listing(MODEL))),
"gateway-2": _poller(repeat(_listing(MODEL))),
} | {missing: _poller(repeat(_listing()))}
assert _await(pollers) == NotServableOn(replica=missing, last_result=_listing())
def test_passes_once_every_replica_lists_the_model(self) -> None:
pollers: Final = {
"gateway-1": _poller(repeat(_listing(MODEL))),
"gateway-2": _poller(chain(repeat(_listing(), 2), repeat(_listing(MODEL)))),
}
assert _await(pollers) == Servable()
class TestParseReplicaUrls:
def test_splits_and_trims_the_gateway_addresses(self) -> None:
raw: Final = " http://127.0.0.1:4010/, http://127.0.0.1:4011 "
assert parse_replica_urls(raw, "http://lb") == ("http://127.0.0.1:4010", "http://127.0.0.1:4011")
def test_falls_back_to_the_data_plane_address_when_unset(self) -> None:
assert parse_replica_urls("", "http://lb") == ("http://lb",)