feat(orch): move reserved blocks setting from in-guest to host-side pre-boot hook#2199
Merged
feat(orch): move reserved blocks setting from in-guest to host-side pre-boot hook#2199
Conversation
… rootfs modifications Add a PreBootFn callback parameter to Factory.CreateSandbox() that runs after the rootfs device is ready but before Firecracker boots the VM. This enables host-side filesystem metadata changes (e.g. tune2fs) on the rootfs path without depending on guest-side tooling. Thread the hook through the layer CreateSandbox builder via a new WithPreBootFn option.
…re-boot hook Replace SetReservedBlocksInGuest (tune2fs inside the VM) with a host-side PreBootFn that calls SetReservedBlocksOnHost on the rootfs device path before the guest kernel boots. This eliminates the dependency on e2fsprogs being installed and compatible inside the guest image. The previous in-guest approach was fragile across different build paths: - fromImage builds: provisioning could install e2fsprogs, but older guest versions were incompatible with host-created ext4 features. - fromTemplate builds: provisioning is skipped entirely, so e2fsprogs was only present if the parent template happened to have it. - Cached builds: the base phase SetReservedBlocksOnHost was skipped on cache hits, leaving only the in-guest call which could fail. Reserved blocks are now set at two points: - Base phase: on the host rootfs file after provisioning, so root has protected disk space during user build steps. - Finalize phase: via PreBootFn before every VM boot, regardless of build path or cache state, using the host's own tune2fs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88394c8df9
ℹ️ 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".
packages/orchestrator/pkg/template/build/phases/finalize/builder.go
Outdated
Show resolved
Hide resolved
djeebus
reviewed
Mar 22, 2026
packages/orchestrator/pkg/template/build/phases/finalize/builder.go
Outdated
Show resolved
Hide resolved
Gather CreateSandbox options up front and spread them, rather than always passing WithPreBootFn which could be nil.
…ootFn Apply the host-side SetReservedBlocksOnHost pre-boot hook to the steps phase as well, not just finalize. When the first uncached layer creates a new sandbox, the PreBootFn sets reserved blocks before the guest boots. This protects intermediate build sandboxes from disk-full conditions during user RUN commands, not just the final template.
Move the inline SetReservedBlocksOnHost call in buildLayerFromOCI to use the same WithPreBootFn mechanism as the steps and finalize phases. This is a pure refactor — same file, same timing (before VM boot), just consistent with the rest of the pipeline.
dobrac
approved these changes
Mar 23, 2026
Contributor
dobrac
left a comment
dobrac
left a comment
There was a problem hiding this comment.
This is great! It would be better to run the reserve for the first uncached layer (like envd) instead, but this is enough to fix the behavior for now.
…reBootFn setup Consolidate the identical reserved blocks PreBootFn setup from base, steps, and finalize phases into a single ReservedBlocksOptions() helper in the layer package. Each call site now collapses from 5 lines to 1.
Contributor
Author
I think this should be addressed now in the latest push. |
dobrac
approved these changes
Mar 23, 2026
packages/orchestrator/pkg/template/build/phases/finalize/builder.go
Outdated
Show resolved
Hide resolved
Only set reserved blocks in finalize when the source layer was cached, since otherwise a prior step phase already set them via its own CreateSandbox PreBootFn.
This was referenced Mar 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Narrows the issue introduced in #2082 where setting reserved disk space required running
tune2fsinside the guest VM. This approach failed when the guest image didn't havee2fsprogsinstalled (e.g.from_templatebuilds inheriting from older parents) or when the gueste2fsprogsversion was incompatible with the host's ext4 features.Obsoletes #2194, which tried to fix this by adding
e2fsprogstoprovision.sh. This would force cache invalidation for all existing templates but still didn't cover thefrom_templatepath where provisioning is skipped entirely.Instead, a
PreBootFnhook is added toFactory.CreateSandbox()that runs after the rootfs device is ready but before Firecracker boots. The finalize phase uses this hook to callSetReservedBlocksOnHostusing the host'stune2fs, which is always available and compatible. The in-guestSetReservedBlocksInGuestis removed.Close: #2194
Note
Medium Risk
Touches sandbox creation by adding a pre-boot callback that can mutate the rootfs before Firecracker starts, which could affect boot behavior or disk state if misused. Risk is limited to template build sandboxes and is gated by a feature flag for reserved disk space.
Overview
Adds an optional
PreBootFnhook to sandbox creation that runs after the rootfs device is prepared but before Firecracker boots, enabling host-side filesystem adjustments. Template builds now use this hook (behindBuildReservedDiskSpaceMB) to set ext4 reserved blocks via hosttune2fsinstead of runningtune2fsinside the guest, and the in-guest reserved-blocks command path is removed while feature-flag plumbing is threaded through the build step/user builders.Written by Cursor Bugbot for commit 21cd01a. This will update automatically on new commits. Configure here.