Skip to content

Commit 9fe1eae

Browse files
committed
Add new validator to fix a bug in ResumeActor
1 parent 6a75b36 commit 9fe1eae

2 files changed

Lines changed: 63 additions & 2 deletions

File tree

‎cmd/ateapi/internal/controlapi/functional_test.go‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,6 +1374,63 @@ func TestCreateActor_DefaultsRequiredWorkerPoolFromTemplate(t *testing.T) {
13741374
}
13751375
}
13761376

1377+
func TestResumeActor_LegacyActorUsesTemplateRequiredWorkerPool(t *testing.T) {
1378+
ns := namespaceForTest("ns-legacy-template-required-pool")
1379+
tc := setupTest(t, ns)
1380+
defer tc.cleanup()
1381+
1382+
createWorkerPool(t, tc, ns, "pool-a", map[string]string{"group": ns})
1383+
createWorkerPool(t, tc, ns, "pool-b", map[string]string{"group": ns})
1384+
1385+
actorTemplate := &atev1alpha1.ActorTemplate{
1386+
ObjectMeta: metav1.ObjectMeta{
1387+
Name: "tmpl-required-pool",
1388+
Namespace: ns,
1389+
},
1390+
Spec: atev1alpha1.ActorTemplateSpec{
1391+
PauseImage: "pause@sha256:abc",
1392+
SnapshotsConfig: atev1alpha1.SnapshotsConfig{
1393+
Location: "gs://fake-fake-fake",
1394+
},
1395+
Containers: []atev1alpha1.Container{
1396+
{Name: "main", Image: "main@sha256:abc", Command: []string{"/main"}},
1397+
},
1398+
RequiredWorkerPoolName: "pool-a",
1399+
},
1400+
}
1401+
if _, err := tc.substrateClient.ApiV1alpha1().ActorTemplates(ns).Create(context.Background(), actorTemplate, metav1.CreateOptions{}); err != nil {
1402+
t.Fatalf("failed to create actor template: %v", err)
1403+
}
1404+
1405+
createWorkerPod(t, tc, ns, "worker-b", "node1", "pool-b")
1406+
1407+
_, err := tc.persistence.CreateActor(context.Background(), &ateapipb.Actor{
1408+
Metadata: &ateapipb.ResourceMetadata{
1409+
Atespace: testAtespace,
1410+
Name: "legacy-id1",
1411+
},
1412+
Status: ateapipb.Actor_STATUS_SUSPENDED,
1413+
ActorTemplateNamespace: ns,
1414+
ActorTemplateName: "tmpl-required-pool",
1415+
})
1416+
if err != nil {
1417+
t.Fatalf("CreateActor in persistence failed: %v", err)
1418+
}
1419+
1420+
_, err = tc.client.ResumeActor(context.Background(), &ateapipb.ResumeActorRequest{
1421+
Actor: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "legacy-id1"},
1422+
})
1423+
assertGrpcError(t, err, codes.FailedPrecondition, "no free workers available")
1424+
1425+
getResp, err := tc.client.GetActor(context.Background(), &ateapipb.GetActorRequest{Actor: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "legacy-id1"}})
1426+
if err != nil {
1427+
t.Fatalf("GetActor failed: %v", err)
1428+
}
1429+
if got := getResp.GetWorkerPoolName(); got != "" {
1430+
t.Errorf("expected actor to remain unassigned, got worker_pool_name=%q", got)
1431+
}
1432+
}
1433+
13771434
func TestResumeActor_RequiredWorkerPoolDoesNotFallback(t *testing.T) {
13781435
ns := namespaceForTest("ns-required-pool-no-fallback")
13791436
tc := setupTest(t, ns)

‎cmd/ateapi/internal/controlapi/workflow_resume.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat
170170
}
171171

172172
var assignedWorker *ateapipb.Worker
173+
requiredWorkerPool := state.Actor.GetRequiredWorkerPoolName()
174+
if requiredWorkerPool == "" {
175+
requiredWorkerPool = state.ActorTemplate.Spec.RequiredWorkerPoolName
176+
}
173177

174178
// Check if we already have a worker assigned from a previous failed attempt.
175179
// This can happen if ateapi crashed after updating worker with actor assignment,
@@ -181,7 +185,7 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat
181185
if worker.Assignment.Actor.Atespace != input.Atespace || worker.Assignment.Actor.Name != input.ActorName {
182186
continue
183187
}
184-
eligible, err := isWorkerEligibleForActor(worker, state.ActorTemplate.Spec.SandboxClass, state.ActorTemplate.Spec.WorkerSelector, state.Actor.GetWorkerSelector(), state.Actor.GetRequiredWorkerPoolName())
188+
eligible, err := isWorkerEligibleForActor(worker, state.ActorTemplate.Spec.SandboxClass, state.ActorTemplate.Spec.WorkerSelector, state.Actor.GetWorkerSelector(), requiredWorkerPool)
185189
if err != nil {
186190
return fmt.Errorf("while checking worker eligibility: %w", err)
187191
}
@@ -208,7 +212,7 @@ func (s *AssignWorkerStep) Execute(ctx context.Context, input *ResumeInput, stat
208212
}(releaseWorker)
209213
}
210214
if assignedWorker == nil {
211-
pickedWorker, err := s.findFreeWorker(workers, state.ActorTemplate.Spec.SandboxClass, state.ActorTemplate.Spec.WorkerSelector, state.Actor.GetWorkerSelector(), state.Actor.GetRequiredWorkerPoolName(), state.Actor.GetLatestSnapshotInfo().GetLocal().GetNodeVmsWithLocalSnapshots())
215+
pickedWorker, err := s.findFreeWorker(workers, state.ActorTemplate.Spec.SandboxClass, state.ActorTemplate.Spec.WorkerSelector, state.Actor.GetWorkerSelector(), requiredWorkerPool, state.Actor.GetLatestSnapshotInfo().GetLocal().GetNodeVmsWithLocalSnapshots())
212216
if err != nil {
213217
return err
214218
}

0 commit comments

Comments
 (0)