Skip to content

[rayci] pin the cache epoch for the whole build - #497

Open
elliot-barn wants to merge 1 commit into
mainfrom
elliot-barn-pin-cache-epoch
Open

[rayci] pin the cache epoch for the whole build#497
elliot-barn wants to merge 1 commit into
mainfrom
elliot-barn-pin-cache-epoch

Conversation

@elliot-barn

Copy link
Copy Markdown
Collaborator

First step of the digest-addressing migration
(plan). Standalone and
useful on its own; nothing depends on it yet.

The bug today

wanda resolves the cache epoch from the wall clock at the moment it starts
(wanda/wanda/main.go), and each wanda step in a build starts at a different time. A build whose
steps straddle an epoch boundary therefore computes two different cache keys for the same image:
the early steps look up z-<digest with epoch A>, the later ones z-<digest with epoch B>. The
second set misses the cache and rebuilds images that were just built.

Rare and currently cheap — one spurious rebuild. It stops being cheap in the next phase, where
the pipeline generator predicts the cache tag and bakes it into the step's image reference: if the
generator resolves epoch A and the builder resolves epoch B, the job pulls a tag nobody wrote and
the build fails.

What this does

Resolve the epoch once, in makeBuildInfo, and hand it to every wanda step via
RAYCI_WANDA_EPOCH. All steps in a build now agree by construction.

raycicmd calls wanda.DefaultCacheEpoch() rather than reimplementing the rule. That is the
point of the import: one definition, so the pipeline and the builders cannot drift apart on it.

Precedence in wanda is unchanged except for the new middle rung:

  1. an explicit -epoch flag, if passed
  2. RAYCI_WANDA_EPOCH, if set ← new
  3. wanda.DefaultCacheEpoch() from the clock

So a pipeline generated by an older rayci sets no variable and keeps today's behaviour exactly.

RAYCI_CACHE_EPOCH overrides the resolved value, which is also what keeps the tests off the
clock.

Testing

go test ./... passes for every package. New coverage: TestWandaStep_cacheEpoch (the variable
is emitted when pinned, and left unset when not, so wanda keeps its own default rather than
receiving an empty epoch) and TestMakeCacheEpoch (override wins; default matches wanda's;
an empty override falls back). TestMakeBuildInfo now pins the epoch so its struct comparison
does not depend on what day it runs.

raycilint's TestFetchRef fails on this checkout, unrelated — the sandbox has git 2.25.1 and
the test uses git init --initial-branch, which needs 2.28+.

Rollout

No behaviour change for existing pipelines until a repo bumps .rayciversion to a release
containing this. Product is on 0.36.0 and ray on 0.46.0, so they stage independently.

🤖 Generated with Claude Code

wanda resolves the cache epoch from the wall clock when it starts
(wanda/wanda/main.go), so a build whose wanda steps straddle an epoch
boundary computes two different cache keys for the same image. Today
that costs a spurious rebuild. It becomes a correctness problem once the
pipeline generator has to predict the cache tag, since generator and
builder would disagree about the address.

Resolve it once in makeBuildInfo and hand it to every wanda step via
RAYCI_WANDA_EPOCH. raycicmd calls wanda.DefaultCacheEpoch rather than
reimplementing the rule, so the two can never drift. An explicit -epoch
flag still wins, and an unset variable still falls back to the clock, so
older pipelines are unaffected.

RAYCI_CACHE_EPOCH overrides the resolved value, which also keeps the
tests off the clock.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to pin and propagate the Wanda cache epoch across build steps to prevent cache key mismatches on epoch boundaries. It resolves the cache epoch once during build initialization (using RAYCI_CACHE_EPOCH or falling back to Wanda's default) and passes it to each Wanda step via the RAYCI_WANDA_EPOCH environment variable. The review feedback suggests tightening a test assertion in raycicmd/build_info_test.go to explicitly verify that an empty override falls back to wanda.DefaultCacheEpoch() instead of just checking that the returned value is non-empty.

Comment on lines +65 to +67
if got := makeCacheEpoch(env); got == "" {
t.Error("makeCacheEpoch() = \"\", want a resolved epoch")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test assertion here is weak because it only checks that the returned epoch is not empty. If makeCacheEpoch had a bug and returned an invalid non-empty string, the test would still pass. It should be tightened to assert that it actually falls back to wanda.DefaultCacheEpoch().

Suggested change
if got := makeCacheEpoch(env); got == "" {
t.Error("makeCacheEpoch() = \"\", want a resolved epoch")
}
if got, want := makeCacheEpoch(env), wanda.DefaultCacheEpoch(); got != want {
t.Errorf("makeCacheEpoch() = %q, want %q", got, want)
}

@elliot-barn
elliot-barn requested a review from thomasdesr August 18, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant