Remove the empty .bundle/<name> directory on destroy - #6317
Draft
shreyas-goenka wants to merge 2 commits into
Draft
Remove the empty .bundle/<name> directory on destroy#6317shreyas-goenka wants to merge 2 commits into
shreyas-goenka wants to merge 2 commits into
Conversation
`bundle destroy` deleted `~/.bundle/<name>/<target>` but never the `<name>` parent, so every destroy left an empty directory behind. Deployments that use a fresh bundle name each time never reuse those directories, so they accumulate and count against the workspace child-node limit; the CLI's own CI workspace reached ~27k of them per day. Delete the parent too, non-recursively so it survives while another target of the bundle is still deployed there. The delete is restricted to the `~/.bundle/<name>/<target>` layout the CLI generates, since `root_path` is user-configurable and the parent of an arbitrary path is not ours to remove. The fake workspace accepted a non-recursive delete of a non-empty directory, which the real API rejects with DIRECTORY_NOT_EMPTY. Model that, so the sibling-target case is covered by the local suite. Co-authored-by: Isaac
Co-authored-by: Isaac
Collaborator
Integration test reportCommit: ccf0ff7
8 interesting tests: 4 SKIP, 3 RECOVERED, 1 flaky
Top 6 slowest tests (at least 2 minutes):
|
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.
Why
bundle destroydeletes~/.bundle/<name>/<target>but never the<name>parent, so every destroy leaves an empty directory behind:Deployments that use a fresh bundle name each time never reuse those directories, so nothing ever reclaims them. Our own CI workspace is the worst case: the acceptance suite deploys ~1,056 bundles per run under names like
ci<RUN_ID>x<rand>, across ~26 runs a day. That workspace's.bundlereached 48k+ nodes and triggered a sev0 on webapp MySQL (ES-2138924); its child-node limit had already been raised from 10k to 100k to accommodate it.The nightly workspace sweep does reclaim them, so this is a sawtooth rather than unbounded growth — but of the 27,243 directories one pass removed, 26,982 were empty leftovers of exactly this shape and only 261 were genuinely leaked deployments. This removes the cause instead of relying on the sweep.
Related:
acceptance/bundle/destroy/force-lock-node-limitalready exists to cope withMAX_CHILD_NODE_SIZE_EXCEEDEDonce the limit is hit.What
Delete the parent directory too, non-recursively, so it is removed only when nothing else is deployed under it — a sibling target of the same bundle keeps it alive. Every failure (sibling present, already gone) is expected and ignored at debug level.
The delete is restricted to the
~/.bundle/<name>/<target>layout the CLI generates.workspace.root_pathis user-configurable, and the parent of an arbitrary path is not ours to remove — without the guard, aroot_pathof/Workspace/Shared/foowould make us try to delete/Workspace/Shared.Testserver
The fake workspace accepted a non-recursive delete of a non-empty directory, which the real API rejects with
DIRECTORY_NOT_EMPTY. Modelled that, so the sibling-target behaviour is covered by the local suite rather than only on cloud.Tests
acceptance/bundle/destroy/all-resources— extended:.bundle/test-bundleis gone after destroy.acceptance/bundle/destroy/sibling-target— new: deploydev+prod, destroydev→ directory kept; destroyprod→ directory deleted.libs/testserverunit test for the newDIRECTORY_NOT_EMPTYbehaviour.bundle/user_agent/simpleandbundle/resource_deps/remote_app_urlpick up the extraworkspace/deletecall.Full local acceptance suite passes (1143/1146 dirs, both engines). The two unrelated failures on my box —
fipsandbundle/templates/lakeflow-integrations— fail identically without this change (non-FIPS local build; uv cannot reach the network).This pull request and its description were written by Isaac.