Skip to content

Commit 78c1ddb

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 e4b12ae commit 78c1ddb

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
@@ -438,12 +438,14 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
438438
});
439439

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

448450
const missingInstanceCount = Math.max(0, scaleUp - newRunners);
449451

0 commit comments

Comments
 (0)