From 6c45f08e16f0fbd6b5109cdca92e6883910464b5 Mon Sep 17 00:00:00 2001 From: zoezhao Date: Sat, 5 Sep 2026 12:56:31 -0700 Subject: [PATCH] ateapi: resolve the resume boot source once in loadActorForResume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resume path decided the boot source in two places that agreed only by hand-mirrored ordering. loadActorForResume picked a snapshot URI (the actor's own, else the template's golden), a content scope, and a template-replaced flag; ensureAteletRestored then re-derived the rest in every branch: local-vs-durable from LocalSnapshotInfo, cold boot from an empty SnapshotURI, the telemetry snapshot kind from the actor's external snapshot record, and the wire scope from a three-case switch repeated per branch. Resolve everything in loadActorForResume as a first-match rule list: updated template, paused actor, own durable snapshot, then golden fallback or cold boot (with an explicit boot request skipping only the golden fallback, as before). resumeSnapshotSource now carries the resolved restoreKind and wire scope; ensureAteletRestored dispatches on the kind, maps the telemetry labels from it, and fails with Internal on an unhandled kind instead of silently cold booting. The golden snapshot's validation and URI parsing, previously duplicated between the no-snapshot fallback and the Golden data-resume policy, collapse into resolveGoldenSnapshot; the per-branch scope fallbacks fold into resolveRestoreScope. A paused actor's restore scope now comes from pausedContentScope — the scope recorded on the checkpoint, with the template's onPause standing in for legacy checkpoints — the same derivation suspend uses. GoldenSnapshotURI becomes GoldenForDataSnapshotURI, since it only ever names the guest half of a data-only combined restore, never the boot snapshot itself. A golden boot now gets its own log line (it used to log as a durable-snapshot restore) and an explicitly FULL wire scope (it used to echo the golden's recorded content scope). Four behavior changes ride along, all on paths where the old code resolved or demanded state the restore would not use: - An actor whose ActorTemplate was updated restores its data alone, but the old resolution still consulted the Golden resume policy for its Data-scoped capture and failed with FailedPrecondition (or DataLoss) when the template recorded no valid golden. Rule 1 now returns before the policy is consulted. - The template update was only detected when the actor held a durable snapshot. An actor resuming from a local pause checkpoint alone restored at the template's onPause scope, carrying guest state built on the previous template; it now gets the same data-only restore as every other actor whose template was updated. - The golden fallback is only resolved for an actor with no snapshot of its own. A paused actor without a durable snapshot used to run the fallback resolution anyway — its local checkpoint restore ignored the result, but a legacy-scoped or unparseable golden failed the resume. - A paused actor's shadowed durable snapshot URI was parsed even though the local restore never consumes it, so an unparseable URI failed the resume with DataLoss. The URI is now parsed exactly when the durable snapshot is the boot source. Tests pin the rule list: the updated-template bypass of the Golden policy, boot skipping only the golden fallback, the golden fallback's FULL scope, the recorded pause scope outranking the template's onPause, DataLoss on an unparseable durable snapshot URI only when it is the boot source, and the kind-to-telemetry mapping. --- .../internal/controlapi/workflow_resume.go | 336 +++++++---- .../controlapi/workflow_resume_test.go | 532 +++++++++++++++--- 2 files changed, 682 insertions(+), 186 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/workflow_resume.go b/cmd/ateapi/internal/controlapi/workflow_resume.go index c3b6fca08d..b5df23c22b 100644 --- a/cmd/ateapi/internal/controlapi/workflow_resume.go +++ b/cmd/ateapi/internal/controlapi/workflow_resume.go @@ -34,23 +34,71 @@ import ( "k8s.io/apimachinery/pkg/util/wait" ) +// restoreKind identifies which boot source the restore step drives. Resolved +// once by loadActorForResume so the restore step and its telemetry dispatch +// on it instead of re-deriving the precedence from raw actor fields. +type restoreKind int + +const ( + // restoreColdBoot boots from the template spec: no snapshot anywhere. + // The zero value, so a zero resumeSnapshotSource means cold boot. + restoreColdBoot restoreKind = iota + // restoreLocalSnapshot restores the actor's local pause checkpoint on + // the worker node. Outranks any durable snapshot the actor also holds. + restoreLocalSnapshot + // restoreDurableSnapshot restores the actor's own durable snapshot at + // SnapshotURI. + restoreDurableSnapshot + // restoreGoldenSnapshot restores the template's golden snapshot at + // SnapshotURI as the actor's own content. + restoreGoldenSnapshot +) + +// telemetrySnapshotKind maps the restore kind to the snapshot-kind label of +// the resume lifecycle metric. +func (k restoreKind) telemetrySnapshotKind() string { + switch k { + case restoreLocalSnapshot: + return ateattr.SnapshotKindLocal + case restoreDurableSnapshot: + return ateattr.SnapshotKindLatest + case restoreGoldenSnapshot: + return ateattr.SnapshotKindGolden + case restoreColdBoot: + return ateattr.SnapshotKindBoot + default: + // An unhandled kind fails ensureAteletRestored; stay within the + // registered label values here. + return ateattr.SnapshotKindBoot + } +} + // resumeSnapshotSource is the boot source resolved once by loadActorForResume // and passed by value to the restore step — never mutated after resolution. type resumeSnapshotSource struct { - // SnapshotURI is the storage location of the durable snapshot to restore - // from: the actor's own latest snapshot when one exists, the template's - // golden snapshot otherwise. Zero means cold boot from the spec (unless - // the actor holds a local snapshot, which takes precedence at restore). + // Kind selects the restore path; the remaining fields only carry what + // that path consumes. + Kind restoreKind + // SnapshotURI is the storage location of the external snapshot a + // restoreDurableSnapshot or restoreGoldenSnapshot restore boots from; + // zero for every other kind. A paused actor's shadowed durable snapshot + // is never parsed or carried: the local checkpoint outranks it. SnapshotURI resources.SnapshotURI - Scope ateapipb.SnapshotContentScope - // GoldenSnapshotURI is the storage location of the ActorTemplate's golden - // snapshot. Populated only when the template's onResume configuration - // selects the golden snapshot as the boot source for the pending restore: - // restore then combines the golden snapshot with the actor's data. - GoldenSnapshotURI resources.SnapshotURI - // TemplateReplaced is true when the snapshot's recorded template UID - // differs from the actor's current template. - TemplateReplaced bool + // GoldenForDataSnapshotURI is the storage location of the ActorTemplate's + // golden snapshot. Populated only when the template's onResume + // configuration selects the golden snapshot for a pending data-only + // restore: the restore then combines the golden snapshot with the + // actor's data. + GoldenForDataSnapshotURI resources.SnapshotURI + // WireScope is the scope of the restore operation, not of any stored + // snapshot: + // - DATA_ON_GOLDEN: the restore rides the golden snapshot. + // - DATA: the restore discards the snapshot's guest state and carries + // the actor's durable data alone. + // - FULL or unspecified: the restore uses a FULL snapshot. + // + // Meaningless for a cold boot: RunRequest carries no scope. + WireScope ateletpb.SnapshotScope } // restoreTelemetry labels the restore operation for the resume lifecycle @@ -154,7 +202,22 @@ func validateGoldenSnapshotScope(snapshot *ateapipb.ExternalSnapshot) error { } // loadActorForResume fetches the current actor record and its template, and -// resolves the boot source for the pending restore. +// resolves the boot source for the pending restore. The source is decided by +// the first matching rule: +// +// 1. The snapshot of an actor whose ActorTemplate was updated holds guest +// state from the previous template, so the restore carries the actor's +// durable data alone. +// 2. A paused actor restores its local pause checkpoint at the scope the +// checkpoint was captured with; a Data-scoped pause rides the golden +// when the onResume policy selects it. +// 3. An actor on its own template restores its snapshot at the captured +// scope; a Data-scoped capture rides the golden when the onResume +// policy selects it. +// 4. An actor with no snapshot restores the template's golden when one +// exists, and cold boots from the spec otherwise. An explicit boot +// request skips the golden fallback and always cold boots; it has no +// effect on an actor with a snapshot of its own. func (w *ActorWorkflow) loadActorForResume(ctx context.Context, actorRef resources.ActorRef, boot bool) (_ *ateapipb.Actor, _ *ateapipb.ActorTemplate, _ resumeSnapshotSource, err error) { ctx, done := stepSpan(ctx, "LoadActorForResume") defer func() { err = done(err) }() @@ -179,59 +242,122 @@ func (w *ActorWorkflow) loadActorForResume(ctx context.Context, actorRef resourc if err != nil { return nil, nil, src, err } - goldenSnapshotStatus := actorTemplate.GetStatus().GetGoldenSnapshotStatus() - if uri := actor.GetStatus().GetExternalSnapshot().GetSnapshotUri(); uri != "" { - if src.SnapshotURI, err = resources.ParseSnapshotURI(uri); err != nil { + external := actor.GetStatus().GetExternalSnapshot() + switch { + case actor.GetStatus().GetLocalSnapshotInfo() != nil: + // The local checkpoint outranks any durable snapshot it shadows, so + // the shadowed URI is never parsed: an unparseable URI must not fail + // a restore that would not consume it. + src.Kind = restoreLocalSnapshot + case external.GetSnapshotUri() != "": + src.Kind = restoreDurableSnapshot + if src.SnapshotURI, err = resources.ParseSnapshotURI(external.GetSnapshotUri()); err != nil { return nil, nil, src, status.Errorf(codes.DataLoss, "Actor %s external snapshot: %v", actorRef, err) } - src.Scope = actor.GetStatus().GetExternalSnapshot().GetContentScope() - // The Actor records the template its guest state was built on; a - // different UID on its current template means it was repointed since - // the capture. - builtOnTemplateUID := actor.GetStatus().GetCurrentActorTemplateUid() - src.TemplateReplaced = builtOnTemplateUID != "" && builtOnTemplateUID != actorTemplate.GetMetadata().GetUid() - } else if goldenURI := goldenSnapshotStatus.GetGoldenSnapshot().GetSnapshotUri(); goldenURI != "" && !boot { - if err := validateGoldenSnapshotScope(goldenSnapshotStatus.GetGoldenSnapshot()); err != nil { + } + + // Rule 1: an actor's template has been updated, so the restore carries + // the actor's durable data alone. + builtOnTemplateUID := actor.GetStatus().GetCurrentActorTemplateUid() + if src.Kind != restoreColdBoot && builtOnTemplateUID != "" && builtOnTemplateUID != actorTemplate.GetMetadata().GetUid() { + src.WireScope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA + return actor, actorTemplate, src, nil + } + + // Rule 2: a paused actor restores its local pause checkpoint at the + // scope the pause recorded. + if src.Kind == restoreLocalSnapshot { + scope := pausedContentScope(actor.GetStatus().GetLocalSnapshotInfo(), actorTemplate) + if src.GoldenForDataSnapshotURI, src.WireScope, err = resolveRestoreScope(scope, actorTemplate); err != nil { return nil, nil, src, err } - if src.SnapshotURI, err = resources.ParseSnapshotURI(goldenURI); err != nil { - return nil, nil, src, status.Errorf(codes.DataLoss, "golden external snapshot %q: %v", goldenURI, err) - } - src.Scope = goldenSnapshotStatus.GetGoldenSnapshot().GetContentScope() - } - - // The template's onResume configuration selects the boot source for the - // pending restore. When it names the golden snapshot, resolve the golden - // snapshot's location so the restore can combine the golden's guest - // state with the actor's data. The pending - // restore is data-only when the actor is paused with a Data pause scope - // (the local snapshot takes precedence at restore), or when its durable - // snapshot holds Data. Valid Full snapshots restore from their own - // content and ignore the policy. - if actorTemplate.GetSnapshotsConfig().GetOnResume().GetFromData() == ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN { - dataOnly := false - if actor.GetStatus().GetLocalSnapshotInfo() != nil { - dataOnly = effectiveContentScope(actorTemplate.GetSnapshotsConfig().GetOnPause()) == ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA - } else if actor.GetStatus().GetExternalSnapshot().GetSnapshotUri() != "" { - dataOnly = src.Scope == ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA - } - if dataOnly { - goldenURI := goldenSnapshotStatus.GetGoldenSnapshot().GetSnapshotUri() - if goldenURI == "" { - return nil, nil, src, status.Error(codes.FailedPrecondition, "a Golden data resume requires the ActorTemplate golden snapshot, which is not available") - } - if err := validateGoldenSnapshotScope(goldenSnapshotStatus.GetGoldenSnapshot()); err != nil { - return nil, nil, src, err - } - if src.GoldenSnapshotURI, err = resources.ParseSnapshotURI(goldenURI); err != nil { - return nil, nil, src, status.Errorf(codes.DataLoss, "golden external snapshot %q: %v", goldenURI, err) - } + return actor, actorTemplate, src, nil + } + + // Rule 3: the actor restores its own durable snapshot at the captured + // scope; the onResume policy only governs data-only restores, so a valid + // Full (or legacy Unspecified) capture restores from its own content. + if src.Kind == restoreDurableSnapshot { + if src.GoldenForDataSnapshotURI, src.WireScope, err = resolveRestoreScope(external.GetContentScope(), actorTemplate); err != nil { + return nil, nil, src, err } + return actor, actorTemplate, src, nil } + // Rule 4: an actor with no snapshot of its own restores the template's + // golden snapshot as its own content when one exists, and cold boots + // from the spec otherwise (the wire scope is inert then: RunRequest + // carries no scope). An explicit boot request skips the golden fallback + // without resolving it, so an unusable golden cannot block the boot. + if boot { + return actor, actorTemplate, src, nil + } + goldenURI, err := resolveGoldenSnapshot(actorTemplate) + if errors.Is(err, errNoGoldenSnapshot) { + return actor, actorTemplate, src, nil + } + if err != nil { + return nil, nil, src, err + } + src.Kind = restoreGoldenSnapshot + src.SnapshotURI = goldenURI + // A golden snapshot is always FULL scope. + src.WireScope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL return actor, actorTemplate, src, nil } +// errNoGoldenSnapshot reports that the ActorTemplate records no golden +// snapshot. Callers decide what that means for their resume path: a gRPC +// status naming the path that needed the golden, or a fallback source. +var errNoGoldenSnapshot = errors.New("ActorTemplate has no golden snapshot") + +// resolveGoldenSnapshot resolves the storage location of the template's +// golden snapshot, validating that it can serve as a boot source or as the +// guest half of a combined restore. Returns errNoGoldenSnapshot when the +// template records no golden snapshot. +func resolveGoldenSnapshot(actorTemplate *ateapipb.ActorTemplate) (resources.SnapshotURI, error) { + golden := actorTemplate.GetStatus().GetGoldenSnapshotStatus().GetGoldenSnapshot() + if golden.GetSnapshotUri() == "" { + return resources.SnapshotURI{}, errNoGoldenSnapshot + } + if err := validateGoldenSnapshotScope(golden); err != nil { + return resources.SnapshotURI{}, err + } + uri, err := resources.ParseSnapshotURI(golden.GetSnapshotUri()) + if err != nil { + return resources.SnapshotURI{}, status.Errorf(codes.DataLoss, "golden external snapshot %q: %v", golden.GetSnapshotUri(), err) + } + return uri, nil +} + +// resolveRestoreScope maps a captured snapshot content scope to the wire +// scope of the pending restore. A Data-scoped capture rides the template's +// golden snapshot when the onResume policy selects it — then the returned +// URI locates the golden half of the combined restore, and is zero otherwise. +func resolveRestoreScope(scope ateapipb.SnapshotContentScope, actorTemplate *ateapipb.ActorTemplate) (goldenForData resources.SnapshotURI, _ ateletpb.SnapshotScope, _ error) { + wireScope := actorSnapshotContentScopeToAtelet(scope) + onResumeGolden := actorTemplate.GetSnapshotsConfig().GetOnResume().GetFromData() == ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN + if wireScope != ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA || !onResumeGolden { + return resources.SnapshotURI{}, wireScope, nil + } + goldenURI, err := resolveGoldenForDataResume(actorTemplate) + if err != nil { + return resources.SnapshotURI{}, wireScope, err + } + return goldenURI, ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN, nil +} + +// resolveGoldenForDataResume resolves the golden snapshot a Golden data +// resume rides, failing with FailedPrecondition when the template records +// none. +func resolveGoldenForDataResume(actorTemplate *ateapipb.ActorTemplate) (resources.SnapshotURI, error) { + uri, err := resolveGoldenSnapshot(actorTemplate) + if errors.Is(err, errNoGoldenSnapshot) { + return resources.SnapshotURI{}, status.Error(codes.FailedPrecondition, "a Golden data resume requires the ActorTemplate golden snapshot, which is not available") + } + return uri, err +} + // ensureVolumesCreated provisions any initial actor volumes that are in // PENDING state, persisting the resulting volume state (even when creation // partially failed, so progress is not lost) and returning the stored copy. @@ -639,13 +765,13 @@ func (w *ActorWorkflow) ensureVolumesAttached(ctx context.Context, actor *ateapi return nil } -// ensureAteletRestored brings the workload up on the assigned worker: -// restoring the actor's local snapshot when one exists, else its durable (or -// golden) snapshot, else cold-booting from the template spec. This is the -// atelet reentrancy seam (#372): the request is keyed by the actor UID and -// the worker pod UID, so a re-entered workflow re-sends the same semantic -// request; once atelet's Restore/Run are idempotent on those keys this step -// becomes fully reentrant with no changes here. +// ensureAteletRestored brings the workload up on the assigned worker, +// dispatching on the resolved src.Kind: the actor's local pause checkpoint, +// an external (durable or golden) snapshot, or a cold boot from the template +// spec. This is the atelet reentrancy seam (#372): the request is keyed by +// the actor UID and the worker pod UID, so a re-entered workflow re-sends the +// same semantic request; once atelet's Restore/Run are idempotent on those +// keys this step becomes fully reentrant with no changes here. func (w *ActorWorkflow) ensureAteletRestored(ctx context.Context, actorRef resources.ActorRef, actor *ateapipb.Actor, actorTemplate *ateapipb.ActorTemplate, src resumeSnapshotSource) (tele restoreTelemetry, err error) { ctx, done := stepSpan(ctx, "CallAteletRestore") defer func() { err = done(err) }() @@ -670,9 +796,16 @@ func (w *ActorWorkflow) ensureAteletRestored(ctx context.Context, actorRef resou return tele, err } - if local := actor.GetStatus().GetLocalSnapshotInfo(); local != nil { + tele.SnapshotKind = src.Kind.telemetrySnapshotKind() + if src.Kind != restoreColdBoot { + // Not set for a cold boot: RunRequest carries no scope, and mapping + // the meaningless src.WireScope would label boot series as unknown. + tele.WireSnapshotScope = ateattr.SnapshotScopeValue(src.WireScope) + } + + switch src.Kind { + case restoreLocalSnapshot: slog.InfoContext(ctx, "Actor has snapshot; Restoring from snapshot") - tele.SnapshotKind = ateattr.SnapshotKindLocal req := &ateletpb.RestoreRequest{ TargetAteomUid: assignment.GetWorkerPodUid(), @@ -688,44 +821,21 @@ func (w *ActorWorkflow) ensureAteletRestored(ctx context.Context, actorRef resou } req.Type = ateletpb.CheckpointType_CHECKPOINT_TYPE_LOCAL req.Config = &ateletpb.RestoreRequest_LocalConfig{ - LocalConfig: &ateletpb.LocalCheckpointConfiguration{SnapshotName: local.GetSnapshotName()}, - } - // The wire scope describes the restore OPERATION: DATA_ON_GOLDEN when - // loadActorForResume resolved a golden URI per the template's onResume - // configuration, else what the pause captured. - switch { - case src.TemplateReplaced: - req.Scope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA - case !src.GoldenSnapshotURI.IsZero(): - req.Scope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN - req.GoldenSnapshotUri = src.GoldenSnapshotURI.String() - default: - req.Scope = actorSnapshotContentScopeToAtelet(actorTemplate.GetSnapshotsConfig().GetOnPause()) + LocalConfig: &ateletpb.LocalCheckpointConfiguration{SnapshotName: actor.GetStatus().GetLocalSnapshotInfo().GetSnapshotName()}, + } + req.Scope = src.WireScope + // Empty unless this is a Golden data resume. + if src.WireScope == ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN { + req.GoldenSnapshotUri = src.GoldenForDataSnapshotURI.String() } - tele.WireSnapshotScope = ateattr.SnapshotScopeValue(req.Scope) - _, err = client.Restore(ctx, req) return tele, maybeCrashActor(ctx, w.store, actorRef, err, "while restoring workload", ateattr.OperationResume) - } else if !src.SnapshotURI.IsZero() { - slog.InfoContext(ctx, "Actor has durable snapshot; Restoring from snapshot") - // Mirrors loadActorForResume's source resolution: the durable URI is - // the actor's own snapshot when one exists, the golden otherwise. - tele.SnapshotKind = ateattr.SnapshotKindGolden - if actor.GetStatus().GetExternalSnapshot().GetSnapshotUri() != "" { - tele.SnapshotKind = ateattr.SnapshotKindLatest - } - var scope ateletpb.SnapshotScope - var goldenSnapshotURI string - switch { - case src.TemplateReplaced: - scope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA - case !src.GoldenSnapshotURI.IsZero(): - scope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN - goldenSnapshotURI = src.GoldenSnapshotURI.String() - default: - scope = actorSnapshotContentScopeToAtelet(src.Scope) + case restoreDurableSnapshot, restoreGoldenSnapshot: + if src.Kind == restoreGoldenSnapshot { + slog.InfoContext(ctx, "Actor has no snapshot; Restoring from the ActorTemplate golden snapshot") + } else { + slog.InfoContext(ctx, "Actor has durable snapshot; Restoring from snapshot") } - tele.WireSnapshotScope = ateattr.SnapshotScopeValue(scope) req := &ateletpb.RestoreRequest{ TargetAteomUid: assignment.GetWorkerPodUid(), Atespace: actor.GetMetadata().GetAtespace(), @@ -739,19 +849,19 @@ func (w *ActorWorkflow) ensureAteletRestored(ctx context.Context, actorRef resou SnapshotUri: src.SnapshotURI.String(), }, }, - Scope: scope, - // Empty unless this is a Golden data resume. - GoldenSnapshotUri: goldenSnapshotURI, - ActorUid: actor.GetMetadata().Uid, - EgressGateway: egressGateway, - CpuMilli: cpuMilli, - MemoryBytes: memBytes, + Scope: src.WireScope, + ActorUid: actor.GetMetadata().Uid, + EgressGateway: egressGateway, + CpuMilli: cpuMilli, + MemoryBytes: memBytes, + } + if src.WireScope == ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN { + req.GoldenSnapshotUri = src.GoldenForDataSnapshotURI.String() } _, err = client.Restore(ctx, req) return tele, maybeCrashActor(ctx, w.store, actorRef, err, "while restoring durable snapshot", ateattr.OperationResume) - } else { - slog.InfoContext(ctx, "Actor has no snapshot; ActorTemplate has no golden snapshot; Booting from ActorTemplate spec") - tele.SnapshotKind = ateattr.SnapshotKindBoot + case restoreColdBoot: + slog.InfoContext(ctx, "Actor has no snapshot; Booting from ActorTemplate spec") // Booting from scratch: resolve the sandbox binaries from the // template's SandboxConfig and send them so atelet can fetch and @@ -777,6 +887,10 @@ func (w *ActorWorkflow) ensureAteletRestored(ctx context.Context, actorRef resou } _, err = client.Run(ctx, req) return tele, maybeCrashActor(ctx, w.store, actorRef, err, "while creating workload from spec", ateattr.OperationResume) + default: + // Fail loud rather than cold boot an actor whose snapshot a new, + // unhandled kind was meant to restore. + return tele, status.Errorf(codes.Internal, "unhandled restore kind %d for Actor %s", src.Kind, actorRef) } } @@ -800,7 +914,7 @@ func (w *ActorWorkflow) finalizeRunning(ctx context.Context, actorRef resources. storedActor, err := w.store.UpdateActor(ctx, actorRef, store.PreconditionFrom(latestActor), func(toUpdate *ateapipb.Actor) error { toUpdate.Status.State = ateapipb.ActorState_ACTOR_STATE_RUNNING - // Recorded at sprint start so the next resume can detect a repointed + // Recorded at sprint start so the next resume can detect an updated // template by UID; the snapshot it restores was taken under this one. toUpdate.Status.CurrentActorTemplateUid = actorTemplate.GetMetadata().GetUid() return nil diff --git a/cmd/ateapi/internal/controlapi/workflow_resume_test.go b/cmd/ateapi/internal/controlapi/workflow_resume_test.go index f5797d61a3..df412d2165 100644 --- a/cmd/ateapi/internal/controlapi/workflow_resume_test.go +++ b/cmd/ateapi/internal/controlapi/workflow_resume_test.go @@ -27,6 +27,8 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store/storetest" "github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache" + "github.com/agent-substrate/substrate/internal/ateattr" + "github.com/agent-substrate/substrate/internal/proto/ateletpb" "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc/codes" @@ -95,7 +97,7 @@ func TestResumeActor_RunningFastPathDoesNotAcquireLease(t *testing.T) { // TestFinalizeRunning_RecordsSprintTemplate verifies committing RUNNING stamps // the template the sprint booted with, overwriting the previous sprint's -// record, so the next resume can detect a repointed template by UID. +// record, so the next resume can detect an updated template by UID. func TestFinalizeRunning_RecordsSprintTemplate(t *testing.T) { ctx := context.Background() persistence := newTestPersistence(t) @@ -886,6 +888,80 @@ func TestValidateAssignedWorker_WorkerOwnership(t *testing.T) { } } +// resumeSeed describes the stored state a loadActorForResume test builds: the +// actor's snapshot state, and the ns/tmpl1 template's snapshots config and +// recorded golden snapshot. +type resumeSeed struct { + // paused stores the actor PAUSED with a local pause checkpoint; noSnapshot + // leaves the actor without any snapshot; otherwise a durable snapshot + // captured at contentScope is seeded. + paused bool + noSnapshot bool + contentScope ateapipb.SnapshotContentScope + // pausedScope is the content scope recorded on the pause checkpoint; the + // zero value seeds a legacy checkpoint with no recorded scope. + pausedScope ateapipb.SnapshotContentScope + // onPause and fromData seed the template's SnapshotsConfig. + onPause ateapipb.SnapshotContentScope + fromData ateapipb.ResumeSource + // goldenURI and goldenScope are the template's recorded golden external + // snapshot; an empty URI means the template has none. + goldenURI string + goldenScope ateapipb.SnapshotContentScope +} + +// seedResumeTemplate creates the ns atespace and the ns/tmpl1 template the +// seed describes, returning the stored template (for its assigned UID). +func seedResumeTemplate(t *testing.T, ctx context.Context, persistence store.Interface, seed resumeSeed) *ateapipb.ActorTemplate { + t.Helper() + storetest.MustCreateAtespace(t, ctx, persistence, "ns") + tmpl := &ateapipb.ActorTemplate{ + Metadata: &ateapipb.ResourceMetadata{Atespace: "ns", Name: "tmpl1"}, + SnapshotsConfig: &ateapipb.SnapshotsConfig{ + OnPause: seed.onPause, + OnResume: &ateapipb.OnResumeConfig{FromData: seed.fromData}, + }, + } + if seed.goldenURI != "" { + tmpl.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ + GoldenSnapshot: &ateapipb.ExternalSnapshot{SnapshotUri: seed.goldenURI, ContentScope: seed.goldenScope}, + }} + } + stored, err := persistence.CreateActorTemplate(ctx, tmpl) + if err != nil { + t.Fatalf("create template: %v", err) + } + return stored +} + +// seedResumeActor stores the actor the seed describes, bound to ns/tmpl1, and +// returns the durable snapshot URI it was seeded with ("" when none was). +// opts mutate the actor after the seed's snapshot state is applied. +func seedResumeActor(t *testing.T, ctx context.Context, persistence store.Interface, actorRef resources.ActorRef, seed resumeSeed, opts ...func(*ateapipb.Actor)) string { + t.Helper() + durableSnapshotURI := "" + var seedOpts []func(*ateapipb.Actor) + actorState := ateapipb.ActorState_ACTOR_STATE_SUSPENDED + switch { + case seed.paused: + actorState = ateapipb.ActorState_ACTOR_STATE_PAUSED + seedOpts = append(seedOpts, func(a *ateapipb.Actor) { + a.Status.LocalSnapshotInfo = &ateapipb.LocalSnapshotInfo{SnapshotName: "pause-1", ContentScope: seed.pausedScope} + }) + case !seed.noSnapshot: + durableSnapshotURI = someActorSnapshotURI(t, testStorageLocation, actorRef.Atespace, "snap-1") + seedOpts = append(seedOpts, func(a *ateapipb.Actor) { + a.Status.ExternalSnapshot = &ateapipb.ExternalSnapshot{ + SnapshotUri: durableSnapshotURI, + ContentScope: seed.contentScope, + } + }) + } + seedOpts = append(seedOpts, opts...) + seedWorkflowActor(t, ctx, persistence, actorRef, "ns", "tmpl1", actorState, seedOpts...) + return durableSnapshotURI +} + // TestLoadActorForResume_OnGoldenDataResume verifies the golden-location // plumbing: when the template's onResume.fromData is Golden, a pending // data-only restore (a Data durable snapshot, or a paused actor whose @@ -898,9 +974,12 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { name string fromData ateapipb.ResumeSource // paused seeds the actor with LocalSnapshotInfo (a pause checkpoint) - // instead of a durable snapshot; onPause is the template's pause - // scope, contentScope the durable snapshot's recorded content. + // instead of a durable snapshot; pausedScope is the scope recorded on + // that checkpoint (zero for a legacy checkpoint without one); onPause + // is the template's pause scope, contentScope the durable snapshot's + // recorded content. paused bool + pausedScope ateapipb.SnapshotContentScope onPause ateapipb.SnapshotContentScope contentScope ateapipb.SnapshotContentScope // goldenURI and goldenScope are the template's recorded golden @@ -910,6 +989,7 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope ateapipb.SnapshotContentScope wantCode codes.Code wantGoldenURI string + wantWireScope ateletpb.SnapshotScope }{ { name: "resolves golden location for Data durable snapshot", @@ -919,8 +999,11 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantCode: codes.OK, wantGoldenURI: goldenSnapshotURI, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN, }, { + // A legacy checkpoint records no scope, so the template's onPause + // stands in for it. name: "resolves golden location for paused actor with Data onPause", fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, paused: true, @@ -929,6 +1012,36 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantCode: codes.OK, wantGoldenURI: goldenSnapshotURI, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN, + }, + { + // The scope recorded on the checkpoint is authoritative: a Data + // capture rides the golden even when the template's onPause says + // Full. + name: "recorded Data pause scope outranks Full onPause", + fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, + paused: true, + pausedScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + onPause: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantCode: codes.OK, + wantGoldenURI: goldenSnapshotURI, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN, + }, + { + // The reverse direction: a Full capture restores from its own + // content even when the template's onPause says Data. + name: "recorded Full pause scope outranks Data onPause", + fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, + paused: true, + pausedScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + onPause: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantCode: codes.OK, + wantGoldenURI: "", + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL, }, { // A Full pause snapshot restores from its own content; the policy @@ -941,6 +1054,20 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantCode: codes.OK, wantGoldenURI: "", + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL, + }, + { + // A Data pause checkpoint under a non-Golden policy restores its + // data alone: no golden ride, plain Data wire scope. + name: "leaves golden location empty for paused actor with Data onPause under ColdBoot fromData", + fromData: ateapipb.ResumeSource_RESUME_SOURCE_COLD_BOOT, + paused: true, + onPause: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantCode: codes.OK, + wantGoldenURI: "", + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA, }, { name: "fails when golden snapshot is not Full", @@ -974,6 +1101,7 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantCode: codes.OK, wantGoldenURI: "", + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL, }, { name: "leaves golden location empty under ColdBoot fromData", @@ -983,6 +1111,7 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantCode: codes.OK, wantGoldenURI: "", + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA, }, } @@ -991,41 +1120,70 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { ctx := context.Background() persistence := newTestPersistence(t) - var seedOpts []func(*ateapipb.Actor) - if tt.paused { - seedOpts = append(seedOpts, func(a *ateapipb.Actor) { - a.Status.LocalSnapshotInfo = &ateapipb.LocalSnapshotInfo{SnapshotName: "pause-1"} - }) - } else { - seedOpts = append(seedOpts, func(a *ateapipb.Actor) { - a.Status.ExternalSnapshot = &ateapipb.ExternalSnapshot{ - SnapshotUri: someActorSnapshotURI(t, testStorageLocation, actorRef.Atespace, "snap-1"), - ContentScope: tt.contentScope, - } - }) + seed := resumeSeed{ + paused: tt.paused, + pausedScope: tt.pausedScope, + contentScope: tt.contentScope, + onPause: tt.onPause, + fromData: tt.fromData, + goldenURI: tt.goldenURI, + goldenScope: tt.goldenScope, } - actorState := ateapipb.ActorState_ACTOR_STATE_SUSPENDED - if tt.paused { - actorState = ateapipb.ActorState_ACTOR_STATE_PAUSED - } - seedWorkflowActor(t, ctx, persistence, actorRef, "ns", "tmpl1", actorState, seedOpts...) + seedResumeTemplate(t, ctx, persistence, seed) + seedResumeActor(t, ctx, persistence, actorRef, seed) - storetest.MustCreateAtespace(t, ctx, persistence, "ns") - tmpl := &ateapipb.ActorTemplate{ - Metadata: &ateapipb.ResourceMetadata{Atespace: "ns", Name: "tmpl1"}, - SnapshotsConfig: &ateapipb.SnapshotsConfig{ - OnPause: tt.onPause, - OnResume: &ateapipb.OnResumeConfig{FromData: tt.fromData}, - }, + w := &ActorWorkflow{store: persistence} + _, _, src, err := w.loadActorForResume(ctx, actorRef, false) + if got := status.Code(err); got != tt.wantCode { + t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, tt.wantCode, err) + } + if err != nil { + return + } + if got := src.GoldenForDataSnapshotURI.String(); got != tt.wantGoldenURI { + t.Errorf("src.GoldenForDataSnapshotURI = %q, want %q", got, tt.wantGoldenURI) } - if tt.goldenURI != "" { - tmpl.Status = &ateapipb.ActorTemplateStatus{GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ - GoldenSnapshot: &ateapipb.ExternalSnapshot{SnapshotUri: tt.goldenURI, ContentScope: tt.goldenScope}, - }} + if src.WireScope != tt.wantWireScope { + t.Errorf("src.WireScope = %v, want %v", src.WireScope, tt.wantWireScope) } - if _, err := persistence.CreateActorTemplate(ctx, tmpl); err != nil { - t.Fatalf("create template: %v", err) + wantKind := restoreDurableSnapshot + if tt.paused { + wantKind = restoreLocalSnapshot + } + if src.Kind != wantKind { + t.Errorf("src.Kind = %v, want %v", src.Kind, wantKind) } + }) + } +} + +// TestLoadActorForResume_UnparseableDurableSnapshotURI pins that a recorded +// durable snapshot URI that fails to parse fails the resume with DataLoss +// only when that snapshot is the boot source. A paused actor's local +// checkpoint outranks the durable snapshot it shadows, so the shadowed URI +// is never parsed and cannot fail a restore that does not consume it. +func TestLoadActorForResume_UnparseableDurableSnapshotURI(t *testing.T) { + actorRef := resources.ActorRef{Atespace: "team-a", Name: "id1"} + + tests := []struct { + name string + paused bool + wantCode codes.Code + }{ + {name: "paused actor ignores an unparseable shadowed durable snapshot", paused: true, wantCode: codes.OK}, + {name: "suspended actor with an unparseable durable snapshot", wantCode: codes.DataLoss}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + persistence := newTestPersistence(t) + + seed := resumeSeed{paused: tt.paused, noSnapshot: true} + seedResumeTemplate(t, ctx, persistence, seed) + seedResumeActor(t, ctx, persistence, actorRef, seed, func(a *ateapipb.Actor) { + a.Status.ExternalSnapshot = &ateapipb.ExternalSnapshot{SnapshotUri: "not-a-snapshot-uri"} + }) w := &ActorWorkflow{store: persistence} _, _, src, err := w.loadActorForResume(ctx, actorRef, false) @@ -1035,11 +1193,11 @@ func TestLoadActorForResume_OnGoldenDataResume(t *testing.T) { if err != nil { return } - if got := src.GoldenSnapshotURI.String(); got != tt.wantGoldenURI { - t.Errorf("src.GoldenSnapshotURI = %q, want %q", got, tt.wantGoldenURI) + if src.Kind != restoreLocalSnapshot { + t.Errorf("src.Kind = %v, want %v", src.Kind, restoreLocalSnapshot) } - if !tt.paused && src.Scope != tt.contentScope { - t.Errorf("src.Scope = %v, want %v", src.Scope, tt.contentScope) + if !src.SnapshotURI.IsZero() { + t.Errorf("src.SnapshotURI = %q, want zero: the shadowed durable snapshot is not the boot source", src.SnapshotURI.String()) } }) } @@ -1056,22 +1214,13 @@ func TestLoadActorForResume_GoldenFallbackRejectsNonFullGolden(t *testing.T) { persistence := newTestPersistence(t) actorRef := resources.ActorRef{Atespace: "team-a", Name: "id1"} - seedWorkflowActor(t, ctx, persistence, actorRef, "ns", "tmpl1", ateapipb.ActorState_ACTOR_STATE_SUSPENDED) - - storetest.MustCreateAtespace(t, ctx, persistence, "ns") - if _, err := persistence.CreateActorTemplate(ctx, &ateapipb.ActorTemplate{ - Metadata: &ateapipb.ResourceMetadata{Atespace: "ns", Name: "tmpl1"}, - Status: &ateapipb.ActorTemplateStatus{ - GoldenSnapshotStatus: &ateapipb.GoldenSnapshotStatus{ - GoldenSnapshot: &ateapipb.ExternalSnapshot{ - SnapshotUri: someActorSnapshotURI(t, "gs://bucket/golden-root", "ate-golden", "golden-1"), - ContentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, - }, - }, - }, - }); err != nil { - t.Fatalf("create template: %v", err) + seed := resumeSeed{ + noSnapshot: true, + goldenURI: someActorSnapshotURI(t, "gs://bucket/golden-root", "ate-golden", "golden-1"), + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, } + seedResumeTemplate(t, ctx, persistence, seed) + seedResumeActor(t, ctx, persistence, actorRef, seed) w := &ActorWorkflow{store: persistence} _, _, _, err := w.loadActorForResume(ctx, actorRef, false) @@ -1083,10 +1232,64 @@ func TestLoadActorForResume_GoldenFallbackRejectsNonFullGolden(t *testing.T) { } } -// TestLoadActorForResume_TemplateReplaced covers the detection of a repointed -// actor: the actor records the template UID its guest state was built on, and -// a mismatch with its current template marks the source TemplateReplaced, -// forcing the restore to data-only. +// TestLoadActorForResume_GoldenFallbackRestoresGoldenAsFull covers the +// golden-fallback success path: an actor with no snapshot of its own boots +// from the template's golden snapshot as its own content, always at FULL +// wire scope (a golden snapshot is FULL by construction; an Unspecified +// recorded scope just predates the scope field). +func TestLoadActorForResume_GoldenFallbackRestoresGoldenAsFull(t *testing.T) { + goldenSnapshotURI := someActorSnapshotURI(t, "gs://bucket/golden-root", "ate-golden", "golden-1") + actorRef := resources.ActorRef{Atespace: "team-a", Name: "id1"} + + tests := []struct { + name string + goldenScope ateapipb.SnapshotContentScope + }{ + {name: "Full golden", goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL}, + {name: "legacy golden without a recorded scope", goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_UNSPECIFIED}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + persistence := newTestPersistence(t) + + seed := resumeSeed{ + noSnapshot: true, + goldenURI: goldenSnapshotURI, + goldenScope: tt.goldenScope, + } + seedResumeTemplate(t, ctx, persistence, seed) + seedResumeActor(t, ctx, persistence, actorRef, seed) + + w := &ActorWorkflow{store: persistence} + _, _, src, err := w.loadActorForResume(ctx, actorRef, false) + if err != nil { + t.Fatalf("loadActorForResume: %v", err) + } + if got := src.SnapshotURI.String(); got != goldenSnapshotURI { + t.Errorf("src.SnapshotURI = %q, want the golden %q", got, goldenSnapshotURI) + } + if !src.GoldenForDataSnapshotURI.IsZero() { + t.Errorf("src.GoldenForDataSnapshotURI = %q, want empty", src.GoldenForDataSnapshotURI) + } + if src.WireScope != ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL { + t.Errorf("src.WireScope = %v, want %v", src.WireScope, ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL) + } + if src.Kind != restoreGoldenSnapshot { + t.Errorf("src.Kind = %v, want %v", src.Kind, restoreGoldenSnapshot) + } + }) + } +} + +// TestLoadActorForResume_TemplateReplaced covers the detection of an actor +// whose ActorTemplate was updated: the actor records the template UID its +// guest state was built on, and a mismatch with its current template means +// the snapshot's guest state must not be restored, forcing the restore +// operation to data-only. Such a restore never rides the golden snapshot, so +// the onResume policy must not be consulted: a Golden policy with no usable +// golden must not block the resume. func TestLoadActorForResume_TemplateReplaced(t *testing.T) { actorRef := resources.ActorRef{Atespace: "team-a", Name: "id1"} @@ -1097,12 +1300,43 @@ func TestLoadActorForResume_TemplateReplaced(t *testing.T) { // "" leaves the field unset (an actor from before it was recorded). builtOnTemplateUID string noSnapshot bool - want bool + // paused seeds a PAUSED actor holding only a local pause checkpoint + // instead of a durable snapshot; onPause is the template's pause + // scope, contentScope the durable snapshot's recorded content. + paused bool + onPause ateapipb.SnapshotContentScope + contentScope ateapipb.SnapshotContentScope + // fromData seeds the template's onResume policy. No case seeds a + // golden snapshot, so a case that consulted the Golden policy would + // fail with FailedPrecondition instead of resolving its scope. + fromData ateapipb.ResumeSource + // wantWireScope is the resolved restore scope: DATA for the snapshot + // of an actor whose template was updated, FULL for a snapshot + // restored as-is, and unspecified (inert) for a cold boot with no + // snapshot or golden. + wantWireScope ateletpb.SnapshotScope }{ - {name: "snapshot taken under the current template", builtOnTemplateUID: "current", want: false}, - {name: "snapshot taken under a replaced template", builtOnTemplateUID: "some-other-uid", want: true}, - {name: "snapshot without a recorded template UID", builtOnTemplateUID: "", want: false}, - {name: "no durable snapshot", noSnapshot: true, want: false}, + {name: "snapshot taken under the current template", builtOnTemplateUID: "current", contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL}, + {name: "snapshot taken under a replaced template", builtOnTemplateUID: "some-other-uid", contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA}, + {name: "snapshot without a recorded template UID", builtOnTemplateUID: "", contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL}, + {name: "no durable snapshot", noSnapshot: true, wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_UNSPECIFIED}, + {name: "pause checkpoint taken under the current template", paused: true, builtOnTemplateUID: "current", wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL}, + {name: "pause checkpoint taken under a replaced template", paused: true, builtOnTemplateUID: "some-other-uid", wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA}, + { + name: "Data snapshot from an updated template under the Golden policy skips the absent golden", + builtOnTemplateUID: "some-other-uid", + contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA, + }, + { + name: "Data pause checkpoint from an updated template under the Golden policy skips the absent golden", + paused: true, + builtOnTemplateUID: "some-other-uid", + onPause: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA, + }, } for _, tt := range tests { @@ -1110,13 +1344,14 @@ func TestLoadActorForResume_TemplateReplaced(t *testing.T) { ctx := context.Background() persistence := newTestPersistence(t) - storetest.MustCreateAtespace(t, ctx, persistence, "ns") - tmpl, err := persistence.CreateActorTemplate(ctx, &ateapipb.ActorTemplate{ - Metadata: &ateapipb.ResourceMetadata{Atespace: "ns", Name: "tmpl1"}, - }) - if err != nil { - t.Fatalf("create template: %v", err) + seed := resumeSeed{ + paused: tt.paused, + noSnapshot: tt.noSnapshot, + contentScope: tt.contentScope, + onPause: tt.onPause, + fromData: tt.fromData, } + tmpl := seedResumeTemplate(t, ctx, persistence, seed) if tmpl.GetMetadata().GetUid() == "" { t.Fatal("created template has no UID; the matching case would be vacuous") } @@ -1128,27 +1363,174 @@ func TestLoadActorForResume_TemplateReplaced(t *testing.T) { uid = tmpl.GetMetadata().GetUid() } seedOpts = append(seedOpts, func(a *ateapipb.Actor) { - a.Status.ExternalSnapshot = &ateapipb.ExternalSnapshot{ - SnapshotUri: someActorSnapshotURI(t, testStorageLocation, actorRef.Atespace, "snap-1"), - ContentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, - } a.Status.CurrentActorTemplateUid = uid }) } - seedWorkflowActor(t, ctx, persistence, actorRef, "ns", "tmpl1", ateapipb.ActorState_ACTOR_STATE_SUSPENDED, seedOpts...) + seedResumeActor(t, ctx, persistence, actorRef, seed, seedOpts...) w := &ActorWorkflow{store: persistence} _, _, src, err := w.loadActorForResume(ctx, actorRef, false) if err != nil { t.Fatalf("loadActorForResume: %v", err) } - if src.TemplateReplaced != tt.want { - t.Errorf("src.TemplateReplaced = %v, want %v", src.TemplateReplaced, tt.want) + if src.WireScope != tt.wantWireScope { + t.Errorf("src.WireScope = %v, want %v", src.WireScope, tt.wantWireScope) + } + // No case rides the golden, whatever the onResume policy says. + if !src.GoldenForDataSnapshotURI.IsZero() { + t.Errorf("src.GoldenForDataSnapshotURI = %v, want zero", src.GoldenForDataSnapshotURI) + } + wantKind := restoreDurableSnapshot + switch { + case tt.paused: + wantKind = restoreLocalSnapshot + case tt.noSnapshot: + wantKind = restoreColdBoot + } + if src.Kind != wantKind { + t.Errorf("src.Kind = %v, want %v", src.Kind, wantKind) + } + }) + } +} + +// TestLoadActorForResume_BootSkipsGoldenFallback covers the explicit boot +// request: it only suppresses the golden-snapshot fallback for an actor with +// no snapshot of its own, without resolving the golden (an unusable golden +// must not block a boot). An actor's own local or durable snapshot restores +// exactly as it would without the flag. +func TestLoadActorForResume_BootSkipsGoldenFallback(t *testing.T) { + goldenSnapshotURI := someActorSnapshotURI(t, "gs://bucket/golden-root", "ate-golden", "golden-1") + actorRef := resources.ActorRef{Atespace: "team-a", Name: "id1"} + + tests := []struct { + name string + // paused seeds a local pause checkpoint; noSnapshot leaves the actor + // without a durable snapshot; otherwise a durable snapshot captured + // with contentScope is seeded. + paused bool + noSnapshot bool + contentScope ateapipb.SnapshotContentScope + // fromData seeds the template's onResume policy; goldenURI and + // goldenScope the template's recorded golden snapshot. + fromData ateapipb.ResumeSource + goldenURI string + goldenScope ateapipb.SnapshotContentScope + // wantSnapshotURI reports whether the actor's own durable snapshot + // must be the boot source; a boot never resolves the golden as one. + wantSnapshotURI bool + // wantGoldenForDataURI is the golden location of a pending + // data-on-golden restore, empty for every other restore. + wantGoldenForDataURI string + wantWireScope ateletpb.SnapshotScope + }{ + { + name: "Full durable snapshot restores as-is", + contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantSnapshotURI: true, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL, + }, + { + name: "Data durable snapshot still rides the golden under the Golden policy", + contentScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + fromData: ateapipb.ResumeSource_RESUME_SOURCE_GOLDEN, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantSnapshotURI: true, + wantGoldenForDataURI: goldenSnapshotURI, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA_ON_GOLDEN, + }, + { + name: "no snapshot skips the golden fallback and cold boots", + noSnapshot: true, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_UNSPECIFIED, + }, + { + name: "no snapshot with an unusable golden still cold boots", + noSnapshot: true, + goldenURI: goldenSnapshotURI, + goldenScope: ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_UNSPECIFIED, + }, + { + name: "paused actor restores its checkpoint at the pause scope", + paused: true, + wantWireScope: ateletpb.SnapshotScope_SNAPSHOT_SCOPE_FULL, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + persistence := newTestPersistence(t) + + seed := resumeSeed{ + paused: tt.paused, + noSnapshot: tt.noSnapshot, + contentScope: tt.contentScope, + fromData: tt.fromData, + goldenURI: tt.goldenURI, + goldenScope: tt.goldenScope, + } + seedResumeTemplate(t, ctx, persistence, seed) + durableSnapshotURI := seedResumeActor(t, ctx, persistence, actorRef, seed) + + w := &ActorWorkflow{store: persistence} + _, _, src, err := w.loadActorForResume(ctx, actorRef, true) + if err != nil { + t.Fatalf("loadActorForResume: %v", err) + } + wantSnapshotURI := "" + if tt.wantSnapshotURI { + wantSnapshotURI = durableSnapshotURI + } + if got := src.SnapshotURI.String(); got != wantSnapshotURI { + t.Errorf("src.SnapshotURI = %q, want %q", got, wantSnapshotURI) + } + if got := src.GoldenForDataSnapshotURI.String(); got != tt.wantGoldenForDataURI { + t.Errorf("src.GoldenForDataSnapshotURI = %q, want %q", got, tt.wantGoldenForDataURI) + } + if src.WireScope != tt.wantWireScope { + t.Errorf("src.WireScope = %v, want %v", src.WireScope, tt.wantWireScope) + } + wantKind := restoreDurableSnapshot + switch { + case tt.paused: + wantKind = restoreLocalSnapshot + case tt.noSnapshot: + wantKind = restoreColdBoot + } + if src.Kind != wantKind { + t.Errorf("src.Kind = %v, want %v", src.Kind, wantKind) } }) } } +// TestRestoreKindTelemetrySnapshotKind pins the restore-kind to metric-label +// mapping: these are the recorded values of the resume lifecycle metric's +// snapshot-kind attribute. +func TestRestoreKindTelemetrySnapshotKind(t *testing.T) { + tests := []struct { + kind restoreKind + want string + }{ + {restoreColdBoot, ateattr.SnapshotKindBoot}, + {restoreLocalSnapshot, ateattr.SnapshotKindLocal}, + {restoreDurableSnapshot, ateattr.SnapshotKindLatest}, + {restoreGoldenSnapshot, ateattr.SnapshotKindGolden}, + } + for _, tt := range tests { + if got := tt.kind.telemetrySnapshotKind(); got != tt.want { + t.Errorf("restoreKind(%d).telemetrySnapshotKind() = %q, want %q", tt.kind, got, tt.want) + } + } +} + func TestLoadActorForResume_RunningActorShortCircuits(t *testing.T) { ctx := context.Background() persistence := newTestPersistence(t) @@ -1171,7 +1553,7 @@ func TestLoadActorForResume_RunningActorShortCircuits(t *testing.T) { if tmpl != nil { t.Errorf("expected nil template, got %v", tmpl) } - if !src.SnapshotURI.IsZero() || !src.GoldenSnapshotURI.IsZero() { + if !src.SnapshotURI.IsZero() || !src.GoldenForDataSnapshotURI.IsZero() { t.Errorf("expected empty snapshot source, got %+v", src) } }