From 6c5fb0ef6f14775d33ff0e0ce5dff8dde0742c52 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Wed, 2 Sep 2026 15:21:15 -0700 Subject: [PATCH] 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. --- litellm/proxy/proxy_server.py | 64 +++++++ tests/test_litellm/proxy/test_proxy_server.py | 162 +++++++++++++++--- 2 files changed, 201 insertions(+), 25 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 38488c03922..1e0792cb8a5 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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: diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index b18fc373cae..7322c6e62a5 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -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",