Add ActorTemplate and ActorTemplateVersion as substrate resources; Add CRUD methods in redis package - #824
Add ActorTemplate and ActorTemplateVersion as substrate resources; Add CRUD methods in redis package#824Zoe Zhao (zoez7) wants to merge 7 commits into
Conversation
8546ab7 to
47fb420
Compare
…ne proto
Introduce ActorTemplate (identity + placement policy) and
ActorTemplateVersion (immutable workload definition), along with the
supporting SandboxClass/SandboxConfig, SnapshotsConfig, ResumeSource,
and OnResumeConfig messages. Both resources are flat: workload and
status fields live directly on the message, like Actor.
The version State enum (nested in ActorTemplateVersion, like
Actor.Status) mirrors the existing ActorTemplateVersion CRD PhaseType
(RESUME_GOLDEN_ACTOR -> WAIT_GOLDEN_ACTOR -> {READY | FAILED}, with
UNSPECIFIED for the initial empty phase).
47fb420 to
2216177
Compare
Adds per-resource store methods and the ateredis implementation for the two new global-scoped resources, following the Atespace/Actor patterns: SetNX creates with server-owned metadata, WATCH+version CAS update for ActorTemplate, and SCAN-based listing. ListActorTemplateVersions filters by parent after fetching (the parent lives in the value, not the key); matches count toward the page so pages stay full. DeleteActorTemplate rejects while any version names it as parent. DeleteActorTemplateVersion rejects while it is its parent's default_version_on_create and deletes the golden snapshot recorded in its status before removing the version record.
| // the stored resource with advanced metadata (version, update_time). The | ||
| // input is not mutated. Returns ErrNotFound if missing, or | ||
| // ErrVersionConflict on version mismatch. | ||
| UpdateActorTemplate(ctx context.Context, template *ateapipb.ActorTemplate, expectedVersion int64) (*ateapipb.ActorTemplate, error) |
There was a problem hiding this comment.
Can you sync with Luiz Oliveira (@laoj2)? We are reworking the way store should expose Update methods. See #763
2216177 to
8c86364
Compare
| // Returns store.ErrNotFound if the template does not exist, or | ||
| // store.ErrFailedPrecondition while any ActorTemplateVersion still names it | ||
| // as parent. | ||
| func (s *Persistence) DeleteActorTemplate(ctx context.Context, name string) (*ateapipb.ActorTemplate, error) { |
There was a problem hiding this comment.
I think here's a TOCTOU problem - if DeleteActorTemplate happens after the ActorTemplate check in but before the actual creation of CreateActorTemplateVersion, then we would have a orphaned ATV.
This is similar to what I realize in DeleteAtespace and CreateActor in #396, this might need a same pattern to have a terminating marker that can prevent another operator from proceeding.
Maybe add a todo for that TOCTOU?
There was a problem hiding this comment.
I was using API level locks, but I think adding deletion markers here makes sense too in case there is a bug in the API level locking. Added in 091cafb.
29b39b8 to
1dff0cc
Compare
1dff0cc to
091cafb
Compare
Part of #477 .
All ATV fields are immutable. SandboxConifg will be frozen into ATV at creation time.
Adds per-resource store methods and the ateredis package for the AT and ATV as global resources.