fix(pause): preserve sandbox when snapshot fails, behind a flag

A pause whose snapshot failed after the guest was suspended (e.g. a
rootfs-diff fsync EIO, or a memfd ENOMEM under host memory/hugepage
pressure) destroyed the sandbox: Server.Pause armed an unconditional
deferred stop before snapshotting, so the error path tore down the
already-suspended VM, and the API had already removed the routing entry
and store record. The sandbox was left unrecoverable with no snapshot
(#3658).

Behind PauseRefusalRestoreFlag (off by default, so today's
destroy-on-failure behaviour is unchanged), keep the sandbox recoverable:

- sandbox: decouple the resume-on-error cleanup from WithMaintainSandbox
  via a new WithResumeOnFailure() option. It arms the same resume-in-place
  cleanup (health checks restarted, guest clock re-synced) that the
  in-place checkpoint uses, but only on failure - a successful pause still
  suspends and leaves the VM for the caller to stop.
- Server.Pause: pass resumeOnFailure when the flag is on, and arm the
  deferred stop only after the snapshot succeeds. On snapshot error the
  guest has been resumed in place, so re-register it in the live map
  (MarkRunning) and return FailedPrecondition with a "sandbox preserved"
  marker. ErrSandboxLost (the resume itself failed) falls through to the
  destroy path and returns Internal unchanged.
- api: classify that FailedPrecondition+marker as ErrPausePreservedSandbox
  (pause_instance.go) and map it in DeleteInstance to the existing
  restoreRefusedPause path, restoring the store record and route instead
  of removing them.

Tests: TestSnapshotInstance_PreservedSandboxIsClassified and
_PlainFailedPreconditionNotPreserved cover the API classification (a
plain FailedPrecondition is not misclassified). All three packages build;
the full internal/orchestrator suite passes.

End-to-end validation with the flag enabled against real Firecracker
(inject a snapshot fault, assert the sandbox survives and stays routable)
requires a KVM host and is a follow-up.

Addresses the pause half of #3658 (checkpoint half: #3666).

Signed-off-by: adababys <8872412+adababys@users.noreply.github.com>

Author: adababys

packages/api/internal/orchestrator/delete_instance.go

@@ -182,6 +182,45 @@ func (o *Orchestrator) RemoveSandbox(ctx context.Context, teamID uuid.UUID, sand
 			return PauseQueueExhaustedError{}
 		}
 
+		// The node's snapshot failed but it resumed the sandbox in place
+		// (e2b-dev/infra#3658). The VM is alive and back in the node's live map,
+		// so restore the store record and route — exactly like a retryable
+		// refusal — instead of removing them and orphaning a healthy sandbox.
+		// Gated by the same restoreOnRefusal flag the node used to decide to
+		// preserve; if it is off the node would have taken the destroy path and
+		// never returned this error.
+		if errors.Is(err, ErrPausePreservedSandbox) {
+			if restoreOnRefusal {
+				outcome := o.restoreRefusedPause(context.WithoutCancel(ctx), transition)
+				o.recordRefusalRestore(ctx, outcome, opts.Eviction)
+				switch outcome {
+				case restoreOutcomeRestored:
+					preserveRecord = true
+					err = sandbox.ErrTransitionRestored
+				case restoreOutcomeSuperseded:
+					preserveRecord = true
+					err = sandbox.ErrTransitionRestored
+
+					return fmt.Errorf("%w: %w", ErrSandboxNotFound, sandbox.ErrExecutionMismatch)
+				}
+			}
+
+			logger.L().Info(ctx, "Pause snapshot failed but the node preserved the sandbox",
+				logger.WithSandboxID(sbx.SandboxID),
+				zap.Bool("restored", preserveRecord),
+			)
+
+			if !preserveRecord {
+				// The sandbox is alive on the node but we could not restore its
+				// record/route, so it would be an unrouteable orphan: kill it.
+				o.killRefusedSandbox(ctx, sbx)
+
+				return ErrSandboxOperationFailed
+			}
+
+			return nil
+		}
+
 		if errors.Is(err, ErrRefusedRouteLost) {
 			// The record is going and the route is already gone: kill the VM
 			// now rather than leaving it to the orphan reconciler.

packages/api/internal/orchestrator/errors.go

@@ -9,4 +9,9 @@ var (
 	// ErrRefusedRouteLost: the node refused the pause retryably but the edge
 	// could not put the sandbox's route back, so the sandbox cannot be kept.
 	ErrRefusedRouteLost = errors.New("pause refused by the node and the edge could not restore its route")
+	// ErrPausePreservedSandbox: the node's snapshot failed but it resumed the
+	// sandbox in place instead of destroying it (e2b-dev/infra#3658). Handled
+	// like a retryable refusal: the store record and route are restored so the
+	// still-healthy sandbox stays usable, rather than removed.
+	ErrPausePreservedSandbox = errors.New("pause snapshot failed but the node preserved the sandbox")
 )

packages/api/internal/orchestrator/pause_instance.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/gogo/status"
 	"github.com/google/uuid"
@@ -118,6 +119,17 @@ func snapshotInstance(ctx context.Context, node *nodemanager.Node, sbx sandbox.S
 		return ErrRefusedRouteLost
 	}
 
+	// The node's snapshot failed but it resumed the sandbox in place instead of
+	// destroying it (e2b-dev/infra#3658). The orchestrator signals this with
+	// FailedPrecondition and a stable "sandbox preserved" marker in the message.
+	// Classify it so DeleteInstance restores the record + route rather than
+	// removing them, keeping the still-healthy sandbox usable.
+	if st.Code() == codes.FailedPrecondition && strings.Contains(st.Message(), "sandbox preserved after snapshot failure") {
+		logger.L().Warn(ctx, "Pause snapshot failed but the node preserved the sandbox", logger.WithSandboxID(sbx.SandboxID), zap.String("node_message", st.Message()))
+
+		return ErrPausePreservedSandbox
+	}
+
 	return fmt.Errorf("failed to pause sandbox '%s': %w", sbx.SandboxID, err)
 }
 

packages/api/internal/orchestrator/pause_instance_failure_test.go

@@ -128,3 +128,38 @@ func TestPauseSandbox_FailsBuildWhenPauseQueueExhausted(t *testing.T) {
 	assert.Equal(t, string(types.BuildStatusFailed), buildStatus)
 	assert.True(t, hasFinishedAt, "a terminal build must record finished_at")
 }
+
+// A FailedPrecondition carrying the "sandbox preserved after snapshot failure"
+// marker (emitted by the node when WithResumeOnFailure resumed the VM instead
+// of destroying it, e2b-dev/infra#3658) must be classified as
+// ErrPausePreservedSandbox so DeleteInstance restores the record + route.
+func TestSnapshotInstance_PreservedSandboxIsClassified(t *testing.T) {
+	t.Parallel()
+
+	node := nodemanager.NewTestNode("node-preserved", api.NodeStatusReady, 0, 8)
+	node.SetSandboxClient(&pauseFailingSandboxClient{
+		err: status.Error(codes.FailedPrecondition, "sandbox preserved after snapshot failure for 'sbx-x': fsync: input/output error"),
+	})
+
+	sbx := sandbox.Sandbox{SandboxID: "sbx-x", ClusterID: consts.LocalClusterID}
+
+	err := snapshotInstance(t.Context(), node, sbx, "tmpl", "build", false, true)
+	require.ErrorIs(t, err, ErrPausePreservedSandbox)
+}
+
+// A plain FailedPrecondition WITHOUT the preserved marker (e.g. an envd-version
+// precondition) must NOT be misclassified as a preserved sandbox.
+func TestSnapshotInstance_PlainFailedPreconditionNotPreserved(t *testing.T) {
+	t.Parallel()
+
+	node := nodemanager.NewTestNode("node-plain-fp", api.NodeStatusReady, 0, 8)
+	node.SetSandboxClient(&pauseFailingSandboxClient{
+		err: status.Error(codes.FailedPrecondition, "envd version too old"),
+	})
+
+	sbx := sandbox.Sandbox{SandboxID: "sbx-y", ClusterID: consts.LocalClusterID}
+
+	err := snapshotInstance(t.Context(), node, sbx, "tmpl", "build", false, true)
+	require.Error(t, err)
+	require.NotErrorIs(t, err, ErrPausePreservedSandbox)
+}

packages/orchestrator/pkg/sandbox/sandbox.go

@@ -1915,6 +1915,7 @@ type pauseOptions struct {
 	filesystemSnapshot bool
 	deferRootfsExport  bool
 	maintainSandbox    bool
+	resumeOnFailure    bool
 }
 
 type PauseOption func(*pauseOptions)
@@ -1928,6 +1929,22 @@ func WithMaintainSandbox() PauseOption {
 	return func(o *pauseOptions) { o.maintainSandbox = true }
 }
 
+// WithResumeOnFailure keeps the sandbox recoverable across a FAILED pause: if
+// the snapshot fails after the guest has been suspended, the VM is resumed in
+// place (health checks restarted, guest clock re-synced) instead of being left
+// frozen for the caller to tear down, so a transient snapshot error (e.g. a
+// rootfs-diff fsync EIO) no longer destroys an otherwise-healthy sandbox (see
+// e2b-dev/infra#3658).
+//
+// Unlike WithMaintainSandbox, this does NOT resume on success: a successful
+// pause still suspends the guest and leaves it for the caller to stop. It only
+// arms the same resume-on-error cleanup that the in-place checkpoint uses, for
+// the destroy path. The two compose: maintainSandbox implies resume on every
+// outcome; resumeOnFailure alone resumes only on failure.
+func WithResumeOnFailure() PauseOption {
+	return func(o *pauseOptions) { o.resumeOnFailure = true }
+}
+
 // WithFilesystemSnapshot makes the pause produce a filesystem-only snapshot:
 // guest memory is not snapshotted, only the filesystem (rootfs) is persisted.
 // Resuming such a snapshot reboots the guest instead of restoring memory state.
@@ -2130,8 +2147,13 @@ func (s *Sandbox) Pause(
 	// assignment).
 	memExportDeferred := false
 	var freezeStart time.Time
-	resumeOnError := pauseOpts.maintainSandbox
-	if pauseOpts.maintainSandbox {
+	// resumeOnError arms the resume-in-place cleanup below for BOTH the in-place
+	// checkpoint (maintainSandbox: resume on every outcome) and the recoverable
+	// destroy path (resumeOnFailure: resume only when the pause fails). The
+	// cleanup runs only on the error path (see the top-level deferred cleanup.Run
+	// guarded by e != nil), so a successful pause never resumes here regardless.
+	resumeOnError := pauseOpts.maintainSandbox || pauseOpts.resumeOnFailure
+	if resumeOnError {
 		cleanup.Add(ctx, func(ctx context.Context) error {
 			if !resumeOnError {
 				return nil
@@ -2186,14 +2208,17 @@ func (s *Sandbox) Pause(
 	}
 
 	freezeStart = time.Now()
-	if pauseOpts.maintainSandbox {
+	if resumeOnError {
 		// The pause PATCH is the one state flip whose failure is AMBIGUOUS: a
 		// request-ctx cancellation (client disconnect) can kill the round-trip
 		// after FC already applied it. So it runs immune to request
 		// cancellation under the same state-flip bound as the resume, and the
 		// cleanup above resumes on EVERY outcome (see the pre-arm rule at its
 		// registration). pauseLanded — the metric/clock gate — is set only on
 		// a successful return, the one case the guest is KNOWN to have frozen.
+		// Both the in-place checkpoint and the recoverable destroy path
+		// (resumeOnFailure) need this: each arms a resume that must be able to
+		// unfreeze the guest even if the caller's context died.
 		pauseCtx, cancelPause := context.WithTimeout(context.WithoutCancel(ctx), inPlaceStateFlipTimeout)
 		err := s.process.Pause(pauseCtx)
 		cancelPause()
@@ -2202,9 +2227,8 @@ func (s *Sandbox) Pause(
 		}
 		pauseLanded = true
 	} else {
-		// Destroy path: no resume cleanup exists (resumeOnError is false), so
-		// the ambiguity above has no consumer; keep the plain request-scoped
-		// call.
+		// Plain destroy path with no resume cleanup: the ambiguity above has no
+		// consumer, so keep the plain request-scoped call.
 		if err := s.process.Pause(ctx); err != nil {
 			return nil, fmt.Errorf("failed to pause VM: %w", err)
 		}

packages/orchestrator/pkg/server/sandboxes.go

@@ -997,22 +997,59 @@ func (s *Server) Pause(ctx context.Context, in *orchestrator.SandboxPauseRequest
 	// guest and can close the sandbox, which would read as a crash.
 	sbx.SetStopReason(sandbox.StopReasonPaused)
 
-	// Stop the old sandbox in background after we're done
-	defer s.stopSandboxAsync(context.WithoutCancel(ctx), sbx)
+	// When enabled, a snapshot that fails AFTER the guest was suspended (e.g. a
+	// rootfs-diff fsync EIO or a memfd ENOMEM) resumes the VM in place instead
+	// of leaving it frozen for the deferred stop to tear down, so a transient
+	// snapshot error no longer destroys an otherwise-healthy sandbox
+	// (e2b-dev/infra#3658). Gated by the same flag as pause-refusal restore:
+	// both keep the sandbox recoverable when a pause could not be persisted, and
+	// the API path that restores the store record + route already keys off it.
+	// Off by default preserves today's destroy-on-failure behaviour.
+	preserveOnFailure := s.featureFlags.BoolFlag(ctx, featureflags.PauseRefusalRestoreFlag)
 
 	// Defer the rootfs reflink off the pause critical path when enabled: pause is a
 	// suspend, so nothing reads the diff until a later resume (which waits on the
 	// upload anyway). NBD provider only; falls back to synchronous export otherwise.
 	deferRootfsExport := s.featureFlags.BoolFlag(ctx, featureflags.DeferRootfsExportFlag)
 
 	// Fire and forget - upload completes in the background
-	res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), map[string]string{storage.ObjectMetadataTemplateID: in.GetTemplateId()}, storage.ObjectOriginPause, in.GetFilesystemOnly(), deferRootfsExport, false)
+	res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), map[string]string{storage.ObjectMetadataTemplateID: in.GetTemplateId()}, storage.ObjectOriginPause, in.GetFilesystemOnly(), deferRootfsExport, false, preserveOnFailure)
 	if err != nil {
 		telemetry.ReportCriticalError(ctx, "error snapshotting sandbox", err, telemetry.WithSandboxID(in.GetSandboxId()))
 
+		// With preserveOnFailure, sbx.Pause (WithResumeOnFailure) resumed the
+		// guest in place on any error short of ErrSandboxLost, so the VM is
+		// still alive. Put it back in the live map — MarkStopping removed it
+		// before the snapshot — and tell the API the sandbox was preserved via
+		// FailedPrecondition, so it restores the store record and route instead
+		// of removing them. ErrSandboxLost means the resume itself failed and
+		// Pause already tore the VM down: fall through to the destroy path.
+		if preserveOnFailure && !errors.Is(err, sandbox.ErrSandboxLost) {
+			if markErr := s.sandboxFactory.Sandboxes.MarkRunning(ctx, sbx); markErr != nil {
+				// Could not re-register the resumed VM; it would be an
+				// unrouteable orphan. Stop it and report the original error.
+				sbxlogger.E(sbx).Error(ctx, "failed to restore resumed sandbox to live map after snapshot failure", zap.Error(markErr))
+				sbx.SetStopReason(sandbox.StopReasonKilled)
+				s.stopSandboxAsync(context.WithoutCancel(ctx), sbx)
+
+				return nil, status.Errorf(codes.Internal, "error snapshotting sandbox '%s': %s", in.GetSandboxId(), err)
+			}
+
+			return nil, status.Errorf(codes.FailedPrecondition, "sandbox preserved after snapshot failure for '%s': %s", in.GetSandboxId(), err)
+		}
+
+		// Default (flag off) or the resume itself failed (ErrSandboxLost): the
+		// VM is frozen or already gone, so stop it as before.
+		s.stopSandboxAsync(context.WithoutCancel(ctx), sbx)
+
 		return nil, status.Errorf(codes.Internal, "error snapshotting sandbox '%s': %s", in.GetSandboxId(), err)
 	}
 
+	// Snapshot succeeded: stop the old sandbox in background after we're done.
+	// Armed here rather than before the snapshot so a preserved failure above
+	// does not also stop the sandbox it just resumed.
+	defer s.stopSandboxAsync(context.WithoutCancel(ctx), sbx)
+
 	s.uploadSnapshotAsync(ctx, sbx, res)
 
 	// Best-effort: the local snapshot is now in the cache and the remote upload
@@ -1283,7 +1320,8 @@ func (s *Server) checkpointInPlace(ctx context.Context, sbx *sandbox.Sandbox, in
 		storage.ObjectOriginSnapshotTemplate,
 		false, // filesystemOnly: full-memory checkpoint (fs-only in-place is a follow-up)
 		deferRootfsExport,
-		true, // maintainSandbox: resume in place
+		true,  // maintainSandbox: resume in place
+		false, // resumeOnFailure: maintainSandbox already resumes on every outcome
 	)
 	if err != nil {
 		telemetry.ReportCriticalError(ctx, "error snapshotting sandbox for checkpoint", err, telemetry.WithSandboxID(in.GetSandboxId()))
@@ -1368,7 +1406,7 @@ func (s *Server) checkpointResumeFresh(ctx context.Context, sbx *sandbox.Sandbox
 	// Checkpoint resumes a fresh sandbox from the new build immediately, so the
 	// diff must be materialized synchronously — never defer the rootfs export
 	// here, and never maintain the paused sandbox.
-	res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), in.GetMetadata(), storage.ObjectOriginSnapshotTemplate, false, false, false)
+	res, err := s.snapshotAndCacheSandbox(ctx, sbx, in.GetBuildId(), in.GetMetadata(), storage.ObjectOriginSnapshotTemplate, false, false, false, false)
 	if err != nil {
 		telemetry.ReportCriticalError(ctx, "error snapshotting sandbox for checkpoint", err, telemetry.WithSandboxID(in.GetSandboxId()))
 
@@ -1552,6 +1590,7 @@ func (s *Server) snapshotAndCacheSandbox(
 	filesystemOnly bool,
 	deferRootfsExport bool,
 	maintainSandbox bool,
+	resumeOnFailure bool,
 ) (*snapshotResult, error) {
 	meta, err := sbx.Template.Metadata()
 	if err != nil {
@@ -1574,6 +1613,9 @@ func (s *Server) snapshotAndCacheSandbox(
 	if maintainSandbox {
 		pauseOpts = append(pauseOpts, sandbox.WithMaintainSandbox())
 	}
+	if resumeOnFailure {
+		pauseOpts = append(pauseOpts, sandbox.WithResumeOnFailure())
+	}
 
 	snapshot, err := sbx.Pause(ctx, meta, sandbox.SnapshotUseCasePause, pauseOpts...)
 	if err != nil {