Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions packages/api/internal/orchestrator/delete_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions packages/api/internal/orchestrator/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
12 changes: 12 additions & 0 deletions packages/api/internal/orchestrator/pause_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/gogo/status"
"github.com/google/uuid"
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
36 changes: 30 additions & 6 deletions packages/orchestrator/pkg/sandbox/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -1915,6 +1915,7 @@ type pauseOptions struct {
filesystemSnapshot bool
deferRootfsExport bool
maintainSandbox bool
resumeOnFailure bool
}

type PauseOption func(*pauseOptions)
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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)
}
Expand Down
52 changes: 47 additions & 5 deletions packages/orchestrator/pkg/server/sandboxes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()))
Expand Down Expand Up @@ -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()))

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down