📝 Description
When a repository has a concurrency_limit, each running PipelineRun holds
one slot, and the slot is given back when the run finishes. The handover
happens at the end of reportFinalStatus, which promotes the next queued run.
ReconcileKind never gets there when provider detection fails. At
pkg/reconciler/reconciler.go:267 a failure to detect the git provider logs a
message and returns nil. Returning nil tells the work queue the item is
done, so nothing retries, reportFinalStatus never runs, and the finished
run's slot stays occupied forever.
Provider detection is not a pure local check. For GitHub App installs it calls
InitAppClient, which talks to the network and can fail for a moment. If that
moment coincides with a queued run finishing, the repository permanently loses
one slot of capacity. With concurrency_limit: 1 the queue stops moving
entirely until someone restarts the watcher.
A restart does not even help fully: the finished run still carries the
started state annotation, so the queue rebuild counts it as running again
(see the companion issue about InitQueues).
The neighbouring failure paths in reportFinalStatus (secret fetch, client
setup) all return their error and recover on retry. This one is the odd one
out.
Found during a post-merge review of #2890.
🛠️ Suggested fix
Return the error so the rate-limited work queue retries:
return fmt.Errorf("detect provider: %w", err)
Releasing the slot directly before returning nil is not a good alternative,
because it frees capacity without waking the queue, and nothing else will.
🧪 Testing Strategy
📝 Description
When a repository has a
concurrency_limit, each running PipelineRun holdsone slot, and the slot is given back when the run finishes. The handover
happens at the end of
reportFinalStatus, which promotes the next queued run.ReconcileKindnever gets there when provider detection fails. Atpkg/reconciler/reconciler.go:267a failure to detect the git provider logs amessage and returns
nil. Returningniltells the work queue the item isdone, so nothing retries,
reportFinalStatusnever runs, and the finishedrun's slot stays occupied forever.
Provider detection is not a pure local check. For GitHub App installs it calls
InitAppClient, which talks to the network and can fail for a moment. If thatmoment coincides with a queued run finishing, the repository permanently loses
one slot of capacity. With
concurrency_limit: 1the queue stops movingentirely until someone restarts the watcher.
A restart does not even help fully: the finished run still carries the
startedstate annotation, so the queue rebuild counts it as running again(see the companion issue about
InitQueues).The neighbouring failure paths in
reportFinalStatus(secret fetch, clientsetup) all return their error and recover on retry. This one is the odd one
out.
Found during a post-merge review of #2890.
🛠️ Suggested fix
Return the error so the rate-limited work queue retries:
Releasing the slot directly before returning
nilis not a good alternative,because it frees capacity without waking the queue, and nothing else will.
🧪 Testing Strategy