client: add reset option for local cache exporter#6612
client: add reset option for local cache exporter#6612jirimoravcik wants to merge 1 commit intomoby:masterfrom
reset option for local cache exporter#6612Conversation
There was a problem hiding this comment.
Pull request overview
Adds a reset=true option to the local cache exporter so exported cache directories can be cleaned up (unreferenced blobs removed) after export, preventing unbounded growth. This fits into the client-side cache export flow that already updates the local index.json after a solve.
Changes:
- Parse a new
resetattribute fortype=localcache exports and trigger a post-export cleanup. - Implement
resetCacheStoreto compute referenced blobs fromindex.jsonand delete unreferenced blobs. - Add unit + integration tests and document the new option in README.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
client/solve.go |
Parses reset attr, tracks stores to reset, and implements blob cleanup after export. |
client/solve_resetcache_test.go |
Unit tests for resetCacheStore behavior (currently image-manifest oriented). |
client/client_test.go |
Integration test for reset=true behavior with local cache export. |
README.md |
Documents the new reset=<true|false> option for local cache exporter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d06efaa to
20a5427
Compare
|
Fixed the lint, other errors seem to be |
client/solve.go
Outdated
|
|
||
| referenced := make(map[digest.Digest]struct{}) | ||
|
|
||
| for _, manifestDesc := range index.Manifests { |
There was a problem hiding this comment.
This should use images.Dispatch for walking the descriptors instead of handwritten logic.
Also makes sure it works with image-manifest=false case that has nested indexes.
There was a problem hiding this comment.
Hey @tonistiigi, thanks for the review.
I've updated the code to use images.Dispatch.
I also did a deep dive into the codebase and fixed the logic for image-manifest=false, which can have nested indexes, now the logic is recursive (which is handled nicely by images.Dispatch.
Using images.IsIndexType and images.IsManifestType to determine the type - that should be future-proof.
I also added more tests that cover nested indexes and Docker media types.
Let me know if there's anything else to fix or improve
Add `reset=true` attribute to the local cache exporter that removes unreferenced blobs from the cache directory after export, preventing unbounded growth. Signed-off-by: Jiří Moravčík <jiri.moravcik@gmail.com>
20a5427 to
195fa8e
Compare
Add
reset=trueattribute to the local cache exporter that removes unreferenced blobs from the cache directory after export, preventing unbounded growth.The logic is very simple:
index.jsonto get all the referenced manifests or indexesI think
cleanupcould also work as the attribute name.Resolves #1896