Freeze fs on old-envd guests via the envd exec API - #3420
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit a4cbc64. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2565a16 to
49db8eb
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9c471f2. Configure here.
A filesystem-only pause must quiesce the guest rootfs before snapshotting to close the sync->pause write race. Today only envd >= 0.6.6 (native /fsfreeze) does a real FIFREEZE; older guests fall back to a plain `sync`, which does not block writes and can capture a torn ext4 journal. For those guests, when the fsfreeze-via-exec flag is on, run `fsfreeze -f /` through the envd process API instead — the same FIFREEZE, no native endpoint needed, running entirely inside the guest (no host attack surface). Falls back to `sync` per-pause if the guest lacks the fsfreeze binary or the freeze fails. On the pause-failure rollback the rootfs is thawed via the same API (bounded, best-effort). Gated behind featureflags.FsFreezeViaExecFlag (off by default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Babis Chalios <babis.chalios@e2b.dev>
9c471f2 to
a4cbc64
Compare
…h telemetry (#3445) # feat(orch): persist an fs-quiesced flag on filesystem-only pause, with telemetry ## Why A filesystem-only snapshot (`pause(memory:false)`) is only safe to cold-boot — and to rewrite offline — if its rootfs was **frozen** (`FIFREEZE`) at pause, not merely `sync`'d. The pause path already knows which happened (`guestPrepareFsForPause` picks native `fsfreeze` for envd ≥ 0.6.6, `fsfreeze-exec` for older envd, or a `sync` fallback), but that fact is **thrown away**: it lives only in a transient span/histogram attribute and is never persisted with the snapshot. So nothing downstream can tell, for a given snapshot, whether its rootfs is crash-consistent, and there is no fleet-level signal for *how many* fs-only snapshots are frozen. This is the building block for a later offline envd-upgrade feature (separate PR): to upgrade an envd-unaware, paused sandbox by rewriting its rootfs and cold-booting, that feature must restrict itself to snapshots it *knows* were frozen — otherwise it would have to repair a possibly-torn journal. This PR records and measures the fact; it does not consume it. ## What **Persist the flag in the snapshot's own metadata.** A new `FsQuiesced bool` field (`json:"fs_quiesced,omitempty"`) on `metadata.Template`, with `IsFsQuiesced()` / `MarkFsQuiesced()` helpers, sitting right next to `filesystem_only` in the per-snapshot `metadata.json`. `guestPrepareFsForPause` now returns whether a real freeze ran (`true` for both the native and the exec-API paths; `false` only for the `sync` fallback), and `Sandbox.Pause` stamps `m.MarkFsQuiesced(filesystemSnapshot && frozen)`. It is re-stamped on every pause, so it never goes stale, and is deliberately *not* carried by the copy-constructors (like `filesystem_only`). **Telemetry** — two signals so the flag is observable both in aggregate and per-pause: | signal | where | what it shows | |---|---|---| | `orchestrator.sandbox.pause.fs_quiesced{quiesced=true\|false}` counter | Mimir | count of fs-only pauses split by frozen vs sync — the eligible-snapshot population | | `fs_quiesced` bool attribute on the `sandbox-snapshot` span | Tempo (sampled) | per-pause value, for inspecting individual snapshots | The counter increments only on fs-only pauses, so the headline query is the eligible fraction: ```promql sum(rate(orchestrator_sandbox_pause_fs_quiesced_total{quiesced="true"}[$__rate_interval])) / sum(rate(orchestrator_sandbox_pause_fs_quiesced_total[$__rate_interval])) ``` The counter is registered in `telemetry` (`meters.go`) with description + unit map entries, guarded by a meter-map test. ## Validation - **Unit** (`go test`, orchestrator + shared): metadata round-trip test asserting `fs_quiesced` survives serialize→deserialize on a filesystem-only snapshot (no dedicated metadata version needed), that a sync-fallback fs-only snapshot stays not-quiesced, and that a legacy snapshot without the field deserializes as not-quiesced (the safe default); the `#3420` fsfreeze-pause test adapted + strengthened to assert the new `frozen` return (`true` on a successful native freeze, `false` on an aborted one); a meter-map guard test. Build + `go vet` clean, `gofmt` clean. - **Dev-cluster e2e:** deployed this build to a dev orchestrator node, funneled placement to it, created a sandbox (guest envd 0.6.13, native `FIFREEZE`), and did a real `memory:false` pause. The metric appeared in dev Mimir through the actual OTEL pipeline: `sum by (quiesced)(orchestrator_sandbox_pause_fs_quiesced_total)` → `{quiesced="true"} = 1`. Reverted afterwards. ## Key decisions - **No new metadata version.** The field is only meaningful on fs-only snapshots, which `MarkFilesystemOnly` already stamps at `>= FilesystemOnlyVersion` — a version `deserialize()` fully unmarshals — so the flag survives without its own version bump. Verified by the round-trip test. - **Backward-compatible by construction.** `omitempty` + the `false` zero value mean a pre-existing/legacy snapshot (no field) reads as *not* quiesced. For the future consumer that is the safe "not eligible, wait for a fresh freezing pause" default; no migration. - **Both freeze paths count as quiesced.** `quiesced=true` means "a real `FIFREEZE` ran," native (≥0.6.6) or exec-API (<0.6.6) alike — both yield an equally crash-consistent rootfs; only `sync` is `false`. The native-vs-exec split remains available on the existing `guest_sync.duration` `method` label, so it is not duplicated here. - **Signature change:** `guestPrepareFsForPause` now returns `(frozen bool, err error)`; its sole caller (`Sandbox.Pause`) and the existing fsfreeze test are updated in this PR. - **Deliberately scoped to record + measure.** The consumer of the flag — the offline rootfs `envd` swap, the reboot-time gate, and the version-remap flag — is intentionally *not* in this PR; it lands separately on top of this building block. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Nikita Kalyazin <nikita.kalyazin@e2b.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Babis Chalios <mail@bchalios.io>
A filesystem-only pause must quiesce the guest rootfs before snapshotting to close the sync->pause write race. Today only envd >= 0.6.6 (native /fsfreeze) does a real FIFREEZE; older guests fall back to a plain `sync`, which does not block writes and can capture a torn ext4 journal. For those guests, when the fsfreeze-via-exec flag is on, run `fsfreeze -f /` through the envd process API instead — the same FIFREEZE, no native endpoint needed, running entirely inside the guest (no host attack surface). Falls back to `sync` per-pause if the guest lacks the fsfreeze binary or the freeze fails. On the pause-failure rollback the rootfs is thawed via the same API (bounded, best-effort). Gated behind featureflags.FsFreezeViaExecFlag (off by default). Signed-off-by: Babis Chalios <babis.chalios@e2b.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h telemetry (#3445) # feat(orch): persist an fs-quiesced flag on filesystem-only pause, with telemetry ## Why A filesystem-only snapshot (`pause(memory:false)`) is only safe to cold-boot — and to rewrite offline — if its rootfs was **frozen** (`FIFREEZE`) at pause, not merely `sync`'d. The pause path already knows which happened (`guestPrepareFsForPause` picks native `fsfreeze` for envd ≥ 0.6.6, `fsfreeze-exec` for older envd, or a `sync` fallback), but that fact is **thrown away**: it lives only in a transient span/histogram attribute and is never persisted with the snapshot. So nothing downstream can tell, for a given snapshot, whether its rootfs is crash-consistent, and there is no fleet-level signal for *how many* fs-only snapshots are frozen. This is the building block for a later offline envd-upgrade feature (separate PR): to upgrade an envd-unaware, paused sandbox by rewriting its rootfs and cold-booting, that feature must restrict itself to snapshots it *knows* were frozen — otherwise it would have to repair a possibly-torn journal. This PR records and measures the fact; it does not consume it. ## What **Persist the flag in the snapshot's own metadata.** A new `FsQuiesced bool` field (`json:"fs_quiesced,omitempty"`) on `metadata.Template`, with `IsFsQuiesced()` / `MarkFsQuiesced()` helpers, sitting right next to `filesystem_only` in the per-snapshot `metadata.json`. `guestPrepareFsForPause` now returns whether a real freeze ran (`true` for both the native and the exec-API paths; `false` only for the `sync` fallback), and `Sandbox.Pause` stamps `m.MarkFsQuiesced(filesystemSnapshot && frozen)`. It is re-stamped on every pause, so it never goes stale, and is deliberately *not* carried by the copy-constructors (like `filesystem_only`). **Telemetry** — two signals so the flag is observable both in aggregate and per-pause: | signal | where | what it shows | |---|---|---| | `orchestrator.sandbox.pause.fs_quiesced{quiesced=true\|false}` counter | Mimir | count of fs-only pauses split by frozen vs sync — the eligible-snapshot population | | `fs_quiesced` bool attribute on the `sandbox-snapshot` span | Tempo (sampled) | per-pause value, for inspecting individual snapshots | The counter increments only on fs-only pauses, so the headline query is the eligible fraction: ```promql sum(rate(orchestrator_sandbox_pause_fs_quiesced_total{quiesced="true"}[$__rate_interval])) / sum(rate(orchestrator_sandbox_pause_fs_quiesced_total[$__rate_interval])) ``` The counter is registered in `telemetry` (`meters.go`) with description + unit map entries, guarded by a meter-map test. ## Validation - **Unit** (`go test`, orchestrator + shared): metadata round-trip test asserting `fs_quiesced` survives serialize→deserialize on a filesystem-only snapshot (no dedicated metadata version needed), that a sync-fallback fs-only snapshot stays not-quiesced, and that a legacy snapshot without the field deserializes as not-quiesced (the safe default); the `#3420` fsfreeze-pause test adapted + strengthened to assert the new `frozen` return (`true` on a successful native freeze, `false` on an aborted one); a meter-map guard test. Build + `go vet` clean, `gofmt` clean. - **Dev-cluster e2e:** deployed this build to a dev orchestrator node, funneled placement to it, created a sandbox (guest envd 0.6.13, native `FIFREEZE`), and did a real `memory:false` pause. The metric appeared in dev Mimir through the actual OTEL pipeline: `sum by (quiesced)(orchestrator_sandbox_pause_fs_quiesced_total)` → `{quiesced="true"} = 1`. Reverted afterwards. ## Key decisions - **No new metadata version.** The field is only meaningful on fs-only snapshots, which `MarkFilesystemOnly` already stamps at `>= FilesystemOnlyVersion` — a version `deserialize()` fully unmarshals — so the flag survives without its own version bump. Verified by the round-trip test. - **Backward-compatible by construction.** `omitempty` + the `false` zero value mean a pre-existing/legacy snapshot (no field) reads as *not* quiesced. For the future consumer that is the safe "not eligible, wait for a fresh freezing pause" default; no migration. - **Both freeze paths count as quiesced.** `quiesced=true` means "a real `FIFREEZE` ran," native (≥0.6.6) or exec-API (<0.6.6) alike — both yield an equally crash-consistent rootfs; only `sync` is `false`. The native-vs-exec split remains available on the existing `guest_sync.duration` `method` label, so it is not duplicated here. - **Signature change:** `guestPrepareFsForPause` now returns `(frozen bool, err error)`; its sole caller (`Sandbox.Pause`) and the existing fsfreeze test are updated in this PR. - **Deliberately scoped to record + measure.** The consumer of the flag — the offline rootfs `envd` swap, the reboot-time gate, and the version-remap flag — is intentionally *not* in this PR; it lands separately on top of this building block. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Nikita Kalyazin <nikita.kalyazin@e2b.dev> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Babis Chalios <mail@bchalios.io>

A filesystem-only pause must quiesce the guest rootfs before snapshotting to close the sync->pause write race. Today only envd >= 0.6.6 (native /fsfreeze) does a real FIFREEZE; older guests fall back to a plain
sync, which does not block writes and can capture a torn ext4 journal.For those guests, when the fsfreeze-via-exec flag is on, run
fsfreeze -f /through the envd process API instead — the same FIFREEZE, no native endpoint needed, running entirely inside the guest (no host attack surface). Falls back tosyncper-pause if the guest lacks the fsfreeze binary or the freeze fails. On the pause-failure rollback the rootfs is thawed via the same API (bounded, best-effort).Gated behind featureflags.FsFreezeViaExecFlag (off by default).