Add cache save and restore using github.com/buildkite/zstash #3551

wolfeidau · 2025-10-22T05:48:06Z

Description

Adding buildkite cache support to the agent.

Context

https://linear.app/buildkite/issue/MDC-723/add-the-restore-and-save-commands-to-the-buildkite-agent

Changes

This change introduces some new sub commands.

buildkite-agent cache save
buildkite-agent cache restore

It is currently using some conventions from zstash which will change as it is integrated more with the agent.

This change is just adding these subcommands to the agent for use from a plugin.

Testing

Currently most of the testing for this feature is in github.com/buildkite/zstash given this is just a minimal integration currently.

Disclosures / Credits

Claude Code was used to write and review code in this PR.

DrJosh9000 · 2025-10-22T06:11:40Z

Tagging myself so I'm subscribed

wolfeidau · 2025-10-22T22:09:32Z

I am working on reducing the footprint of the library via buildkite/zstash#79 and buildkite/zstash#80

zhming0

I did a preliminary review. My comments are mainly recommendations.

I don't see any red flag. But I have doubt about the concept of cache id being overly prominent in the code base.

clicommand/cache_restore.go

zhming0 · 2025-10-24T04:16:52Z

clicommand/cache_restore.go

+      paths:
+        - node_modules


It might be wise to we document the behavior when it comes to symlink in the folder.

@zhming0 yeah this will need to be captured as it also overlaps with security, which makes it very complicated.

zhming0 · 2025-10-24T04:20:43Z

clicommand/cache_restore.go

+The command automatically uses the following environment variables when available:
+  - BUILDKITE_BRANCH (for branch scoping)
+  - BUILDKITE_PIPELINE_SLUG (for pipeline scoping)
+  - BUILDKITE_ORGANIZATION_SLUG (for organization scoping)`


[question] If these aren't available, are these information by default extracted from job token?

@zhming0 at the moment this is just run from the CLI so it depends on the environment variables, this will change once we add some configuration to the pipeline yaml as we can then trigger it within the agent lifecycle without needing to invoke the cli.

My main question was on the security aspect. If in the backend we validate BUILDKITE_BRANCH against agent job token then I think it's ✅ .

clicommand/cache_restore.go

internal/cache/cache.go

zhming0 · 2025-10-24T04:33:51Z

internal/cache/cache.go

+}
+
+// setupCacheClient creates a cache client and determines which cache IDs to process
+func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash.Cache, []string, error) {


[non-blocking] as we will eventually introduce more ways to specify cache configuration, I am not sure if we should use cache id as the first level concept here. Maybe we should turn it into a concrete CacheConfig object and pass that around.

Good point, will add validation of cache ids.

I think my main point is that maybe this method should return a list of cache configurations instead of cache ids. Cache ID as a concept doesn't seem to serve much purpose beyond system boundary. They might be more of an inconvenience when we want to refactor this down the track.

Like this method, maybe it can just return

Suggested change

func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash.Cache, []string, error) {

func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash.Cache, []CacheConfiguration, error) {

@zhming0 sorry commented on the wrong change here, cache ids are used to selectively restore caches in a step. The common example is a rails app wiht ruby and node deps, being able to only restore node or ruby deps or both in a step is useful.

This model follows other ci providers in their structure.

A good example of a bigger cache is something we experimented here https://github.com/bk-playground/kitesocial/blob/main/.buildkite/cache.hosted.yml#L1-L9 which has a docker cache.

I acknowledge the value of cache ids in boundary/edge (in people's pipeline yaml as you mentioned), but I don't see a great value preserving this concept beyond that.

What I mean is that maybe we can just take the cache IDs, parse them into into config configuration at the earliest code path, so we don't need to pass cache ids around internall?

DrJosh9000

First thoughts

clicommand/global.go

zhming0

Great starting point. I have doubt about some details but none of them are blocking. So it's a LGTM from me.

This change introduces some new sub commands. * buildkite-agent cache save * buildkite-agent cache restore It is currently using some conventions from zstash which will change as it is integrated more with the agent.

This structure allows additional options at the top level of the configuration.

wolfeidau · 2025-10-29T02:02:01Z

@DrJosh9000 did you have any concerns with me merging this?

DrJosh9000 · 2025-10-29T02:23:04Z

@wolfeidau I planned to take another look this afternoon, but don't let me stop you if you want it in now

wolfeidau · 2025-10-29T02:26:26Z

@DrJosh9000 if you find anything happy to remediate via a follow up PR!

wolfeidau force-pushed the mdc-723-add-the-restore-and-save-commands-to-the-buildkite-agent branch 2 times, most recently from 5cd1e8e to db78c9f Compare October 22, 2025 05:55

DrJosh9000 self-requested a review October 22, 2025 06:11

wolfeidau marked this pull request as ready for review October 23, 2025 05:40

wolfeidau requested review from a team and zhming0 October 23, 2025 21:28

wolfeidau force-pushed the mdc-723-add-the-restore-and-save-commands-to-the-buildkite-agent branch from 8240d88 to bb98e0a Compare October 24, 2025 00:41

zhming0 reviewed Oct 24, 2025

View reviewed changes

DrJosh9000 reviewed Oct 26, 2025

View reviewed changes

clicommand/global.go Outdated Show resolved Hide resolved

clicommand/global.go Outdated Show resolved Hide resolved

clicommand/global.go Show resolved Hide resolved

wolfeidau force-pushed the mdc-723-add-the-restore-and-save-commands-to-the-buildkite-agent branch 2 times, most recently from c7790ce to f762297 Compare October 27, 2025 04:48

wolfeidau requested review from DrJosh9000 and zhming0 October 27, 2025 04:55

zhming0 approved these changes Oct 28, 2025

View reviewed changes

wolfeidau added 14 commits October 29, 2025 12:34

Add cache save and restore using github.com/buildkite/zstash

e88a982

This change introduces some new sub commands. * buildkite-agent cache save * buildkite-agent cache restore It is currently using some conventions from zstash which will change as it is integrated more with the agent.

fix: update the zstash library to latest main

706e715

fix: ensure we capture registry slug so we can provide it later

4de343e

refactor: centralised flags and fixed config test

70aa3cc

fix: adjust logging and fixes for testing

22e9124

refactor: moved the cache logic into it's own package and added tests

da52bdf

feat: add progress updates during save or restore

a3f6391

fix: updated to latest zstash and added progress logging

0fb60cc

chore: I have hidden the cache sub command added notes to help

04ceb3d

chore: bump zstash to v0.5.0 which includes quite a few fixes

fa8728a

fix: validate cache ids and return list of invalid ones in error

5f0e982

fix: corrected description build>job

db27790

feat: added a top level configuration key for deps to enable extension

527e40c

This structure allows additional options at the top level of the configuration.

fix: as suggested switched to using StringSlice and cache_shared.go

04c0aa1

wolfeidau force-pushed the mdc-723-add-the-restore-and-save-commands-to-the-buildkite-agent branch from f762297 to 04c0aa1 Compare October 29, 2025 01:41

wolfeidau merged commit 0d2e270 into main Oct 29, 2025
1 check passed

wolfeidau deleted the mdc-723-add-the-restore-and-save-commands-to-the-buildkite-agent branch October 29, 2025 02:25

DrJosh9000 mentioned this pull request Nov 5, 2025

Bump version and changelog for v3.111.0 #3567

Merged

	func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash.Cache, []string, error) {
	func setupCacheClient(ctx context.Context, l logger.Logger, cfg Config) (*zstash.Cache, []CacheConfiguration, error) {

Add cache save and restore using github.com/buildkite/zstash #3551

Add cache save and restore using github.com/buildkite/zstash #3551

Uh oh!

Conversation

wolfeidau commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Changes

Testing

Disclosures / Credits

Uh oh!

DrJosh9000 commented Oct 22, 2025

Uh oh!

wolfeidau commented Oct 22, 2025

Uh oh!

zhming0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhming0 Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhming0 Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrJosh9000 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhming0 left a comment

Choose a reason for hiding this comment

Uh oh!

wolfeidau commented Oct 29, 2025

Uh oh!

DrJosh9000 commented Oct 29, 2025

Uh oh!

Uh oh!

wolfeidau commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wolfeidau commented Oct 22, 2025 •

edited

Loading

zhming0 Oct 26, 2025 •

edited

Loading

zhming0 Oct 26, 2025 •

edited

Loading