Skip to content

Commit 8f6bca9

Browse files
authored
Fix scaleUpChron org filter for fleets with a separate scale-config org (#8370)
**Impact:** scaleUpChron **Risk:** low ## What The chron scale-up now matches queued jobs against the fleet's own org (`authGHOrg`, falling back to `scaleConfigOrg`) instead of always comparing against `scaleConfigOrg`. ## Why meta-pytorch runs its runners under `authGHOrg` but pulls the runner catalog from a shared scale-config in a different `scaleConfigOrg`. The org filter only ever matched `scaleConfigOrg`, so every queued job for the fleet's real org was filtered out and `scaleUpChron` never scaled anything — leaving jobs stuck in the queue. The new precedence mirrors what `authRepo` already does a few lines above (`authGHOrg || scaleConfigOrg`), keeping single-org fleets (empty `authGHOrg`) behaving exactly as before. Signed-off-by: Jean Schmidt <contato@jschmidt.me>
1 parent e2d4883 commit 8f6bca9

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,24 @@ const hudQueryInvalidOrgResponse = [
7373
max_queue_time_minutes: 32,
7474
},
7575
];
76+
const hudQueryDivergentOrgResponse = [
77+
{
78+
runner_label: 'test_runner_type1',
79+
org: 'meta_org',
80+
repo: 'meta_repo',
81+
num_queued_jobs: 1,
82+
min_queue_time_minutes: 31,
83+
max_queue_time_minutes: 31,
84+
},
85+
{
86+
runner_label: 'test_runner_type1',
87+
org: 'scale_org',
88+
repo: 'scale_repo',
89+
num_queued_jobs: 1,
90+
min_queue_time_minutes: 31,
91+
max_queue_time_minutes: 31,
92+
},
93+
];
7694

7795
const runnerTypeValid = 'test_runner_type1';
7896
const runnerTypeInvalid = 'runner_type_invalid';
@@ -160,6 +178,69 @@ describe('scaleUpChron', () => {
160178
expect(mockedScaleUp).toBeCalledTimes(1);
161179
});
162180

181+
it("backfills the fleet's own org (authGHOrg) even when scaleConfigOrg differs", async () => {
182+
const mockedScaleUp = mocked(scaleUp).mockResolvedValue(undefined);
183+
184+
jest.clearAllMocks();
185+
jest.spyOn(Config, 'Instance', 'get').mockImplementation(
186+
() =>
187+
({
188+
...baseCfg,
189+
authGHOrg: 'meta_org',
190+
scaleConfigOrg: 'scale_org',
191+
} as unknown as Config),
192+
);
193+
194+
mocked(shuffleArrayInPlace).mockImplementation((a) => a);
195+
mocked(getRepo).mockReturnValue({ owner: 'meta_org', repo: 'meta_repo' });
196+
mocked(getRunnerTypes).mockResolvedValue(
197+
new Map([[runnerTypeValid, { runnerTypeName: 'test_runner_type1' } as RunnerType]]),
198+
);
199+
mocked(expBackOff).mockResolvedValue({ data: hudQueryDivergentOrgResponse });
200+
201+
await scaleUpChron(metrics);
202+
203+
expect(mockedScaleUp).toBeCalledTimes(1);
204+
expect(mockedScaleUp).toHaveBeenCalledWith(
205+
'aws:sqs',
206+
expect.objectContaining({
207+
repositoryOwner: 'meta_org',
208+
repositoryName: 'meta_repo',
209+
runnerLabels: ['test_runner_type1'],
210+
}),
211+
expect.anything(),
212+
);
213+
});
214+
215+
it('falls back to scaleConfigOrg when authGHOrg is empty string (shared-catalog fleet)', async () => {
216+
const mockedScaleUp = mocked(scaleUp).mockResolvedValue(undefined);
217+
218+
jest.clearAllMocks();
219+
jest.spyOn(Config, 'Instance', 'get').mockImplementation(
220+
() =>
221+
({
222+
...baseCfg,
223+
authGHOrg: '',
224+
} as unknown as Config),
225+
);
226+
227+
mocked(shuffleArrayInPlace).mockImplementation((a) => a);
228+
mocked(getRepo).mockReturnValue({ owner: 'test_org1', repo: 'test_repo1' });
229+
mocked(getRunnerTypes).mockResolvedValue(
230+
new Map([[runnerTypeValid, { runnerTypeName: 'test_runner_type1' } as RunnerType]]),
231+
);
232+
mocked(expBackOff).mockResolvedValue({ data: hudQueryValidResponse });
233+
234+
await scaleUpChron(metrics);
235+
236+
expect(mockedScaleUp).toBeCalledTimes(1);
237+
expect(mockedScaleUp).toHaveBeenCalledWith(
238+
'aws:sqs',
239+
expect.objectContaining({ repositoryOwner: 'test_org1' }),
240+
expect.anything(),
241+
);
242+
});
243+
163244
it('scaled up throws error', async () => {
164245
const mockedScaleUp = mocked(scaleUp).mockRejectedValue(Error('error'));
165246
const scaleUpChronInstanceFailureNonRetryableSpy = jest.spyOn(metrics, 'scaleUpChronInstanceFailureNonRetryable');

terraform-aws-github-runner/modules/runners/lambdas/runners/src/scale-runners/scale-up-chron.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export async function scaleUpChron(metrics: ScaleUpChronMetrics): Promise<void>
4444
);
4545
return (
4646
runner.max_queue_time_minutes >= Config.Instance.scaleUpMaxQueueTimeMinutes &&
47-
runner.org === Config.Instance.scaleConfigOrg
47+
runner.org === (Config.Instance.authGHOrg || Config.Instance.scaleConfigOrg)
4848
);
4949
})
5050
.filter((requested_runner) => {

0 commit comments

Comments
 (0)