Write to cache when building a template

[djeebus]

Note

Adds an NFS-backed cache for template builds behind a feature flag, introduces atomic file writes, updates storage interfaces, and enhances cache metrics/tracing with comprehensive tests.

• Build/Orchestrator:
  • Conditionally wrap templateStorage with storage.NewCachedProvider during build via useNFSCache, gated by featureflags.BuildingFeatureFlagName and SharedChunkCacheDir.
• Feature Flags:
  • Add BuildingFeatureFlagName (use-nfs-for-building-templates).
• Storage:
  • Interfaces: Extract WriteFromFSCtx; update ObjectProvider and SeekableObjectProvider to use it.
  • Atomic writes/locking: Add lock.AtomicFile with OpenFile for atomic, lock-guarded writes; tests included.
  • Cache provider:
    • Async local delete on DeleteObjectsWithPrefix.
    • Add error recording helpers and OpenTelemetry tracing integration.
    • Add cache metrics counters (ops, bytes) with hit/miss and operation tags.
  • CachedObjectProvider:
    • Read from cache if available; otherwise read remote and asynchronously write to cache.
    • On Write/WriteFromFileSystem, write to remote and cache concurrently; use lock.OpenFile for atomic cache writes.
  • CachedSeekableObjectProvider:
    • Improve ReadAt/Size to report cache hit/miss, record errors, and async backfill cache.
    • Implement concurrent chunked caching from filesystem with bounded concurrency; write local size metadata atomically.
  • Utils: Add moveWithoutReplace and cleanup helpers; refine EOF handling.
  • Tests: New and updated tests for atomic file behavior and cache read/write paths.

^{Written by Cursor Bugbot for commit 83bcfe4. This will update automatically on new commits. Configure here.}

ready for a new review!

[ValentaTomas]

@djeebus Let's split to PR preparing for this change (refactor) and adding the caching behavior.

[djeebus]

@djeebus Let's split to PR preparing for this change (refactor) and adding the caching behavior.

OK, I've removed all the nice-to-haves, the rest will go in other PRs.

[djeebus]

There is currently a mismatch between reading/writing whole files and reading/writing chunked files. We need to do things:

1. Line things up so that the template writes the same files that the orchestrator expects to read
2. Find a way to throw errors if the function calls don't line up across structs

[djeebus]

This is blocked by #1361 ; will refactor after that gets merged.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Avoid async caching from caller buffer after Write returns

The new Write implementation spawns a goroutine that streams bytes.NewReader(p) into the cache while immediately returning the result of inner.Write. Because p is provided by the caller, the io.Writer contract allows the caller to reuse or mutate the slice as soon as Write returns; the goroutine may therefore read modified data or race with the caller and cache corrupted bytes even when the remote write succeeded. Copy the data or defer launching the cache write until after the payload is safely owned to prevent serving incorrect cached content.

Useful? React with 👍 / 👎.

[ValentaTomas]

My one concern here is that because some teams are building tens of thousands of templates, this might start ejecting things disproportionately.

[cursor]

Bug: NFS cache wrapping not applied to storage operations

The templateStorage variable is wrapped with NewCachedProvider at lines 229-231 to enable NFS caching, but the unwrapped builder.templateStorage is then passed to layerExecutor (line 241), baseBuilder (line 251), and postProcessingBuilder (line 293). Additionally, the unwrapped builder.templateStorage is used at line 327 for getRootfsSize. This means the cache wrapping is never actually used for any storage operations, defeating the entire purpose of the caching feature introduced in this PR.

 

[cursor]

Bug: File descriptor leak in WriteFromFileSystem cache operation

In WriteFromFileSystem, a file is opened at line 93 but never closed. The *os.File is passed as an io.Reader to writeFileToCache, which cannot close it since it only accepts the io.Reader interface. The goroutine runs asynchronously and leaves the file handle open, causing file descriptor leaks over time.

 

[djeebus]

Okay, I'll work on getting a cache effectiveness dashboard added. We can monitor that to see if cache effectiveness goes down, and disable via flag if it doesn't improve things. Does that work?