mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-07 21:14:11 +02:00
Backport #37461 by @silverwind Fixes #37446. The job-status resolver in `checkJobsOfCurrentRunAttempt` only considered `needs` and job-level concurrency when transitioning jobs out of `Blocked`. When something drove the resolver against a run blocked solely by workflow-level concurrency — for example, a sibling run in the same group entering the queue and triggering `EmitJobsIfReadyByRun` — the run's job silently became `Waiting` while another run still held the concurrency group, and the runner could pick it up, defeating the concurrency guarantee. The fix bails out of the resolver when the run's latest attempt is still blocked by run-level concurrency. `checkRunConcurrency` re-evaluates when the holding run finishes. Covered by a unit test (`Test_checkJobsOfCurrentRunAttempt_RunLevelConcurrencyKeepsJobsBlocked` in `services/actions/job_emitter_test.go`) that sets up a Running holder attempt and a Blocked sibling attempt in the same concurrency group directly in the DB, calls `checkJobsOfCurrentRunAttempt`, and asserts the blocked job stays `Blocked`. Fails on master, passes with the fix. --- This PR was written with the help of Claude Opus 4.7 --------- Co-authored-by: silverwind <me@silverwind.io> Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
parent
eeb4d8ffa2
commit
b79529015e
@ -199,6 +199,18 @@ func checkJobsOfRun(ctx context.Context, run *actions_model.ActionRun) (jobs, up
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
// The resolver below only considers needs and job-level concurrency, so a run blocked
|
||||
// solely by run-level concurrency would have its jobs unblocked here. checkRunConcurrency
|
||||
// re-evaluates when the holding run finishes.
|
||||
if run.Status.IsBlocked() {
|
||||
shouldBlock, err := shouldBlockRunByConcurrency(ctx, run)
|
||||
if err != nil {
|
||||
return nil, nil, fmt.Errorf("shouldBlockRunByConcurrency: %w", err)
|
||||
}
|
||||
if shouldBlock {
|
||||
return jobs, nil, nil
|
||||
}
|
||||
}
|
||||
vars, err := actions_model.GetVariablesOfRun(ctx, run)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
|
||||
@ -201,3 +201,55 @@ func Test_checkRunConcurrency_NoDuplicateConcurrencyGroupCheck(t *testing.T) {
|
||||
assert.Equal(t, jobBBlocked.ID, jobs[0].ID)
|
||||
}
|
||||
}
|
||||
|
||||
// Test_checkJobsOfRun_RunLevelConcurrencyKeepsJobsBlocked verifies that
|
||||
// the resolver does not transition a job out of Blocked while another run still holds
|
||||
// the workflow-level concurrency group. Regression for #37446.
|
||||
func Test_checkJobsOfRun_RunLevelConcurrencyKeepsJobsBlocked(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
ctx := t.Context()
|
||||
|
||||
const group = "test-run-level-concurrency-keeps-blocked"
|
||||
|
||||
// Holder run: Running run in the concurrency group.
|
||||
holderRun := &actions_model.ActionRun{
|
||||
RepoID: 4, OwnerID: 1, TriggerUserID: 1,
|
||||
WorkflowID: "test.yml", Index: 9911, Ref: "refs/heads/main",
|
||||
Status: actions_model.StatusRunning,
|
||||
ConcurrencyGroup: group,
|
||||
}
|
||||
assert.NoError(t, db.Insert(ctx, holderRun))
|
||||
|
||||
// Blocked run: Blocked run in the same group, with one Blocked job that has
|
||||
// no needs and no job-level concurrency. Without the run-level guard in
|
||||
// checkJobsOfRun, the resolver would transition this job to Waiting.
|
||||
blockedRun := &actions_model.ActionRun{
|
||||
RepoID: 4, OwnerID: 1, TriggerUserID: 1,
|
||||
WorkflowID: "test.yml", Index: 9912, Ref: "refs/heads/main",
|
||||
Status: actions_model.StatusBlocked,
|
||||
ConcurrencyGroup: group,
|
||||
}
|
||||
assert.NoError(t, db.Insert(ctx, blockedRun))
|
||||
blockedJob := &actions_model.ActionRunJob{
|
||||
RunID: blockedRun.ID,
|
||||
RepoID: 4, OwnerID: 1, JobID: "job1", Name: "job1",
|
||||
Status: actions_model.StatusBlocked,
|
||||
WorkflowPayload: []byte(`
|
||||
name: test
|
||||
on: push
|
||||
jobs:
|
||||
job1:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo
|
||||
`),
|
||||
}
|
||||
assert.NoError(t, db.Insert(ctx, blockedJob))
|
||||
|
||||
_, updated, err := checkJobsOfRun(ctx, blockedRun)
|
||||
assert.NoError(t, err)
|
||||
assert.Empty(t, updated)
|
||||
|
||||
refreshed := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: blockedJob.ID})
|
||||
assert.Equal(t, actions_model.StatusBlocked, refreshed.Status)
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user