Repository navigation
Publish ActorTemplate golden snapshots as tags - #1523
Merged
Luiz Oliveira (laoj2) merged 3 commits intoSep 16, 2026
Merged
Luiz Oliveira (laoj2) merged 3 commits into
Luiz Oliveira (laoj2) merged 3 commits into
Conversation
Eitan Yarmush (EItanya)
requested review from
Julian Gutierrez Oschmann (juli4n),
Luiz Oliveira (laoj2) and
Zoe Zhao (zoez7)
and removed request for
Luiz Oliveira (laoj2)
September 6, 2026 12:16
| } | ||
| if err == nil { | ||
| if tag.GetStatus().GetActorTemplateUid() != tmpl.GetMetadata().GetUid() || resources.ActorRefFromObjectRef(tag.GetSourceActor()) != resources.ActorRefFromObjectRef(goldenActorRef) { | ||
| return 0, fmt.Errorf("golden tag belongs to another actor or template") |
Collaborator
There was a problem hiding this comment.
what about failing this with r.fail, so we don't retry forever if the existing golden tag matches another actor?
Copy each warmed golden actor snapshot into a published tag before deleting the temporary actor. Resolve that tag as the default when creating actors, and use it for data-only golden restores. Clean up golden resources with their template and recover interrupted tag builds. Signed-off-by: Eitan Yarmush <[email protected]>
Record ownership conflicts as terminal template failures so reconciliation does not retry indefinitely. Keep conflicting tags untouched and cover actor and template mismatches, repeated reconciliation, and periodic resync. Signed-off-by: Eitan Yarmush <[email protected]>
Eitan Yarmush (EItanya)
force-pushed
the
fix/golden-template-tag
branch
from
September 15, 2026 17:00
7971143 to
c817631
Compare
Luiz Oliveira (laoj2)
previously approved these changes
Sep 15, 2026
| return Validate_ListActorTemplatesRequest(ctx, op, nil, req, nil) | ||
| } | ||
|
|
||
| func (s *RPCService) DeleteActorTemplate(ctx context.Context, req *ateapipb.DeleteActorTemplateRequest) (*ateapipb.ActorTemplate, error) { |
Collaborator
There was a problem hiding this comment.
can we add a unit test for DeleteActorTemplate?
| } | ||
| } | ||
|
|
||
| func TestReconcileOne_GoldenTagRecovery(t *testing.T) { |
Collaborator
There was a problem hiding this comment.
can we make these tests table-driven? It's a bit hard to read right now the per-case inputs/expectations. For example, we could have a struct like this:
tests := []struct {
name string
// tag is the golden tag an earlier pass left behind, if any.
tag *ateapipb.Tag
// actorDeleted seeds a pass that died after deleting the golden actor.
actorDeleted bool
// createTagErr, deleteActorErr and deleteTagErr fail one step of the
// first pass. The retry runs with every step working again.
createTagErr error
deleteActorErr error
deleteTagErr error
// wantCreateTags is how many copies the whole scenario must cost.
wantCreateTags int
}{
Exercise golden resource cleanup, partial failures, and retries through DeleteActorTemplate. Declare recovery and ownership-conflict inputs in tables and assert the number of tag copy attempts. Signed-off-by: Eitan Yarmush <[email protected]>
Luiz Oliveira (laoj2)
approved these changes
Sep 16, 2026
2 tasks done
Julian Gutierrez Oschmann (juli4n)
pushed a commit
that referenced
this pull request
Sep 16, 2026
#1522 added a resyncInterval parameter to NewActorTemplateReconciler and #1523 added this call site. They landed without seeing each other, so `main` doesn't compile. SEt the resyncInterval to 7s, to match what we have in other tests - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1507
Golden snapshots currently remain owned by the temporary golden actor, so another resume/suspend cycle or actor deletion can collect a snapshot still referenced by its template. The controller now copies the warmed snapshot into a published tag, deletes the golden actor, and records the tag reference on the template. Interrupted tag creation and cleanup remain retryable; template deletion cleans up both resources.
CreateActorresolves an explicitsourceTagor the template's golden tag into the actor's initial snapshot. Actors created before the golden tag is ready retain their cold-boot behavior. The golden tag uses the template UID as its name inate-golden. The proto replacesgolden_snapshotwithgolden_tagat field 1, without backward compatibility.This PR is based directly on
mainand does not depend on #1521.Follow-up recommendation: move the create → resume → wait → suspend → tag → delete sequence into a golden-template workflow using the existing workflow conventions. The reconciler now coordinates multiple recoverable steps; it could retain scheduling and retries while delegating that sequence to the workflow. This refactor is outside this PR.
Validation: full
env -u NO_COLOR make verifypassed after rebasing ontomain. After the final proto field-number change, bindings were regenerated and the control API unit/functional tests plus proto-format and Go-format checks passed.