mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-05 08:07:05 +00:00
fix(proxy): build coordination Redis from REDIS_* env vars unconditionally (#39410)
Coordination Redis (spend counters, budget-window enforcement, and the reset_spend cache-eviction broadcast) previously only attached when a deployment set general_settings.coordination_redis or litellm_settings.cache. Bare REDIS_HOST/REDIS_PORT env vars alone did nothing, so a multi-replica proxy with no cache block got no cross-pod coordination at all: a key reset on one pod never cleared another pod's stale budget enforcement. The inferred Redis is pinged before being adopted, and a malformed REDIS_CLUSTER_NODES/REDIS_SENTINEL_NODES value is tolerated too: env vars can be set for an unrelated reason with nothing reachable there, and guessing wrong must not turn a previously harmless in-memory-only proxy into one that fails to boot or raises on its next cache write.
This commit is contained in:
parent
0c0b432cf1
commit
6c5fb0ef6f
2 changed files with 201 additions and 25 deletions
|
|
@ -4744,6 +4744,63 @@ class ProxyConfig:
|
|||
)
|
||||
return coordination_redis_cache
|
||||
|
||||
@staticmethod
|
||||
async def _init_coordination_redis_env_fallback(litellm_settings: Mapping[str, object]) -> RedisCache | None:
|
||||
"""
|
||||
Last-resort coordination Redis, tried after an explicit
|
||||
`general_settings.coordination_redis` block and `litellm_settings.cache`
|
||||
have both had a chance to resolve one. Without this, a deployment that
|
||||
only exports REDIS_HOST/REDIS_PORT (no cache block, no coordination_redis
|
||||
block) gets NO cross-pod coordination at all: spend counters, budget-window
|
||||
enforcement, and the reset_spend cache-eviction broadcast all silently stay
|
||||
per-pod local, so a key reset on one pod never clears another pod's stale
|
||||
enforcement.
|
||||
|
||||
Unlike the explicit block and cache-backend paths (a deliberate opt-in, so a
|
||||
bad connection target or a malformed REDIS_CLUSTER_NODES/REDIS_SENTINEL_NODES
|
||||
value should fail loudly), this one is inferred from bare env vars that may be
|
||||
set for an unrelated reason -- e.g. a REDIS_HOST left over from a different
|
||||
job/service, or a REDIS_CLUSTER_NODES value nothing here ever asked to be
|
||||
parsed. Wrongly guessing "coordination available" must not turn a previously
|
||||
harmless in-memory-only proxy into one that fails to boot or raises on every
|
||||
cache write, so a malformed value or a failed/slow ping are both treated the
|
||||
same as no REDIS_* vars at all.
|
||||
"""
|
||||
try:
|
||||
env_coordination_redis_cache: Final = _build_redis_usage_cache_from_environment()
|
||||
except Exception as e: # noqa: BLE001 # a malformed inferred Redis env var must not block startup
|
||||
verbose_proxy_logger.warning(
|
||||
"coordination_redis: could not build a Redis client from REDIS_* environment variables "
|
||||
"(%s); cross-pod coordination stays in-memory. Set general_settings.coordination_redis "
|
||||
"explicitly to require it.",
|
||||
e,
|
||||
)
|
||||
return None
|
||||
if env_coordination_redis_cache is None:
|
||||
return None
|
||||
try:
|
||||
reachable: Final = await asyncio.wait_for(env_coordination_redis_cache.ping(), timeout=2.0)
|
||||
except Exception as e: # noqa: BLE001 # an unreachable inferred Redis must not block startup or writes
|
||||
verbose_proxy_logger.warning(
|
||||
"coordination_redis: REDIS_* environment variables named a Redis that is not reachable "
|
||||
"(%s); cross-pod coordination stays in-memory. Set general_settings.coordination_redis "
|
||||
"explicitly to require it.",
|
||||
e,
|
||||
)
|
||||
return None
|
||||
if not reachable:
|
||||
return None
|
||||
_attach_redis_usage_cache(
|
||||
env_coordination_redis_cache,
|
||||
enable_redis_auth_cache=litellm_settings.get("enable_redis_auth_cache", False) is True,
|
||||
)
|
||||
verbose_proxy_logger.info(
|
||||
"coordination_redis: using a standalone Redis built from REDIS_* "
|
||||
"environment variables for usage tracking, rate limiting, and "
|
||||
"cross-pod coordination."
|
||||
)
|
||||
return env_coordination_redis_cache
|
||||
|
||||
def _init_cache(
|
||||
self,
|
||||
cache_params: dict,
|
||||
|
|
@ -5412,6 +5469,13 @@ class ProxyConfig:
|
|||
reset_audit_log_callback_cache()
|
||||
_in_memory_loggers[:] = [cb for cb in _in_memory_loggers if not isinstance(cb, S3V2Logger)]
|
||||
|
||||
if redis_usage_cache is None:
|
||||
env_coordination_redis_cache: Final = await self._init_coordination_redis_env_fallback(
|
||||
litellm_settings=litellm_settings
|
||||
)
|
||||
if env_coordination_redis_cache is not None:
|
||||
_set_redis_usage_cache(env_coordination_redis_cache)
|
||||
|
||||
## GENERAL SERVER SETTINGS (e.g. master key,..) # do this after initializing litellm, to ensure sentry logging works for proxylogging
|
||||
general_settings = config.get("general_settings", {})
|
||||
if general_settings is None:
|
||||
|
|
|
|||
|
|
@ -10783,11 +10783,54 @@ def test_update_config_redacts_all_environment_variable_values(_update_config_se
|
|||
class _EnvBuiltRedisCache(RedisCache):
|
||||
"""RedisCache stand-in that records its constructor kwargs and never
|
||||
opens a network connection, so tests can assert which connection params
|
||||
the proxy used to build its coordination Redis."""
|
||||
the proxy used to build its coordination Redis. `ping()` reports reachable
|
||||
by default, matching a real Redis the env fallback should adopt."""
|
||||
|
||||
def __init__(self, **kwargs):
|
||||
self.init_kwargs = kwargs
|
||||
|
||||
async def ping(self) -> bool:
|
||||
return True
|
||||
|
||||
|
||||
class _UnreachableRedisCache(_EnvBuiltRedisCache):
|
||||
"""Same stand-in, but `ping()` fails like a REDIS_* env var naming a Redis
|
||||
that is not actually reachable (wrong host, no service running, ...)."""
|
||||
|
||||
async def ping(self) -> bool:
|
||||
raise ConnectionError("connection refused")
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _patched_coordination_redis_module_state(
|
||||
*,
|
||||
spend_cache: DualCache,
|
||||
config_cache: types.SimpleNamespace,
|
||||
redis_cache_class: type = _EnvBuiltRedisCache,
|
||||
):
|
||||
"""Stub every `litellm.proxy.proxy_server` global that
|
||||
`_attach_redis_usage_cache` (and its callers) can write to, shared by the
|
||||
whole coordination-Redis test family below.
|
||||
|
||||
Centralizing this is not just DRY: `_attach_redis_usage_cache` always sets
|
||||
`cli_sso_session_cache.redis_cache` unconditionally, and a call site that
|
||||
forgets to patch that one real (persistent) global leaks a throwaway
|
||||
Redis stand-in into it for the rest of the pytest session, breaking
|
||||
unrelated tests that run later. One patched-state helper means a new call
|
||||
site cannot forget a global this family already knows to isolate.
|
||||
"""
|
||||
with (
|
||||
patch.object(proxy_server_module, "redis_usage_cache", None),
|
||||
patch.object(proxy_server_module, "spend_counter_cache", spend_cache),
|
||||
patch.object(proxy_server_module, "user_api_key_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "cli_sso_session_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "llm_router", None),
|
||||
patch.object(proxy_server_module, "litellm_config_cache", config_cache),
|
||||
patch.object(proxy_server_module, "RedisCache", redis_cache_class),
|
||||
patch.object(proxy_server_module, "RedisClusterCache", _EnvBuiltClusterCache),
|
||||
):
|
||||
yield
|
||||
|
||||
|
||||
def _run_init_cache_with_backend(cache_backend, redis_env_kwargs):
|
||||
"""Run ProxyConfig._init_cache with a stubbed response-cache backend and a
|
||||
|
|
@ -10799,12 +10842,7 @@ def _run_init_cache_with_backend(cache_backend, redis_env_kwargs):
|
|||
fresh_config_cache = types.SimpleNamespace(redis_cache=None)
|
||||
|
||||
with (
|
||||
patch.object(proxy_server_module, "redis_usage_cache", None),
|
||||
patch.object(proxy_server_module, "spend_counter_cache", fresh_spend_cache),
|
||||
patch.object(proxy_server_module, "user_api_key_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "llm_router", None),
|
||||
patch.object(proxy_server_module, "litellm_config_cache", fresh_config_cache),
|
||||
patch.object(proxy_server_module, "RedisCache", _EnvBuiltRedisCache),
|
||||
_patched_coordination_redis_module_state(spend_cache=fresh_spend_cache, config_cache=fresh_config_cache),
|
||||
patch(
|
||||
"litellm._redis._redis_kwargs_from_environment",
|
||||
return_value=redis_env_kwargs,
|
||||
|
|
@ -10879,12 +10917,7 @@ def _run_init_coordination_redis(config, env=None):
|
|||
fresh_config_cache = types.SimpleNamespace(redis_cache=None)
|
||||
|
||||
with (
|
||||
patch.object(proxy_server_module, "redis_usage_cache", None),
|
||||
patch.object(proxy_server_module, "spend_counter_cache", fresh_spend_cache),
|
||||
patch.object(proxy_server_module, "user_api_key_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "litellm_config_cache", fresh_config_cache),
|
||||
patch.object(proxy_server_module, "RedisCache", _EnvBuiltRedisCache),
|
||||
patch.object(proxy_server_module, "RedisClusterCache", _EnvBuiltClusterCache),
|
||||
_patched_coordination_redis_module_state(spend_cache=fresh_spend_cache, config_cache=fresh_config_cache),
|
||||
mock.patch.dict(os.environ, env or {}, clear=False),
|
||||
):
|
||||
built = proxy_server_module.ProxyConfig()._init_coordination_redis(config=config)
|
||||
|
|
@ -10972,13 +11005,7 @@ def test_explicit_coordination_redis_takes_precedence_over_cache_backend():
|
|||
mock_litellm_cache.cache = cache_backend
|
||||
|
||||
with (
|
||||
patch.object(proxy_server_module, "redis_usage_cache", None),
|
||||
patch.object(proxy_server_module, "spend_counter_cache", fresh_spend_cache),
|
||||
patch.object(proxy_server_module, "user_api_key_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "llm_router", None),
|
||||
patch.object(proxy_server_module, "litellm_config_cache", fresh_config_cache),
|
||||
patch.object(proxy_server_module, "RedisCache", _EnvBuiltRedisCache),
|
||||
patch.object(proxy_server_module, "RedisClusterCache", _EnvBuiltClusterCache),
|
||||
_patched_coordination_redis_module_state(spend_cache=fresh_spend_cache, config_cache=fresh_config_cache),
|
||||
patch("litellm.Cache", return_value=mock_litellm_cache),
|
||||
):
|
||||
litellm.cache = None
|
||||
|
|
@ -10996,6 +11023,95 @@ def test_explicit_coordination_redis_takes_precedence_over_cache_backend():
|
|||
assert fresh_spend_cache.redis_cache is usage_cache
|
||||
|
||||
|
||||
async def _run_init_coordination_redis_env_fallback(
|
||||
litellm_settings, redis_env_kwargs, redis_cache_class=_EnvBuiltRedisCache
|
||||
):
|
||||
"""Run ProxyConfig._init_coordination_redis_env_fallback against a
|
||||
stubbed module state and a controlled REDIS_* environment, returning
|
||||
(built, spend_counter redis)."""
|
||||
fresh_spend_cache = DualCache()
|
||||
fresh_config_cache = types.SimpleNamespace(redis_cache=None)
|
||||
|
||||
with (
|
||||
_patched_coordination_redis_module_state(
|
||||
spend_cache=fresh_spend_cache, config_cache=fresh_config_cache, redis_cache_class=redis_cache_class
|
||||
),
|
||||
patch(
|
||||
"litellm._redis._redis_kwargs_from_environment",
|
||||
return_value=redis_env_kwargs,
|
||||
),
|
||||
):
|
||||
built = await proxy_server_module.ProxyConfig._init_coordination_redis_env_fallback(
|
||||
litellm_settings=litellm_settings
|
||||
)
|
||||
return built, fresh_spend_cache.redis_cache
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_init_coordination_redis_env_fallback_builds_from_environment():
|
||||
"""A deployment with no coordination_redis block and no litellm_settings.cache
|
||||
but with bare REDIS_HOST/REDIS_PORT env vars must still get a coordination
|
||||
Redis: otherwise spend counters, budget-window enforcement, and the
|
||||
reset_spend cache-eviction broadcast stay per-pod local and a reset issued
|
||||
on one pod never clears another pod's stale enforcement."""
|
||||
built, spend_redis = await _run_init_coordination_redis_env_fallback(
|
||||
litellm_settings={},
|
||||
redis_env_kwargs={"host": "env-fallback-host", "port": "6390"},
|
||||
)
|
||||
|
||||
assert isinstance(built, _EnvBuiltRedisCache)
|
||||
assert built.init_kwargs["host"] == "env-fallback-host"
|
||||
assert spend_redis is built
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_init_coordination_redis_env_fallback_without_redis_env_returns_none():
|
||||
"""With no REDIS_* connection info at all, the fallback must leave the
|
||||
coordination Redis unset rather than building a broken client."""
|
||||
built, spend_redis = await _run_init_coordination_redis_env_fallback(
|
||||
litellm_settings={},
|
||||
redis_env_kwargs={},
|
||||
)
|
||||
|
||||
assert built is None
|
||||
assert spend_redis is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_init_coordination_redis_env_fallback_unreachable_stays_in_memory():
|
||||
"""REDIS_* env vars can name a Redis that is not actually reachable (wrong
|
||||
host, leftover from an unrelated job/service). Guessing "coordination
|
||||
available" from bare env vars must not turn a previously harmless
|
||||
in-memory-only proxy into one that raises on its next cache write, so an
|
||||
unreachable ping must leave everything exactly as if no REDIS_* vars were
|
||||
set at all."""
|
||||
built, spend_redis = await _run_init_coordination_redis_env_fallback(
|
||||
litellm_settings={},
|
||||
redis_env_kwargs={"host": "unreachable-host", "port": "6390"},
|
||||
redis_cache_class=_UnreachableRedisCache,
|
||||
)
|
||||
|
||||
assert built is None
|
||||
assert spend_redis is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_init_coordination_redis_env_fallback_malformed_cluster_nodes_stays_in_memory():
|
||||
"""REDIS_CLUSTER_NODES can be set to a malformed value nothing here ever
|
||||
asked to be parsed. Unlike the explicit coordination_redis block (a
|
||||
deliberate opt-in, so a bad value there should fail loudly), this
|
||||
inferred fallback must not abort proxy startup over it -- it has to
|
||||
decline the same way it does for an absent or unreachable Redis."""
|
||||
with mock.patch.dict(os.environ, {"REDIS_CLUSTER_NODES": "not-valid-json"}, clear=False):
|
||||
built, spend_redis = await _run_init_coordination_redis_env_fallback(
|
||||
litellm_settings={},
|
||||
redis_env_kwargs={},
|
||||
)
|
||||
|
||||
assert built is None
|
||||
assert spend_redis is None
|
||||
|
||||
|
||||
def test_env_fallback_builds_cluster_client_from_cluster_nodes_env():
|
||||
"""A deployment whose only Redis env is REDIS_CLUSTER_NODES must still get
|
||||
a coordination Redis from the env fallback, and it must be a cluster
|
||||
|
|
@ -11037,11 +11153,7 @@ async def test_startup_applies_coordination_redis_saved_in_database():
|
|||
fresh_config_cache = types.SimpleNamespace(redis_cache=None)
|
||||
|
||||
with (
|
||||
patch.object(proxy_server_module, "spend_counter_cache", fresh_spend_cache),
|
||||
patch.object(proxy_server_module, "user_api_key_cache", DualCache()),
|
||||
patch.object(proxy_server_module, "litellm_config_cache", fresh_config_cache),
|
||||
patch.object(proxy_server_module, "RedisCache", _EnvBuiltRedisCache),
|
||||
patch.object(proxy_server_module, "RedisClusterCache", _EnvBuiltClusterCache),
|
||||
_patched_coordination_redis_module_state(spend_cache=fresh_spend_cache, config_cache=fresh_config_cache),
|
||||
patch.object(
|
||||
proxy_server_module,
|
||||
"get_persisted_coordination_redis_settings",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue