Skip to content

fix(orchestrator): scope rootfs hash to provision default#3129

Merged
dobrac merged 3 commits into
mainfrom
dobrac/honolulu
Jun 29, 2026
Merged

fix(orchestrator): scope rootfs hash to provision default#3129
dobrac merged 3 commits into
mainfrom
dobrac/honolulu

Conversation

@dobrac

@dobrac dobrac commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Adjusts the rootfs file hash behavior added in #3043.

The rollout invalidation process should not include the embedded rootfs file hash as an independent base-layer cache-key input. Rollout invalidation is controlled by the provision version instead.

This keeps the embedded rootfs file hash computation, but uses it only as part of the fallback provision version combined with the provision script hash. When the BuildProvisionVersion feature flag is set, that explicit provision version remains the rollout control.

@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes base-layer cache key composition for template builds; mis-scoping could leave stale baked units cached in prod or over-invalidate in dev, but the logic is narrow and flag-gated.

Overview
Rollout invalidation for base template layers no longer treats embedded rootfs template content as its own cache-key ingredient. That hash is still computed, but in dev/fallback it is folded into the default provision version together with the provision script hash so local builds still pick up baked unit changes. When BuildProvisionVersion is set explicitly, only that flag value drives invalidation—the rootfs hash does not independently force base-layer cache misses in production.

Reviewed by Cursor Bugbot for commit cad1b11. Bugbot is set up for automated code reviews on this repo. Configure here.

@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 removes the FilesHash function and its usage from the base builder hashing logic, simplifying the rootfs package by removing unused imports and code. There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...rchestrator/pkg/template/build/phases/base/hash.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Clean, self-contained revert — the removed FilesHash had no other callers and provisionVersion remains in the cache key as the documented invalidation mechanism.

Extended reasoning...

Overview

This PR reverts the embedded rootfs file hash from the base-layer cache key that was added in #3043. Two files change: rootfs.go drops the filesHash var and exported FilesHash() function (plus three now-unused imports: crypto/sha256, encoding/hex, io/fs), and hash.go removes the call to rootfs.FilesHash() from the cache.HashKeys argument list and its import. Net diff is roughly -25 lines.

Security risks

None. This is a cache key composition change in a build path. Removing an input from a cache key can only result in more cache hits (potentially serving older base layers for unchanged file content), not in any auth/permission/data-exposure issue. The rollout rationale — that provisionVersion (still present in the hash inputs) is the intended lever for invalidating baked unit files — is sensible and consistent with the description.

Level of scrutiny

Low. This is a mechanical revert with clear intent stated in the description. I verified with grep that rootfs.FilesHash has no other callers in the repo (the other FilesHash matches are in unrelated packages — steps, commands, API generated code — and were not affected by #3043). The remaining cache key inputs (index.Version(), provisionVersion, DiskSizeMB, baseSource) are unchanged.

Other factors

The bug hunting system found no issues. There are no outstanding human reviewer comments (only the Cursor Bugbot summary placeholder). The change reverses behavior introduced very recently and the reasoning aligns with how rollouts are described to work via the provision version feature flag.

@dobrac dobrac changed the title fix(orchestrator): revert rootfs file hash cache key fix(orchestrator): scope rootfs hash to provision default Jun 29, 2026
@dobrac dobrac enabled auto-merge (squash) June 29, 2026 05:00
@dobrac dobrac merged commit 475f955 into main Jun 29, 2026
54 checks passed
@dobrac dobrac deleted the dobrac/honolulu branch June 29, 2026 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants