From 80e08e7db02534e42c3116f5d97af0b2a9db4cf5 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Wed, 11 Feb 2026 22:54:50 +0200 Subject: [PATCH] Fix max runners check and leaky JIT runners The check for max runners was added to CreateInstance(), but we crete the JIT runners before we run the function to add a runner to the DB. The defer function to clean up the JIT runner was being run after the error return generated by CreateInstance. So the cleanup code never ran. Additionally we would know that max runners was reached only after creating the JIT runner. Which kills rate limits. Signed-off-by: Gabriel Adrian Samfira --- runner/pool/pool.go | 46 ++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 3fbd3ab3..ce0f052e 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -887,6 +887,19 @@ func (r *basePoolManager) AddRunner(ctx context.Context, poolID string, aditiona } } + defer func() { + if err != nil { + if runner != nil { + runnerCleanupErr := r.ghcli.RemoveEntityRunner(r.ctx, runner.GetID()) + if err != nil { + slog.With(slog.Any("error", runnerCleanupErr)).ErrorContext( + ctx, "failed to remove runner", + "gh_runner_id", runner.GetID()) + } + } + } + }() + createParams := params.CreateInstanceParams{ Name: name, Status: commonParams.InstancePendingCreate, @@ -906,32 +919,10 @@ func (r *basePoolManager) AddRunner(ctx context.Context, poolID string, aditiona createParams.AgentID = runner.GetID() } - instance, err := r.store.CreateInstance(r.ctx, poolID, createParams) - if err != nil { + if _, err := r.store.CreateInstance(r.ctx, poolID, createParams); err != nil { return fmt.Errorf("error creating instance: %w", err) } - defer func() { - if err != nil { - if instance.ID != "" { - if err := r.DeleteRunner(instance, false, false); err != nil { - slog.With(slog.Any("error", err)).ErrorContext( - ctx, "failed to cleanup instance", - "runner_name", instance.Name) - } - } - - if runner != nil { - runnerCleanupErr := r.ghcli.RemoveEntityRunner(r.ctx, runner.GetID()) - if err != nil { - slog.With(slog.Any("error", runnerCleanupErr)).ErrorContext( - ctx, "failed to remove runner", - "gh_runner_id", runner.GetID()) - } - } - } - }() - return nil } @@ -1247,6 +1238,15 @@ func (r *basePoolManager) addRunnerToPool(pool params.Pool, aditionalLabels []st return fmt.Errorf("pool %s is disabled", pool.ID) } + poolInstanceCount, err := r.store.PoolInstanceCount(r.ctx, pool.ID) + if err != nil { + return fmt.Errorf("failed to list pool instances: %w", err) + } + + if poolInstanceCount >= int64(pool.MaxRunners) { + return fmt.Errorf("max workers (%d) reached for pool %s", pool.MaxRunners, pool.ID) + } + if err := r.AddRunner(r.ctx, pool.ID, aditionalLabels); err != nil { return fmt.Errorf("failed to add new instance for pool %s: %w", pool.ID, err) }