Skip to content

Commit 38080d8

Browse files
committed
PM-4299: allow rerun dispatch when ECS listing is denied
What was broken Changing the tester on an active Marathon Match config triggered the latest-submission rerun path, but ECS scorer launches could all fail before RunTask when the service role lacked permission for ListTasks. Work Manager then showed the saved scorer config while previous scores remained because no new runner tasks were launched. Root cause (if identifiable) A later ECS active-task de-duplication and concurrency check made ListTasks/DescribeTasks mandatory for every scorer launch. In dev, that optional inspection path is not authorized, so the PM-4299 rerun trigger selected submissions but could not dispatch them. What was changed Made active ECS task inspection best-effort. When ECS denies the inspection call, the service logs a warning and continues with direct RunTask dispatch while preserving duplicate-task reuse and concurrency checks whenever inspection is available. Any added/updated tests Added an EcsService unit test covering the AccessDenied ListTasks fallback and verifying the scorer task still launches and persists the runner log mapping.
1 parent b7546e6 commit 38080d8

2 files changed

Lines changed: 199 additions & 43 deletions

File tree

src/shared/modules/global/ecs.service.spec.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,85 @@ describe('EcsService', () => {
310310
).toBe(false);
311311
});
312312

313+
it('launches scorer tasks when active task listing is not permitted', async () => {
314+
const { service, m2mService, prisma, send } = createService();
315+
m2mService.getM2MToken.mockResolvedValue('launch-token');
316+
send.mockImplementation((command) => {
317+
if (command instanceof ListTasksCommand) {
318+
return Promise.reject(
319+
Object.assign(
320+
new Error('User is not authorized to perform: ecs:ListTasks'),
321+
{
322+
name: 'AccessDeniedException',
323+
},
324+
),
325+
);
326+
}
327+
if (command instanceof RunTaskCommand) {
328+
return Promise.resolve({
329+
tasks: [
330+
{
331+
taskArn:
332+
'arn:aws:ecs:us-east-1:123456789012:task/cluster/task-123',
333+
},
334+
],
335+
});
336+
}
337+
if (command instanceof DescribeTaskDefinitionCommand) {
338+
return Promise.resolve({
339+
taskDefinition: {
340+
containerDefinitions: [
341+
{
342+
name: 'tc-mm-runner',
343+
logConfiguration: {
344+
options: {
345+
'awslogs-group': '/ecs/mm-runner',
346+
'awslogs-stream-prefix': 'mm',
347+
},
348+
},
349+
},
350+
],
351+
},
352+
});
353+
}
354+
355+
throw new Error(`Unexpected command ${command.constructor.name}`);
356+
});
357+
358+
const result = await service.launchScorerTask(
359+
'challenge-1',
360+
'submission-1',
361+
baseTaskConfig,
362+
basePhaseConfig,
363+
undefined,
364+
{ memberId: 'member-1' },
365+
);
366+
367+
expect(result.taskId).toBe('task-123');
368+
expect(result.logStreamName).toBe('mm/tc-mm-runner/task-123');
369+
expect(m2mService.getM2MToken).toHaveBeenCalledTimes(1);
370+
expect(
371+
send.mock.calls.some(([command]) => command instanceof RunTaskCommand),
372+
).toBe(true);
373+
expect(
374+
send.mock.calls.some(([command]) => command instanceof StopTaskCommand),
375+
).toBe(false);
376+
expect(mockLogger.warn).toHaveBeenCalledWith(
377+
expect.objectContaining({
378+
message:
379+
'Unable to inspect active ECS scorer tasks; launching without duplicate or concurrency checks.',
380+
taskDefinitionName: 'mm-ecs-runner',
381+
}),
382+
);
383+
expect(prisma.submissionRunnerLog.upsert).toHaveBeenCalledWith(
384+
expect.objectContaining({
385+
where: {
386+
taskArn: 'arn:aws:ecs:us-east-1:123456789012:task/cluster/task-123',
387+
},
388+
}),
389+
);
390+
});
391+
313392
it('passes Auth0 refresh settings to the ECS runner for long system scoring callbacks', async () => {
314393
const { service, m2mService, send } = createService();
315394
m2mService.getM2MToken.mockResolvedValue('launch-token');

src/shared/modules/global/ecs.service.ts

Lines changed: 120 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ export class EcsService {
9595
* AWS_REGION, MARATHON_MATCH_API_URL, REVIEW_TYPE_ID, and Auth0 M2M settings
9696
* used by the runner to refresh tokens during long scoring tasks. Optional
9797
* ECS_SCORER_MAX_CONCURRENT_TASKS controls the global pending/running scorer
98-
* task cap and defaults to 20.
99-
* @throws Error when required ENV vars are missing, token fetch fails, the scorer task cap is reached,
100-
* or ECS launch/cancellation fails.
98+
* task cap when the role can list active ECS tasks and defaults to 20.
99+
* @throws Error when required ENV vars are missing, token fetch fails, the scorer task cap is reached
100+
* after active task lookup succeeds, or ECS launch/cancellation fails.
101101
*/
102102
async launchScorerTask(
103103
challengeId: string,
@@ -130,53 +130,58 @@ export class EcsService {
130130

131131
try {
132132
return await this.runWithScorerLaunchLock(async () => {
133-
const activeTasks = await this.listActiveScorerTasks(
133+
const activeTasks = await this.listActiveScorerTasksIfPermitted(
134134
cluster,
135135
taskDefinitionName,
136136
containerName,
137137
);
138-
const launchableActiveTasks =
139-
await this.stopSupersededMemberScorerTasks(cluster, activeTasks, {
138+
const launchableActiveTasks = activeTasks
139+
? await this.stopSupersededMemberScorerTasks(cluster, activeTasks, {
140+
challengeId,
141+
submissionId,
142+
memberId,
143+
})
144+
: null;
145+
let maxConcurrentScorerTasks: number | null = null;
146+
147+
if (launchableActiveTasks) {
148+
const duplicateTask = this.findDuplicateActiveScorerTask(
149+
launchableActiveTasks,
140150
challengeId,
141151
submissionId,
142-
memberId,
143-
});
144-
const duplicateTask = this.findDuplicateActiveScorerTask(
145-
launchableActiveTasks,
146-
challengeId,
147-
submissionId,
148-
scoringPhase.configType,
149-
);
150-
151-
if (duplicateTask) {
152-
const launchResult = await this.buildLaunchResultFromActiveTask(
153-
duplicateTask,
154-
taskDefinition,
155-
containerName,
152+
scoringPhase.configType,
156153
);
157-
await this.persistSubmissionRunnerLogMapping({
158-
challengeId,
159-
submissionId,
160-
scoringPhase,
161-
launchResult,
162-
});
163-
this.logger.log({
164-
message: 'Skipped duplicate ECS scorer task launch',
165-
challengeId,
166-
submissionId,
167-
memberId: memberId ?? null,
168-
phaseConfigType: scoringPhase.configType,
169-
taskArn: launchResult.taskArn,
170-
taskId: launchResult.taskId,
171-
});
172-
return launchResult;
173-
}
174154

175-
const maxConcurrentScorerTasks = this.getMaxConcurrentScorerTasks();
176-
if (launchableActiveTasks.length >= maxConcurrentScorerTasks) {
177-
throw new Error(
178-
`ECS scorer task concurrency limit reached (${launchableActiveTasks.length}/${maxConcurrentScorerTasks}). Deferring submission ${submissionId} to Kafka retry/back-pressure instead of launching another task.`,
179-
);
155+
if (duplicateTask) {
156+
const launchResult = await this.buildLaunchResultFromActiveTask(
157+
duplicateTask,
158+
taskDefinition,
159+
containerName,
160+
);
161+
await this.persistSubmissionRunnerLogMapping({
162+
challengeId,
163+
submissionId,
164+
scoringPhase,
165+
launchResult,
166+
});
167+
this.logger.log({
168+
message: 'Skipped duplicate ECS scorer task launch',
169+
challengeId,
170+
submissionId,
171+
memberId: memberId ?? null,
172+
phaseConfigType: scoringPhase.configType,
173+
taskArn: launchResult.taskArn,
174+
taskId: launchResult.taskId,
175+
});
176+
return launchResult;
177+
}
178+
179+
maxConcurrentScorerTasks = this.getMaxConcurrentScorerTasks();
180+
if (launchableActiveTasks.length >= maxConcurrentScorerTasks) {
181+
throw new Error(
182+
`ECS scorer task concurrency limit reached (${launchableActiveTasks.length}/${maxConcurrentScorerTasks}). Deferring submission ${submissionId} to Kafka retry/back-pressure instead of launching another task.`,
183+
);
184+
}
180185
}
181186

182187
const token = await this.getM2MTokenForScorerLaunch();
@@ -300,7 +305,8 @@ export class EcsService {
300305
logStreamPrefix: logStreamPrefix ?? null,
301306
logStreamName: logStreamName ?? null,
302307
cloudWatchLogsConsoleUrl: cloudWatchLogsConsoleUrl ?? null,
303-
activeScorerTaskCountBeforeLaunch: launchableActiveTasks.length,
308+
activeScorerTaskCountBeforeLaunch:
309+
launchableActiveTasks?.length ?? null,
304310
maxConcurrentScorerTasks,
305311
});
306312

@@ -522,6 +528,43 @@ export class EcsService {
522528
return parsedLimit;
523529
}
524530

531+
/**
532+
* Lists active scorer tasks when ECS permissions allow it; otherwise logs and
533+
* returns null so scorer dispatch can continue through RunTask.
534+
* @param cluster ECS cluster name or ARN.
535+
* @param taskDefinitionName Task definition family configured for scoring.
536+
* @param containerName Runner container name whose overrides identify scorer tasks.
537+
* @returns Active scorer tasks, or null when listing/describing active tasks is not authorized.
538+
* @throws Error for non-permission ECS lookup failures.
539+
*/
540+
private async listActiveScorerTasksIfPermitted(
541+
cluster: string,
542+
taskDefinitionName: string,
543+
containerName: string,
544+
): Promise<ActiveScorerTask[] | null> {
545+
try {
546+
return await this.listActiveScorerTasks(
547+
cluster,
548+
taskDefinitionName,
549+
containerName,
550+
);
551+
} catch (error) {
552+
if (!this.isEcsPermissionError(error)) {
553+
throw error;
554+
}
555+
556+
this.logger.warn({
557+
message:
558+
'Unable to inspect active ECS scorer tasks; launching without duplicate or concurrency checks.',
559+
cluster,
560+
taskDefinitionName,
561+
containerName,
562+
error: error instanceof Error ? error.message : String(error),
563+
});
564+
return null;
565+
}
566+
}
567+
525568
/**
526569
* Lists active scorer tasks for the configured task family and container.
527570
* @param cluster ECS cluster name or ARN.
@@ -586,6 +629,40 @@ export class EcsService {
586629
return activeTasks;
587630
}
588631

632+
/**
633+
* Detects AWS authorization failures for optional ECS inspection calls.
634+
* @param error Error thrown by the AWS SDK.
635+
* @returns True when the error represents a denied ECS action.
636+
* Used by scorer launches to fall back to direct RunTask dispatch when active
637+
* task inspection is not permitted in an environment.
638+
*/
639+
private isEcsPermissionError(error: unknown): boolean {
640+
const typedError = error as {
641+
name?: unknown;
642+
Code?: unknown;
643+
code?: unknown;
644+
message?: unknown;
645+
};
646+
const values = [
647+
typedError.name,
648+
typedError.Code,
649+
typedError.code,
650+
typedError.message,
651+
]
652+
.map((value) =>
653+
typeof value === 'string' ? value.trim().toLowerCase() : '',
654+
)
655+
.filter(Boolean);
656+
657+
return values.some(
658+
(value) =>
659+
value.includes('accessdenied') ||
660+
value.includes('access denied') ||
661+
value.includes('not authorized') ||
662+
value.includes('unauthorized'),
663+
);
664+
}
665+
589666
/**
590667
* Converts an ECS task description into active scorer task metadata.
591668
* @param cluster ECS cluster used for the task.

0 commit comments

Comments
 (0)