Skip to content

test(e2e): read a deleted key back as deleted, not as a 404 - #41551

Merged
yuneng-berri merged 1 commit into
mainfrom
litellm_cadence_319f427_key_lifecycle_delete
Sep 17, 2026
Merged

yuneng-berri merged 1 commit into
mainfrom
litellm_cadence_319f427_key_lifecycle_delete

Conversation

@yuneng-berri

Copy link
Copy Markdown
Contributor

Culprit: 2bfa10b
Test: management/test_key_lifecycle_e2e.py::TestKeyLifecycle::test_delete_revokes_info_and_chat_on_every_replica

The behavior change: #41311 made /key/info serve a deleted key from the archive with status: "deleted" instead of answering 404, so a test that polled for a 404 could never settle

Found by the CI cadence triage routine on the scheduled e2e build at 319f427c, https://buildkite.com/berriai-1/litellm-e2e/builds/236. cc @yassin-berriai, who requested and merged #41311

TLDR

Problem this solves:

  • The delete step polls /key/info for a 404 that no longer comes
  • It burns its whole 120s deadline, then fails the scheduled e2e run

How it solves it:

  • Wait for status: "deleted" instead of a 404
  • Model the status field the endpoint now returns

User Flow

This is a test-only change, so the end user here is whoever reads the nightly e2e result

Before: the scheduled e2e run reports the key lifecycle suite red, and the message points at replica propagation rather than at the real cause

  1. The 00:00 UTC scheduled build runs the management suite
  2. test_delete_revokes_info_and_chat_on_every_replica spends 120s polling GET /key/info on every replica
  3. It fails with never converged within 120.0s of the write (control/data-plane propagation issue), quoting a healthy HTTP 200 as the last read
  4. Whoever triages it starts by looking for a stale replica, which is not what happened

After: the same run is green, and the test now states the contract the gateway actually provides

  1. The scheduled build runs the same suite
  2. The delete step polls GET /key/info and settles as soon as every replica reports the key with status: "deleted"
  3. The step still confirms that chat with the deleted key is rejected with 401 on every replica
  4. The suite passes, and a future change that let a deleted key read back as live or serve traffic would fail it again

Relevant issues

Affected release

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

This PR changes only an e2e test, so the proof is that test running against three real gateways deployed from three different commits: the culprit's parent, the culprit, and the cadence sha. The edited test has to be red on the first and green on the other two, or the edit would be accommodating the change rather than tracking it. Each run deploys a full ephemeral stack and drives the real management routes, no mocks

Shared setup: E2E_PATHS=management/test_key_lifecycle_e2e.py, LITELLM_SHA=44a0e16c81 (this branch, carrying the edited test), GATEWAY_SHA varied per run

