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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
198 changes: 100 additions & 98 deletions benchmarking/locust/common/ateapi_pb2.py

Large diffs are not rendered by default.

11 changes: 5 additions & 6 deletions cmd/ateapi/internal/actoridentity/actoridentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,15 +363,14 @@ func (s *Server) authorizeActor(ctx context.Context, caller *ateletCaller, actor
// An actor placed on a worker always carries its placement fields. Missing
// placement is a control-plane bug rather than a client error, so it is not
// folded into deny().
podNamespace, podName, pool := actor.GetAteomPodNamespace(), actor.GetAteomPodName(), actor.GetWorkerPoolName()
if podNamespace == "" || podName == "" || pool == "" {
slog.ErrorContext(ctx, "MintCert: running actor has incomplete placement",
slog.Any("actor", actorRef), slog.String("podNamespace", podNamespace),
slog.String("podName", podName), slog.String("workerPool", pool))
assignment := actor.GetWorkerAssignment()
if assignment == nil {
slog.ErrorContext(ctx, "MintCert: running actor has no worker assignment", slog.Any("actor", actorRef))
return nil, status.Errorf(codes.FailedPrecondition, "actor has no worker assigned")
}
podNamespace, podName := assignment.GetWorkerNamespace(), assignment.GetWorkerPod()

worker, err := s.store.GetWorker(ctx, podNamespace, pool, podName)
worker, err := s.store.GetWorker(ctx, podNamespace, assignment.GetWorkerPool(), podName)
if err != nil {
if errors.Is(err, store.ErrNotFound) {
return nil, deny("worker hosting the actor not found", slog.String("workerPod", podNamespace+"/"+podName))
Expand Down
11 changes: 7 additions & 4 deletions cmd/ateapi/internal/actoridentity/actoridentity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ type actorFixture struct {
// unassigned seeds the worker with no assignment at all, as pause, suspend
// and crash leave it once they have released it.
unassigned bool
// noPlacement seeds the actor with none of its worker fields set.
// noPlacement seeds the actor with no worker assignment.
noPlacement bool
// noWorker skips seeding the worker record entirely.
noWorker bool
Expand All @@ -194,9 +194,12 @@ func seedActor(t *testing.T, ctx context.Context, st store.Interface, f actorFix
ActorTemplateName: "counter",
}
if !f.noPlacement {
actor.AteomPodNamespace = testPodNS
actor.AteomPodName = testWorkerPod
actor.WorkerPoolName = testPool
actor.WorkerAssignment = &ateapipb.WorkerAssignment{
WorkerNamespace: testPodNS,
WorkerPool: testPool,
WorkerPod: testWorkerPod,
WorkerPodUid: "worker-uid",
}
}
if _, err := st.CreateActor(ctx, actor); err != nil {
t.Fatalf("seed actor: %v", err)
Expand Down
17 changes: 5 additions & 12 deletions cmd/ateapi/internal/controlapi/crash.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,7 @@ func crashActor(ctx context.Context, st store.Interface, actorRef resources.Acto

// InProgressSnapshot is kept for debugging; failed workflow
// steps must never promote it to an ActorSnapshot.
actor.AteomPodNamespace = ""
actor.AteomPodName = ""
actor.AteomPodIp = ""
actor.AteomPodUid = ""
actor.WorkerPoolName = ""
actor.WorkerAssignment = nil

if _, err := st.UpdateActor(ctx, actor, actor.GetMetadata().GetVersion()); err != nil {
errCollected = append(errCollected, fmt.Errorf("while marking actor crashed: %w", err))
Expand All @@ -78,17 +74,14 @@ func crashActor(ctx context.Context, st store.Interface, actorRef resources.Acto
// releaseWorker clears the worker's assignment if it still points at the given
// actor. A missing worker or an already-cleared assignment is not an error.
func releaseWorker(ctx context.Context, st store.Interface, actor *ateapipb.Actor) error {
podNamespace := actor.GetAteomPodNamespace()
podName := actor.GetAteomPodName()
podUid := actor.GetAteomPodUid()
poolName := actor.GetWorkerPoolName()

if podNamespace == "" || podName == "" || poolName == "" {
assignment := actor.GetWorkerAssignment()
if assignment == nil {
slog.WarnContext(ctx, "Actor's worker assignment is already cleared")
return nil
}
podUid := assignment.GetWorkerPodUid()

worker, err := st.GetWorker(ctx, podNamespace, poolName, podName)
worker, err := st.GetWorker(ctx, assignment.GetWorkerNamespace(), assignment.GetWorkerPool(), assignment.GetWorkerPod())
if errors.Is(err, store.ErrNotFound) {
// No need to release if the worker is not found.
slog.WarnContext(ctx, "Worker already gone while crashing actor, skipping release", slog.String("worker", podUid))
Expand Down
28 changes: 11 additions & 17 deletions cmd/ateapi/internal/controlapi/crash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ import (
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: actorRef.Name, Atespace: actorRef.Atespace},
Status: ateapipb.Actor_STATUS_RUNNING,
AteomPodNamespace: "ns",
AteomPodName: "pod",
AteomPodIp: "1.2.3.4",
AteomPodUid: "uid",
WorkerPoolName: "pool",
Metadata: &ateapipb.ResourceMetadata{Name: actorRef.Name, Atespace: actorRef.Atespace},
Status: ateapipb.Actor_STATUS_RUNNING,
WorkerAssignment: &ateapipb.WorkerAssignment{
WorkerNamespace: "ns",
WorkerPool: "pool",
WorkerPod: "pod",
WorkerPodUid: "uid",
WorkerPodIp: "1.2.3.4",
},
InProgressSnapshot: "gs://snapshots/actor-1/reserved",
}); err != nil {
t.Fatalf("seed actor: %v", err)
Expand Down Expand Up @@ -94,16 +96,8 @@ func assertCrashed(t *testing.T, ctx context.Context, st store.Interface, actorR
if got.GetInProgressSnapshot() == "" {
t.Error(`InProgressSnapshot = "", want preserved`)
}
for field, val := range map[string]string{
"AteomPodNamespace": got.GetAteomPodNamespace(),
"AteomPodName": got.GetAteomPodName(),
"AteomPodIp": got.GetAteomPodIp(),
"AteomPodUid": got.GetAteomPodUid(),
"WorkerPoolName": got.GetWorkerPoolName(),
} {
if val != "" {
t.Errorf("%s = %q, want cleared", field, val)
}
if got.GetWorkerAssignment() != nil {
t.Errorf("WorkerAssignment = %v, want cleared", got.GetWorkerAssignment())
}
}

Expand Down
60 changes: 29 additions & 31 deletions cmd/ateapi/internal/controlapi/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -761,9 +761,6 @@ func TestCreateActor_Success(t *testing.T) {
ActorTemplateName: "tmpl1",
WorkerSelector: &ateapipb.Selector{MatchLabels: map[string]string{"tier": "free"}},
Status: ateapipb.Actor_STATUS_RUNNING,
AteomPodNamespace: "caller-ns",
AteomPodName: "caller-pod",
WorkerPoolName: "caller-pool",
}})
if err != nil {
t.Fatalf("CreateActor failed: %v", err)
Expand Down Expand Up @@ -1672,12 +1669,14 @@ func TestResumeActor(t *testing.T) {
ActorTemplateNamespace: ns,
ActorTemplateName: "tmpl1",
Status: ateapipb.Actor_STATUS_RUNNING,
AteomPodNamespace: ns,
AteomPodName: "worker-1",
AteomPodIp: "127.0.0.1",
WorkerPoolName: "pool1",
WorkerAssignment: &ateapipb.WorkerAssignment{
WorkerNamespace: ns,
WorkerPool: "pool1",
WorkerPod: "worker-1",
WorkerPodIp: "127.0.0.1",
},
}
if diff := cmp.Diff(want, getResp, protocmp.Transform(), ignoreUID, ignoreVersion, ignoreTimestamps, protocmp.IgnoreFields(&ateapipb.Actor{}, "ateom_pod_uid")); diff != "" {
if diff := cmp.Diff(want, getResp, protocmp.Transform(), ignoreUID, ignoreVersion, ignoreTimestamps, protocmp.IgnoreFields(&ateapipb.WorkerAssignment{}, "worker_pod_uid")); diff != "" {
t.Errorf("GetActor response mismatch (-want +got):\n%s", diff)
}

Expand Down Expand Up @@ -1874,11 +1873,11 @@ func TestResumeActor_MultiPoolSelector(t *testing.T) {
if err != nil {
t.Fatalf("GetActor failed: %v", err)
}
if got := getResp.GetAteomPodName(); got != "worker-b" {
if got := getResp.GetWorkerAssignment().GetWorkerPod(); got != "worker-b" {
t.Errorf("expected actor to be assigned to worker-b (pool-b, matching narrowed selector), got %q", got)
}
if got := getResp.GetWorkerPoolName(); got != "pool-b" {
t.Errorf("expected actor's worker_pool_name to be pool-b, got %q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPool(); got != "pool-b" {
t.Errorf("expected actor's worker_assignment.worker_pool to be pool-b, got %q", got)
}
}

Expand Down Expand Up @@ -1924,8 +1923,8 @@ func TestResumeActor_RequiresBothSelectorsToMatch(t *testing.T) {
if err != nil {
t.Fatalf("GetActor failed: %v", err)
}
if got := getResp.GetWorkerPoolName(); got != "pool-both" {
t.Errorf("expected actor to be assigned to pool-both (the only pool matching both selectors), got worker_pool_name=%q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPool(); got != "pool-both" {
t.Errorf("expected actor to be assigned to pool-both (the only pool matching both selectors), got worker_assignment.worker_pool=%q", got)
}
}

Expand Down Expand Up @@ -2180,7 +2179,7 @@ func TestSuspendActor(t *testing.T) {
ignoreUID,
ignoreVersion,
ignoreTimestamps,
protocmp.IgnoreFields(&ateapipb.Actor{}, "ateom_pod_uid", "latest_snapshot"),
protocmp.IgnoreFields(&ateapipb.Actor{}, "latest_snapshot"),
); diff != "" {
t.Errorf("GetActor response mismatch (-want +got):\n%s", diff)
}
Expand Down Expand Up @@ -2272,7 +2271,6 @@ func TestPauseActor(t *testing.T) {
ignoreUID,
ignoreVersion,
ignoreTimestamps,
protocmp.IgnoreFields(&ateapipb.Actor{}, "ateom_pod_uid"),
protocmp.FilterField(&ateapipb.LocalSnapshotInfo{}, "snapshot_prefix", cmp.Comparer(func(x, y string) bool {
return strings.HasPrefix(y, x)
})),
Expand Down Expand Up @@ -2483,7 +2481,7 @@ func TestResumeActor_CrashesIfAssignedWorkerIsDraining(t *testing.T) {
if err != nil {
t.Fatalf("GetActor failed: %v", err)
}
assignedPod := getResp.GetAteomPodName()
assignedPod := getResp.GetWorkerAssignment().GetWorkerPod()
if assignedPod == "" {
t.Fatalf("expected actor to be bound to a worker after the failed attempt")
}
Expand Down Expand Up @@ -2530,7 +2528,7 @@ func TestResumeActor_CrashesIfAssignedWorkerIsDraining(t *testing.T) {
if got := getResp.GetStatus(); got != ateapipb.Actor_STATUS_CRASHED {
t.Errorf("expected actor status CRASHED, got %v", got)
}
if got := getResp.GetAteomPodName(); got != "" {
if got := getResp.GetWorkerAssignment().GetWorkerPod(); got != "" {
t.Errorf("expected actor pod name to be empty, got %q", got)
}

Expand Down Expand Up @@ -2599,11 +2597,11 @@ func TestUpdateActor_ReassignsPoolAcrossSuspendResume(t *testing.T) {
if err != nil {
t.Fatalf("GetActor failed: %v", err)
}
if got := getResp.GetWorkerPoolName(); got != "pool-a" {
t.Fatalf("expected actor to first resume onto pool-a, got worker_pool_name=%q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPool(); got != "pool-a" {
t.Fatalf("expected actor to first resume onto pool-a, got worker_assignment.worker_pool=%q", got)
}
if got := getResp.GetAteomPodName(); got != "worker-a" {
t.Fatalf("expected actor to first resume onto worker-a, got ateom_pod_name=%q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPod(); got != "worker-a" {
t.Fatalf("expected actor to first resume onto worker-a, got worker_assignment.worker_pod=%q", got)
}

if _, err := tc.client.UpdateActor(context.Background(), &ateapipb.UpdateActorRequest{
Expand All @@ -2626,11 +2624,11 @@ func TestUpdateActor_ReassignsPoolAcrossSuspendResume(t *testing.T) {
if err != nil {
t.Fatalf("GetActor failed: %v", err)
}
if got := getResp.GetWorkerPoolName(); got != "pool-b" {
t.Errorf("expected actor to resume onto pool-b after selector update, got worker_pool_name=%q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPool(); got != "pool-b" {
t.Errorf("expected actor to resume onto pool-b after selector update, got worker_assignment.worker_pool=%q", got)
}
if got := getResp.GetAteomPodName(); got != "worker-b" {
t.Errorf("expected actor to resume onto worker-b after selector update, got ateom_pod_name=%q", got)
if got := getResp.GetWorkerAssignment().GetWorkerPod(); got != "worker-b" {
t.Errorf("expected actor to resume onto worker-b after selector update, got worker_assignment.worker_pod=%q", got)
}
if got := getResp.GetStatus(); got != ateapipb.Actor_STATUS_RUNNING {
t.Errorf("expected actor status RUNNING after second resume, got %v", got)
Expand Down Expand Up @@ -2796,8 +2794,8 @@ func TestResumeActor_DanglingWorker(t *testing.T) {
if actor.GetStatus() != ateapipb.Actor_STATUS_RESUMING {
t.Fatalf("expected status RESUMING, got %v", actor.GetStatus())
}
if actor.GetAteomPodName() != "worker-a" {
t.Fatalf("expected worker-a assigned, got %v", actor.GetAteomPodName())
if actor.GetWorkerAssignment().GetWorkerPod() != "worker-a" {
t.Fatalf("expected worker-a assigned, got %v", actor.GetWorkerAssignment().GetWorkerPod())
}

deleteWorkerPod(t, tc, ns, "worker-a")
Expand Down Expand Up @@ -2828,8 +2826,8 @@ func TestResumeActor_DanglingWorker(t *testing.T) {
if actor.GetStatus() != ateapipb.Actor_STATUS_CRASHED {
t.Errorf("expected status CRASHED, got %v", actor.GetStatus())
}
if actor.GetAteomPodName() != "" {
t.Errorf("expected worker to be unassigned, got %v", actor.GetAteomPodName())
if actor.GetWorkerAssignment().GetWorkerPod() != "" {
t.Errorf("expected worker to be unassigned, got %v", actor.GetWorkerAssignment().GetWorkerPod())
}
}

Expand Down Expand Up @@ -2884,8 +2882,8 @@ func TestSuspendActor_DanglingWorker(t *testing.T) {
if getResp.GetStatus() != ateapipb.Actor_STATUS_CRASHED {
t.Errorf("expected status CRASHED, got %v", getResp.GetStatus())
}
if getResp.GetAteomPodNamespace() != "" {
t.Errorf("expected ateom_pod_namespace to be empty, got %v", getResp.GetAteomPodNamespace())
if getResp.GetWorkerAssignment() != nil {
t.Errorf("expected worker_assignment to be cleared, got %v", getResp.GetWorkerAssignment())
}
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/ateapi/internal/controlapi/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func lifecycleOpAttrs(actor *ateapipb.Actor, template *atev1alpha1.ActorTemplate
ateattr.TemplateNameKey.String(actor.GetActorTemplateName()),
ateattr.TemplateNamespaceKey.String(actor.GetActorTemplateNamespace()),
}
if pool := actor.GetWorkerPoolName(); pool != "" {
if pool := actor.GetWorkerAssignment().GetWorkerPool(); pool != "" {
attrs = append(attrs, ateattr.WorkerPoolNameKey.String(pool))
}
if template != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/ateapi/internal/controlapi/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestLifecycleOpDurationShape(t *testing.T) {
actor := &ateapipb.Actor{
ActorTemplateName: "support-agent",
ActorTemplateNamespace: "ate-agents",
WorkerPoolName: "pool-a",
WorkerAssignment: &ateapipb.WorkerAssignment{WorkerPool: "pool-a"},
}
template := &atev1alpha1.ActorTemplate{
Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor},
Expand Down
9 changes: 3 additions & 6 deletions cmd/ateapi/internal/controlapi/syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,8 @@ func (s *WorkerPoolSyncer) releaseActorOnDeadWorker(ctx context.Context, namespa
return err
}
// Skip if a concurrent SuspendActor already cleared the pointer.
if actor.GetAteomPodNamespace() != namespace || actor.GetAteomPodName() != podName {
assignment := actor.GetWorkerAssignment()
if assignment.GetWorkerNamespace() != namespace || assignment.GetWorkerPod() != podName {
return nil
}
// If the actor is suspended, it's already been released.
Expand All @@ -310,12 +311,8 @@ func (s *WorkerPoolSyncer) releaseActorOnDeadWorker(ctx context.Context, namespa
}

actor.Status = ateapipb.Actor_STATUS_CRASHED
actor.AteomPodNamespace = ""
actor.AteomPodName = ""
actor.AteomPodIp = ""
actor.AteomPodUid = ""
actor.WorkerAssignment = nil
actor.InProgressSnapshot = ""
actor.WorkerPoolName = ""

_, err = s.persistence.UpdateActor(ctx, actor, actor.GetMetadata().GetVersion())
return err
Expand Down
Loading
Loading