fix(app): tear down serverless endpoints on flash app delete - #371
Open
justinwlin wants to merge 3 commits into
Open
fix(app): tear down serverless endpoints on flash app delete#371justinwlin wants to merge 3 commits into
justinwlin wants to merge 3 commits into
Conversation
flash app delete removed the app record but left the deployed serverless endpoints running and billable. The orphans were invisible to flash app list (record gone) and accumulated silently in CI. Discover endpoints server-side from each environment and delete them before removing the app record. On any endpoint failure, keep the app record, name the endpoint, print the runpodctl remediation, and exit non-zero. Endpoints already deleted are treated as removed so the command is safe to retry after a partial teardown.
test_deploy_all_background spawned an untracked daemon thread that could
lose the scheduling race and run the real ResourceManager after the mock
patch and fixtures unwound. Its MagicMock(spec=ServerlessResource)
resources were then cached for cleanup, and _save_resources failed to
cloudpickle spec'd mocks ("args[0] from __newobj__ args has the wrong
class"), truncating the shared state file and breaking unrelated tests
downstream (REG008 PicklingError / "Ran out of input").
deploy_all_background now returns the spawned thread so callers can
await completion, and the test joins it while the get_or_deploy_resource
patch is still active, making the cross-test pollution structurally
impossible rather than a scheduling lottery.
- Extract shared _delete_endpoint_idempotent helper into
core/api/runpod.py; ServerlessResource._do_undeploy and
FlashApp.delete_endpoints both delegate to it
- Distinguish the apps.py failure message when endpoint teardown blocks
app deletion ('could not remove all endpoints...; app was not deleted')
from an actual app-record deletion failure
- Cover the id-less endpoint remediation branch in CLI tests, and the
defensive delete_endpoints branches (None/missing environment detail,
success=False-without-exception hard failure with no re-check)
- Pin the empty-list contract of deploy_all_background returning None
justinwlin
marked this pull request as ready for review
August 25, 2026 19:29
justinwlin
requested review from
KAJdev and
deanq
and removed request for
deanq
August 25, 2026 19:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #367 · Internal: CON-1232
flash app deleteremoved the app record, printed an unqualified success message, and exited 0 — while the serverless endpoints it deployed kept running and stayed billable. The orphans are invisible toflash app list(the app record is genuinely gone) and only show up inrunpodctlor the console, so CI teardown loops accumulate them silently.Fix
Delete the endpoints as part of
app delete, before the app record is removed. Endpoints are discovered server-side from each flash environment, so this works without local resource tracking (e.g. in CI).If any endpoint cannot be removed, the app record is kept, the command exits 1, and each survivor is named with its remediation:
Verified
pytest tests/unit/cli/test_apps.py tests/unit/core/resources/test_app.pypytest tests/unit/cli/ tests/unit/resources/test_app.py tests/unit/core/resources/test_app.pyruff check+ruff format --check, touched filesmypy, touched filesmypy note
12 pre-existing errors in
core/resources/app.py; typecheck is opt-in viaquality-check-strict.What changed
core/resources/app.py— newFlashApp.delete_endpoints(). Lists the app's environments server-side, deletes every registered endpoint via the existing GraphQLdeleteEndpointhelper, returns(removed, failed). Already-deleted endpoints count as removed (mirrorsServerlessResource._do_undeploy), so retry after a partial teardown is safe.cli/commands/apps.py—delete_flash_appruns teardown first, prints each deleted endpoint, and on partial failure blocks the app deletion with named endpoints, remediation, and a non-zero exit.tests/unit/cli/test_apps.py(endpoints removed; endpoint failure blocks app delete and prints remediation) and addedTestFlashAppDeleteEndpointsintests/unit/core/resources/test_app.py(multi-env teardown, failure reporting, idempotent retry, id-less endpoint guard).Live testing
2026-08-25, real Runpod account. Fixture: app
con370-live, endpoint4kuwiutqisjt2f, deployed via #370's build fromrunpod/serverless-hello-world:0.4.1.GET /v1/endpoints/4kuwiutqisjt2f→404 endpoint not found. Actually deleted, not just unregistered — the exact silent-orphan case from the issue.FlashAppNotFoundError.Out of scope
.flash/resources.pkl;flash undeploy --cleanup-stalealready handles stale local state.FlashAppNotFoundErrorrenders as a full traceback. Verified pre-existing on a build without this PR, not a regression. Cheap follow-up: catch it in the CLI and printapp 'x' not found.