Before (95abc9f, the culprit's parent)

  1. Ran https://buildkite.com/berriai-1/litellm-e2e-pr/builds/499 with GATEWAY_SHA=95abc9fb0b67385eec60bcebe1312a0e180e5371
  2. Observed 1 failed, 5 passed in 143.79s, the one failure being this test:
    FAILED management/test_key_lifecycle_e2e.py::TestKeyLifecycle::test_delete_revokes_info_and_chat_on_every_replica
    - AssertionError: GET /key/info on http://e2e-pr-499-router.litellm.svc.cluster.local never converged
      within 120.0s of the write; last read: kind='unknown' status_code=404
      body='{"error":{"message":"Key not found in database","type":"not_found_error","param":"key","code":"404"}}'
    
  3. That is the old behavior answering 404, which the edited test correctly refuses to accept. The other five tests in the file pass, so the edit does not disturb them

After (44a0e16)

Gateway at the culprit, 2bfa10b

  1. Ran https://buildkite.com/berriai-1/litellm-e2e-pr/builds/500 with GATEWAY_SHA=2bfa10b6d96876de8cef7696720c3aed0cf43cc4
  2. Observed build state passed, the whole file green

Gateway at the cadence sha, 319f427

  1. Ran https://buildkite.com/berriai-1/litellm-e2e-pr/builds/501 with GATEWAY_SHA=319f427c403043756917aea39cab89021a7f92e7
  2. Observed build state passed, the whole file green

The same endpoint behavior is visible by hand with curl, and #41311's own description records it: /key/info on a deleted key returns {"status":"deleted","key_alias":"...","deleted_at":"..."} at 200 where it used to return 404. The QA runbook below reproduces that against a local proxy

Type

✅ Test

Caveats (if any)

Low

  • KeyInfo.status is typed str | None rather than a Literal of the four statuses, matching SpendLogRow.status in the same file. A Literal would turn any future fifth status into a validation error on unrelated reads
  • The equivalent teardown in the integration suite is fixed separately in test: fix seven tests left stale by #41311, #41337, #39996, #41310, #41289 and #41315 #41527. Same culprit and same behavior, different lane and different files, so the two do not overlap. Fold them together if you would rather review one change

QA runbook

  • management/test_key_lifecycle_e2e.py::TestKeyLifecycle::test_delete_revokes_info_and_chat_on_every_replica - deleting a key stops it serving traffic on every replica, and /key/info reports it as deleted rather than pretending it never existed
    • Start a proxy with a database: python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --use_v2_migration_resolver, master key sk-1234
    • Register a mock deployment: curl -X POST localhost:4000/model/new -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' -d '{"model_name":"qa-key-lifecycle","litellm_params":{"model":"gpt-4o-mini","mock_response":"ok"}}'
    • Make a key for it: curl -X POST localhost:4000/key/generate -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' -d '{"models":["qa-key-lifecycle"],"key_alias":"qa-key-lifecycle"}' and keep key and token_id
    • Warm it: curl -X POST localhost:4000/v1/chat/completions -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model":"qa-key-lifecycle","messages":[{"role":"user","content":"hi"}]}' and expect 200
    • Delete it: curl -X POST localhost:4000/key/delete -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' -d "{\"keys\":[\"$KEY\"]}"
    • Read it back: curl -s "localhost:4000/key/info?key=$TOKEN_ID" -H "Authorization: Bearer sk-1234" | jq '.info.status' and expect "deleted" at HTTP 200, which is what this test now waits for and what it used to expect a 404 for
    • Confirm it stopped serving: repeat the chat call with $KEY and expect 401, which is the assertion after the poll and is unchanged by this PR
    • Sanity check: this test makes sense to add and is not hand-wavey (e.g., assert actual expected spend instead of just spend > 0) or potentially flaky

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_01XvoU8fByocGJcZMBD1CrzP


Generated by Claude Code

/key/info now serves a deleted key from the archive with status deleted
instead of answering 404, so the delete test's convergence predicate never
settled and the read timed out against a 200 it kept discarding.

The predicate now waits for status deleted through the same
_key_info_everywhere helper the rest of the file uses, and KeyInfo carries
the status field. The chat-rejection assertion after it is unchanged, so
the test still proves the key stops serving.
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The test-only change appears safe to merge and accurately tracks the current deleted-key contract

Summary

This PR updates the key lifecycle E2E test to match the archived-key response contract

  • Models the optional key status returned by /key/info
  • Polls every replica until deleted keys report status: "deleted"
  • Retains independent polling that verifies deleted keys cannot serve chat requests

Reviews (1) · Last reviewed commit: "test(e2e): read a deleted key back as de..."

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor Author

CI red on lanes already red on main, not this diff (tests/e2e only). Most are fixed by #41527; litellm_router_testing predates the range. Details in #41553.


Generated by Claude Code

@yuneng-berri
yuneng-berri merged commit 54fa790 into main Sep 17, 2026
121 of 133 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_cadence_319f427_key_lifecycle_delete branch September 17, 2026 03:48

This branch is waiting to be deployed

1 waiting deployment
e2e-changed — 44a0e16c Waiting Sep 17, 2026 by yuneng-berri via Run changed e2e tests against the stage-mirror stack #6081
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants