test(e2e): add models_mgmt suite covering add/update/delete persistence and route permissions - #32272
test(e2e): add models_mgmt suite covering add/update/delete persistence and route permissions#32272mubashir1osmani wants to merge 8 commits into
Conversation
…eway.create_model The split-transport routing table listed only /model/info as a control-plane prefix, so /model/new and /model/delete were sent to the data-plane gateway, which does not serve management routes and 404s them. Every suite that registers deployments at runtime (llm_translation, batches, access_control) failed on the split stage deployment because of this. Widen the prefix to /model/ so all model-management routes reach the control plane while /models stays on the data plane. Separately, batch_client.py and several llm_translation tests call gateway.create_model, but Gateway never had that method, so all 17 batch tests errored at fixture setup with AttributeError. Add create_model/delete_model to Gateway (with the optional mode that batches needs) and make EndpointsClient delegate to it instead of carrying its own copy. Regression tests cover both: the routing predicate for management vs LLM paths and the Gateway model-management surface via a typed fake Transport. Both fail on the previous code
The recording fake always answered with {"model_id": ...} even when the
caller asked for NoBody, which only validated because pydantic ignores extra
fields by default. Return an empty payload for response types that carry no
fields so a future extra="forbid" on NoBody cannot turn the delete test into
a ValidationError inside the fake
…ce and route permissions
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adds a new
Confidence Score: 4/5Safe to merge — all changes are confined to the e2e test tree with no production code modifications. The core logic is sound and backed by parametrized unit tests plus a live stage run. Two items warrant attention: the immediate (non-polled) tpm assertion after create assumes the control-plane /model/info has no caching layer, and the deliberate omission of cache token-cost fields from DeploymentParams could cause a hard-to-diagnose miss if a future test reads them through litellm_params. Neither is a blocker. tests/e2e/models_mgmt/test_models_mgmt_e2e.py (immediate tpm assertion), tests/e2e/models.py (DeploymentParams field coverage)
|
| Filename | Overview |
|---|---|
| tests/e2e/transport.py | Replaces the narrow /model/info control-plane prefix with /model/ so all management endpoints (new/delete/update/info) are routed to the control plane in split deployments; correct and well-tested. |
| tests/e2e/models.py | Adds DeploymentParams as the new type for ModelInfoEntry.litellm_params; safe for existing callers but silently drops cache token-cost fields. |
| tests/e2e/e2e_gateway.py | Promotes create_model/delete_model into Gateway as canonical shared helpers; structurally clean. |
| tests/e2e/models_mgmt/test_models_mgmt_e2e.py | New suite covering add/update/delete lifecycle; tpm assertion after create is immediate (no poll), relying on control-plane synchrony. |
| tests/e2e/models_mgmt/models_mgmt_client.py | Well-structured client with strict delete, update_model_tpm, and raw-status helpers. |
| tests/e2e/test_e2e_gateway.py | Unit regression tests for Gateway.create_model/delete_model via a typed recording transport. |
| tests/e2e/test_transport.py | Comprehensive parametrized tests for is_control_plane_path covering all management and LLM routes. |
| tests/e2e/llm_translation/endpoints_client.py | Delegates create_model/delete_model to Gateway helpers, eliminating duplicate implementation. |
| tests/e2e/models_mgmt/conftest.py | Minimal conftest registering the covers marker and session-scoped client fixture. |
Reviews (1): Last reviewed commit: "test(e2e): add models_mgmt suite coverin..." | Re-trigger Greptile
Greptile SummaryThis PR adds the
Confidence Score: 4/5Safe to merge; all changes are confined to the test harness and do not touch production code. The new tests/e2e/models.py — the
|
| Filename | Overview |
|---|---|
| tests/e2e/models.py | Introduces DeploymentParams for ModelInfoEntry.litellm_params; drops cache_read_input_token_cost and cache_creation_input_token_cost that were present in the old CustomPricing type, and adds ModelMode, ModelUpdateParams, and ModelUpdateBody. |
| tests/e2e/transport.py | Broadens /model/info control-plane prefix to /model/ so all model management routes route to the control plane; /models (OpenAI list-models route) is unaffected. |
| tests/e2e/e2e_gateway.py | Centralises create_model and delete_model into Gateway; delete_model warns on failure (teardown-safe). |
| tests/e2e/models_mgmt/models_mgmt_client.py | New client for the models-mgmt suite; delete_model uses unwrap (hard-fail during act phase); create_model_status uses send to surface raw HTTP status for permission tests. |
| tests/e2e/models_mgmt/test_models_mgmt_e2e.py | Four live e2e tests covering add/update/delete lifecycle and route-permission contract; polling pattern is correct; teardown uses warn-only delete. |
| tests/e2e/test_e2e_gateway.py | Unit tests for Gateway.create_model/delete_model using a typed fake transport; pins the surface so signature drift is caught before a live run. |
| tests/e2e/test_transport.py | New unit tests for is_control_plane_path; verifies management routes route to control plane and LLM routes stay on data plane. |
| tests/e2e/models_mgmt/conftest.py | Minimal conftest: session-scoped client fixture and covers marker registration; lifecycle fixtures come from the parent conftest. |
| tests/e2e/llm_translation/endpoints_client.py | Delegates create_model/delete_model to the newly centralised Gateway methods, removing duplicated raw-transport calls. |
Comments Outside Diff (2)
-
tests/e2e/models.py, line 301-311 (link)DeploymentParamssilently dropscache_read_input_token_costandcache_creation_input_token_costthat existed inCustomPricing. Withextra="ignore", any cache-pricing values the proxy returns inlitellm_paramsare silently swallowed, making it impossible for a test to assert on them. The PR description callsDeploymentParamsa "superset of the pricing fields the custom-pricing tests read", but it's actually a strict subset ofCustomPricingon those two fields. -
tests/e2e/models_mgmt/models_mgmt_client.py, line 25 (link)Unused import:
ModelNewBodyModelNewBodyis imported here but constructed inline insidecreate_model_status. Static analysers may flag this depending on how they trace usage through inline construction — worth a double-check to confirm it is load-bearing.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "test(e2e): add models_mgmt suite coverin..." | Re-trigger Greptile
DeploymentParams carried only the four fields the models_mgmt tests read, so asserting any other configured knob off /model/info (rate limits, timeouts, routing weight, provider regions, per-second pricing, behavior flags) meant widening the model first. Mirror litellm's LiteLLM_Params field for field with typed optionals, keeping extra=ignore and the CustomPricing-compatible pricing names. mock_response, model_info, the *_router_config blobs, and configurable_clientside_auth_params stay out because they have no typed JSON shape to pin. Verified all 9 stage deployments parse cleanly
…rriAI/litellm into litellm_e2e_models_mgmt_suite
The add test asserted the deployment appeared in /model/info with a single immediate read, which flakes if a read cache ever fronts the endpoint; reuse the suite's poll helper like the update and delete paths already do
|
Folded into the management suite instead of a separate models_mgmt folder, per review direction; the four tests, the ManagementClient model methods, and the models.py types now land via the fold PR referenced above, re-verified live on stage as part of the full 12-test management run |
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewStacked on #32261 (needs Gateway.create_model and the /model/ control-plane routing); merge that first
Screenshots / Proof of Fix
Run live against the split stage deployment (kubectl port-forward litellm-gateway:4000 and litellm-backend:4001, master key from the cluster secret via env):
Post-run /model/info leak check confirmed zero deployments named e2e-mgmt* or e2e-probe* remained
Type
Test
Changes
tests/e2e/CLAUDE.md documents a models_mgmt suite for the model-management routes, but the directory never existed; the mgmt.* registry rows had no live coverage and the routes were only exercised as fixture plumbing by other suites. This adds the suite with four tests covering the add/update/delete lifecycle contract and the route-permission contract: add persists to /model/info and the deployment actually serves chat through the data plane; updating tpm persists without the merge clobbering the backend model or key; delete removes the deployment from /model/info and traffic is rejected with a 400 naming the model; and a key restricted to llm_api_routes gets exactly 403 with the route-permission marker from /model/new, with the model verifiably not created
Two live contracts the tests encode: control-plane writes reach the data plane on its periodic DB sync (roughly 30s), so traffic-facing assertions poll to a deadline; and a deleted model is rejected with the router's no-healthy-deployments 400 rather than the unknown-model shape, so the client accepts either form as long as it names the model
models.py gains the typed pieces the suite needs: tpm on LiteLLMParamsBody, a DeploymentParams model for /model/info litellm_params (superset of the pricing fields the custom-pricing tests read), and ModelUpdateParams/ModelUpdateBody for /model/update