🐛 OCPBUGS-76453: clean up orphaned temp dirs in catalog storage#2537
Conversation
LocalDirV1.Store creates a temp dir (.{catalog}-{random}) and renames it
into place atomically. If the process is interrupted before the deferred
RemoveAll runs, the temp dir persists. Each restart adds another, eventually
filling the disk.
Primary fix: remove any matching temp dirs at the start of Store before
creating a new one. Since Store holds the write mutex, any dirs found are
guaranteed to be from a previous run.
Defense-in-depth: add /var/cache/catalogs to GarbageCollector's list of
scanned paths (CachePath string -> CachePaths []string), so orphaned temp
dirs are also removed by the periodic GC cycle.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Todd Short <tshort@redhat.com>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR addresses orphaned temporary directories left behind when LocalDirV1.Store is interrupted before its deferred cleanup runs, preventing disk usage from growing across restarts. It also extends periodic garbage collection so it can clean orphaned temp dirs under catalog storage in addition to the unpack cache.
Changes:
- Clean up any leftover
.{catalog}-*temp directories at the start ofLocalDirV1.Store. - Update
GarbageCollectorto support scanning multiple cache directories (CachePath→CachePaths) and include the catalog storage directory in catalogd’s GC configuration. - Add unit tests covering both the
Storecleanup behavior and GC cleanup of orphaned temp dirs in the storage path.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/catalogd/storage/localdir.go | Adds pre-Store cleanup of orphaned .{catalog}-* temp dirs under the storage root. |
| internal/catalogd/storage/localdir_test.go | Adds a test ensuring orphaned temp dirs for the same catalog are removed while unrelated ones remain. |
| internal/catalogd/garbagecollection/garbage_collector.go | Reworks GC config to accept multiple paths and runs GC per path with path-aware logging. |
| internal/catalogd/garbagecollection/garbage_collector_test.go | Adds a test ensuring GC removes orphaned storage temp dirs while preserving the real catalog directory. |
| cmd/catalogd/main.go | Configures GC to scan both unpack cache and catalog storage directories. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2537 +/- ##
==========================================
+ Coverage 68.62% 68.78% +0.16%
==========================================
Files 131 131
Lines 9288 9301 +13
==========================================
+ Hits 6374 6398 +24
+ Misses 2427 2419 -8
+ Partials 487 484 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/approved |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pedjak, rashmigottipati The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/override codecov/patch |
|
@camilamacedo86: Overrode contexts on behalf of camilamacedo86: codecov/patch DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override patch |
|
@camilamacedo86: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-merge-bot
Bot
merged commit 95afce0
into
operator-framework:main
LocalDirV1.Store creates a temp dir (.{catalog}-{random}) and renames it into place atomically. If the process is interrupted before the deferred RemoveAll runs, the temp dir persists. Each restart adds another, eventually filling the disk.
Primary fix: remove any matching temp dirs at the start of Store before creating a new one. Since Store holds the write mutex, any dirs found are guaranteed to be from a previous run.
Defense-in-depth: add /var/cache/catalogs to GarbageCollector's list of scanned paths (CachePath string -> CachePaths []string), so orphaned temp dirs are also removed by the periodic GC cycle.
Description
Reviewer Checklist