From 2dc1fc62665d89fe9e852a90da084a2e0fd8efae Mon Sep 17 00:00:00 2001 From: Julian Gutierrez Oschmann Date: Mon, 27 Jul 2026 15:12:01 -0400 Subject: [PATCH] Introduce an `ActorRef` type to bundle actor atespace and name. The pattern of passing a tuple of atespace and actor name together is across the whole codebase. This simplifies both callers and callees by leting them pass a single field that bundles both. The ActorRef is the actor specific, typed, in-process version of the ObjectRef we have in our gRPC API. --- cmd/ateapi/internal/controlapi/crash.go | 13 +- cmd/ateapi/internal/controlapi/crash_test.go | 65 +++-- .../internal/controlapi/create_actor.go | 2 +- .../internal/controlapi/delete_actor.go | 20 +- .../internal/controlapi/functional_test.go | 9 +- cmd/ateapi/internal/controlapi/get_actor.go | 5 +- cmd/ateapi/internal/controlapi/pause_actor.go | 7 +- .../internal/controlapi/resume_actor.go | 7 +- .../internal/controlapi/span_identity.go | 5 +- .../internal/controlapi/span_identity_test.go | 5 +- .../internal/controlapi/suspend_actor.go | 7 +- cmd/ateapi/internal/controlapi/syncer.go | 2 +- cmd/ateapi/internal/controlapi/syncer_test.go | 3 +- .../internal/controlapi/update_actor.go | 5 +- cmd/ateapi/internal/controlapi/workflow.go | 28 +-- .../internal/controlapi/workflow_pause.go | 28 +-- .../controlapi/workflow_pause_test.go | 27 ++- .../internal/controlapi/workflow_resume.go | 55 ++--- .../controlapi/workflow_resume_test.go | 29 +-- .../internal/controlapi/workflow_suspend.go | 26 +- .../controlapi/workflow_suspend_test.go | 21 +- .../controlapi/workflow_testutil_test.go | 5 +- .../internal/store/ateredis/ateredis.go | 22 +- .../internal/store/ateredis/ateredis_test.go | 25 +- cmd/ateapi/internal/store/store.go | 7 +- cmd/atelet/main.go | 27 ++- cmd/atenet/internal/router/extproc.go | 15 +- cmd/atenet/internal/router/extproc_in.go | 15 +- cmd/atenet/internal/router/extproc_in_test.go | 52 ++-- cmd/atenet/internal/router/resumer.go | 17 +- cmd/atenet/internal/router/resumer_test.go | 11 +- cmd/ateom-gvisor/main.go | 23 +- cmd/ateom-microvm/checkpoint.go | 8 +- cmd/ateom-microvm/restore.go | 12 +- cmd/ateom-microvm/run.go | 21 +- cmd/kubectl-ate/internal/cmd/delete_actor.go | 7 +- cmd/kubectl-ate/internal/cmd/get_actors.go | 4 +- cmd/kubectl-ate/internal/cmd/logs_actors.go | 50 ++-- .../internal/cmd/logs_actors_test.go | 222 ++++++++---------- cmd/kubectl-ate/internal/cmd/pause_actor.go | 4 +- cmd/kubectl-ate/internal/cmd/resume_actor.go | 4 +- cmd/kubectl-ate/internal/cmd/suspend_actor.go | 4 +- demos/sandbox/client/main.go | 15 +- internal/actorlog/logger.go | 22 +- internal/actorlog/logger_test.go | 12 +- internal/ateattr/ateattr.go | 7 +- internal/ateattr/ateattr_test.go | 20 +- internal/e2e/router_client.go | 9 +- internal/e2e/suites/demo/demo_test.go | 22 +- internal/e2e/suites/identity/identity_test.go | 3 +- internal/resources/actor.go | 36 +-- internal/resources/actor_test.go | 66 ------ internal/resources/actorref.go | 91 +++++++ internal/resources/actorref_test.go | 113 +++++++++ 54 files changed, 726 insertions(+), 614 deletions(-) delete mode 100644 internal/resources/actor_test.go create mode 100644 internal/resources/actorref.go create mode 100644 internal/resources/actorref_test.go diff --git a/cmd/ateapi/internal/controlapi/crash.go b/cmd/ateapi/internal/controlapi/crash.go index f126051c3..95c54c931 100644 --- a/cmd/ateapi/internal/controlapi/crash.go +++ b/cmd/ateapi/internal/controlapi/crash.go @@ -22,6 +22,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/internal/ateerrors" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -29,26 +30,26 @@ import ( // maybeCrashActor inspects err returned by an atelet RPC, it crashes // the actor if the err carries the actorCrashed=true metadata directive. -func maybeCrashActor(ctx context.Context, st store.Interface, atespace, actorName string, err error, wrapMsg string) error { +func maybeCrashActor(ctx context.Context, st store.Interface, actorRef resources.ActorRef, err error, wrapMsg string) error { if err == nil { return nil } if ateerrors.ActorCrashRequested(err) { slog.ErrorContext(ctx, "Setting Actor to crashed due to error", slog.Any("error", err)) - if cerr := crashActor(ctx, st, atespace, actorName); cerr != nil { + if cerr := crashActor(ctx, st, actorRef); cerr != nil { slog.ErrorContext(ctx, "Failed to crash actor", slog.Any("cerr", cerr)) return cerr } - return status.Errorf(codes.DataLoss, "actor %s crashed", actorName) + return status.Errorf(codes.DataLoss, "actor %s crashed", actorRef.Name) } return fmt.Errorf("%s: %w", wrapMsg, err) } // crashActor moves the actor to CRASHED state and frees the worker it was // assigned to, if any, so the worker can host other actors. -func crashActor(ctx context.Context, st store.Interface, atespace, actorName string) error { - actor, err := st.GetActor(ctx, atespace, actorName) +func crashActor(ctx context.Context, st store.Interface, actorRef resources.ActorRef) error { + actor, err := st.GetActor(ctx, actorRef) if err != nil { return fmt.Errorf("while loading actor to crash: %w", err) } @@ -102,7 +103,7 @@ func releaseWorker(ctx context.Context, st store.Interface, actor *ateapipb.Acto return nil } // Only free it if it still belongs to us - if wass.GetActor().GetAtespace() != actor.GetMetadata().GetAtespace() || wass.GetActor().GetName() != actor.GetMetadata().GetName() { + if resources.ActorRefFromObjectRef(wass.GetActor()) != resources.ActorRefFromActor(actor) { slog.WarnContext(ctx, "Worker already assigned to another Actor", slog.String("worker", podUid)) return nil } diff --git a/cmd/ateapi/internal/controlapi/crash_test.go b/cmd/ateapi/internal/controlapi/crash_test.go index 9e4c7e340..641e11df2 100644 --- a/cmd/ateapi/internal/controlapi/crash_test.go +++ b/cmd/ateapi/internal/controlapi/crash_test.go @@ -23,6 +23,7 @@ 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/internal/ateerrors" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -30,10 +31,10 @@ import ( // seedActor stores a running actor with all worker-binding fields populated, so // tests can assert they are cleared when the actor crashes. -func seedActor(t *testing.T, ctx context.Context, st store.Interface, atespace, actorName string) { +func seedActor(t *testing.T, ctx context.Context, st store.Interface, actorRef resources.ActorRef) { t.Helper() if _, err := st.CreateActor(ctx, &ateapipb.Actor{ - Metadata: &ateapipb.ResourceMetadata{Name: actorName, Atespace: atespace}, + Metadata: &ateapipb.ResourceMetadata{Name: actorRef.Name, Atespace: actorRef.Atespace}, Status: ateapipb.Actor_STATUS_RUNNING, AteomPodNamespace: "ns", AteomPodName: "pod", @@ -47,17 +48,17 @@ func seedActor(t *testing.T, ctx context.Context, st store.Interface, atespace, } // seedWorker registers the worker referenced by seedActor's binding fields, -// assigned to the given actor in atespace (unassigned if assignedActor is ""). -func seedWorker(t *testing.T, ctx context.Context, st store.Interface, atespace, assignedActor string) { +// assigned to the given actor (unassigned if assigned is the zero ActorRef). +func seedWorker(t *testing.T, ctx context.Context, st store.Interface, actorRef resources.ActorRef) { t.Helper() worker := &ateapipb.Worker{ WorkerNamespace: "ns", WorkerPool: "pool", WorkerPod: "pod", } - if assignedActor != "" { + if actorRef != (resources.ActorRef{}) { worker.Assignment = &ateapipb.Assignment{ - Actor: &ateapipb.ObjectRef{Atespace: atespace, Name: assignedActor}, + Actor: actorRef.ToObjectRef(), } } if err := st.CreateWorker(ctx, worker); err != nil { @@ -67,10 +68,10 @@ func seedWorker(t *testing.T, ctx context.Context, st store.Interface, atespace, // seedUnboundActor stores a running actor whose worker-binding fields were // already cleared, e.g. by a prior release. -func seedUnboundActor(t *testing.T, ctx context.Context, st store.Interface, atespace, actorName string) { +func seedUnboundActor(t *testing.T, ctx context.Context, st store.Interface, actorRef resources.ActorRef) { t.Helper() if _, err := st.CreateActor(ctx, &ateapipb.Actor{ - Metadata: &ateapipb.ResourceMetadata{Name: actorName, Atespace: atespace}, + Metadata: &ateapipb.ResourceMetadata{Name: actorRef.Name, Atespace: actorRef.Atespace}, Status: ateapipb.Actor_STATUS_RUNNING, InProgressSnapshot: "gs://snapshots/actor-1/reserved", }); err != nil { @@ -80,11 +81,11 @@ func seedUnboundActor(t *testing.T, ctx context.Context, st store.Interface, ate // assertCrashed reloads the actor and verifies it is CRASHED with its worker // binding cleared. -func assertCrashed(t *testing.T, ctx context.Context, st store.Interface, atespace, actorName string) { +func assertCrashed(t *testing.T, ctx context.Context, st store.Interface, actorRef resources.ActorRef) { t.Helper() - got, err := st.GetActor(ctx, atespace, actorName) + got, err := st.GetActor(ctx, actorRef) if err != nil { - t.Fatalf("GetActor(%q, %q) = %v, want nil", atespace, actorName, err) + t.Fatalf("GetActor(%v) = %v, want nil", actorRef, err) } if got.GetStatus() != ateapipb.Actor_STATUS_CRASHED { t.Errorf("status = %v, want %v", got.GetStatus(), ateapipb.Actor_STATUS_CRASHED) @@ -107,10 +108,7 @@ func assertCrashed(t *testing.T, ctx context.Context, st store.Interface, atespa } func TestCrashActor(t *testing.T) { - const ( - atespace = "team-a" - actorName = "actor-1" - ) + actorRef := resources.ActorRef{Atespace: "team-a", Name: "actor-1"} tests := []struct { name string @@ -127,20 +125,20 @@ func TestCrashActor(t *testing.T) { if err != nil { t.Fatalf("crashActor() = %v, want nil", err) } - assertCrashed(t, ctx, st, atespace, actorName) + assertCrashed(t, ctx, st, actorRef) }, }, { name: "releases worker assigned to crashed actor", seed: true, setup: func(t *testing.T, ctx context.Context, st store.Interface) { - seedWorker(t, ctx, st, atespace, actorName) + seedWorker(t, ctx, st, actorRef) }, check: func(t *testing.T, ctx context.Context, st store.Interface, err error) { if err != nil { t.Fatalf("crashActor() = %v, want nil", err) } - assertCrashed(t, ctx, st, atespace, actorName) + assertCrashed(t, ctx, st, actorRef) worker, gerr := st.GetWorker(ctx, "ns", "pool", "pod") if gerr != nil { t.Fatalf("GetWorker() = %v, want nil", gerr) @@ -154,13 +152,13 @@ func TestCrashActor(t *testing.T) { name: "keeps worker assigned to another actor", seed: true, setup: func(t *testing.T, ctx context.Context, st store.Interface) { - seedWorker(t, ctx, st, atespace, "actor-2") + seedWorker(t, ctx, st, resources.ActorRef{Atespace: actorRef.Atespace, Name: "actor-2"}) }, check: func(t *testing.T, ctx context.Context, st store.Interface, err error) { if err != nil { t.Fatalf("crashActor() = %v, want nil", err) } - assertCrashed(t, ctx, st, atespace, actorName) + assertCrashed(t, ctx, st, actorRef) worker, gerr := st.GetWorker(ctx, "ns", "pool", "pod") if gerr != nil { t.Fatalf("GetWorker() = %v, want nil", gerr) @@ -174,14 +172,14 @@ func TestCrashActor(t *testing.T) { name: "skips release for actor with no worker binding", seed: false, setup: func(t *testing.T, ctx context.Context, st store.Interface) { - seedUnboundActor(t, ctx, st, atespace, actorName) - seedWorker(t, ctx, st, atespace, actorName) + seedUnboundActor(t, ctx, st, actorRef) + seedWorker(t, ctx, st, actorRef) }, check: func(t *testing.T, ctx context.Context, st store.Interface, err error) { if err != nil { t.Fatalf("crashActor() = %v, want nil", err) } - assertCrashed(t, ctx, st, atespace, actorName) + assertCrashed(t, ctx, st, actorRef) // Without a binding the worker cannot be looked up, so its // assignment must be left untouched even though it names // the crashed actor. @@ -218,24 +216,21 @@ func TestCrashActor(t *testing.T) { defer cleanup() if tt.seed { - seedActor(t, ctx, st, atespace, actorName) + seedActor(t, ctx, st, actorRef) } if tt.setup != nil { tt.setup(t, ctx, st) } - err := crashActor(ctx, st, atespace, actorName) + err := crashActor(ctx, st, actorRef) tt.check(t, ctx, st, err) }) } } func TestMaybeCrashActor(t *testing.T) { - const ( - atespace = "team-a" - actorName = "actor-1" - wrapMsg = "calling atelet" - ) + const wrapMsg = "calling atelet" + actorRef := resources.ActorRef{Atespace: "team-a", Name: "actor-1"} crashErr := ateerrors.NewGRPCError(context.Background(), codes.NotFound, ateerrors.ReasonTerminalFileSystemError, ateerrors.ActorCrashedMetadata(), errors.New("boom")) // A structured error carrying a reason but no actorCrashed directive must be @@ -271,7 +266,7 @@ func TestMaybeCrashActor(t *testing.T) { if got := status.Code(err); got != codes.DataLoss { t.Errorf("status code = %v, want %v", got, codes.DataLoss) } - assertCrashed(t, ctx, st, atespace, actorName) + assertCrashed(t, ctx, st, actorRef) }, }, { @@ -305,7 +300,7 @@ func TestMaybeCrashActor(t *testing.T) { t.Errorf("maybeCrashActor() error = %q, want prefix %q", err, wrapMsg) } // The actor must not have been crashed. - got, gerr := st.GetActor(ctx, atespace, actorName) + got, gerr := st.GetActor(ctx, actorRef) if gerr != nil { t.Fatalf("GetActor() = %v, want nil", gerr) } @@ -329,7 +324,7 @@ func TestMaybeCrashActor(t *testing.T) { t.Errorf("maybeCrashActor() error = %q, want prefix %q", err, wrapMsg) } // The actor must not have been crashed. - got, gerr := st.GetActor(ctx, atespace, actorName) + got, gerr := st.GetActor(ctx, actorRef) if gerr != nil { t.Fatalf("GetActor() = %v, want nil", gerr) } @@ -347,10 +342,10 @@ func TestMaybeCrashActor(t *testing.T) { defer cleanup() if tt.seed { - seedActor(t, ctx, st, atespace, actorName) + seedActor(t, ctx, st, actorRef) } - err := maybeCrashActor(ctx, st, atespace, actorName, tt.err, wrapMsg) + err := maybeCrashActor(ctx, st, actorRef, tt.err, wrapMsg) tt.check(t, ctx, st, err) }) } diff --git a/cmd/ateapi/internal/controlapi/create_actor.go b/cmd/ateapi/internal/controlapi/create_actor.go index d022b70d8..966a6d2e9 100644 --- a/cmd/ateapi/internal/controlapi/create_actor.go +++ b/cmd/ateapi/internal/controlapi/create_actor.go @@ -37,7 +37,7 @@ func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequ templateNamespace := in.GetActorTemplateNamespace() templateName := in.GetActorTemplateName() - setSpanActorRefAttributes(ctx, in.GetMetadata().GetAtespace(), in.GetMetadata().GetName()) + setSpanActorRefAttributes(ctx, resources.ActorRefFromActor(in)) template, err := s.actorTemplateLister.ActorTemplates(templateNamespace).Get(templateName) if err != nil { diff --git a/cmd/ateapi/internal/controlapi/delete_actor.go b/cmd/ateapi/internal/controlapi/delete_actor.go index f0ea3a3c2..047a7a73c 100644 --- a/cmd/ateapi/internal/controlapi/delete_actor.go +++ b/cmd/ateapi/internal/controlapi/delete_actor.go @@ -31,15 +31,13 @@ func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequ if err := validateDeleteActorRequest(req); err != nil { return nil, err } - setSpanActorRefAttributes(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) + setSpanActorRefAttributes(ctx, actorRef) - atespace := req.GetActor().GetAtespace() - name := req.GetActor().GetName() - - actor, err := s.persistence.GetActor(ctx, atespace, name) + actor, err := s.persistence.GetActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", name) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } return nil, fmt.Errorf("while fetching actor: %w", err) } @@ -49,17 +47,17 @@ func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequ return nil, status.Errorf(codes.Internal, "while deleting actor volumes: %v", err) } - deleted, err := s.persistence.DeleteActor(ctx, atespace, name) + deleted, err := s.persistence.DeleteActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } if errors.Is(err, store.ErrFailedPrecondition) { - current, getErr := s.persistence.GetActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + current, getErr := s.persistence.GetActor(ctx, actorRef) if getErr == nil { - return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended (status: %v)", req.GetActor().GetName(), current.GetStatus()) + return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended (status: %v)", actorRef.Name, current.GetStatus()) } - return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended", req.GetActor().GetName()) + return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended", actorRef.Name) } if errors.Is(err, store.ErrVersionConflict) { return nil, status.Error(codes.Aborted, "concurrent update conflict, please retry") diff --git a/cmd/ateapi/internal/controlapi/functional_test.go b/cmd/ateapi/internal/controlapi/functional_test.go index e95a65589..7199551c6 100644 --- a/cmd/ateapi/internal/controlapi/functional_test.go +++ b/cmd/ateapi/internal/controlapi/functional_test.go @@ -31,6 +31,7 @@ import ( "github.com/agent-substrate/substrate/internal/ateinterceptors" "github.com/agent-substrate/substrate/internal/envtestbins" "github.com/agent-substrate/substrate/internal/proto/ateletpb" + "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/client/clientset/versioned" "github.com/agent-substrate/substrate/pkg/client/informers/externalversions" @@ -1473,7 +1474,7 @@ func TestResumeActor_Reentrancy(t *testing.T) { } // Verify actor state is RESUMING in Redis! - actor, err := tc.persistence.GetActor(context.Background(), testAtespace, name) + actor, err := tc.persistence.GetActor(context.Background(), resources.ActorRef{Atespace: testAtespace, Name: name}) if err != nil { t.Fatalf("failed to get actor from store: %v", err) } @@ -1497,7 +1498,7 @@ func TestResumeActor_Reentrancy(t *testing.T) { } // Verify actor state is RUNNING! - actor, err = tc.persistence.GetActor(context.Background(), testAtespace, name) + actor, err = tc.persistence.GetActor(context.Background(), resources.ActorRef{Atespace: testAtespace, Name: name}) if err != nil { t.Fatalf("failed to get actor from store: %v", err) } @@ -2473,7 +2474,7 @@ func TestResumeActor_DanglingWorker(t *testing.T) { } // Verify actor state is RUNNING with worker B assigned - actor, err = tc.persistence.GetActor(context.Background(), testAtespace, name) + actor, err = tc.persistence.GetActor(context.Background(), resources.ActorRef{Atespace: testAtespace, Name: name}) if err != nil { t.Fatalf("failed to get actor from store: %v", err) } @@ -2626,7 +2627,7 @@ func TestDeleteActor_Crashed(t *testing.T) { t.Fatalf("CreateActor failed: %v", err) } - actor, err := tc.persistence.GetActor(context.Background(), testAtespace, "id1") + actor, err := tc.persistence.GetActor(context.Background(), resources.ActorRef{Atespace: testAtespace, Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } diff --git a/cmd/ateapi/internal/controlapi/get_actor.go b/cmd/ateapi/internal/controlapi/get_actor.go index 979d2799d..a507f4ebd 100644 --- a/cmd/ateapi/internal/controlapi/get_actor.go +++ b/cmd/ateapi/internal/controlapi/get_actor.go @@ -31,9 +31,10 @@ func (s *Service) GetActor(ctx context.Context, req *ateapipb.GetActorRequest) ( if err := validateGetActorRequest(req); err != nil { return nil, err } - actor, err := s.persistence.GetActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) + actor, err := s.persistence.GetActor(ctx, actorRef) if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } else if err != nil { return nil, fmt.Errorf("while getting actor from DB: %w", err) } diff --git a/cmd/ateapi/internal/controlapi/pause_actor.go b/cmd/ateapi/internal/controlapi/pause_actor.go index 1a5f3cffa..7d44e448b 100644 --- a/cmd/ateapi/internal/controlapi/pause_actor.go +++ b/cmd/ateapi/internal/controlapi/pause_actor.go @@ -30,15 +30,16 @@ func (s *Service) PauseActor(ctx context.Context, req *ateapipb.PauseActorReques if err := validatePauseActorRequest(req); err != nil { return nil, err } - setSpanActorRefAttributes(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) + setSpanActorRefAttributes(ctx, actorRef) - actor, err := s.actorWorkflow.PauseActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actor, err := s.actorWorkflow.PauseActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrVersionConflict) { return nil, status.Error(codes.Aborted, "concurrent update conflict, please retry") } if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } return nil, err } diff --git a/cmd/ateapi/internal/controlapi/resume_actor.go b/cmd/ateapi/internal/controlapi/resume_actor.go index 3bbf7a0d4..9ab253e93 100644 --- a/cmd/ateapi/internal/controlapi/resume_actor.go +++ b/cmd/ateapi/internal/controlapi/resume_actor.go @@ -30,15 +30,16 @@ func (s *Service) ResumeActor(ctx context.Context, req *ateapipb.ResumeActorRequ if err := validateResumeActorRequest(req); err != nil { return nil, err } - setSpanActorRefAttributes(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) + setSpanActorRefAttributes(ctx, actorRef) - actor, err := s.actorWorkflow.ResumeActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName(), req.GetBoot()) + actor, err := s.actorWorkflow.ResumeActor(ctx, actorRef, req.GetBoot()) if err != nil { if errors.Is(err, store.ErrVersionConflict) { return nil, status.Error(codes.Aborted, "concurrent update conflict, please retry") } if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } return nil, err } diff --git a/cmd/ateapi/internal/controlapi/span_identity.go b/cmd/ateapi/internal/controlapi/span_identity.go index f7a16bcc6..a344daedd 100644 --- a/cmd/ateapi/internal/controlapi/span_identity.go +++ b/cmd/ateapi/internal/controlapi/span_identity.go @@ -20,6 +20,7 @@ import ( "go.opentelemetry.io/otel/trace" "github.com/agent-substrate/substrate/internal/ateattr" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -31,6 +32,6 @@ func setSpanActorAttributes(ctx context.Context, a *ateapipb.Actor) { // setSpanActorRefAttributes is setSpanActorAttributes for the identity subset known // before the Actor record resolves, so a failed lookup still carries who/where. -func setSpanActorRefAttributes(ctx context.Context, atespace, name string) { - trace.SpanFromContext(ctx).SetAttributes(ateattr.ActorRefAttributes(atespace, name)...) +func setSpanActorRefAttributes(ctx context.Context, actorRef resources.ActorRef) { + trace.SpanFromContext(ctx).SetAttributes(ateattr.ActorRefAttributes(actorRef)...) } diff --git a/cmd/ateapi/internal/controlapi/span_identity_test.go b/cmd/ateapi/internal/controlapi/span_identity_test.go index 46caf7583..615d94d0e 100644 --- a/cmd/ateapi/internal/controlapi/span_identity_test.go +++ b/cmd/ateapi/internal/controlapi/span_identity_test.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel/sdk/trace/tracetest" "github.com/agent-substrate/substrate/internal/ateattr" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -90,7 +91,7 @@ func TestSetSpanActorRefAttributes(t *testing.T) { t.Parallel() attrs := recordRootSpanAttrs(t, func(ctx context.Context) { - setSpanActorRefAttributes(ctx, "team-a", "a1") + setSpanActorRefAttributes(ctx, resources.ActorRef{Atespace: "team-a", Name: "a1"}) }) assertSpanStr(t, attrs, ateattr.AtespaceKey, "team-a") @@ -107,5 +108,5 @@ func TestSetSpanActorRefAttributes(t *testing.T) { func TestSetSpanActorAttributes_NoRecordingSpanIsNoop(t *testing.T) { t.Parallel() setSpanActorAttributes(context.Background(), &ateapipb.Actor{Metadata: &ateapipb.ResourceMetadata{Name: "a1"}}) - setSpanActorRefAttributes(context.Background(), "team-a", "a1") + setSpanActorRefAttributes(context.Background(), resources.ActorRef{Atespace: "team-a", Name: "a1"}) } diff --git a/cmd/ateapi/internal/controlapi/suspend_actor.go b/cmd/ateapi/internal/controlapi/suspend_actor.go index ce66177ab..fd30953e8 100644 --- a/cmd/ateapi/internal/controlapi/suspend_actor.go +++ b/cmd/ateapi/internal/controlapi/suspend_actor.go @@ -30,15 +30,16 @@ func (s *Service) SuspendActor(ctx context.Context, req *ateapipb.SuspendActorRe if err := validateSuspendActorRequest(req); err != nil { return nil, err } - setSpanActorRefAttributes(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) + setSpanActorRefAttributes(ctx, actorRef) - actor, err := s.actorWorkflow.SuspendActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actor, err := s.actorWorkflow.SuspendActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrVersionConflict) { return nil, status.Error(codes.Aborted, "concurrent update conflict, please retry") } if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } return nil, err } diff --git a/cmd/ateapi/internal/controlapi/syncer.go b/cmd/ateapi/internal/controlapi/syncer.go index 6320523cd..0499dac8c 100644 --- a/cmd/ateapi/internal/controlapi/syncer.go +++ b/cmd/ateapi/internal/controlapi/syncer.go @@ -210,7 +210,7 @@ func (s *WorkerPoolSyncer) releaseActorOnDeadWorker(ctx context.Context, namespa if worker.Assignment == nil { return nil } - actor, err := s.persistence.GetActor(ctx, worker.Assignment.Actor.Atespace, worker.Assignment.Actor.Name) + actor, err := s.persistence.GetActor(ctx, resources.ActorRefFromObjectRef(worker.Assignment.Actor)) if err != nil { if errors.Is(err, store.ErrNotFound) { return nil diff --git a/cmd/ateapi/internal/controlapi/syncer_test.go b/cmd/ateapi/internal/controlapi/syncer_test.go index 6452bfaf3..ac29e1d54 100644 --- a/cmd/ateapi/internal/controlapi/syncer_test.go +++ b/cmd/ateapi/internal/controlapi/syncer_test.go @@ -24,6 +24,7 @@ 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/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" atefake "github.com/agent-substrate/substrate/pkg/client/clientset/versioned/fake" "github.com/agent-substrate/substrate/pkg/client/informers/externalversions" @@ -271,7 +272,7 @@ func TestSyncer_DeleteBoundWorker_ClearsActor(t *testing.T) { } var got *ateapipb.Actor if err := wait.PollUntilContextTimeout(ctx, 50*time.Millisecond, 2*time.Second, true, func(c context.Context) (bool, error) { - a, gerr := persistence.GetActor(c, "team-orphan", actorName) + a, gerr := persistence.GetActor(c, resources.ActorRef{Atespace: "team-orphan", Name: actorName}) if gerr != nil { return false, gerr } diff --git a/cmd/ateapi/internal/controlapi/update_actor.go b/cmd/ateapi/internal/controlapi/update_actor.go index aba4a8fe3..e8bce4707 100644 --- a/cmd/ateapi/internal/controlapi/update_actor.go +++ b/cmd/ateapi/internal/controlapi/update_actor.go @@ -31,11 +31,12 @@ func (s *Service) UpdateActor(ctx context.Context, req *ateapipb.UpdateActorRequ if err := validateUpdateActorRequest(req); err != nil { return nil, err } + actorRef := resources.ActorRefFromObjectRef(req.GetActor()) - actor, err := s.persistence.GetActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName()) + actor, err := s.persistence.GetActor(ctx, actorRef) if err != nil { if errors.Is(err, store.ErrNotFound) { - return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName()) + return nil, status.Errorf(codes.NotFound, "Actor %s not found", actorRef.Name) } return nil, fmt.Errorf("while getting actor: %w", err) } diff --git a/cmd/ateapi/internal/controlapi/workflow.go b/cmd/ateapi/internal/controlapi/workflow.go index 855ec2aa4..f06efa7b6 100644 --- a/cmd/ateapi/internal/controlapi/workflow.go +++ b/cmd/ateapi/internal/controlapi/workflow.go @@ -22,6 +22,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/scheduling" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache" + "github.com/agent-substrate/substrate/internal/resources" listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "go.opentelemetry.io/otel" @@ -164,15 +165,14 @@ func NewActorWorkflow( } // ResumeActor executes the workflow to resume a suspended actor. Idempotent. -func (w *ActorWorkflow) ResumeActor(ctx context.Context, atespace, name string, boot bool) (*ateapipb.Actor, error) { +func (w *ActorWorkflow) ResumeActor(ctx context.Context, actorRef resources.ActorRef, boot bool) (*ateapipb.Actor, error) { input := &ResumeInput{ - ActorName: name, - Atespace: atespace, - Boot: boot, + ActorRef: actorRef, + Boot: boot, } state := &ResumeState{} - ctx, lock, err := w.acquireActorLock(ctx, atespace, name) + ctx, lock, err := w.acquireActorLock(ctx, actorRef) if err != nil { return nil, err } @@ -194,14 +194,13 @@ func (w *ActorWorkflow) ResumeActor(ctx context.Context, atespace, name string, } // SuspendActor executes the workflow to suspend a running actor. Idempotent. -func (w *ActorWorkflow) SuspendActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) { +func (w *ActorWorkflow) SuspendActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) { input := &SuspendInput{ - ActorName: name, - Atespace: atespace, + ActorRef: actorRef, } state := &SuspendState{} - ctx, lock, err := w.acquireActorLock(ctx, atespace, name) + ctx, lock, err := w.acquireActorLock(ctx, actorRef) if err != nil { return nil, err } @@ -223,14 +222,13 @@ func (w *ActorWorkflow) SuspendActor(ctx context.Context, atespace, name string) } // PauseActor executes the workflow to pause a running actor. Idempotent. -func (w *ActorWorkflow) PauseActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) { +func (w *ActorWorkflow) PauseActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) { input := &PauseInput{ - ActorName: name, - Atespace: atespace, + ActorRef: actorRef, } state := &PauseState{} - ctx, lock, err := w.acquireActorLock(ctx, atespace, name) + ctx, lock, err := w.acquireActorLock(ctx, actorRef) if err != nil { return nil, err } @@ -251,8 +249,8 @@ func (w *ActorWorkflow) PauseActor(ctx context.Context, atespace, name string) ( return state.Actor, nil } -func (w *ActorWorkflow) acquireActorLock(ctx context.Context, atespace, name string) (context.Context, *store.Lock, error) { - lockKey := "lock:actor:" + atespace + ":" + name +func (w *ActorWorkflow) acquireActorLock(ctx context.Context, actorRef resources.ActorRef) (context.Context, *store.Lock, error) { + lockKey := "lock:actor:" + actorRef.Atespace + ":" + actorRef.Name lock, err := w.store.AcquireLock(ctx, lockKey) if err != nil { diff --git a/cmd/ateapi/internal/controlapi/workflow_pause.go b/cmd/ateapi/internal/controlapi/workflow_pause.go index a5ea093d8..a60eb4064 100644 --- a/cmd/ateapi/internal/controlapi/workflow_pause.go +++ b/cmd/ateapi/internal/controlapi/workflow_pause.go @@ -24,6 +24,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/internal/proto/ateletpb" + "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" @@ -34,8 +35,7 @@ import ( // PauseInput holds the immutable parameters requested by the client. type PauseInput struct { - ActorName string - Atespace string + ActorRef resources.ActorRef } // PauseState holds the mutable state loaded and modified during execution. @@ -58,7 +58,7 @@ func (s *LoadActorForPauseStep) CheckPrerequisite(ctx context.Context, input *Pa return nil } func (s *LoadActorForPauseStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error { - actor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + actor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } @@ -87,7 +87,7 @@ func (s *MarkPausingStep) IsComplete(ctx context.Context, input *PauseInput, sta func (s *MarkPausingStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error { // The pause edge only exists from RUNNING; PAUSING/PAUSED are fast-forwarded by IsComplete. if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING { - return status.Errorf(codes.FailedPrecondition, "MarkPausingStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RUNNING) + return status.Errorf(codes.FailedPrecondition, "MarkPausingStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RUNNING) } return nil } @@ -116,13 +116,13 @@ func (s *CallAteletPauseStep) IsComplete(ctx context.Context, input *PauseInput, } func (s *CallAteletPauseStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_PAUSING { - return status.Errorf(codes.FailedPrecondition, "CallAteletPauseStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING) + return status.Errorf(codes.FailedPrecondition, "CallAteletPauseStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING) } if state.Actor.GetAteomPodNamespace() == "" || state.Actor.GetAteomPodName() == "" { - if err := crashActor(ctx, s.store, state.Actor.GetMetadata().GetAtespace(), state.Actor.GetMetadata().GetName()); err != nil { + if err := crashActor(ctx, s.store, input.ActorRef); err != nil { slog.ErrorContext(ctx, "Failed to crash actor", slog.String("err", err.Error())) } - return status.Errorf(codes.FailedPrecondition, "CallAteletPauseStep prerequisite not met for Actor: %s. AteomPodNamespace: %s, GetAteomPodName %s", input.ActorName, state.Actor.GetAteomPodNamespace(), state.Actor.GetAteomPodName()) + return status.Errorf(codes.FailedPrecondition, "CallAteletPauseStep prerequisite not met for Actor: %s. AteomPodNamespace: %s, GetAteomPodName %s", input.ActorRef.Name, state.Actor.GetAteomPodNamespace(), state.Actor.GetAteomPodName()) } return nil } @@ -132,7 +132,7 @@ func (s *CallAteletPauseStep) Execute(ctx context.Context, input *PauseInput, st if err != nil { if errors.Is(err, ErrWorkerPodNotFound) { slog.ErrorContext(ctx, "Worker pod gone before checkpoint, crashing actor", "namespace", state.Actor.GetAteomPodNamespace(), "pod", state.Actor.GetAteomPodName(), "in_progress_snapshot", state.Actor.GetInProgressSnapshot()) - if err := crashActor(ctx, s.store, state.Actor.GetMetadata().GetAtespace(), state.Actor.GetMetadata().GetName()); err != nil { + if err := crashActor(ctx, s.store, input.ActorRef); err != nil { slog.ErrorContext(ctx, "Failed to crash actor", slog.String("err", err.Error())) } return fmt.Errorf("actor is CRASHED because its worker pod is gone and no snapshot was written") @@ -167,7 +167,7 @@ func (s *CallAteletPauseStep) Execute(ctx context.Context, input *PauseInput, st } _, err = client.Checkpoint(ctx, req) - return maybeCrashActor(ctx, s.store, input.Atespace, input.ActorName, err, "while checkpointing workload") + return maybeCrashActor(ctx, s.store, input.ActorRef, err, "while checkpointing workload") } func (s *CallAteletPauseStep) RetryBackoff() *wait.Backoff { return nil } @@ -210,12 +210,12 @@ func (s *FinalizePausedStep) IsComplete(ctx context.Context, input *PauseInput, func (s *FinalizePausedStep) CheckPrerequisite(ctx context.Context, input *PauseInput, state *PauseState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_PAUSING { - return status.Errorf(codes.FailedPrecondition, "FinalizePausedStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING) + return status.Errorf(codes.FailedPrecondition, "FinalizePausedStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_PAUSING) } return nil } func (s *FinalizePausedStep) Execute(ctx context.Context, input *PauseInput, state *PauseState) error { - latestActor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + latestActor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } @@ -240,7 +240,7 @@ func (s *FinalizePausedStep) Execute(ctx context.Context, input *PauseInput, sta // Only free it if it still belongs to us if wass := worker.Assignment; wass != nil { - if wass.Actor.Atespace == input.Atespace && wass.Actor.Name == input.ActorName { + if resources.ActorRefFromObjectRef(wass.Actor) == input.ActorRef { worker.Assignment = nil err = s.store.UpdateWorker(ctx, worker, worker.Version) if err != nil { @@ -251,7 +251,7 @@ func (s *FinalizePausedStep) Execute(ctx context.Context, input *PauseInput, sta } // 2. Safely clear ActiveWorker now that the worker object in DB is freed - latestActor, err = s.store.GetActor(ctx, input.Atespace, input.ActorName) + latestActor, err = s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } @@ -261,7 +261,7 @@ func (s *FinalizePausedStep) Execute(ctx context.Context, input *PauseInput, sta // so the actor can never be resumed (the scheduler would search for a // worker on an unknown node forever). Crash it instead of leaving it // stuck in PAUSED. - slog.ErrorContext(ctx, "Node name not found during finalize pause, crashing actor", "actor", input.ActorName) + slog.ErrorContext(ctx, "Node name not found during finalize pause, crashing actor", slog.Any("actor", input.ActorRef)) latestActor.Status = ateapipb.Actor_STATUS_CRASHED } // TODO(dberkov) - what if InProgressSnapshot is empty? That shouldn't be possible. diff --git a/cmd/ateapi/internal/controlapi/workflow_pause_test.go b/cmd/ateapi/internal/controlapi/workflow_pause_test.go index 31b42b150..a1128038b 100644 --- a/cmd/ateapi/internal/controlapi/workflow_pause_test.go +++ b/cmd/ateapi/internal/controlapi/workflow_pause_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store/storetest" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -40,10 +41,10 @@ func TestFinalizePausedStep_WorkerGone(t *testing.T) { defer cleanup() ctx := context.Background() - const atespace, actorName = "team-a", "actor-1" + actorRef := resources.ActorRef{Atespace: "team-a", Name: "actor-1"} actor := &ateapipb.Actor{ - Metadata: &ateapipb.ResourceMetadata{Atespace: atespace, Name: actorName}, + Metadata: &ateapipb.ResourceMetadata{Atespace: actorRef.Atespace, Name: actorRef.Name}, Status: ateapipb.Actor_STATUS_PAUSING, AteomPodNamespace: "default", AteomPodName: "worker-pod-1", @@ -56,13 +57,13 @@ func TestFinalizePausedStep_WorkerGone(t *testing.T) { // Intentionally NOT creating the worker in store, simulates worker already gone. step := &FinalizePausedStep{store: st} - input := &PauseInput{Atespace: atespace, ActorName: actorName} + input := &PauseInput{ActorRef: actorRef} state := &PauseState{} if err := step.Execute(ctx, input, state); err != nil { t.Fatalf("Execute: %v", err) } - got, err := st.GetActor(ctx, atespace, actorName) + got, err := st.GetActor(ctx, actorRef) if err != nil { t.Fatalf("GetActor: %v", err) } @@ -121,9 +122,9 @@ func TestPauseActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", tc.seedStatus) + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", tc.seedStatus) - actor, err := w.PauseActor(ctx, "team-a", "id1") + actor, err := w.PauseActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if tc.wantErr { if got := status.Code(err); got != codes.FailedPrecondition { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.FailedPrecondition, err) @@ -137,7 +138,7 @@ func TestPauseActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { } } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -200,7 +201,7 @@ func TestPauseSteps_CheckPrerequisite(t *testing.T) { // Worker pod fields are populated so CallAteletPauseStep's // missing-worker crash branch is not taken; this test only // verifies status gating. - err := tc.step.CheckPrerequisite(ctx, &PauseInput{ActorName: "id1"}, &PauseState{Actor: &ateapipb.Actor{Status: st, AteomPodNamespace: "ns", AteomPodName: "worker-1"}}) + err := tc.step.CheckPrerequisite(ctx, &PauseInput{ActorRef: resources.ActorRef{Name: "id1"}}, &PauseState{Actor: &ateapipb.Actor{Status: st, AteomPodNamespace: "ns", AteomPodName: "worker-1"}}) assertPrerequisiteResult(t, st, err, tc.allowed == nil || tc.allowed[st]) } }) @@ -246,12 +247,12 @@ func TestCallAteletPauseStep_DanglingWorkerDoesNotRecordPhantomSnapshot(t *testi } step := &CallAteletPauseStep{store: persistence, dialer: newDanglingDialer()} - input := &PauseInput{ActorName: "actor-1", Atespace: "team-a"} + input := &PauseInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "actor-1"}} if err := step.Execute(ctx, input, &PauseState{Actor: created}); err == nil { t.Fatal("Execute: want error for dangling worker, got nil") } - stored, err := persistence.GetActor(ctx, "team-a", "actor-1") + stored, err := persistence.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "actor-1"}) if err != nil { t.Fatalf("GetActor: %v", err) } @@ -282,14 +283,14 @@ func TestPauseActor_CrashesWhenPausingActorMissingWorkerPod(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", ateapipb.Actor_STATUS_PAUSING) + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", ateapipb.Actor_STATUS_PAUSING) - _, err := w.PauseActor(ctx, "team-a", "id1") + _, err := w.PauseActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if got := status.Code(err); got != codes.FailedPrecondition { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.FailedPrecondition, err) } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } diff --git a/cmd/ateapi/internal/controlapi/workflow_resume.go b/cmd/ateapi/internal/controlapi/workflow_resume.go index 3025ae915..3699d02ee 100644 --- a/cmd/ateapi/internal/controlapi/workflow_resume.go +++ b/cmd/ateapi/internal/controlapi/workflow_resume.go @@ -25,6 +25,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/cmd/ateapi/internal/workercache" "github.com/agent-substrate/substrate/internal/proto/ateletpb" + "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" @@ -39,9 +40,8 @@ import ( // ResumeInput holds the immutable parameters requested by the client. type ResumeInput struct { - ActorName string - Atespace string - Boot bool + ActorRef resources.ActorRef + Boot bool } // ResumeState holds the mutable state loaded and modified during execution. @@ -65,10 +65,10 @@ func (s *LoadActorForResumeStep) CheckPrerequisite(ctx context.Context, input *R return nil } func (s *LoadActorForResumeStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error { - actor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + actor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { if errors.Is(err, store.ErrNotFound) { - return status.Errorf(codes.NotFound, "Actor %s not found", input.ActorName) + return status.Errorf(codes.NotFound, "Actor %s not found", input.ActorRef.Name) } return fmt.Errorf("while getting actor from DB: %w", err) } @@ -91,20 +91,20 @@ func (s *LoadActorForResumeStep) Execute(ctx context.Context, input *ResumeInput slog.String("AteomPodName", actor.AteomPodName)) // Crash the actor if its worker assignment is corrupted. We should never be in this state. - if cerr := crashActor(ctx, s.store, input.Atespace, input.ActorName); cerr != nil { + if cerr := crashActor(ctx, s.store, input.ActorRef); cerr != nil { return cerr } - return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorName) + return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorRef.Name) } wk, err := s.store.GetWorker(ctx, actor.AteomPodNamespace, actor.WorkerPoolName, actor.AteomPodName) if err != nil { // Crash the actor if it was assigned to a deleted pod. if errors.Is(err, store.ErrNotFound) { - if cerr := crashActor(ctx, s.store, input.Atespace, input.ActorName); cerr != nil { + if cerr := crashActor(ctx, s.store, input.ActorRef); cerr != nil { return cerr } - return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorName) + return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorRef.Name) } return fmt.Errorf("failed to get already assigned worker for actor %w", err) } @@ -133,7 +133,7 @@ func (s *AssignWorkerStep) CheckPrerequisite(ctx context.Context, input *ResumeI case ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED: return nil default: - return status.Errorf(codes.FailedPrecondition, "AssignWorkerStep prerequisite not met for Actor: %s (got: %v, want %s or %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED) + return status.Errorf(codes.FailedPrecondition, "AssignWorkerStep prerequisite not met for Actor: %s (got: %v, want %s or %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED) } } @@ -157,7 +157,7 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat if worker.Assignment == nil { continue } - if worker.Assignment.Actor.Atespace != input.Atespace || worker.Assignment.Actor.Name != input.ActorName { + if resources.ActorRefFromObjectRef(worker.Assignment.Actor) != input.ActorRef { continue } if s.scheduler.Applies(worker, constraints) { @@ -203,10 +203,7 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat Namespace: state.Actor.GetActorTemplateNamespace(), Name: state.Actor.GetActorTemplateName(), }, - Actor: &ateapipb.ObjectRef{ - Name: input.ActorName, - Atespace: state.Actor.GetMetadata().GetAtespace(), - }, + Actor: input.ActorRef.ToObjectRef(), } if err := s.store.UpdateWorker(ctx, assignedWorker, assignedWorker.Version); err != nil { @@ -226,18 +223,18 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat return err } // refresh the version of actor to avoid always failure in rest retries. - fresh, gerr := s.store.GetActor(ctx, input.Atespace, input.ActorName) + fresh, gerr := s.store.GetActor(ctx, input.ActorRef) if gerr != nil { slog.WarnContext(ctx, "Failed to refresh actor after assignment conflict", slog.Any("err", gerr)) return err } switch fresh.GetStatus() { case ateapipb.Actor_STATUS_SUSPENDED, ateapipb.Actor_STATUS_PAUSED: - slog.InfoContext(ctx, "Retrying assignment due to actor version conflict", slog.String("actor", input.Atespace+"/"+input.ActorName)) + slog.InfoContext(ctx, "Retrying assignment due to actor version conflict", slog.Any("actor", input.ActorRef)) state.Actor = fresh return err default: - return status.Errorf(codes.Aborted, "actor %s is %s and can no longer be resumed", input.ActorName, fresh.GetStatus()) + return status.Errorf(codes.Aborted, "actor %s is %s and can no longer be resumed", input.ActorRef.Name, fresh.GetStatus()) } } state.Actor = updatedActor @@ -329,21 +326,21 @@ func (s *CallAteletRestoreStep) IsComplete(ctx context.Context, input *ResumeInp } func (s *CallAteletRestoreStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RESUMING { - return status.Errorf(codes.FailedPrecondition, "CallAteletRestoreStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING) + return status.Errorf(codes.FailedPrecondition, "CallAteletRestoreStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING) } if state.Worker == nil { return status.Errorf(codes.FailedPrecondition, "Assigned worker is nil") } // Verify if the worker is still assigned to the same Actor. assigned := state.Worker.GetAssignment().GetActor() - if assigned.GetAtespace() != input.Atespace || assigned.GetName() != input.ActorName { + if resources.ActorRefFromObjectRef(assigned) != input.ActorRef { slog.ErrorContext(ctx, "crashing actor because its assigned worker no longer belongs to it", slog.String("worker", state.Worker.GetWorkerPod()), slog.Any("assignment", state.Worker.GetAssignment())) - if cerr := crashActor(ctx, s.store, input.Atespace, input.ActorName); cerr != nil { + if cerr := crashActor(ctx, s.store, input.ActorRef); cerr != nil { return fmt.Errorf("while crashing actor: %w", cerr) } - return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorName) + return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorRef.Name) } constraints, err := schedulingConstraints(state.Actor, state.ActorTemplate) if err != nil { @@ -360,10 +357,10 @@ func (s *CallAteletRestoreStep) CheckPrerequisite(ctx context.Context, input *Re if err := s.store.UpdateWorker(ctx, release, release.Version); err != nil { return fmt.Errorf("while releasing stale worker assignment: %w", err) } - if cerr := crashActor(ctx, s.store, input.Atespace, input.ActorName); cerr != nil { + if cerr := crashActor(ctx, s.store, input.ActorRef); cerr != nil { return fmt.Errorf("while crashing actor: %w", cerr) } - return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorName) + return status.Errorf(codes.Aborted, "actor %s crashed", input.ActorRef.Name) } return nil } @@ -413,7 +410,7 @@ func (s *CallAteletRestoreStep) Execute(ctx context.Context, input *ResumeInput, } _, err = client.Restore(ctx, req) - return maybeCrashActor(ctx, s.store, input.Atespace, input.ActorName, err, "while restoring workload") + return maybeCrashActor(ctx, s.store, input.ActorRef, err, "while restoring workload") } else if state.ActorTemplate.Status.GoldenSnapshot != "" && !input.Boot { slog.InfoContext(ctx, "Actor has no snapshot; ActorTemplate has golden snapshot; Restoring from golden snapshot") @@ -436,7 +433,7 @@ func (s *CallAteletRestoreStep) Execute(ctx context.Context, input *ResumeInput, ActorUid: state.Actor.GetMetadata().Uid, } _, err = client.Restore(ctx, req) - return maybeCrashActor(ctx, s.store, input.Atespace, input.ActorName, err, "while creating workload from golden snapshot") + return maybeCrashActor(ctx, s.store, input.ActorRef, err, "while creating workload from golden snapshot") } else { slog.InfoContext(ctx, "Actor has no snapshot; ActorTemplate has no golden snapshot; Booting from ActorTemplate spec") @@ -459,7 +456,7 @@ func (s *CallAteletRestoreStep) Execute(ctx context.Context, input *ResumeInput, ActorUid: state.Actor.GetMetadata().Uid, } _, err = client.Run(ctx, req) - return maybeCrashActor(ctx, s.store, input.Atespace, input.ActorName, err, "while creating workload from spec") + return maybeCrashActor(ctx, s.store, input.ActorRef, err, "while creating workload from spec") } // Unreachable } @@ -476,12 +473,12 @@ func (s *FinalizeRunningStep) IsComplete(ctx context.Context, input *ResumeInput } func (s *FinalizeRunningStep) CheckPrerequisite(ctx context.Context, input *ResumeInput, state *ResumeState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RESUMING { - return status.Errorf(codes.FailedPrecondition, "FinalizeRunningStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING) + return status.Errorf(codes.FailedPrecondition, "FinalizeRunningStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RESUMING) } return nil } func (s *FinalizeRunningStep) Execute(ctx context.Context, input *ResumeInput, state *ResumeState) error { - latestActor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + latestActor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } diff --git a/cmd/ateapi/internal/controlapi/workflow_resume_test.go b/cmd/ateapi/internal/controlapi/workflow_resume_test.go index d69c5067d..60c2ddf51 100644 --- a/cmd/ateapi/internal/controlapi/workflow_resume_test.go +++ b/cmd/ateapi/internal/controlapi/workflow_resume_test.go @@ -25,6 +25,7 @@ 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/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc/codes" @@ -67,7 +68,7 @@ func TestAssignWorkerStep_SkipsWorkerAssignedInOtherAtespace(t *testing.T) { Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}, }, } - err := step.Execute(ctx, &ResumeInput{ActorName: "shared", Atespace: "team-a"}, state) + err := step.Execute(ctx, &ResumeInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "shared"}}, state) if status.Code(err) != codes.FailedPrecondition { t.Fatalf("Execute() error = %v, want FailedPrecondition (no free workers)", err) } @@ -134,7 +135,7 @@ func TestAssignWorkerStep_ReleasesIneligibleStaleWorkerInBackground(t *testing.T Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}, }, } - if err := step.Execute(ctx, &ResumeInput{ActorName: "id1", Atespace: "team-a"}, state); err != nil { + if err := step.Execute(ctx, &ResumeInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "id1"}}, state); err != nil { t.Fatalf("Execute() error = %v, want nil (release must not fail the resume)", err) } @@ -232,7 +233,7 @@ func TestAssignWorkerStep_RetryAfterConflictPicksFreshWorker(t *testing.T) { Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}, }, } - if err := step.Execute(ctx, &ResumeInput{ActorName: "id1", Atespace: "team-a"}, state); err != nil { + if err := step.Execute(ctx, &ResumeInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "id1"}}, state); err != nil { t.Fatalf("Execute() on retry = %v, want nil (must re-pick a free worker)", err) } if got := state.Worker.GetWorkerPod(); got != "fallback-pod" { @@ -254,7 +255,7 @@ func TestAssignWorkerStep_RetryAfterConflictPicksFreshWorker(t *testing.T) { t.Errorf("fallback worker assignment = %v, want actor %q", storedFallback.GetAssignment(), "id1") } - storedActor, err := persistence.GetActor(ctx, "team-a", "id1") + storedActor, err := persistence.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor: %v", err) } @@ -433,7 +434,7 @@ func TestResumeActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", tc.seedStatus, func(a *ateapipb.Actor) { + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", tc.seedStatus, func(a *ateapipb.Actor) { a.AteomPodNamespace = "wns" a.AteomPodName = "wpod" a.AteomPodIp = "1.2.3.4" @@ -441,7 +442,7 @@ func TestResumeActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { a.WorkerPoolName = "pool1" }) - actor, err := w.ResumeActor(ctx, "team-a", "id1", false) + actor, err := w.ResumeActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}, false) if tc.wantErr { if got := status.Code(err); got != codes.FailedPrecondition { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.FailedPrecondition, err) @@ -455,7 +456,7 @@ func TestResumeActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { } } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -527,7 +528,7 @@ func TestResumeSteps_CheckPrerequisite(t *testing.T) { }, ActorTemplate: &atev1alpha1.ActorTemplate{Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}}, } - err := tc.step.CheckPrerequisite(ctx, &ResumeInput{ActorName: "id1"}, state) + err := tc.step.CheckPrerequisite(ctx, &ResumeInput{ActorRef: resources.ActorRef{Name: "id1"}}, state) assertPrerequisiteResult(t, st, err, tc.allowed == nil || tc.allowed[st]) } }) @@ -543,16 +544,16 @@ func TestResumeActor_CrashesOnCorruptWorkerAssignment(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", ateapipb.Actor_STATUS_RESUMING, func(a *ateapipb.Actor) { + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", ateapipb.Actor_STATUS_RESUMING, func(a *ateapipb.Actor) { a.AteomPodName = "worker-1" // AteomPodUid and WorkerPoolName left empty }) - _, err := w.ResumeActor(ctx, "team-a", "id1", false) + _, err := w.ResumeActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}, false) if got := status.Code(err); got != codes.Aborted { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.Aborted, err) } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -644,7 +645,7 @@ func TestCallAteletRestoreStep_CheckPrerequisite_WorkerOwnership(t *testing.T) { t.Fatalf("GetWorker: %v", err) } - seedWorkflowActor(t, ctx, persistence, "team-a", "shared", "ns", "tmpl1", ateapipb.Actor_STATUS_RESUMING) + seedWorkflowActor(t, ctx, persistence, resources.ActorRef{Atespace: "team-a", Name: "shared"}, "ns", "tmpl1", ateapipb.Actor_STATUS_RESUMING) step := &CallAteletRestoreStep{store: persistence, scheduler: scheduling.New(nil)} state := &ResumeState{ @@ -655,12 +656,12 @@ func TestCallAteletRestoreStep_CheckPrerequisite_WorkerOwnership(t *testing.T) { Worker: seeded, ActorTemplate: &atev1alpha1.ActorTemplate{Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}}, } - err = step.CheckPrerequisite(ctx, &ResumeInput{Atespace: "team-a", ActorName: "shared"}, state) + err = step.CheckPrerequisite(ctx, &ResumeInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "shared"}}, state) if got := status.Code(err); got != tt.wantCode { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, tt.wantCode, err) } - actor, err := persistence.GetActor(ctx, "team-a", "shared") + actor, err := persistence.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "shared"}) if err != nil { t.Fatalf("GetActor: %v", err) } diff --git a/cmd/ateapi/internal/controlapi/workflow_suspend.go b/cmd/ateapi/internal/controlapi/workflow_suspend.go index 706c97891..d8bb4839c 100644 --- a/cmd/ateapi/internal/controlapi/workflow_suspend.go +++ b/cmd/ateapi/internal/controlapi/workflow_suspend.go @@ -25,6 +25,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/internal/proto/ateletpb" + "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" @@ -35,8 +36,7 @@ import ( // SuspendInput holds the immutable parameters requested by the client. type SuspendInput struct { - ActorName string - Atespace string + ActorRef resources.ActorRef } // SuspendState holds the mutable state loaded and modified during execution. @@ -59,7 +59,7 @@ func (s *LoadActorForSuspendStep) CheckPrerequisite(ctx context.Context, input * return nil } func (s *LoadActorForSuspendStep) Execute(ctx context.Context, input *SuspendInput, state *SuspendState) error { - actor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + actor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } @@ -87,14 +87,14 @@ func (s *MarkSuspendingStep) IsComplete(ctx context.Context, input *SuspendInput } func (s *MarkSuspendingStep) CheckPrerequisite(ctx context.Context, input *SuspendInput, state *SuspendState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING { - return status.Errorf(codes.FailedPrecondition, "MarkSuspendingStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RUNNING) + return status.Errorf(codes.FailedPrecondition, "MarkSuspendingStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_RUNNING) } return nil } func (s *MarkSuspendingStep) Execute(ctx context.Context, input *SuspendInput, state *SuspendState) error { state.Actor.Status = ateapipb.Actor_STATUS_SUSPENDING snapshotID := time.Now().Format(time.RFC3339) + "-" + rand.Text() - state.Actor.InProgressSnapshot = strings.TrimSuffix(state.ActorTemplate.Spec.SnapshotsConfig.Location, "/") + "/" + input.ActorName + "/" + snapshotID + state.Actor.InProgressSnapshot = strings.TrimSuffix(state.ActorTemplate.Spec.SnapshotsConfig.Location, "/") + "/" + input.ActorRef.Name + "/" + snapshotID updatedActor, err := s.store.UpdateActor(ctx, state.Actor, state.Actor.GetMetadata().GetVersion()) if err != nil { return err @@ -117,10 +117,10 @@ func (s *CallAteletSuspendStep) IsComplete(ctx context.Context, input *SuspendIn } func (s *CallAteletSuspendStep) CheckPrerequisite(ctx context.Context, input *SuspendInput, state *SuspendState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_SUSPENDING { - return status.Errorf(codes.FailedPrecondition, "CallAteletSuspendStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDING) + return status.Errorf(codes.FailedPrecondition, "CallAteletSuspendStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDING) } if state.Actor.GetAteomPodNamespace() == "" || state.Actor.GetAteomPodName() == "" { - if err := crashActor(ctx, s.store, state.Actor.GetMetadata().GetAtespace(), state.Actor.GetMetadata().GetName()); err != nil { + if err := crashActor(ctx, s.store, input.ActorRef); err != nil { slog.ErrorContext(ctx, "Failed to crash actor", slog.String("err", err.Error())) } return fmt.Errorf("actor is CRASHED because it was in SUSPENDING state but has no active worker") @@ -132,7 +132,7 @@ func (s *CallAteletSuspendStep) Execute(ctx context.Context, input *SuspendInput if err != nil { if errors.Is(err, ErrWorkerPodNotFound) { slog.ErrorContext(ctx, "Worker pod gone before checkpoint, crashing actor", "namespace", state.Actor.GetAteomPodNamespace(), "pod", state.Actor.GetAteomPodName(), "in_progress_snapshot", state.Actor.GetInProgressSnapshot()) - if err := crashActor(ctx, s.store, state.Actor.GetMetadata().GetAtespace(), state.Actor.GetMetadata().GetName()); err != nil { + if err := crashActor(ctx, s.store, input.ActorRef); err != nil { slog.ErrorContext(ctx, "Failed to crash actor", slog.String("err", err.Error())) } return fmt.Errorf("actor is CRASHED because its worker pod is gone and no snapshot was written") @@ -167,7 +167,7 @@ func (s *CallAteletSuspendStep) Execute(ctx context.Context, input *SuspendInput } _, err = client.Checkpoint(ctx, req) - return maybeCrashActor(ctx, s.store, input.Atespace, input.ActorName, err, "while checkpointing workload") + return maybeCrashActor(ctx, s.store, input.ActorRef, err, "while checkpointing workload") } func (s *CallAteletSuspendStep) RetryBackoff() *wait.Backoff { return nil } @@ -203,12 +203,12 @@ func (s *FinalizeSuspendedStep) IsComplete(ctx context.Context, input *SuspendIn } func (s *FinalizeSuspendedStep) CheckPrerequisite(ctx context.Context, input *SuspendInput, state *SuspendState) error { if state.Actor.GetStatus() != ateapipb.Actor_STATUS_SUSPENDING { - return status.Errorf(codes.FailedPrecondition, "FinalizeSuspendedStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorName, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDING) + return status.Errorf(codes.FailedPrecondition, "FinalizeSuspendedStep prerequisite not met for Actor: %s (got: %v, want %s)", input.ActorRef.Name, state.Actor.GetStatus(), ateapipb.Actor_STATUS_SUSPENDING) } return nil } func (s *FinalizeSuspendedStep) Execute(ctx context.Context, input *SuspendInput, state *SuspendState) error { - latestActor, err := s.store.GetActor(ctx, input.Atespace, input.ActorName) + latestActor, err := s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } @@ -229,7 +229,7 @@ func (s *FinalizeSuspendedStep) Execute(ctx context.Context, input *SuspendInput } else { // Only free it if it still belongs to us if wass := worker.Assignment; wass != nil { - if wass.Actor.Atespace == input.Atespace && wass.Actor.Name == input.ActorName { + if resources.ActorRefFromObjectRef(wass.Actor) == input.ActorRef { worker.Assignment = nil err = s.store.UpdateWorker(ctx, worker, worker.Version) if err != nil { @@ -240,7 +240,7 @@ func (s *FinalizeSuspendedStep) Execute(ctx context.Context, input *SuspendInput } // 2. Safely clear ActiveWorker now that the worker object in DB is freed - latestActor, err = s.store.GetActor(ctx, input.Atespace, input.ActorName) + latestActor, err = s.store.GetActor(ctx, input.ActorRef) if err != nil { return err } diff --git a/cmd/ateapi/internal/controlapi/workflow_suspend_test.go b/cmd/ateapi/internal/controlapi/workflow_suspend_test.go index 387087d83..91e02419c 100644 --- a/cmd/ateapi/internal/controlapi/workflow_suspend_test.go +++ b/cmd/ateapi/internal/controlapi/workflow_suspend_test.go @@ -21,6 +21,7 @@ import ( "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store/ateredis" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store/storetest" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/alicebob/miniredis/v2" "github.com/redis/go-redis/v9" @@ -66,9 +67,9 @@ func TestSuspendActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", tc.seedStatus) + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", tc.seedStatus) - actor, err := w.SuspendActor(ctx, "team-a", "id1") + actor, err := w.SuspendActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if tc.wantErr { if got := status.Code(err); got != codes.FailedPrecondition { t.Fatalf("status.Code(err) = %v, want %v (err: %v)", got, codes.FailedPrecondition, err) @@ -82,7 +83,7 @@ func TestSuspendActorWorkflow_RejectedAndIdempotentPaths(t *testing.T) { } } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -145,7 +146,7 @@ func TestSuspendSteps_CheckPrerequisite(t *testing.T) { // Worker pod fields are populated so CallAteletSuspendStep's // missing-worker crash branch is not taken; this test only // verifies status gating. - err := tc.step.CheckPrerequisite(ctx, &SuspendInput{ActorName: "id1"}, &SuspendState{Actor: &ateapipb.Actor{Status: st, AteomPodNamespace: "ns", AteomPodName: "worker-1"}}) + err := tc.step.CheckPrerequisite(ctx, &SuspendInput{ActorRef: resources.ActorRef{Name: "id1"}}, &SuspendState{Actor: &ateapipb.Actor{Status: st, AteomPodNamespace: "ns", AteomPodName: "worker-1"}}) assertPrerequisiteResult(t, st, err, tc.allowed == nil || tc.allowed[st]) } }) @@ -161,13 +162,13 @@ func TestSuspendActor_CrashesWhenSuspendingActorMissingWorkerPod(t *testing.T) { defer cleanup() w := newTestActorWorkflow(t, st, "ns", "tmpl1") - seedWorkflowActor(t, ctx, st, "team-a", "id1", "ns", "tmpl1", ateapipb.Actor_STATUS_SUSPENDING) + seedWorkflowActor(t, ctx, st, resources.ActorRef{Atespace: "team-a", Name: "id1"}, "ns", "tmpl1", ateapipb.Actor_STATUS_SUSPENDING) - if _, err := w.SuspendActor(ctx, "team-a", "id1"); err == nil { + if _, err := w.SuspendActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}); err == nil { t.Fatal("SuspendActor succeeded, want error for SUSPENDING actor with no worker pod") } - got, err := st.GetActor(ctx, "team-a", "id1") + got, err := st.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -237,12 +238,12 @@ func TestCallAteletSuspendStep_DanglingWorkerDoesNotRecordPhantomSnapshot(t *tes } step := &CallAteletSuspendStep{store: persistence, dialer: newDanglingDialer()} - input := &SuspendInput{ActorName: "actor-1", Atespace: "team-a"} + input := &SuspendInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "actor-1"}} if err := step.Execute(ctx, input, &SuspendState{Actor: created}); err == nil { t.Fatal("Execute: want error for dangling worker, got nil") } - stored, err := persistence.GetActor(ctx, "team-a", "actor-1") + stored, err := persistence.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "actor-1"}) if err != nil { t.Fatalf("GetActor: %v", err) } @@ -311,7 +312,7 @@ func TestFinalizeSuspendedStep_ReleasesOnlyOwnWorker(t *testing.T) { } step := &FinalizeSuspendedStep{store: persistence} - input := &SuspendInput{ActorName: "shared", Atespace: "team-a"} + input := &SuspendInput{ActorRef: resources.ActorRef{Atespace: "team-a", Name: "shared"}} if err := step.Execute(ctx, input, &SuspendState{}); err != nil { t.Fatalf("Execute: %v", err) } diff --git a/cmd/ateapi/internal/controlapi/workflow_testutil_test.go b/cmd/ateapi/internal/controlapi/workflow_testutil_test.go index c6962694a..eebcc09cf 100644 --- a/cmd/ateapi/internal/controlapi/workflow_testutil_test.go +++ b/cmd/ateapi/internal/controlapi/workflow_testutil_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" + "github.com/agent-substrate/substrate/internal/resources" atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" listersv1alpha1 "github.com/agent-substrate/substrate/pkg/client/listers/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" @@ -47,10 +48,10 @@ func newTestActorWorkflow(t *testing.T, st store.Interface, tmplNamespace, tmplN // seedWorkflowActor stores an actor with the given status, bound to the given // template (pass the same tmplNamespace/tmplName as newTestActorWorkflow). // opts mutate the actor before it is stored. -func seedWorkflowActor(t *testing.T, ctx context.Context, st store.Interface, atespace, id, tmplNamespace, tmplName string, actorStatus ateapipb.Actor_Status, opts ...func(*ateapipb.Actor)) { +func seedWorkflowActor(t *testing.T, ctx context.Context, st store.Interface, actorRef resources.ActorRef, tmplNamespace, tmplName string, actorStatus ateapipb.Actor_Status, opts ...func(*ateapipb.Actor)) { t.Helper() actor := &ateapipb.Actor{ - Metadata: &ateapipb.ResourceMetadata{Name: id, Atespace: atespace}, + Metadata: &ateapipb.ResourceMetadata{Name: actorRef.Name, Atespace: actorRef.Atespace}, Status: actorStatus, ActorTemplateNamespace: tmplNamespace, ActorTemplateName: tmplName, diff --git a/cmd/ateapi/internal/store/ateredis/ateredis.go b/cmd/ateapi/internal/store/ateredis/ateredis.go index 467707e06..e5ada1ff8 100644 --- a/cmd/ateapi/internal/store/ateredis/ateredis.go +++ b/cmd/ateapi/internal/store/ateredis/ateredis.go @@ -53,6 +53,7 @@ import ( "time" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/google/uuid" "github.com/redis/go-redis/v9" @@ -89,8 +90,11 @@ func NewPersistence(redisClient *redis.ClusterClient) *Persistence { } } -func actorDBKey(atespace, name string) string { - return "actor:" + atespace + ":" + name +// actorDBKey returns the Redis key an actor is stored under. The encoding is +// "actor::" and must not change: existing databases hold keys +// in this form. +func actorDBKey(actorRef resources.ActorRef) string { + return "actor:" + actorRef.Atespace + ":" + actorRef.Name } // actorScanPattern returns the SCAN match pattern for listing actors. An empty @@ -312,8 +316,8 @@ func (s *Persistence) DebugClearAll(ctx context.Context) error { return err } -func (s *Persistence) GetActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) { - dbKey := actorDBKey(atespace, name) +func (s *Persistence) GetActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) { + dbKey := actorDBKey(actorRef) dbActorBytes, err := s.rdb.Get(ctx, dbKey).Bytes() if err != nil { @@ -328,7 +332,7 @@ func (s *Persistence) GetActor(ctx context.Context, atespace, name string) (*ate return nil, fmt.Errorf("while unmarshaling actor: %w", err) } - if actor.GetMetadata().GetName() != name || actor.GetMetadata().GetAtespace() != atespace { + if resources.ActorRefFromActor(actor) != actorRef { return nil, fmt.Errorf("(impossible) mismatch between stored name/atespace and key") } @@ -336,7 +340,7 @@ func (s *Persistence) GetActor(ctx context.Context, atespace, name string) (*ate } func (s *Persistence) CreateActor(ctx context.Context, actor *ateapipb.Actor) (*ateapipb.Actor, error) { - dbKey := actorDBKey(actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + dbKey := actorDBKey(resources.ActorRefFromActor(actor)) // Clone so we don't stomp the caller's copy, then attach fresh server-owned // metadata carrying the caller-specified identity. @@ -483,8 +487,8 @@ func (s *Persistence) DeleteWorker(ctx context.Context, namespace, pool, pod str return nil } -func (s *Persistence) DeleteActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) { - dbKey := actorDBKey(atespace, name) +func (s *Persistence) DeleteActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) { + dbKey := actorDBKey(actorRef) var deleted *ateapipb.Actor err := s.rdb.Watch(ctx, func(tx *redis.Tx) error { currentVal, err := tx.Get(ctx, dbKey).Bytes() @@ -526,7 +530,7 @@ func (s *Persistence) DeleteActor(ctx context.Context, atespace, name string) (* } func (s *Persistence) UpdateActor(ctx context.Context, actor *ateapipb.Actor, expectedVersion int64) (*ateapipb.Actor, error) { - dbKey := actorDBKey(actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + dbKey := actorDBKey(resources.ActorRefFromActor(actor)) // Clone because we will update the version field, and we don't want to // stomp the caller's copy. diff --git a/cmd/ateapi/internal/store/ateredis/ateredis_test.go b/cmd/ateapi/internal/store/ateredis/ateredis_test.go index fcdbf5834..931949f94 100644 --- a/cmd/ateapi/internal/store/ateredis/ateredis_test.go +++ b/cmd/ateapi/internal/store/ateredis/ateredis_test.go @@ -31,6 +31,7 @@ import ( "google.golang.org/protobuf/testing/protocmp" "github.com/agent-substrate/substrate/cmd/ateapi/internal/store" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -65,7 +66,7 @@ var ( func TestGetActor_NotFound(t *testing.T) { _, s, ctx := setupTest(t) - _, err := s.GetActor(ctx, testAtespace, "non-existent") + _, err := s.GetActor(ctx, resources.ActorRef{Atespace: testAtespace, Name: "non-existent"}) if !errors.Is(err, store.ErrNotFound) { t.Errorf("expected ErrNotFound, got %v", err) } @@ -103,7 +104,7 @@ func TestCreateActor_Success(t *testing.T) { } // The returned resource is exactly what GetActor reads back. - got, err := s.GetActor(ctx, actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + got, err := s.GetActor(ctx, resources.ActorRefFromActor(actor)) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -183,7 +184,7 @@ func TestUpdateActor_Success(t *testing.T) { } // The returned resource is exactly what GetActor reads back. - got, err := s.GetActor(ctx, actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + got, err := s.GetActor(ctx, resources.ActorRefFromActor(actor)) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -208,13 +209,13 @@ func TestUpdateActor_Conflict(t *testing.T) { } // Fetch instance 1 - actor1, err := s.GetActor(ctx, actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + actor1, err := s.GetActor(ctx, resources.ActorRefFromActor(actor)) if err != nil { t.Fatalf("GetActor failed: %v", err) } // Fetch instance 2 (stale after actor1 updates) - actor2, err := s.GetActor(ctx, actor.GetMetadata().GetAtespace(), actor.GetMetadata().GetName()) + actor2, err := s.GetActor(ctx, resources.ActorRefFromActor(actor)) if err != nil { t.Fatalf("GetActor failed: %v", err) } @@ -434,7 +435,7 @@ func TestDeleteActor(t *testing.T) { t.Fatalf("CreateActor failed: %v", err) } - deleted, err := s.DeleteActor(ctx, testAtespace, "session-1") + deleted, err := s.DeleteActor(ctx, resources.ActorRef{Atespace: testAtespace, Name: "session-1"}) if tt.wantErr != nil { if !errors.Is(err, tt.wantErr) { t.Errorf("DeleteActor: expected %v, got %v", tt.wantErr, err) @@ -449,7 +450,7 @@ func TestDeleteActor(t *testing.T) { t.Errorf("deleted actor name = %q, want session-1", got) } - if _, err := s.GetActor(ctx, testAtespace, "session-1"); !errors.Is(err, store.ErrNotFound) { + if _, err := s.GetActor(ctx, resources.ActorRef{Atespace: testAtespace, Name: "session-1"}); !errors.Is(err, store.ErrNotFound) { t.Errorf("expected ErrNotFound after delete, got %v", err) } }) @@ -459,7 +460,7 @@ func TestDeleteActor(t *testing.T) { func TestDeleteActor_NotFound(t *testing.T) { _, s, ctx := setupTest(t) - _, err := s.DeleteActor(ctx, testAtespace, "non-existent") + _, err := s.DeleteActor(ctx, resources.ActorRef{Atespace: testAtespace, Name: "non-existent"}) if !errors.Is(err, store.ErrNotFound) { t.Errorf("expected ErrNotFound deleting non-existent actor, got %v", err) } @@ -1186,13 +1187,13 @@ func TestListActors_ScopedByAtespace(t *testing.T) { } // Get is scoped too: right atespace hits, wrong/empty atespace misses. - if _, err := s.GetActor(ctx, "team-a", "a1"); err != nil { + if _, err := s.GetActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "a1"}); err != nil { t.Errorf("GetActor(team-a, a1) failed: %v", err) } - if _, err := s.GetActor(ctx, "team-b", "a1"); !errors.Is(err, store.ErrNotFound) { + if _, err := s.GetActor(ctx, resources.ActorRef{Atespace: "team-b", Name: "a1"}); !errors.Is(err, store.ErrNotFound) { t.Errorf("GetActor(team-b, a1) = %v, want ErrNotFound", err) } - if _, err := s.GetActor(ctx, "", "a1"); !errors.Is(err, store.ErrNotFound) { + if _, err := s.GetActor(ctx, resources.ActorRef{Atespace: "", Name: "a1"}); !errors.Is(err, store.ErrNotFound) { t.Errorf("GetActor(empty, a1) = %v, want ErrNotFound", err) } } @@ -1370,7 +1371,7 @@ func TestDeleteAtespace_EmptyAfterActorsRemoved(t *testing.T) { if _, err := s.DeleteAtespace(ctx, "team-a"); !errors.Is(err, store.ErrFailedPrecondition) { t.Fatalf("expected rejection while non-empty, got %v", err) } - if _, err := s.DeleteActor(ctx, "team-a", "id1"); err != nil { + if _, err := s.DeleteActor(ctx, resources.ActorRef{Atespace: "team-a", Name: "id1"}); err != nil { t.Fatalf("DeleteActor failed: %v", err) } if _, err := s.DeleteAtespace(ctx, "team-a"); err != nil { diff --git a/cmd/ateapi/internal/store/store.go b/cmd/ateapi/internal/store/store.go index 3b6e8426c..556eb1c3c 100644 --- a/cmd/ateapi/internal/store/store.go +++ b/cmd/ateapi/internal/store/store.go @@ -20,6 +20,7 @@ import ( "errors" "sync" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -42,8 +43,8 @@ var ( // Interface defines the contract for the persistence layer storing actor state. type Interface interface { - // Fetches an actor by (atespace, name). Returns ErrNotFound if missing. - GetActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) + // Fetches an actor by reference. Returns ErrNotFound if missing. + GetActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) // Stores a new actor in suspended state and returns the stored resource with // server-assigned metadata (uid, version, timestamps). The input is not @@ -57,7 +58,7 @@ type Interface interface { // Removes an actor and returns the deleted resource. Returns ErrNotFound if // missing, or ErrFailedPrecondition if not suspended. - DeleteActor(ctx context.Context, atespace, name string) (*ateapipb.Actor, error) + DeleteActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) // Lists actors in the given atespace (scoped scan), or across ALL atespaces if atespace is // empty. Returns a page of actors and a next page token. diff --git a/cmd/atelet/main.go b/cmd/atelet/main.go index 4f1e02f63..28a5a6aca 100644 --- a/cmd/atelet/main.go +++ b/cmd/atelet/main.go @@ -239,7 +239,8 @@ func (s *AteomHerder) Run(ctx context.Context, req *ateletpb.RunRequest) (resp * return nil, status.Error(codes.InvalidArgument, err.Error()) } - actorUID, atespace, actorName := req.GetActorUid(), req.GetAtespace(), req.GetActorName() + actorUID := req.GetActorUid() + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} sandboxRec, err := recordFromRequest(req.GetSandboxAssets()) if err != nil { @@ -271,7 +272,7 @@ func (s *AteomHerder) Run(ctx context.Context, req *ateletpb.RunRequest) (resp * return nil, fmt.Errorf("while recording sandbox assets: %w", err) } - if err := s.prepareOCIBundles(ctx, actorUID, actorName, + if err := s.prepareOCIBundles(ctx, actorUID, actorRef.Name, req.GetSpec(), req.GetTargetAteomUid(), ); err != nil { return nil, ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonInvalidContainerConfig) @@ -285,8 +286,8 @@ func (s *AteomHerder) Run(ctx context.Context, req *ateletpb.RunRequest) (resp * // Tell ateom to start the workload. gVisor uses RunscPath; the micro-VM // runtime uses the full RuntimeAssetPaths set. if _, err := client.RunWorkload(ctx, &ateompb.RunWorkloadRequest{ - Atespace: atespace, - ActorName: actorName, + Atespace: actorRef.Atespace, + ActorName: actorRef.Name, ActorTemplateNamespace: req.GetActorTemplateNamespace(), ActorTemplateName: req.GetActorTemplateName(), RunscPath: runscPathFor(assetPaths), @@ -341,7 +342,8 @@ func (s *AteomHerder) Checkpoint(ctx context.Context, req *ateletpb.CheckpointRe return nil, status.Error(codes.InvalidArgument, err.Error()) } - actorUID, atespace, actorName := req.GetActorUid(), req.GetAtespace(), req.GetActorName() + actorUID := req.GetActorUid() + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} // Checkpoint requests no longer carry the sandbox config; recover the // version this actor was started with from the on-node record and re-fetch @@ -367,8 +369,8 @@ func (s *AteomHerder) Checkpoint(ctx context.Context, req *ateletpb.CheckpointRe // exact files it wrote so we ship precisely that set (gVisor's image files, // cloud-hypervisor's snapshot set, ...) rather than a hardcoded list. resp, err := client.CheckpointWorkload(ctx, &ateompb.CheckpointWorkloadRequest{ - Atespace: atespace, - ActorName: actorName, + Atespace: actorRef.Atespace, + ActorName: actorRef.Name, ActorTemplateNamespace: req.GetActorTemplateNamespace(), ActorTemplateName: req.GetActorTemplateName(), RunscPath: runscPathFor(assetPaths), @@ -490,7 +492,8 @@ func (s *AteomHerder) Restore(ctx context.Context, req *ateletpb.RestoreRequest) return nil, status.Error(codes.InvalidArgument, err.Error()) } - actorUID, atespace, actorName := req.GetActorUid(), req.GetAtespace(), req.GetActorName() + actorUID := req.GetActorUid() + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} // Not crashing the actor, because terminal errors here indicate problems with atelet, // node or the disk itself. @@ -577,7 +580,7 @@ func (s *AteomHerder) Restore(ctx context.Context, req *ateletpb.RestoreRequest) return ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonFailedGetExternalObject, ateerrors.ReasonInvalidObjectURL, ateerrors.ReasonTerminalFileSystemError, ateerrors.ReasonInvalidSandboxAsset) } t := time.Now() - if err := s.prepareOCIBundles(gctx, actorUID, actorName, req.GetSpec(), req.GetTargetAteomUid()); err != nil { + if err := s.prepareOCIBundles(gctx, actorUID, actorRef.Name, req.GetSpec(), req.GetTargetAteomUid()); err != nil { return ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonTerminalFileSystemError, ateerrors.ReasonInvalidContainerConfig) } dBundles = time.Since(t) @@ -596,8 +599,8 @@ func (s *AteomHerder) Restore(ctx context.Context, req *ateletpb.RestoreRequest) // all application containers. tAteom := time.Now() if _, err := client.RestoreWorkload(ctx, &ateompb.RestoreWorkloadRequest{ - Atespace: atespace, - ActorName: actorName, + Atespace: actorRef.Atespace, + ActorName: actorRef.Name, ActorTemplateNamespace: req.GetActorTemplateNamespace(), ActorTemplateName: req.GetActorTemplateName(), RunscPath: runscPathFor(assetPaths), @@ -618,7 +621,7 @@ func (s *AteomHerder) Restore(ctx context.Context, req *ateletpb.RestoreRequest) return nil, ateerrors.CrashIfReason(ctx, err, ateerrors.ReasonTerminalFileSystemError) } - slog.InfoContext(ctx, "Restore timing breakdown", slog.String("actor", actorName), + slog.InfoContext(ctx, "Restore timing breakdown", slog.Any("actor", actorRef), slog.Duration("download", dDownload), // rustfs/GCS fetch + decompress (or local copy) slog.Duration("oci_unpack", dBundles), // prepareOCIBundles: unpack the OCI image to the bundle slog.Duration("ateom_restore", dAteom), // ateom.RestoreWorkload (see its own breakdown) diff --git a/cmd/atenet/internal/router/extproc.go b/cmd/atenet/internal/router/extproc.go index 203c668d1..cef6519d2 100644 --- a/cmd/atenet/internal/router/extproc.go +++ b/cmd/atenet/internal/router/extproc.go @@ -142,16 +142,16 @@ func (s *ExtProcServer) handleRequestHeaders( ctx, span := otel.Tracer(routerServiceName).Start(ctx, "ExtProc.RequestHeaders") defer span.End() - atespace, actorName, err := parseActorRef(metadata.host) + actorRef, err := parseActorRef(metadata.host) if err != nil { // Host is invalid, respond with 404. return nil, metadata, "", "", "", invalidHostErr(metadata.host, err) } - slog.InfoContext(ctx, "ResumeActor", slog.String("atespace", atespace), slog.String("actor", actorName)) - actor, err := s.resumer.ResumeActor(ctx, atespace, actorName) + slog.InfoContext(ctx, "ResumeActor", slog.Any("actor", actorRef)) + actor, err := s.resumer.ResumeActor(ctx, actorRef) if err != nil { - return nil, metadata, "", "", "", mapResumeError(actorName, err) + return nil, metadata, "", "", "", mapResumeError(actorRef.Name, err) } // Actor template identity, used as low-cardinality route-latency metric @@ -161,20 +161,19 @@ func (s *ExtProcServer) handleRequestHeaders( workerIP := actor.GetAteomPodIp() slog.InfoContext(ctx, "ResumeActor result", - slog.String("atespace", atespace), - slog.String("actor", actorName), + slog.Any("actor", actorRef), slog.String("status", actor.GetStatus().String()), slog.String("workerIP", workerIP)) if ip := net.ParseIP(workerIP); ip == nil { return nil, metadata, "", tmplNs, tmplName, newReqError(envoy_type.StatusCode_InternalServerError, - "actor %q routing failed", actorName) + "actor %q routing failed", actorRef.Name) } // TODO(bowei) -- handle more than port 80 on the actor. targetAddr := net.JoinHostPort(workerIP, "80") - slog.InfoContext(ctx, "Route ok", slog.String("actor", actorName), slog.String("targetAddr", targetAddr)) + slog.InfoContext(ctx, "Route ok", slog.Any("actor", actorRef), slog.String("targetAddr", targetAddr)) // Route by rewriting the :authority header. mutation := &extprocv3.HeaderMutation{} diff --git a/cmd/atenet/internal/router/extproc_in.go b/cmd/atenet/internal/router/extproc_in.go index 5bfdb4974..459b26b92 100644 --- a/cmd/atenet/internal/router/extproc_in.go +++ b/cmd/atenet/internal/router/extproc_in.go @@ -56,17 +56,18 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { } } -// parseActorRef extracts the (atespace, actor name) an incoming request is -// addressed to from its Host/:authority, which has the form +// parseActorRef extracts the actor an incoming request is addressed to from its +// Host/:authority, which has the form // "..actors.resources.substrate.ate.dev" (optionally with a -// port). The atespace is required because an actor name is only unique within its -// atespace. -func parseActorRef(host string) (atespace, actorName string, err error) { +// port). The atespace is part of the name because an actor name is only unique +// within its atespace. +func parseActorRef(host string) (resources.ActorRef, error) { if strings.Contains(host, ":") { - host, _, err = net.SplitHostPort(host) + h, _, err := net.SplitHostPort(host) if err != nil { - return "", "", err + return resources.ActorRef{}, err } + host = h } return resources.ParseActorDNSName(host) } diff --git a/cmd/atenet/internal/router/extproc_in_test.go b/cmd/atenet/internal/router/extproc_in_test.go index a23d47590..bbb7a8f9e 100644 --- a/cmd/atenet/internal/router/extproc_in_test.go +++ b/cmd/atenet/internal/router/extproc_in_test.go @@ -18,6 +18,7 @@ import ( "reflect" "testing" + "github.com/agent-substrate/substrate/internal/resources" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" ) @@ -121,39 +122,34 @@ func TestExtractMetadata(t *testing.T) { func TestParseActorRef(t *testing.T) { tests := []struct { - name string - host string - wantAtespace string - wantID string - wantErr bool + name string + host string + want resources.ActorRef + wantErr bool }{ { - name: "valid host without port", - host: "my-actor.team-a.actors.resources.substrate.ate.dev", - wantAtespace: "team-a", - wantID: "my-actor", - wantErr: false, + name: "valid host without port", + host: "my-actor.team-a.actors.resources.substrate.ate.dev", + want: resources.ActorRef{Atespace: "team-a", Name: "my-actor"}, + wantErr: false, }, { - name: "valid host with port", - host: "my-actor.team-a.actors.resources.substrate.ate.dev:8443", - wantAtespace: "team-a", - wantID: "my-actor", - wantErr: false, + name: "valid host with port", + host: "my-actor.team-a.actors.resources.substrate.ate.dev:8443", + want: resources.ActorRef{Atespace: "team-a", Name: "my-actor"}, + wantErr: false, }, { - name: "valid host with trailing dot", - host: "my-actor.team-a.actors.resources.substrate.ate.dev.", - wantAtespace: "team-a", - wantID: "my-actor", - wantErr: false, + name: "valid host with trailing dot", + host: "my-actor.team-a.actors.resources.substrate.ate.dev.", + want: resources.ActorRef{Atespace: "team-a", Name: "my-actor"}, + wantErr: false, }, { - name: "valid host with trailing dot and port", - host: "my-actor.team-a.actors.resources.substrate.ate.dev.:8080", - wantAtespace: "team-a", - wantID: "my-actor", - wantErr: false, + name: "valid host with trailing dot and port", + host: "my-actor.team-a.actors.resources.substrate.ate.dev.:8080", + want: resources.ActorRef{Atespace: "team-a", Name: "my-actor"}, + wantErr: false, }, { name: "missing atespace label", @@ -174,13 +170,13 @@ func TestParseActorRef(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - gotAtespace, gotID, err := parseActorRef(tc.host) + got, err := parseActorRef(tc.host) if (err != nil) != tc.wantErr { t.Errorf("parseActorRef(%q) error = %v, wantErr %v", tc.host, err, tc.wantErr) return } - if gotAtespace != tc.wantAtespace || gotID != tc.wantID { - t.Errorf("parseActorRef(%q) = (%q, %q), want (%q, %q)", tc.host, gotAtespace, gotID, tc.wantAtespace, tc.wantID) + if got != tc.want { + t.Errorf("parseActorRef(%q) = %+v, want %+v", tc.host, got, tc.want) } }) } diff --git a/cmd/atenet/internal/router/resumer.go b/cmd/atenet/internal/router/resumer.go index db2231c06..5e111f9df 100644 --- a/cmd/atenet/internal/router/resumer.go +++ b/cmd/atenet/internal/router/resumer.go @@ -18,9 +18,10 @@ import ( "context" "time" + "github.com/agent-substrate/substrate/internal/ateattr" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "golang.org/x/sync/singleflight" "google.golang.org/grpc/codes" @@ -41,17 +42,13 @@ func NewActorResumer(apiClient ateapipb.ControlClient) *ActorResumer { } // ResumeActor ensures the requested actor is running. It deduplicates concurrent -// requests within the process and retries when needed. The actor is addressed by -// (atespace, actorName) since an actor name is only unique within its atespace. -func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName string) (*ateapipb.Actor, error) { +// requests within the process and retries when needed. +func (r *ActorResumer) ResumeActor(ctx context.Context, actorRef resources.ActorRef) (*ateapipb.Actor, error) { ctx, span := otel.Tracer(routerServiceName).Start(ctx, "ResumeActor", - trace.WithAttributes( - attribute.String("atespace", atespace), - attribute.String("actor", actorName), - )) + trace.WithAttributes(ateattr.ActorRefAttributes(actorRef)...)) defer span.End() - ch := r.flight.DoChan(atespace+"/"+actorName, func() (interface{}, error) { + ch := r.flight.DoChan(actorRef.String(), func() (interface{}, error) { // We detach the context from the first caller using a fixed background timeout. // This guarantees that if Caller 1 disconnects or times out, the underlying // resume operation continues running for Caller 2 and Caller 3 without failing. @@ -70,7 +67,7 @@ func (r *ActorResumer) ResumeActor(ctx context.Context, atespace, actorName stri err := wait.ExponentialBackoffWithContext(bgCtx, backoff, func(ctx context.Context) (bool, error) { var err error resumeResp, err = r.apiClient.ResumeActor(ctx, &ateapipb.ResumeActorRequest{ - Actor: &ateapipb.ObjectRef{Atespace: atespace, Name: actorName}, + Actor: actorRef.ToObjectRef(), }) if err == nil { return true, nil diff --git a/cmd/atenet/internal/router/resumer_test.go b/cmd/atenet/internal/router/resumer_test.go index 149db9b0d..c112bf985 100644 --- a/cmd/atenet/internal/router/resumer_test.go +++ b/cmd/atenet/internal/router/resumer_test.go @@ -20,6 +20,7 @@ import ( "testing" "time" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -43,6 +44,8 @@ func TestActorResumer_ResumeActor(t *testing.T) { const testAtespace = "team-a" const expectedIP = "10.0.0.52" + testActorRef := resources.ActorRef{Atespace: testAtespace, Name: testActorName} + t.Run("SuspendedResumedSuccessfully", func(t *testing.T) { var resumeCalled int mock := &resumerMockClient{ @@ -59,7 +62,7 @@ func TestActorResumer_ResumeActor(t *testing.T) { } resumer := NewActorResumer(mock) - actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName) + actor, err := resumer.ResumeActor(context.Background(), testActorRef) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -90,7 +93,7 @@ func TestActorResumer_ResumeActor(t *testing.T) { } resumer := NewActorResumer(mock) - actor, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName) + actor, err := resumer.ResumeActor(context.Background(), testActorRef) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -110,7 +113,7 @@ func TestActorResumer_ResumeActor(t *testing.T) { } resumer := NewActorResumer(mock) - _, err := resumer.ResumeActor(context.Background(), testAtespace, testActorName) + _, err := resumer.ResumeActor(context.Background(), testActorRef) if got := status.Code(err); got != codes.NotFound { t.Errorf("expected gRPC code NotFound, got %v (err=%v)", got, err) } @@ -147,7 +150,7 @@ func TestActorResumer_ResumeActor(t *testing.T) { for i := 0; i < concurrentRequests; i++ { go func(idx int) { defer wg.Done() - results[idx], errs[idx] = resumer.ResumeActor(context.Background(), testAtespace, testActorName) + results[idx], errs[idx] = resumer.ResumeActor(context.Background(), testActorRef) }(i) } wg.Wait() diff --git a/cmd/ateom-gvisor/main.go b/cmd/ateom-gvisor/main.go index 1f1a907c8..9c5c0289b 100644 --- a/cmd/ateom-gvisor/main.go +++ b/cmd/ateom-gvisor/main.go @@ -36,6 +36,7 @@ import ( "github.com/agent-substrate/substrate/internal/imagecache" "github.com/agent-substrate/substrate/internal/proto/ateompb" "github.com/agent-substrate/substrate/internal/readyz" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/internal/serverboot" "github.com/agent-substrate/substrate/internal/version" "github.com/google/nftables" @@ -189,7 +190,8 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload s.lock.Lock() defer s.lock.Unlock() - s.actorLogger.EmitLifecycleLog("Actor starting", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} + s.actorLogger.EmitLifecycleLog("Actor starting", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) // Contract with atelet: // @@ -237,7 +239,7 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload // Create and start each application container, each with its own log pipe so // every line is tagged with the originating container (ate.dev/container_name). for _, ac := range req.GetSpec().GetContainers() { - pw, err := s.actorLogger.StartJSONLogPipe(req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName(), ac.GetName()) + pw, err := s.actorLogger.StartJSONLogPipe(actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName(), ac.GetName()) if err != nil { return nil, fmt.Errorf("while starting json log pipe for %q: %w", ac.GetName(), err) } @@ -258,7 +260,7 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload return nil, fmt.Errorf("while waiting for container readyz: %w", err) } - s.actorLogger.EmitLifecycleLog("Actor started", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + s.actorLogger.EmitLifecycleLog("Actor started", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) return &ateompb.RunWorkloadResponse{}, nil } @@ -267,7 +269,8 @@ func (s *AteomService) CheckpointWorkload(ctx context.Context, req *ateompb.Chec s.lock.Lock() defer s.lock.Unlock() - s.actorLogger.EmitLifecycleLog("Actor checkpointing", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} + s.actorLogger.EmitLifecycleLog("Actor checkpointing", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) // Contract with atelet: // @@ -313,8 +316,7 @@ func (s *AteomService) CheckpointWorkload(ctx context.Context, req *ateompb.Chec // directories after uploading the snapshot. if err := rcmd.cleanupContainersAfterCheckpoint(ctx, req.GetSpec().GetContainers()); err != nil { slog.WarnContext(ctx, "Failed to clean up runsc containers after checkpoint", - "actorName", req.GetActorName(), - "atespace", req.GetAtespace(), + "actor", actorRef, "actorUID", req.GetActorUid(), "err", err) } @@ -338,7 +340,7 @@ func (s *AteomService) CheckpointWorkload(ctx context.Context, req *ateompb.Chec return nil, fmt.Errorf("while listing checkpoint files: %w", err) } - s.actorLogger.EmitLifecycleLog("Actor checkpointed", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + s.actorLogger.EmitLifecycleLog("Actor checkpointed", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) return &ateompb.CheckpointWorkloadResponse{SnapshotFiles: snapshotFiles}, nil } @@ -390,7 +392,8 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore s.lock.Lock() defer s.lock.Unlock() - s.actorLogger.EmitLifecycleLog("Actor restoring", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} + s.actorLogger.EmitLifecycleLog("Actor restoring", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) // Contract with atelet: // @@ -450,7 +453,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore // Create and restore each application container, each with its own log pipe so // every line is tagged with the originating container (ate.dev/container_name). for _, ac := range req.GetSpec().GetContainers() { - pw, err := s.actorLogger.StartJSONLogPipe(req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName(), ac.GetName()) + pw, err := s.actorLogger.StartJSONLogPipe(actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName(), ac.GetName()) if err != nil { return nil, fmt.Errorf("while starting json log pipe for %q: %w", ac.GetName(), err) } @@ -483,7 +486,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore return nil, fmt.Errorf("while waiting for container readyz: %w", err) } - s.actorLogger.EmitLifecycleLog("Actor restored", req.GetAtespace(), req.GetActorName(), req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) + s.actorLogger.EmitLifecycleLog("Actor restored", actorRef, req.GetActorUid(), req.GetActorTemplateNamespace(), req.GetActorTemplateName()) return &ateompb.RestoreWorkloadResponse{}, nil } diff --git a/cmd/ateom-microvm/checkpoint.go b/cmd/ateom-microvm/checkpoint.go index b5ff7a714..66b7fb247 100644 --- a/cmd/ateom-microvm/checkpoint.go +++ b/cmd/ateom-microvm/checkpoint.go @@ -30,6 +30,7 @@ import ( "github.com/agent-substrate/substrate/internal/ateompath" "github.com/agent-substrate/substrate/internal/imagecache" "github.com/agent-substrate/substrate/internal/proto/ateompb" + "github.com/agent-substrate/substrate/internal/resources" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -59,13 +60,12 @@ func (s *AteomService) CheckpointWorkload(ctx context.Context, req *ateompb.Chec s.lock.Lock() defer s.lock.Unlock() - atespace := req.GetAtespace() - name := req.GetActorName() + actorRef := resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()} actorUID := req.GetActorUid() templateNS := req.GetActorTemplateNamespace() templateName := req.GetActorTemplateName() - s.actorLogger.EmitLifecycleLog("Actor checkpointing", atespace, name, actorUID, templateNS, templateName) + s.actorLogger.EmitLifecycleLog("Actor checkpointing", actorRef, actorUID, templateNS, templateName) // Check what the request asks for BEFORE touching the guest: these are // properties of the request, and pausing first would leave the actor @@ -154,7 +154,7 @@ func (s *AteomService) CheckpointWorkload(ctx context.Context, req *ateompb.Chec slog.WarnContext(ctx, "Failed to clean up actor network after checkpoint", slog.Any("err", err)) } - s.actorLogger.EmitLifecycleLog("Actor checkpointed", atespace, name, actorUID, templateNS, templateName) + s.actorLogger.EmitLifecycleLog("Actor checkpointed", actorRef, actorUID, templateNS, templateName) slog.InfoContext(ctx, "Actor checkpointed", slog.String("id", actorUID), slog.Any("snapshot_files", snapshotFiles), slog.String("scope", scope.String()), slog.Duration("pause", dPause), slog.Duration("snapshot", dSnapshot), diff --git a/cmd/ateom-microvm/restore.go b/cmd/ateom-microvm/restore.go index c939af669..e24967fb2 100644 --- a/cmd/ateom-microvm/restore.go +++ b/cmd/ateom-microvm/restore.go @@ -33,6 +33,7 @@ import ( "github.com/agent-substrate/substrate/internal/imagecache" "github.com/agent-substrate/substrate/internal/proto/ateompb" "github.com/agent-substrate/substrate/internal/readyz" + "github.com/agent-substrate/substrate/internal/resources" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -52,8 +53,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore defer s.lock.Unlock() p := actorBootParams{ - atespace: req.GetAtespace(), - actorName: req.GetActorName(), + actorRef: resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()}, actorUID: req.GetActorUid(), templateNS: req.GetActorTemplateNamespace(), templateName: req.GetActorTemplateName(), @@ -64,7 +64,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore durableDir := ateompath.DurableDirVolumeMountsDir(p.actorUID) tStart := time.Now() - s.actorLogger.EmitLifecycleLog("Actor restoring", p.atespace, p.actorName, p.actorUID, p.templateNS, p.templateName) + s.actorLogger.EmitLifecycleLog("Actor restoring", p.actorRef, p.actorUID, p.templateNS, p.templateName) // Restore the durable-dir volumes before anything can observe them: for Full // that means before the share's virtiofsd starts, for Data before the workload @@ -94,7 +94,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore return nil, status.Errorf(codes.InvalidArgument, "unsupported snapshot scope: %v", scope) } - s.actorLogger.EmitLifecycleLog("Actor restored", p.atespace, p.actorName, p.actorUID, p.templateNS, p.templateName) + s.actorLogger.EmitLifecycleLog("Actor restored", p.actorRef, p.actorUID, p.templateNS, p.templateName) return &ateompb.RestoreWorkloadResponse{}, nil } @@ -111,7 +111,7 @@ func (s *AteomService) RestoreWorkload(ctx context.Context, req *ateompb.Restore // config — comes back from the memory snapshot. Durable-dir volumes are host-backed // instead, and the caller has already restored them from the snapshot's tar. func (s *AteomService) restoreFullScope(ctx context.Context, p actorBootParams, restoreDir string, tStart time.Time) (retErr error) { - atespace, name, actorUID := p.atespace, p.actorName, p.actorUID + actorUID := p.actorUID templateNS, templateName := p.templateNS, p.templateName rr := s.resolveRuntime(p.assetPaths) @@ -271,7 +271,7 @@ func (s *AteomService) restoreFullScope(ctx context.Context, p actorBootParams, } else { ra.logAgent = logAC for _, c := range containers { - s.startActorLogForwarding(logAC, atespace, name, actorUID, templateNS, templateName, overlayWorkloadID(c.GetName()), c.GetName()) + s.startActorLogForwarding(logAC, p.actorRef, actorUID, templateNS, templateName, overlayWorkloadID(c.GetName()), c.GetName()) } } diff --git a/cmd/ateom-microvm/run.go b/cmd/ateom-microvm/run.go index 5047f271f..01629d372 100644 --- a/cmd/ateom-microvm/run.go +++ b/cmd/ateom-microvm/run.go @@ -33,6 +33,7 @@ import ( "github.com/agent-substrate/substrate/internal/imagecache" "github.com/agent-substrate/substrate/internal/proto/ateompb" "github.com/agent-substrate/substrate/internal/readyz" + "github.com/agent-substrate/substrate/internal/resources" specs "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" "google.golang.org/grpc/codes" @@ -198,8 +199,7 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload defer s.lock.Unlock() p := actorBootParams{ - atespace: req.GetAtespace(), - actorName: req.GetActorName(), + actorRef: resources.ActorRef{Atespace: req.GetAtespace(), Name: req.GetActorName()}, actorUID: req.GetActorUid(), templateNS: req.GetActorTemplateNamespace(), templateName: req.GetActorTemplateName(), @@ -207,11 +207,11 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload assetPaths: req.GetRuntimeAssetPaths(), } - s.actorLogger.EmitLifecycleLog("Actor starting", p.atespace, p.actorName, p.actorUID, p.templateNS, p.templateName) + s.actorLogger.EmitLifecycleLog("Actor starting", p.actorRef, p.actorUID, p.templateNS, p.templateName) if err := s.coldBootActor(ctx, p); err != nil { return nil, err } - s.actorLogger.EmitLifecycleLog("Actor started", p.atespace, p.actorName, p.actorUID, p.templateNS, p.templateName) + s.actorLogger.EmitLifecycleLog("Actor started", p.actorRef, p.actorUID, p.templateNS, p.templateName) slog.InfoContext(ctx, "Actor started (overlay rootfs)", slog.String("id", p.actorUID)) return &ateompb.RunWorkloadResponse{}, nil } @@ -220,8 +220,7 @@ func (s *AteomService) RunWorkload(ctx context.Context, req *ateompb.RunWorkload // request, or from a Restore request whose snapshot scope covers only the // durable-dir volumes (the workload itself cold-starts). type actorBootParams struct { - atespace string - actorName string + actorRef resources.ActorRef actorUID string templateNS string templateName string @@ -233,7 +232,7 @@ type actorBootParams struct { // containers, registering the result in s.running. The caller holds s.lock and // owns the lifecycle logging. func (s *AteomService) coldBootActor(ctx context.Context, p actorBootParams) (retErr error) { - atespace, name, actorUID := p.atespace, p.actorName, p.actorUID + actorUID := p.actorUID templateNS, templateName := p.templateNS, p.templateName // All of the actor's containers share the one micro-VM (which is the pod @@ -423,7 +422,7 @@ func (s *AteomService) coldBootActor(ctx context.Context, p actorBootParams) (re // that and tag with the display container name. The goroutines read over ac for the // actor's lifetime and exit (io.EOF) when teardownActor closes ac. for _, c := range ctrs { - s.startActorLogForwarding(ac, atespace, name, actorUID, templateNS, templateName, overlayWorkloadID(c.name), c.name) + s.startActorLogForwarding(ac, p.actorRef, actorUID, templateNS, templateName, overlayWorkloadID(c.name), c.name) } return nil @@ -652,9 +651,9 @@ func startOverlayContainer(ctx context.Context, ac *kata.AgentClient, vsockPath // ending WrapContainerLogs. This keeps the agent connection (which ttrpc allows // concurrent Calls on) alive for forwarding while guaranteeing no goroutine outlives // the connection. -func (s *AteomService) startActorLogForwarding(ac *kata.AgentClient, atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, streamID, containerName string) { - go s.actorLogger.WrapContainerLogs(kata.NewStdioReader(context.Background(), ac, streamID, streamID, false), atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, containerName) - go s.actorLogger.WrapContainerLogs(kata.NewStdioReader(context.Background(), ac, streamID, streamID, true), atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, containerName) +func (s *AteomService) startActorLogForwarding(ac *kata.AgentClient, actorRef resources.ActorRef, actorUID, actorTemplateNamespace, actorTemplateName, streamID, containerName string) { + go s.actorLogger.WrapContainerLogs(kata.NewStdioReader(context.Background(), ac, streamID, streamID, false), actorRef, actorUID, actorTemplateNamespace, actorTemplateName, containerName) + go s.actorLogger.WrapContainerLogs(kata.NewStdioReader(context.Background(), ac, streamID, streamID, true), actorRef, actorUID, actorTemplateNamespace, actorTemplateName, containerName) } // dialAgentRetry polls DialAgent until the kata-agent answers the hybrid-vsock diff --git a/cmd/kubectl-ate/internal/cmd/delete_actor.go b/cmd/kubectl-ate/internal/cmd/delete_actor.go index 578b8928e..2f863d535 100644 --- a/cmd/kubectl-ate/internal/cmd/delete_actor.go +++ b/cmd/kubectl-ate/internal/cmd/delete_actor.go @@ -18,6 +18,7 @@ import ( "fmt" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" ) @@ -36,15 +37,15 @@ var deleteActorCmd = &cobra.Command{ } defer c.Close() - name := args[0] + actorRef := resources.ActorRef{Atespace: deleteAtespaceFlag, Name: args[0]} _, err = c.ControlClient.DeleteActor(ctx, &ateapipb.DeleteActorRequest{ - Actor: &ateapipb.ObjectRef{Atespace: deleteAtespaceFlag, Name: name}, + Actor: actorRef.ToObjectRef(), }) if err != nil { return err } - fmt.Printf("actor %q deleted\n", name) + fmt.Printf("actor %q deleted\n", actorRef.Name) return nil }, } diff --git a/cmd/kubectl-ate/internal/cmd/get_actors.go b/cmd/kubectl-ate/internal/cmd/get_actors.go index 0f90a71cf..d99299f47 100644 --- a/cmd/kubectl-ate/internal/cmd/get_actors.go +++ b/cmd/kubectl-ate/internal/cmd/get_actors.go @@ -19,6 +19,7 @@ import ( "github.com/agent-substrate/substrate/cmd/kubectl-ate/internal/printer" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" ) @@ -55,7 +56,8 @@ var getActorsCmd = &cobra.Command{ actors := make([]*ateapipb.Actor, 0, len(args)) for _, actorName := range args { - resp, err := apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: &ateapipb.ObjectRef{Atespace: getActorsAtespaceFlag, Name: actorName}}) + actorRef := resources.ActorRef{Atespace: getActorsAtespaceFlag, Name: actorName} + resp, err := apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: actorRef.ToObjectRef()}) if err != nil { return fmt.Errorf("failed to get actor %q: %w", actorName, err) } diff --git a/cmd/kubectl-ate/internal/cmd/logs_actors.go b/cmd/kubectl-ate/internal/cmd/logs_actors.go index 5badf1ba1..baadcf775 100644 --- a/cmd/kubectl-ate/internal/cmd/logs_actors.go +++ b/cmd/kubectl-ate/internal/cmd/logs_actors.go @@ -28,6 +28,7 @@ import ( "time" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" "google.golang.org/grpc" @@ -80,7 +81,7 @@ func (s *k8sPodLogsStreamer) StreamLogs(ctx context.Context, namespace, podName type LogsActorRunner struct { apiClient AteAPIClient streamer PodLogsStreamer - atespace string + actorRef resources.ActorRef stdout io.Writer stderr io.Writer follow bool @@ -90,7 +91,7 @@ type LogsActorRunner struct { } // Run executes the logs command. -func (r *LogsActorRunner) Run(ctx context.Context, actorName string) error { +func (r *LogsActorRunner) Run(ctx context.Context) error { if r.pollInterval <= 0 { r.pollInterval = 2 * time.Second } @@ -103,13 +104,13 @@ func (r *LogsActorRunner) Run(ctx context.Context, actorName string) error { defer r.apiClient.Close() if r.follow { - return r.runFollow(ctx, actorName) + return r.runFollow(ctx) } - return r.runOneShot(ctx, actorName) + return r.runOneShot(ctx) } -func (r *LogsActorRunner) runOneShot(ctx context.Context, actorName string) error { - actor, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: &ateapipb.ObjectRef{Atespace: r.atespace, Name: actorName}}) +func (r *LogsActorRunner) runOneShot(ctx context.Context) error { + actor, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: r.actorRef.ToObjectRef()}) if err != nil { return fmt.Errorf("failed to get actor: %w", err) } @@ -118,7 +119,7 @@ func (r *LogsActorRunner) runOneShot(ctx context.Context, actorName string) erro namespace := actor.GetAteomPodNamespace() if podName == "" || namespace == "" || actor.GetStatus() != ateapipb.Actor_STATUS_RUNNING { - return fmt.Errorf("actor %s is not currently running on any worker pod", actorName) + return fmt.Errorf("actor %s is not currently running on any worker pod", r.actorRef) } opts := &corev1.PodLogOptions{ @@ -136,7 +137,7 @@ func (r *LogsActorRunner) runOneShot(ctx context.Context, actorName string) erro scanner.Buffer(buf, 1024*1024) // Support up to 1MB lines for scanner.Scan() { line := scanner.Text() - filterAndDisplayLogLine(line, r.atespace, actorName, r.stdout) + filterAndDisplayLogLine(line, r.actorRef, r.stdout) } if err := scanner.Err(); err != nil { return fmt.Errorf("error reading log stream: %w", err) @@ -144,7 +145,7 @@ func (r *LogsActorRunner) runOneShot(ctx context.Context, actorName string) erro return nil } -func (r *LogsActorRunner) runFollow(ctx context.Context, actorName string) error { +func (r *LogsActorRunner) runFollow(ctx context.Context) error { var lastWorkerPod string var lastSeenTime time.Time @@ -155,10 +156,10 @@ func (r *LogsActorRunner) runFollow(ctx context.Context, actorName string) error default: } - actor, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: &ateapipb.ObjectRef{Atespace: r.atespace, Name: actorName}}) + actor, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: r.actorRef.ToObjectRef()}) if err != nil { if status.Code(err) == codes.NotFound { - return fmt.Errorf("actor %s not found: %w", actorName, err) + return fmt.Errorf("actor %s not found: %w", r.actorRef, err) } select { case <-ctx.Done(): @@ -206,14 +207,14 @@ func (r *LogsActorRunner) runFollow(ctx context.Context, actorName string) error } var wg sync.WaitGroup - r.startMigrationMonitor(streamCtx, streamCancel, &wg, actorName, podName) + r.startMigrationMonitor(streamCtx, streamCancel, &wg, podName) scanner := bufio.NewScanner(stream) buf := make([]byte, 0, 64*1024) scanner.Buffer(buf, 1024*1024) // Support up to 1MB lines for scanner.Scan() { line := scanner.Text() - logTime, _ := filterAndDisplayLogLine(line, r.atespace, actorName, r.stdout) + logTime, _ := filterAndDisplayLogLine(line, r.actorRef, r.stdout) if !logTime.IsZero() { lastSeenTime = logTime } @@ -249,7 +250,6 @@ func (r *LogsActorRunner) startMigrationMonitor( ctx context.Context, cancel context.CancelFunc, wg *sync.WaitGroup, - actorName string, currentPod string, ) { wg.Add(1) @@ -262,7 +262,7 @@ func (r *LogsActorRunner) startMigrationMonitor( case <-ctx.Done(): return case <-ticker.C: - resp, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: &ateapipb.ObjectRef{Atespace: r.atespace, Name: actorName}}) + resp, err := r.apiClient.GetActor(ctx, &ateapipb.GetActorRequest{Actor: r.actorRef.ToObjectRef()}) if err == nil { act := resp if act.GetStatus() != ateapipb.Actor_STATUS_RUNNING || act.GetAteomPodName() != currentPod { @@ -278,7 +278,6 @@ func (r *LogsActorRunner) startMigrationMonitor( func runLogsActor(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - actorName := args[0] apiClient, err := ateclient.NewClient(ctx, kubeconfig, k8sContext, endpoint, traceEnabled) if err != nil { @@ -294,7 +293,7 @@ func runLogsActor(cmd *cobra.Command, args []string) error { runner := &LogsActorRunner{ apiClient: apiClient, streamer: &k8sPodLogsStreamer{clientset: k8sClient}, - atespace: logsAtespaceFlag, + actorRef: resources.ActorRef{Atespace: logsAtespaceFlag, Name: args[0]}, stdout: os.Stdout, stderr: os.Stderr, follow: followLogs, @@ -303,10 +302,10 @@ func runLogsActor(cmd *cobra.Command, args []string) error { tickerInterval: 2 * time.Second, } - return runner.Run(ctx, actorName) + return runner.Run(ctx) } -func filterAndDisplayLogLine(line, targetAtespace, targetActorName string, w io.Writer) (time.Time, bool) { +func filterAndDisplayLogLine(line string, target resources.ActorRef, w io.Writer) (time.Time, bool) { var m map[string]any dec := json.NewDecoder(strings.NewReader(line)) dec.UseNumber() @@ -323,13 +322,13 @@ func filterAndDisplayLogLine(line, targetAtespace, targetActorName string, w io. } } - var atespace, actorName string + var emitter resources.ActorRef for _, labelKey := range []string{"logging.googleapis.com/labels", "labels"} { if labelsAny, ok := m[labelKey]; ok { if labels, ok := labelsAny.(map[string]any); ok { if name, ok := labels["ate.dev/actor_name"].(string); ok && name != "" { - actorName = name - atespace, _ = labels["ate.dev/actor_atespace"].(string) + emitter.Name = name + emitter.Atespace, _ = labels["ate.dev/actor_atespace"].(string) break } } @@ -337,9 +336,10 @@ func filterAndDisplayLogLine(line, targetAtespace, targetActorName string, w io. } // Actor names are only unique within an atespace, and a worker pod can host - // actors from different atespaces over time, so match on both. - matched := (actorName != "" && actorName == targetActorName && - targetAtespace != "" && atespace == targetAtespace) + // actors from different atespaces over time, so match on both. A partial + // target never matches: a line missing either label would otherwise be + // attributed to whichever actor was asked for. + matched := emitter == target && target.Atespace != "" && target.Name != "" if !matched { return logTime, false diff --git a/cmd/kubectl-ate/internal/cmd/logs_actors_test.go b/cmd/kubectl-ate/internal/cmd/logs_actors_test.go index 5ff52fddc..0f6dacb96 100644 --- a/cmd/kubectl-ate/internal/cmd/logs_actors_test.go +++ b/cmd/kubectl-ate/internal/cmd/logs_actors_test.go @@ -24,6 +24,7 @@ import ( "testing" "time" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -33,146 +34,131 @@ import ( func TestFilterAndDisplayLogLine(t *testing.T) { tests := []struct { - name string - line string - targetAtespace string - targetActorName string - wantMatched bool - wantTime string - wantOutput string + name string + line string + target resources.ActorRef + wantMatched bool + wantTime string + wantOutput string }{ { - name: "matching actor, JSON log with RFC3339Nano", - line: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38.602878302Z", - wantOutput: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count"}`, + name: "matching actor, JSON log with RFC3339Nano", + line: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38.602878302Z", + wantOutput: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count"}`, }, { - name: "matching actor, plain text log", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: `{"time":"2026-05-16T01:03:38Z","message":"Hello"}`, + name: "matching actor, plain text log", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: `{"time":"2026-05-16T01:03:38Z","message":"Hello"}`, }, { - name: "matching actor, JSON log with no timestamp fallback", - line: `{"level":"error","msg":"Failed","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "", - wantOutput: `{"level":"error","msg":"Failed"}`, + name: "matching actor, JSON log with no timestamp fallback", + line: `{"level":"error","msg":"Failed","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "", + wantOutput: `{"level":"error","msg":"Failed"}`, }, { - name: "matching actor, fallback to standard labels key", - line: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count","labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38.602878302Z", - wantOutput: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count"}`, + name: "matching actor, fallback to standard labels key", + line: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count","labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38.602878302Z", + wantOutput: `{"time":"2026-05-16T01:03:38.602878302Z","level":"info","msg":"Count"}`, }, { - name: "non-matching actor", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-2"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: false, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: "", + name: "non-matching actor", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-2"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: false, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: "", }, { - name: "same actor name in a different atespace", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-2","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: false, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: "", + name: "same actor name in a different atespace", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-2","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: false, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: "", }, { - name: "matching actor name without atespace label", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: false, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: "", + name: "matching actor name without atespace label", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: false, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: "", }, { - name: "empty target atespace does not match empty atespace label", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "", - targetActorName: "act-1", - wantMatched: false, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: "", + name: "empty target atespace does not match empty atespace label", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "", Name: "act-1"}, + wantMatched: false, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: "", }, { - name: "empty target actor name does not match empty name label", - line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":""}}`, - targetAtespace: "space-1", - targetActorName: "", - wantMatched: false, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: "", + name: "empty target actor name does not match empty name label", + line: `{"time":"2026-05-16T01:03:38Z","message":"Hello world","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":""}}`, + target: resources.ActorRef{Atespace: "space-1", Name: ""}, + wantMatched: false, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: "", }, { - name: "invalid json line", - line: "not a json line", - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: false, - wantTime: "", - wantOutput: "", + name: "invalid json line", + line: "not a json line", + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: false, + wantTime: "", + wantOutput: "", }, { - name: "matching actor, flat JSON log", - line: `{"time":"2026-05-16T01:03:38Z","level":"info","msg":"Hello","traceID":"abc-123","err":"timeout","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: `{"time":"2026-05-16T01:03:38Z","err":"timeout","level":"info","msg":"Hello","traceID":"abc-123"}`, + name: "matching actor, flat JSON log", + line: `{"time":"2026-05-16T01:03:38Z","level":"info","msg":"Hello","traceID":"abc-123","err":"timeout","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: `{"time":"2026-05-16T01:03:38Z","err":"timeout","level":"info","msg":"Hello","traceID":"abc-123"}`, }, { - name: "matching actor, severity and message keys", - line: `{"time":"2026-05-16T01:03:38Z","severity":"error","message":"Disk full","custom_tag":"alert","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: `{"time":"2026-05-16T01:03:38Z","custom_tag":"alert","message":"Disk full","severity":"error"}`, + name: "matching actor, severity and message keys", + line: `{"time":"2026-05-16T01:03:38Z","severity":"error","message":"Disk full","custom_tag":"alert","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: `{"time":"2026-05-16T01:03:38Z","custom_tag":"alert","message":"Disk full","severity":"error"}`, }, { - name: "matching actor, 2-field structured log without time", - line: `{"message":"login failed","code":401,"logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "", - wantOutput: `{"code":401,"message":"login failed"}`, + name: "matching actor, 2-field structured log without time", + line: `{"message":"login failed","code":401,"logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "", + wantOutput: `{"code":401,"message":"login failed"}`, }, { - name: "matching actor, JSON log with custom application labels", - line: `{"time":"2026-05-16T01:03:38Z","level":"info","msg":"Hello","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1","app":"my-app"}}`, - targetAtespace: "space-1", - targetActorName: "act-1", - wantMatched: true, - wantTime: "2026-05-16T01:03:38Z", - wantOutput: `{"time":"2026-05-16T01:03:38Z","level":"info","logging.googleapis.com/labels":{"app":"my-app"},"msg":"Hello"}`, + name: "matching actor, JSON log with custom application labels", + line: `{"time":"2026-05-16T01:03:38Z","level":"info","msg":"Hello","logging.googleapis.com/labels":{"ate.dev/actor_atespace":"space-1","ate.dev/actor_name":"act-1","app":"my-app"}}`, + target: resources.ActorRef{Atespace: "space-1", Name: "act-1"}, + wantMatched: true, + wantTime: "2026-05-16T01:03:38Z", + wantOutput: `{"time":"2026-05-16T01:03:38Z","level":"info","logging.googleapis.com/labels":{"app":"my-app"},"msg":"Hello"}`, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { var buf bytes.Buffer - logTime, matched := filterAndDisplayLogLine(tc.line, tc.targetAtespace, tc.targetActorName, &buf) + logTime, matched := filterAndDisplayLogLine(tc.line, tc.target, &buf) if matched != tc.wantMatched { t.Errorf("got matched = %v, want %v", matched, tc.wantMatched) @@ -262,14 +248,14 @@ func TestLogsActorRunner_Run_OneShotSuccess(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, follow: false, } - err := runner.Run(context.Background(), actorName) + err := runner.Run(context.Background()) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -306,14 +292,14 @@ func TestLogsActorRunner_Run_OneShot_ActorNotRunning(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, follow: false, } - err := runner.Run(context.Background(), actorName) + err := runner.Run(context.Background()) if err == nil { t.Fatal("expected error, got nil") } @@ -391,7 +377,7 @@ func TestLogsActorRunner_Run_Follow_SuspendedToRunning(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, @@ -401,7 +387,7 @@ func TestLogsActorRunner_Run_Follow_SuspendedToRunning(t *testing.T) { tickerInterval: 1 * time.Millisecond, } - err := runner.Run(ctx, actorName) + err := runner.Run(ctx) if err != nil && err != context.Canceled { t.Fatalf("unexpected error: %v", err) } @@ -441,7 +427,7 @@ func TestLogsActorRunner_Run_Follow_NotFoundActor(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, @@ -451,7 +437,7 @@ func TestLogsActorRunner_Run_Follow_NotFoundActor(t *testing.T) { tickerInterval: 1 * time.Millisecond, } - err := runner.Run(context.Background(), actorName) + err := runner.Run(context.Background()) if err == nil { t.Fatal("expected error, got nil") } @@ -548,7 +534,7 @@ func TestLogsActorRunner_Run_Follow_ActorMigration(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, @@ -558,7 +544,7 @@ func TestLogsActorRunner_Run_Follow_ActorMigration(t *testing.T) { tickerInterval: 1 * time.Millisecond, } - err := runner.Run(ctx, actorName) + err := runner.Run(ctx) if err != nil && err != context.Canceled { t.Fatalf("unexpected error: %v", err) } @@ -661,7 +647,7 @@ func TestLogsActorRunner_Run_Follow_ActorSuspendedMidStream(t *testing.T) { var stdout, stderr bytes.Buffer runner := &LogsActorRunner{ apiClient: mockAPI, - atespace: "space-1", + actorRef: resources.ActorRef{Atespace: "space-1", Name: actorName}, streamer: mockStreamer, stdout: &stdout, stderr: &stderr, @@ -671,7 +657,7 @@ func TestLogsActorRunner_Run_Follow_ActorSuspendedMidStream(t *testing.T) { tickerInterval: 1 * time.Millisecond, } - err := runner.Run(ctx, actorName) + err := runner.Run(ctx) if err != nil && err != context.Canceled { t.Fatalf("unexpected error: %v", err) } diff --git a/cmd/kubectl-ate/internal/cmd/pause_actor.go b/cmd/kubectl-ate/internal/cmd/pause_actor.go index 6b22749fd..91a4adfd0 100644 --- a/cmd/kubectl-ate/internal/cmd/pause_actor.go +++ b/cmd/kubectl-ate/internal/cmd/pause_actor.go @@ -19,6 +19,7 @@ import ( "github.com/agent-substrate/substrate/cmd/kubectl-ate/internal/printer" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" ) @@ -37,8 +38,9 @@ var pauseActorCmd = &cobra.Command{ } defer apiClient.Close() + actorRef := resources.ActorRef{Atespace: pauseAtespaceFlag, Name: args[0]} resp, err := apiClient.PauseActor(ctx, &ateapipb.PauseActorRequest{ - Actor: &ateapipb.ObjectRef{Atespace: pauseAtespaceFlag, Name: args[0]}, + Actor: actorRef.ToObjectRef(), }) if err != nil { return fmt.Errorf("failed to pause actor: %w", err) diff --git a/cmd/kubectl-ate/internal/cmd/resume_actor.go b/cmd/kubectl-ate/internal/cmd/resume_actor.go index d744688f8..c25cf6d8f 100644 --- a/cmd/kubectl-ate/internal/cmd/resume_actor.go +++ b/cmd/kubectl-ate/internal/cmd/resume_actor.go @@ -19,6 +19,7 @@ import ( "github.com/agent-substrate/substrate/cmd/kubectl-ate/internal/printer" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" ) @@ -38,8 +39,9 @@ var resumeActorCmd = &cobra.Command{ } defer apiClient.Close() + actorRef := resources.ActorRef{Atespace: resumeAtespaceFlag, Name: args[0]} resp, err := apiClient.ResumeActor(ctx, &ateapipb.ResumeActorRequest{ - Actor: &ateapipb.ObjectRef{Atespace: resumeAtespaceFlag, Name: args[0]}, + Actor: actorRef.ToObjectRef(), Boot: bootFlag, }) if err != nil { diff --git a/cmd/kubectl-ate/internal/cmd/suspend_actor.go b/cmd/kubectl-ate/internal/cmd/suspend_actor.go index 3bca211cd..84315e1cb 100644 --- a/cmd/kubectl-ate/internal/cmd/suspend_actor.go +++ b/cmd/kubectl-ate/internal/cmd/suspend_actor.go @@ -19,6 +19,7 @@ import ( "github.com/agent-substrate/substrate/cmd/kubectl-ate/internal/printer" "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/spf13/cobra" ) @@ -37,8 +38,9 @@ var suspendActorCmd = &cobra.Command{ } defer apiClient.Close() + actorRef := resources.ActorRef{Atespace: suspendAtespaceFlag, Name: args[0]} resp, err := apiClient.SuspendActor(ctx, &ateapipb.SuspendActorRequest{ - Actor: &ateapipb.ObjectRef{Atespace: suspendAtespaceFlag, Name: args[0]}, + Actor: actorRef.ToObjectRef(), }) if err != nil { return fmt.Errorf("failed to suspend actor: %w", err) diff --git a/demos/sandbox/client/main.go b/demos/sandbox/client/main.go index 3df1d14e5..0ddbf9155 100644 --- a/demos/sandbox/client/main.go +++ b/demos/sandbox/client/main.go @@ -72,6 +72,7 @@ func main() { if *atespace == "" { log.Fatal("--atespace is required") } + actorRef := resources.ActorRef{Atespace: *atespace, Name: *actorName} ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -93,8 +94,8 @@ func main() { } defer conn.Close() - log.Printf("Resuming actor %s...", *actorName) - _, err = cli.ResumeActor(ctx, &ateapipb.ResumeActorRequest{Actor: &ateapipb.ObjectRef{Atespace: *atespace, Name: *actorName}}) + log.Printf("Resuming actor %s...", actorRef.Name) + _, err = cli.ResumeActor(ctx, &ateapipb.ResumeActorRequest{Actor: actorRef.ToObjectRef()}) if err != nil { log.Fatalf("Failed to resume actor: %v", err) } @@ -102,9 +103,9 @@ func main() { // Ensure we suspend the actor on exit defer func() { - log.Printf("Suspending actor %s...", *actorName) + log.Printf("Suspending actor %s...", actorRef.Name) suspendCtx := context.Background() - _, err := cli.SuspendActor(suspendCtx, &ateapipb.SuspendActorRequest{Actor: &ateapipb.ObjectRef{Atespace: *atespace, Name: *actorName}}) + _, err := cli.SuspendActor(suspendCtx, &ateapipb.SuspendActorRequest{Actor: actorRef.ToObjectRef()}) if err != nil { log.Printf("Failed to suspend actor: %v", err) } else { @@ -152,7 +153,7 @@ func main() { } // Send command to atenet router - output, err := runCommand(ctx, *atenetAddr, *atespace, *actorName, line) + output, err := runCommand(ctx, *atenetAddr, actorRef, line) if err != nil { fmt.Printf("Error: %v\n", err) continue @@ -171,7 +172,7 @@ func main() { } } -func runCommand(ctx context.Context, atenetAddr, atespace, actorName, command string) (*ProcessResponse, error) { +func runCommand(ctx context.Context, atenetAddr string, actorRef resources.ActorRef, command string) (*ProcessResponse, error) { url := fmt.Sprintf("http://%s/process", atenetAddr) reqBody := ProcessRequest{ @@ -184,7 +185,7 @@ func runCommand(ctx context.Context, atenetAddr, atespace, actorName, command st return nil, fmt.Errorf("failed to create request: %w", err) } req.Header.Set("Content-Type", "application/json") - req.Host = resources.ActorDNSName(atespace, actorName) + req.Host = actorRef.DNSName() resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/internal/actorlog/logger.go b/internal/actorlog/logger.go index f477c09e6..ebbf05a78 100644 --- a/internal/actorlog/logger.go +++ b/internal/actorlog/logger.go @@ -27,6 +27,8 @@ import ( "os" "sync" "time" + + "github.com/agent-substrate/substrate/internal/resources" ) // SyncedWriter wraps an io.Writer and synchronizes writes across goroutines. @@ -66,13 +68,13 @@ func NewActorLogger(w io.Writer, isOnGCE bool) *ActorLogger { } // EmitLifecycleLog logs a synthetic actor lifecycle event. -func (al *ActorLogger) EmitLifecycleLog(msg, atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName string) { +func (al *ActorLogger) EmitLifecycleLog(msg string, actorRef resources.ActorRef, actorUID, actorTemplateNamespace, actorTemplateName string) { envelope := map[string]any{ "time": time.Now().Format(time.RFC3339Nano), "message": msg, al.labelsKey: map[string]string{ - "ate.dev/actor_atespace": atespace, - "ate.dev/actor_name": actorName, + "ate.dev/actor_atespace": actorRef.Atespace, + "ate.dev/actor_name": actorRef.Name, "ate.dev/actor_uid": actorUID, "ate.dev/actor_template_namespace": actorTemplateNamespace, "ate.dev/actor_template_name": actorTemplateName, @@ -88,13 +90,13 @@ func (al *ActorLogger) EmitLifecycleLog(msg, atespace, actorName, actorUID, acto // through the logger. containerName tags every line with the originating container; // callers that multiplex multiple containers should give each its own pipe so the // tag is meaningful. -func (al *ActorLogger) StartJSONLogPipe(atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, containerName string) (io.WriteCloser, error) { +func (al *ActorLogger) StartJSONLogPipe(actorRef resources.ActorRef, actorUID, actorTemplateNamespace, actorTemplateName, containerName string) (io.WriteCloser, error) { pr, pw, err := os.Pipe() if err != nil { return nil, err } go func() { - al.WrapContainerLogs(pr, atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, containerName) + al.WrapContainerLogs(pr, actorRef, actorUID, actorTemplateNamespace, actorTemplateName, containerName) pr.Close() }() return pw, nil @@ -103,7 +105,7 @@ func (al *ActorLogger) StartJSONLogPipe(atespace, actorName, actorUID, actorTemp // WrapContainerLogs reads log lines from r, parses them, and logs them in a unified // structured format. containerName is added as the ate.dev/container_name label so // multi-container actors can be demultiplexed. -func (al *ActorLogger) WrapContainerLogs(r io.Reader, atespace, actorName, actorUID, actorTemplateNamespace, actorTemplateName, containerName string) { +func (al *ActorLogger) WrapContainerLogs(r io.Reader, actorRef resources.ActorRef, actorUID, actorTemplateNamespace, actorTemplateName, containerName string) { rdr := bufio.NewReader(r) for { lineBytes, err := rdr.ReadBytes('\n') @@ -130,8 +132,8 @@ func (al *ActorLogger) WrapContainerLogs(r io.Reader, atespace, actorName, actor if unmarshalErr != nil { labels := map[string]string{ - "ate.dev/actor_atespace": atespace, - "ate.dev/actor_name": actorName, + "ate.dev/actor_atespace": actorRef.Atespace, + "ate.dev/actor_name": actorRef.Name, "ate.dev/actor_uid": actorUID, "ate.dev/actor_template_namespace": actorTemplateNamespace, "ate.dev/actor_template_name": actorTemplateName, @@ -151,8 +153,8 @@ func (al *ActorLogger) WrapContainerLogs(r io.Reader, atespace, actorName, actor labels = make(map[string]any) m[al.labelsKey] = labels } - labels["ate.dev/actor_atespace"] = atespace - labels["ate.dev/actor_name"] = actorName + labels["ate.dev/actor_atespace"] = actorRef.Atespace + labels["ate.dev/actor_name"] = actorRef.Name labels["ate.dev/actor_template_namespace"] = actorTemplateNamespace labels["ate.dev/actor_template_name"] = actorTemplateName labels["ate.dev/container_name"] = containerName diff --git a/internal/actorlog/logger_test.go b/internal/actorlog/logger_test.go index 17601636b..653d4d839 100644 --- a/internal/actorlog/logger_test.go +++ b/internal/actorlog/logger_test.go @@ -20,6 +20,8 @@ import ( "strings" "sync" "testing" + + "github.com/agent-substrate/substrate/internal/resources" ) func TestWrapContainerLogs(t *testing.T) { @@ -28,7 +30,7 @@ func TestWrapContainerLogs(t *testing.T) { var buf bytes.Buffer al := NewActorLogger(&buf, false) - al.WrapContainerLogs(rdr, "default", "act-1", "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") + al.WrapContainerLogs(rdr, resources.ActorRef{Atespace: "default", Name: "act-1"}, "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") var m map[string]any if err := json.Unmarshal(buf.Bytes(), &m); err != nil { @@ -81,7 +83,7 @@ func TestWrapContainerLogs_JSONInput(t *testing.T) { var buf bytes.Buffer al := NewActorLogger(&buf, false) - al.WrapContainerLogs(rdr, "default", "act-1", "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") + al.WrapContainerLogs(rdr, resources.ActorRef{Atespace: "default", Name: "act-1"}, "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") dec := json.NewDecoder(&buf) dec.UseNumber() @@ -176,7 +178,7 @@ func TestWrapContainerLogs_MergeLabels(t *testing.T) { var buf bytes.Buffer al := NewActorLogger(&buf, false) // labelsKey will be "labels" - al.WrapContainerLogs(rdr, "default", "act-1", "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") + al.WrapContainerLogs(rdr, resources.ActorRef{Atespace: "default", Name: "act-1"}, "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") var m map[string]any if err := json.Unmarshal(buf.Bytes(), &m); err != nil { @@ -212,7 +214,7 @@ func TestWrapContainerLogs_LabelCollision(t *testing.T) { var buf bytes.Buffer al := NewActorLogger(&buf, false) - al.WrapContainerLogs(rdr, "default", "act-1", "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") + al.WrapContainerLogs(rdr, resources.ActorRef{Atespace: "default", Name: "act-1"}, "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") var m map[string]any if err := json.Unmarshal(buf.Bytes(), &m); err != nil { @@ -242,7 +244,7 @@ func TestWrapContainerLogs_TrailingGarbage(t *testing.T) { var buf bytes.Buffer al := NewActorLogger(&buf, false) - al.WrapContainerLogs(rdr, "default", "act-1", "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") + al.WrapContainerLogs(rdr, resources.ActorRef{Atespace: "default", Name: "act-1"}, "uid-1", "tmpl-ns", "tmpl-1", "ctr-1") var m map[string]any if err := json.Unmarshal(buf.Bytes(), &m); err != nil { diff --git a/internal/ateattr/ateattr.go b/internal/ateattr/ateattr.go index a2cd067ba..670342008 100644 --- a/internal/ateattr/ateattr.go +++ b/internal/ateattr/ateattr.go @@ -21,6 +21,7 @@ package ateattr import ( "go.opentelemetry.io/otel/attribute" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -64,10 +65,10 @@ const ( // ActorRefAttributes returns the subset knowable before the Actor record // resolves: only the (atespace, name) the request addresses. The uid and version // are server-assigned and unknown until the record loads, so they are omitted. -func ActorRefAttributes(atespace, name string) []attribute.KeyValue { +func ActorRefAttributes(actorRef resources.ActorRef) []attribute.KeyValue { return []attribute.KeyValue{ - AtespaceKey.String(atespace), - ActorNameKey.String(name), + AtespaceKey.String(actorRef.Atespace), + ActorNameKey.String(actorRef.Name), } } diff --git a/internal/ateattr/ateattr_test.go b/internal/ateattr/ateattr_test.go index 794d3881c..fc3c61e79 100644 --- a/internal/ateattr/ateattr_test.go +++ b/internal/ateattr/ateattr_test.go @@ -19,6 +19,7 @@ import ( "go.opentelemetry.io/otel/attribute" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" ) @@ -104,24 +105,21 @@ func TestActorAttributes(t *testing.T) { func TestActorRefAttributes(t *testing.T) { tests := []struct { - name string - atespace string - actorName string - want map[attribute.Key]any + name string + actorRef resources.ActorRef + want map[attribute.Key]any }{ { - name: "atespace and actor name only", - atespace: "team-a", - actorName: "support-agent-42", + name: "atespace and actor name only", + actorRef: resources.ActorRef{Atespace: "team-a", Name: "support-agent-42"}, want: map[attribute.Key]any{ AtespaceKey: "team-a", ActorNameKey: "support-agent-42", }, }, { - name: "empty values still produce both keys", - atespace: "", - actorName: "", + name: "zero ref still produces both keys", + actorRef: resources.ActorRef{}, want: map[attribute.Key]any{ AtespaceKey: "", ActorNameKey: "", @@ -131,7 +129,7 @@ func TestActorRefAttributes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assertAttrs(t, toMap(ActorRefAttributes(tt.atespace, tt.actorName)), tt.want) + assertAttrs(t, toMap(ActorRefAttributes(tt.actorRef)), tt.want) }) } } diff --git a/internal/e2e/router_client.go b/internal/e2e/router_client.go index 3c15405fc..058e58852 100644 --- a/internal/e2e/router_client.go +++ b/internal/e2e/router_client.go @@ -70,15 +70,14 @@ func (c *RouterClient) Close() { c.stop() } -// Get issues GET path to (atespace, actorName) through the router, setting the -// actor's mesh Host so the router routes (and resumes) it. The caller must close -// the body. -func (c *RouterClient) Get(ctx context.Context, atespace, actorName, path string) (*http.Response, error) { +// Get issues GET path to actor through the router, setting the actor's mesh Host +// so the router routes (and resumes) it. The caller must close the body. +func (c *RouterClient) Get(ctx context.Context, actorRef resources.ActorRef, path string) (*http.Response, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.baseURL+path, nil) if err != nil { return nil, err } // The router routes on the Host/:authority, not a header. - req.Host = resources.ActorDNSName(atespace, actorName) + req.Host = actorRef.DNSName() return c.http.Do(req) } diff --git a/internal/e2e/suites/demo/demo_test.go b/internal/e2e/suites/demo/demo_test.go index c1c404a1b..e50b6fde4 100644 --- a/internal/e2e/suites/demo/demo_test.go +++ b/internal/e2e/suites/demo/demo_test.go @@ -223,7 +223,7 @@ func runActorLifecycleTestCase(t *testing.T, prefix string, createTemplate func( } waitForActorStatus(ctx, t, clients, actorID, ateapipb.Actor_STATUS_RUNNING) - resp, err := callActor(t, demoAtespace, actorID) + resp, err := callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorID}) if err != nil { t.Fatalf("failed to call actor: %v", err) } @@ -249,7 +249,7 @@ func runActorLifecycleTestCase(t *testing.T, prefix string, createTemplate func( } waitForActorStatus(ctx, t, clients, actorID, ateapipb.Actor_STATUS_RUNNING) - resp, err = callActor(t, demoAtespace, actorID) + resp, err = callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorID}) if err != nil { t.Fatalf("failed to call actor again: %v", err) } @@ -275,7 +275,7 @@ func runActorLifecycleTestCase(t *testing.T, prefix string, createTemplate func( } waitForActorStatus(ctx, t, clients, actorID, ateapipb.Actor_STATUS_RUNNING) - resp, err = callActor(t, demoAtespace, actorID) + resp, err = callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorID}) if err != nil { t.Fatalf("failed to call actor again: %v", err) } @@ -371,7 +371,7 @@ func pauseActor(ctx context.Context, t *testing.T, clients *e2e.Clients, nsObj * } waitForActorStatus(ctx, t, clients, actorName, ateapipb.Actor_STATUS_RUNNING) - resp, err := callActor(t, demoAtespace, actorName) + resp, err := callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorName}) if err != nil { t.Fatalf("failed to call actor: %v", err) } @@ -396,7 +396,7 @@ func pauseActor(ctx context.Context, t *testing.T, clients *e2e.Clients, nsObj * } waitForActorStatus(ctx, t, clients, actorName, ateapipb.Actor_STATUS_RUNNING) - resp, err = callActor(t, demoAtespace, actorName) + resp, err = callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorName}) if err != nil { t.Fatalf("failed to call actor again: %v", err) } @@ -451,7 +451,7 @@ func suspendActor(ctx context.Context, t *testing.T, clients *e2e.Clients, nsObj } waitForActorStatus(ctx, t, clients, actorName, ateapipb.Actor_STATUS_RUNNING) - resp, err := callActor(t, demoAtespace, actorName) + resp, err := callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorName}) if err != nil { t.Fatalf("failed to call actor: %v", err) } @@ -475,7 +475,7 @@ func suspendActor(ctx context.Context, t *testing.T, clients *e2e.Clients, nsObj } waitForActorStatus(ctx, t, clients, actorName, ateapipb.Actor_STATUS_RUNNING) - resp, err = callActor(t, demoAtespace, actorName) + resp, err = callActor(t, resources.ActorRef{Atespace: demoAtespace, Name: actorName}) if err != nil { t.Fatalf("failed to call actor again: %v", err) } @@ -697,13 +697,13 @@ func waitForActorStatus(ctx context.Context, t *testing.T, clients *e2e.Clients, t.Fatalf("timed out waiting for actor %q to reach status %v", actorName, expectedStatus) } -func callActor(t *testing.T, atespace, actorName string) (string, error) { +func callActor(t *testing.T, actorRef resources.ActorRef) (string, error) { t.Helper() deadline := time.Now().Add(30 * time.Second) var lastErr error for time.Now().Before(deadline) { - resp, err := callActorOnce(t, atespace, actorName) + resp, err := callActorOnce(t, actorRef) if err == nil { return resp, nil } @@ -714,7 +714,7 @@ func callActor(t *testing.T, atespace, actorName string) (string, error) { return "", fmt.Errorf("timed out waiting for actor response: %w", lastErr) } -func callActorOnce(t *testing.T, atespace, actorName string) (string, error) { +func callActorOnce(t *testing.T, actorRef resources.ActorRef) (string, error) { t.Helper() clients := e2e.GetClients() @@ -785,7 +785,7 @@ func callActorOnce(t *testing.T, atespace, actorName string) (string, error) { if err != nil { return "", fmt.Errorf("failed to create request: %w", err) } - reqHttp.Host = resources.ActorDNSName(atespace, actorName) + reqHttp.Host = actorRef.DNSName() httpClient := &http.Client{Timeout: 15 * time.Second} resp, err := httpClient.Do(reqHttp) diff --git a/internal/e2e/suites/identity/identity_test.go b/internal/e2e/suites/identity/identity_test.go index f32ced832..abfcf7e9f 100644 --- a/internal/e2e/suites/identity/identity_test.go +++ b/internal/e2e/suites/identity/identity_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/agent-substrate/substrate/internal/e2e" + "github.com/agent-substrate/substrate/internal/resources" "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -182,7 +183,7 @@ func createAndResumeActor(t *testing.T, ctx context.Context, clients *e2e.Client func whoami(t *testing.T, ctx context.Context, rc *e2e.RouterClient, id string) whoamiResponse { t.Helper() - resp, err := rc.Get(ctx, probeNamespace, id, "/whoami") + resp, err := rc.Get(ctx, resources.ActorRef{Atespace: probeNamespace, Name: id}, "/whoami") if err != nil { t.Fatalf("GET /whoami for %q: %v", id, err) } diff --git a/internal/resources/actor.go b/internal/resources/actor.go index fa693f2ed..cf9479835 100644 --- a/internal/resources/actor.go +++ b/internal/resources/actor.go @@ -14,48 +14,14 @@ package resources -import ( - "fmt" - "strings" -) - const ( // ResourceNameRegexPattern is the regular expression pattern for a valid // Substrate resource name. ResourceNameRegexPattern = `[a-z0-9]([-a-z0-9]*[a-z0-9])?` // ActorDNSSuffix is suffix to the DNS name for direct access to Actor - // "..actors.resources.substrate.ate.dev." + // "..actors.resources.substrate.ate.dev" ActorDNSSuffix = "actors.resources.substrate.ate.dev" // GoldenActorAtespace is the reserved system atespace that per-template golden // actors live in. GoldenActorAtespace = "ate-golden" ) - -// ActorDNSName returns the mesh DNS name an actor is reachable at: -// "..actors.resources.substrate.ate.dev". The atespace is -// part of the name because an actor name is only unique within its atespace. -func ActorDNSName(atespace, actorName string) string { - return actorName + "." + atespace + "." + ActorDNSSuffix -} - -// ParseActorDNSName parses a mesh DNS name of the form -// "..actors.resources.substrate.ate.dev" (a trailing dot -// is tolerated) into its atespace and actor name, validating both. It does not -// accept a host:port; callers must strip the port first. -func ParseActorDNSName(name string) (atespace, actorName string, err error) { - rest, found := strings.CutSuffix(strings.TrimSuffix(name, "."), "."+ActorDNSSuffix) - if !found { - return "", "", fmt.Errorf("invalid actor DNS name: must end with %s, got %q", ActorDNSSuffix, name) - } - actorName, atespace, found = strings.Cut(rest, ".") - if !found { - return "", "", fmt.Errorf("invalid actor DNS name: expected ..%s, got %q", ActorDNSSuffix, name) - } - if !IsValidResourceName(actorName) { - return "", "", fmt.Errorf("invalid actor DNS name %q: %q is not a valid actor name", name, actorName) - } - if !IsValidResourceName(atespace) { - return "", "", fmt.Errorf("invalid actor DNS name %q: %q is not a valid atespace", name, atespace) - } - return atespace, actorName, nil -} diff --git a/internal/resources/actor_test.go b/internal/resources/actor_test.go deleted file mode 100644 index a7e780edb..000000000 --- a/internal/resources/actor_test.go +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2026 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package resources - -import ( - "testing" -) - -func TestActorDNSName(t *testing.T) { - got := ActorDNSName("team-a", "act-1") - want := "act-1.team-a." + ActorDNSSuffix - if got != want { - t.Errorf("ActorDNSName() = %q, want %q", got, want) - } - - // Round-trips through ParseActorDNSName. - atespace, actorName, err := ParseActorDNSName(got) - if err != nil || atespace != "team-a" || actorName != "act-1" { - t.Errorf("round-trip = (%q, %q, %v), want (team-a, act-1, )", atespace, actorName, err) - } -} - -func TestParseActorDNSName(t *testing.T) { - tests := []struct { - name string - input string - wantAtespace string - wantActorName string - wantErr bool - }{ - {"valid", "act-1.team-a." + ActorDNSSuffix, "team-a", "act-1", false}, - {"valid trailing dot", "act-1.team-a." + ActorDNSSuffix + ".", "team-a", "act-1", false}, - {"wrong suffix", "act-1.team-a.example.com", "", "", true}, - {"missing atespace", "act-1." + ActorDNSSuffix, "", "", true}, - {"invalid actor name", "ACT-1.team-a." + ActorDNSSuffix, "", "", true}, - {"invalid atespace", "act-1.TEAM." + ActorDNSSuffix, "", "", true}, - {"host:port not accepted", "act-1.team-a." + ActorDNSSuffix + ":8080", "", "", true}, - {"empty", "", "", "", true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - atespace, actorName, err := ParseActorDNSName(tt.input) - if (err != nil) != tt.wantErr { - t.Fatalf("ParseActorDNSName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) - } - if err != nil { - return - } - if atespace != tt.wantAtespace || actorName != tt.wantActorName { - t.Errorf("ParseActorDNSName(%q) = (%q, %q), want (%q, %q)", tt.input, atespace, actorName, tt.wantAtespace, tt.wantActorName) - } - }) - } -} diff --git a/internal/resources/actorref.go b/internal/resources/actorref.go new file mode 100644 index 000000000..7d287ad8a --- /dev/null +++ b/internal/resources/actorref.go @@ -0,0 +1,91 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package resources + +import ( + "fmt" + "log/slog" + "strings" + + "github.com/agent-substrate/substrate/pkg/proto/ateapipb" +) + +// ActorRef identifies an actor by the (atespace, name). +// +// ActorRef is the in-process form of the identity that ateapipb.ObjectRef +// carries on the wire. +type ActorRef struct { + // Atespace is the isolation boundary the actor was created into. Required. + Atespace string + // Name is the actor's name, unique within Atespace. Required. + Name string +} + +func (r ActorRef) String() string { + return r.Atespace + "/" + r.Name +} + +// LogValue implements slog.LogValuer so that slog.Any("actor", ref) records the +// two components as a group ("actor.atespace", "actor.name") rather than +// flattening them into one opaque string. +func (r ActorRef) LogValue() slog.Value { + return slog.GroupValue( + slog.String("atespace", r.Atespace), + slog.String("name", r.Name), + ) +} + +// DNSName returns the mesh DNS name the actor is reachable at. +// This is: "..actors.resources.substrate.ate.dev". +func (r ActorRef) DNSName() string { + return r.Name + "." + r.Atespace + "." + ActorDNSSuffix +} + +// ToObjectRef converts the reference to its wire form. +func (r ActorRef) ToObjectRef() *ateapipb.ObjectRef { + return &ateapipb.ObjectRef{Atespace: r.Atespace, Name: r.Name} +} + +// ActorRefFromObjectRef converts a wire reference to an ActorRef. +func ActorRefFromObjectRef(ref *ateapipb.ObjectRef) ActorRef { + return ActorRef{Atespace: ref.GetAtespace(), Name: ref.GetName()} +} + +// ActorRefFromActor returns the reference addressing the given actor. +func ActorRefFromActor(a *ateapipb.Actor) ActorRef { + return ActorRef{ + Atespace: a.GetMetadata().GetAtespace(), + Name: a.GetMetadata().GetName(), + } +} + +// ParseActorDNSName parses a DNS name for a given actor. +func ParseActorDNSName(name string) (ActorRef, error) { + rest, found := strings.CutSuffix(strings.TrimSuffix(name, "."), "."+ActorDNSSuffix) + if !found { + return ActorRef{}, fmt.Errorf("invalid actor DNS name: must end with %s, got %q", ActorDNSSuffix, name) + } + actorName, atespace, found := strings.Cut(rest, ".") + if !found { + return ActorRef{}, fmt.Errorf("invalid actor DNS name: expected ..%s, got %q", ActorDNSSuffix, name) + } + if !IsValidResourceName(actorName) { + return ActorRef{}, fmt.Errorf("invalid actor DNS name %q: %q is not a valid actor name", name, actorName) + } + if !IsValidResourceName(atespace) { + return ActorRef{}, fmt.Errorf("invalid actor DNS name %q: %q is not a valid atespace", name, atespace) + } + return ActorRef{Atespace: atespace, Name: actorName}, nil +} diff --git a/internal/resources/actorref_test.go b/internal/resources/actorref_test.go new file mode 100644 index 000000000..63b74069f --- /dev/null +++ b/internal/resources/actorref_test.go @@ -0,0 +1,113 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package resources + +import ( + "testing" + + "github.com/agent-substrate/substrate/pkg/proto/ateapipb" +) + +func TestActorRefString(t *testing.T) { + got := ActorRef{Atespace: "team-a", Name: "act-1"}.String() + if want := "team-a/act-1"; got != want { + t.Errorf("String() = %q, want %q", got, want) + } +} + +func TestActorRefDNSName(t *testing.T) { + actorRef := ActorRef{Atespace: "team-a", Name: "act-1"} + + got := actorRef.DNSName() + want := "act-1.team-a.actors.resources.substrate.ate.dev" + if got != want { + t.Errorf("DNSName() = %q, want %q", got, want) + } + + parsed, err := ParseActorDNSName(got) + if err != nil { + t.Fatalf("ParseActorDNSName(%q) error = %v", got, err) + } + if parsed != actorRef { + t.Errorf("round-trip = %+v, want %+v", parsed, actorRef) + } +} + +func TestParseActorDNSName(t *testing.T) { + tests := []struct { + name string + input string + want ActorRef + wantErr bool + }{ + {"valid", "act-1.team-a.actors.resources.substrate.ate.dev", ActorRef{Atespace: "team-a", Name: "act-1"}, false}, + {"valid trailing dot", "act-1.team-a.actors.resources.substrate.ate.dev.", ActorRef{Atespace: "team-a", Name: "act-1"}, false}, + {"wrong suffix", "act-1.team-a.example.com", ActorRef{}, true}, + {"missing atespace", "act-1.actors.resources.substrate.ate.dev", ActorRef{}, true}, + {"invalid actor name", "ACT-1.team-a.actors.resources.substrate.ate.dev", ActorRef{}, true}, + {"invalid atespace", "act-1.TEAM.actors.resources.substrate.ate.dev", ActorRef{}, true}, + {"host:port not accepted", "act-1.team-a.actors.resources.substrate.ate.dev:8080", ActorRef{}, true}, + {"empty", "", ActorRef{}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseActorDNSName(tt.input) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseActorDNSName(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + if got != tt.want { + t.Errorf("ParseActorDNSName(%q) = %+v, want %+v", tt.input, got, tt.want) + } + }) + } +} + +func TestActorRefObjectRefRoundTrip(t *testing.T) { + actorRef := ActorRef{Atespace: "team-a", Name: "act-1"} + + obj := actorRef.ToObjectRef() + if obj.GetAtespace() != "team-a" || obj.GetName() != "act-1" { + t.Errorf("ToObjectRef() = (%q, %q), want (team-a, act-1)", obj.GetAtespace(), obj.GetName()) + } + if got := ActorRefFromObjectRef(obj); got != actorRef { + t.Errorf("round-trip = %+v, want %+v", got, actorRef) + } +} + +func TestActorRefFromActor(t *testing.T) { + tests := []struct { + name string + actor *ateapipb.Actor + want ActorRef + }{ + { + name: "populated", + actor: &ateapipb.Actor{Metadata: &ateapipb.ResourceMetadata{ + Atespace: "team-a", + Name: "act-1", + }}, + want: ActorRef{Atespace: "team-a", Name: "act-1"}, + }, + {"nil actor", nil, ActorRef{}}, + {"nil metadata", &ateapipb.Actor{}, ActorRef{}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ActorRefFromActor(tt.actor); got != tt.want { + t.Errorf("ActorRefFromActor() = %+v, want %+v", got, tt.want) + } + }) + } +}