Skip to content

Commit 8a1ea0b

Browse files
committed
fix(scale-up): prevent negative TotalTargetCapacity when runners exceed maximum
When pool and scale-up lambdas run concurrently, currentRunners can temporarily exceed maximumRunners. This caused the calculation `maximumRunners - currentRunners` to produce a negative value, which was then passed to EC2 CreateFleet API, resulting in: InvalidTargetCapacitySpecification: TotalTargetCapacity should not be negative. This fix wraps the calculation with Math.max(0, ...) to ensure we never attempt to create a negative number of runners. Fixes race condition between pool-lambda and scale-up-lambda.
1 parent a5b332f commit 8a1ea0b

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,24 @@ describe('scaleUp with GHES', () => {
228228
expect(mockOctokit.actions.createRegistrationTokenForRepo).not.toBeCalled();
229229
});
230230

231+
it('does not create runners when current runners exceed maximum (race condition)', async () => {
232+
process.env.RUNNERS_MAXIMUM_COUNT = '5';
233+
process.env.ENABLE_EPHEMERAL_RUNNERS = 'false';
234+
// Simulate race condition where pool lambda created more runners than max
235+
mockListRunners.mockImplementation(async () =>
236+
Array.from({ length: 10 }, (_, i) => ({
237+
instanceId: `i-${i}`,
238+
launchTime: new Date(),
239+
type: 'Org',
240+
owner: TEST_DATA_SINGLE.repositoryOwner,
241+
})),
242+
);
243+
await scaleUpModule.scaleUp(TEST_DATA);
244+
// Should not attempt to create runners (would be negative without fix)
245+
expect(createRunner).not.toBeCalled();
246+
expect(mockOctokit.actions.createRegistrationTokenForOrg).not.toBeCalled();
247+
});
248+
231249
it('does create a runner if maximum is set to -1', async () => {
232250
process.env.RUNNERS_MAXIMUM_COUNT = '-1';
233251
process.env.ENABLE_EPHEMERAL_RUNNERS = 'false';

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,14 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
440440
});
441441

442442
// Calculate how many runners we want to create.
443+
// Use Math.max(0, ...) to ensure we never attempt to create a negative number of runners,
444+
// which can happen when currentRunners exceeds maximumRunners due to pool/scale-up race conditions.
443445
const newRunners =
444446
maximumRunners === -1
445447
? // If we don't have an upper limit, scale up by the number of new jobs.
446448
scaleUp
447449
: // Otherwise, we do have a limit, so work out if `scaleUp` would exceed it.
448-
Math.min(scaleUp, maximumRunners - currentRunners);
450+
Math.max(0, Math.min(scaleUp, maximumRunners - currentRunners));
449451

450452
const missingInstanceCount = Math.max(0, scaleUp - newRunners);
451453

0 commit comments

Comments
 (0)