diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index b6422d7f5ae..41904e3883b 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -324,7 +324,7 @@ async def patch_model( # Clear cache and reload models (uses config setting or defaults to preserving config models for DB updates) live_before_reload = live_model_ids_snapshot() - await clear_cache() + still_desired_ids = await clear_cache() ## CREATE AUDIT LOG ## asyncio.create_task( @@ -344,6 +344,7 @@ async def patch_model( before=live_before_reload, written_models=[(model_id, getattr(updated_model, "model_info", None))], action="update", + still_desired=still_desired_ids, ) return updated_model @@ -429,7 +430,7 @@ async def _set_model_blocked_status( ) live_before_reload = live_model_ids_snapshot() - await clear_cache() + still_desired_ids = await clear_cache() asyncio.create_task( create_object_audit_log( @@ -450,6 +451,7 @@ async def _set_model_blocked_status( before=live_before_reload, written_models=[(data.model_id, getattr(updated_model, "model_info", None))], action=action, + still_desired=still_desired_ids, ) return updated_model @@ -1355,6 +1357,7 @@ async def add_new_model( """ live_before_reload = live_model_ids_snapshot() + still_desired_ids: frozenset[str] | None = None try: _original_litellm_model_name = model_params.model_name if model_params.model_info.team_id is None: @@ -1369,7 +1372,9 @@ async def add_new_model( user_api_key_dict=user_api_key_dict, prisma_client=prisma_client, ) - await proxy_config.add_deployment(prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj) + still_desired_ids = await proxy_config.add_deployment( + prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj + ) # don't let failed slack alert block the /model/new response _alerting = general_settings.get("alerting", []) or [] if "slack" in _alerting: @@ -1414,6 +1419,7 @@ async def add_new_model( before=live_before_reload, written_models=[(model_response.model_id, getattr(model_response, "model_info", None))], action="create", + still_desired=still_desired_ids, ) return model_response @@ -1542,7 +1548,7 @@ async def update_model( # Clear cache and reload models (uses config setting or defaults to preserving config models for DB updates) live_before_reload = live_model_ids_snapshot() - await clear_cache() + still_desired_ids = await clear_cache() ## CREATE AUDIT LOG ## asyncio.create_task( create_object_audit_log( @@ -1569,6 +1575,7 @@ async def update_model( before=live_before_reload, written_models=[(_model_id, getattr(model_response, "model_info", None))], action="update", + still_desired=still_desired_ids, ) return model_response @@ -1814,6 +1821,7 @@ def reload_serving_verdict( before: frozenset[str], written_models: Sequence[tuple[str, object]], written_must_serve: bool, + still_desired: frozenset[str] | None = None, ) -> tuple[tuple[str, ...], tuple[str, ...]]: """Judge a write-triggered reload by diffing the router's serving state instead of trusting any layer of the reload stack to report its own failure. @@ -1828,10 +1836,14 @@ def reload_serving_verdict( this write and blaming it would block unrelated metadata fixes - not written but live before and gone now: collateral degradation of this pod caused by the reload this request triggered (a wholesale re-add failure, or a - newly introduced conflict), always reported + newly introduced conflict), reported only when the db still wants that id + + ``still_desired`` is the db + config id set the reload just reconciled against. An + id absent from it was deleted on purpose, most often by another pod this one had not + yet polled, so the reload dropping it is the reconcile working rather than damage. + Without it (no reconcile ran) every drop is reported, which is the safe direction. Returns (written ids violating their obligation, collateral ids no longer served). - Best effort under concurrent admin writes: the snapshot spans only this request. """ now = live_model_ids_snapshot() written_ids = frozenset(model_id for model_id, _ in written_models) @@ -1843,7 +1855,8 @@ def reload_serving_verdict( ) else: missing = tuple(model_id for model_id, _ in written_models if model_id in before and model_id not in now) - collateral = tuple(sorted(before - now - written_ids)) + dropped = before - now - written_ids + collateral = tuple(sorted(dropped if still_desired is None else dropped & still_desired)) return (missing, collateral) @@ -1851,12 +1864,18 @@ def raise_if_reload_degraded_serving( before: frozenset[str], written_models: Sequence[tuple[str, object]], action: str, + still_desired: frozenset[str] | None = None, ) -> None: """The caller-visible error this pod's model-write endpoints owe their caller when the model they wrote is not being served after the reload they triggered. The DB write is durable either way and every other pod reloads on its own interval; this speaks only for the handling pod.""" - missing, collateral = reload_serving_verdict(before=before, written_models=written_models, written_must_serve=True) + missing, collateral = reload_serving_verdict( + before=before, + written_models=written_models, + written_must_serve=True, + still_desired=still_desired, + ) if not missing and not collateral: return missing_clause = ( @@ -1882,9 +1901,12 @@ def raise_if_reload_degraded_serving( ) -async def clear_cache(): +async def clear_cache() -> frozenset[str] | None: """ Clear router caches and reload models. + + Returns the db + config id set the reload reconciled against, or None when no + reload ran, so callers can pass it to raise_if_reload_degraded_serving. """ from litellm.proxy.proxy_server import ( llm_router, @@ -1896,7 +1918,7 @@ async def clear_cache(): if llm_router is None or prisma_client is None: verbose_proxy_logger.debug("llm_router or prisma_client is None, skipping cache clear") - return + return None try: # Only clear DB models, preserve config models @@ -1943,10 +1965,14 @@ async def clear_cache(): llm_router.quality_routers.pop(model_name, None) # Reload only DB models - await proxy_config.add_deployment(prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj) + still_desired_ids = await proxy_config.add_deployment( + prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj + ) verbose_proxy_logger.debug( f"Cleared {len(db_model_ids)} DB models, preserved {len(config_models)} config models" ) + return still_desired_ids except Exception as e: verbose_proxy_logger.exception(f"Failed to clear cache and reload models. Due to error - {str(e)}") + return None diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index a60ea2da019..15d977b75f2 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -5300,7 +5300,7 @@ class ProxyConfig: _model_info = RouterModelInfo(id=model.model_id, db_model=db_model) return _model_info - async def _delete_deployment(self, db_models: list) -> int: + async def _delete_deployment(self, db_models: list) -> frozenset[str] | None: """ (Helper function of add deployment) -> combined to reduce prisma db calls @@ -5309,14 +5309,16 @@ class ProxyConfig: - Remove any that are missing Return: - - int - returns number of deleted deployments + - frozenset[str] - the ids the db + config say should be served after this + reconcile, so a caller can tell an id this evicted on purpose from one that + went missing. None when no reconcile ran and that set is therefore unknown. """ global user_config_file_path, llm_router combined_id_list = [] ## BASE CASES ## if llm_router is None: - return 0 + return None # NOTE: db_models may be legitimately empty when all DB models have been deleted. # Do NOT short-circuit on len(db_models) == 0 — we must still evict any # DB-sourced deployments that are no longer in the DB. The caller @@ -5337,7 +5339,7 @@ class ProxyConfig: "Skipping deployment cleanup to avoid removing valid models.", str(e), ) - return 0 + return None model_list = config.get("model_list", None) if model_list: for model in model_list: @@ -5361,13 +5363,10 @@ class ProxyConfig: router_model_ids = llm_router.get_model_ids() # Check for model IDs in llm_router not present in combined_id_list and delete them - deleted_deployments = 0 for model_id in router_model_ids: if model_id not in combined_id_list: - is_deleted = llm_router.delete_deployment(id=model_id) - if is_deleted is not None: - deleted_deployments += 1 - return deleted_deployments + llm_router.delete_deployment(id=model_id) + return frozenset(combined_id_list) def _resolve_db_litellm_param(self, key: str, value: object) -> object: if not isinstance(value, str): @@ -5452,9 +5451,11 @@ class ProxyConfig: self, new_models: Optional[Json], proxy_logging_obj: ProxyLogging, - ): + ) -> frozenset[str] | None: global llm_router, llm_model_list, master_key, general_settings + still_desired_ids: frozenset[str] | None = None + # Load config separately so a timeout here doesn't block model loading config_data: dict = {} search_tools = None @@ -5501,7 +5502,7 @@ class ProxyConfig: if search_tools is not None and llm_router is not None: llm_router.search_tools = search_tools ## DELETE MODEL LOGIC - await self._delete_deployment(db_models=models_list) + still_desired_ids = await self._delete_deployment(db_models=models_list) ## ADD MODEL LOGIC self._add_deployment(db_models=models_list) @@ -5527,6 +5528,8 @@ class ProxyConfig: proxy_logging_obj=proxy_logging_obj, ) + return still_desired_ids + def _add_callback_from_db_to_in_memory_litellm_callbacks( self, callback: str, @@ -6159,14 +6162,20 @@ class ProxyConfig: self, prisma_client: PrismaClient, proxy_logging_obj: ProxyLogging, - ): + ) -> frozenset[str] | None: """ - Check db for new models - Check if model id's in router already - If not, add to router + + Returns the ids the db + config say should be served after the reconcile, or + None when no reconcile ran. Callers that judge their own reload need it to tell + a deliberate eviction from a deployment that went missing. """ global llm_router, llm_model_list, master_key, general_settings + still_desired_ids: frozenset[str] | None = None + try: # warm the config cache so the per-param reads below all hit await prefetch_config_params( @@ -6186,7 +6195,9 @@ class ProxyConfig: new_models = await self._get_models_from_db(prisma_client=prisma_client) # update llm router - await self._update_llm_router(new_models=new_models, proxy_logging_obj=proxy_logging_obj) + still_desired_ids = await self._update_llm_router( + new_models=new_models, proxy_logging_obj=proxy_logging_obj + ) db_general_settings = await get_config_param(prisma_client, "general_settings") @@ -6204,6 +6215,8 @@ class ProxyConfig: "litellm.proxy.proxy_server.py::ProxyConfig:add_deployment - {}".format(str(e)) ) + return still_desired_ids + async def _init_non_llm_objects_in_db(self, prisma_client: PrismaClient): """ Use this to read non-llm objects from the db and initialize them diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index 1e8add52f74..8dbf38555b3 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -839,7 +839,7 @@ class TestUpdateModel: ), patch( "litellm.proxy.management_endpoints.model_management_endpoints.clear_cache", - new=AsyncMock(return_value=True), + new=AsyncMock(return_value=None), ) as mock_clear_cache, ): await update_model( @@ -3223,7 +3223,7 @@ class TestPatchModelBlockedAuthGate: ), patch( "litellm.proxy.management_endpoints.model_management_endpoints.clear_cache", - new=AsyncMock(return_value=True), + new=AsyncMock(return_value=None), ), ): result = await patch_model( @@ -3314,6 +3314,72 @@ class TestWriteSurfacesReloadDrop: before=frozenset({"m-live", "m-collateral"}), written_models=[("m-live", None)], action="update" ) + def test_a_model_the_db_no_longer_has_is_not_collateral(self, monkeypatch): + """Another pod deleting a model is not this pod's reload breaking. + + A pod that has not yet polled the delete still lists the id when the write + snapshots `before`; the reload it triggers then evicts the id because the db no + longer has it. That eviction is the reconcile working, so it must not fail the + write. `still_desired` is the db + config set the reload reconciled against, so + an id missing from it drops out of the collateral diff. + + The cases below, in order: an id the db no longer wants is not collateral and the + write succeeds; an id the db still wants that stopped serving is real degradation + and still raises, so a genuinely broken reload is caught; and with no reconcile at + all the desired set is unknown, so every drop is reported. + """ + import litellm + from litellm.proxy._types import ProxyException + from litellm.proxy.management_endpoints.model_management_endpoints import ( + raise_if_reload_degraded_serving, + reload_serving_verdict, + ) + + live_router = litellm.Router( + model_list=[ + { + "model_name": "gpt-4o", + "litellm_params": {"model": "gpt-4o"}, + "model_info": {"id": "m-live", "db_model": True}, + } + ] + ) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", live_router) + + _, collateral = reload_serving_verdict( + before=frozenset({"m-live", "m-deleted-elsewhere"}), + written_models=[("m-live", None)], + written_must_serve=True, + still_desired=frozenset({"m-live"}), + ) + assert collateral == () + + assert ( + raise_if_reload_degraded_serving( + before=frozenset({"m-live", "m-deleted-elsewhere"}), + written_models=[("m-live", None)], + action="create", + still_desired=frozenset({"m-live"}), + ) + is None + ) + + with pytest.raises(ProxyException, match="m-should-be-serving"): + raise_if_reload_degraded_serving( + before=frozenset({"m-live", "m-should-be-serving"}), + written_models=[("m-live", None)], + action="create", + still_desired=frozenset({"m-live", "m-should-be-serving"}), + ) + + with pytest.raises(ProxyException, match="m-deleted-elsewhere"): + raise_if_reload_degraded_serving( + before=frozenset({"m-live", "m-deleted-elsewhere"}), + written_models=[("m-live", None)], + action="create", + still_desired=None, + ) + class TestModelInfoAsMapping: """The model_info column reaches consumers as a dict or as its JSON string; this is diff --git a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py index 8d1d8185e4d..99287b92b8f 100644 --- a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py @@ -1404,12 +1404,12 @@ def test_ProxyConfig_get_model_info_with_id_missing_model_id_raises(monkeypatch) @pytest.mark.asyncio -async def test_ProxyConfig__delete_deployment_empty_returns_zero(monkeypatch): +async def test_ProxyConfig__delete_deployment_no_router_returns_none(monkeypatch): monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", None) pc = ProxyConfig() result = await pc._delete_deployment(db_models=[]) - snapshot = {"deleted": result, "router_was": "none", "empty_db_models": True} - assert snapshot == {"deleted": 0, "router_was": "none", "empty_db_models": True} + snapshot = {"still_desired": result, "router_was": "none", "empty_db_models": True} + assert snapshot == {"still_desired": None, "router_was": "none", "empty_db_models": True} @pytest.mark.asyncio diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index b9a33bd2cef..1ecf7e8b83a 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -2459,13 +2459,11 @@ async def test_delete_deployment_type_mismatch(): patch("litellm.proxy.proxy_server.user_config_file_path", "test_config.yaml"), ): # Call the function under test - deleted_count = await pc._delete_deployment(db_models=[]) + still_desired = await pc._delete_deployment(db_models=[]) # The two SHA-hash models have no corresponding entry in combined_id_list # and must be evicted. - assert ( - deleted_count == 2 - ), f"Expected 2 deletions (SHA-hash models), got {deleted_count}" + assert len(deleted_ids) == 2, f"Expected 2 deletions (SHA-hash models), got {deleted_ids}" assert ( "a96e12e76b36a57cfae57a41288eb41567629cac89b4828c6f7074afc3534695" in deleted_ids @@ -2485,6 +2483,12 @@ async def test_delete_deployment_type_mismatch(): "12345679" not in deleted_ids ), f"Model 12345679 should NOT be deleted. Deleted IDs: {deleted_ids}" + assert still_desired is not None + assert {"12345678", "12345679"} <= still_desired, ( + "the int-keyed config models must come back as strings in the desired set, so a " + f"caller judging its own reload reads them as wanted rather than evicted; got {still_desired}" + ) + @pytest.mark.asyncio async def test_get_config_from_file(tmp_path, monkeypatch): @@ -9119,10 +9123,13 @@ class TestDeleteDeploymentSync: with patch.object( proxy_config, "get_config", AsyncMock(return_value={"model_list": []}) ): - count = await proxy_config._delete_deployment(db_models=[]) + still_desired = await proxy_config._delete_deployment(db_models=[]) mock_router.delete_deployment.assert_called_once_with(id="model-id-to-evict") - assert count == 1 + assert still_desired == frozenset(), ( + "an empty db and an empty config want nothing, which must stay distinct from " + f"the None returned when no reconcile ran at all; got {still_desired}" + ) @pytest.mark.asyncio async def test_update_llm_router_skips_update_on_db_fetch_failure(self): diff --git a/tests/test_litellm/proxy/test_update_llm_router_resilience.py b/tests/test_litellm/proxy/test_update_llm_router_resilience.py index fd0df4805e6..a7dc9c1783e 100644 --- a/tests/test_litellm/proxy/test_update_llm_router_resilience.py +++ b/tests/test_litellm/proxy/test_update_llm_router_resilience.py @@ -115,8 +115,10 @@ class TestDeleteDeploymentResilience: """Test _delete_deployment handles get_config failures gracefully.""" @pytest.mark.asyncio - async def test_returns_zero_when_get_config_times_out(self): - """Should return 0 (no deletions) when get_config fails, not raise.""" + async def test_returns_none_when_get_config_times_out(self): + """Should return None (no reconcile ran, desired set unknown) when get_config + fails, not raise. A caller judging its own reload must not read that as "the db + wants nothing" and blame the reload for every model it serves.""" proxy_config = ProxyConfig() db_models = [_make_db_model("gpt-5.1", "db-id-1")] @@ -136,8 +138,8 @@ class TestDeleteDeploymentResilience: ): result = await proxy_config._delete_deployment(db_models=db_models) - # Should safely return 0 instead of raising - assert result == 0 + # Should safely return None instead of raising + assert result is None # Should NOT have deleted any deployments mock_router.delete_deployment.assert_not_called() @@ -175,5 +177,8 @@ class TestDeleteDeploymentResilience: result = await proxy_config._delete_deployment(db_models=db_models) # "stale-id" should have been deleted (not in db_models or config) - assert result == 1 mock_router.delete_deployment.assert_called_once_with(id="stale-id") + assert result == frozenset({"db-id-1", "config-id-1"}), ( + "the returned set must be what the db + config still want, so a caller can " + f"tell that eviction apart from a deployment that went missing; got {result}" + ) diff --git a/tests/test_litellm/test_model_block_unblock.py b/tests/test_litellm/test_model_block_unblock.py index dc0098e405e..ff66bedf0dc 100644 --- a/tests/test_litellm/test_model_block_unblock.py +++ b/tests/test_litellm/test_model_block_unblock.py @@ -36,7 +36,7 @@ def _setup_model_block_mocks(monkeypatch, *, updated_blocked: bool): mock_router = MagicMock() mock_router.get_model_ids.return_value = [model_id] - mock_clear_cache = AsyncMock(return_value=True) + mock_clear_cache = AsyncMock(return_value=None) mock_audit_log = AsyncMock(return_value=None) monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client)