Skip to content

Commit edda2ec

Browse files
Zettat123silverwind
authored andcommitted
Fix bugs in rerunning jobs (go-gitea#29955)
Fix go-gitea#28761 Fix go-gitea#27884 Fix go-gitea#28093 ## Changes ### Rerun all jobs When rerun all jobs, status of the jobs with `needs` will be set to `blocked` instead of `waiting`. Therefore, these jobs will not run until the required jobs are completed. ### Rerun a single job When a single job is rerun, its dependents should also be rerun, just like GitHub does (go-gitea#28761 (comment)). In this case, only the specified job will be set to `waiting`, its dependents will be set to `blocked` to wait the job. ### Show warning if every job has `needs` If every job in a workflow has `needs`, all jobs will be blocked and no job can be run. So I add a warning message. <img src="https://github.com/go-gitea/gitea/assets/15528715/88f43511-2360-465d-be96-ee92b57ff67b" width="480px" />
1 parent a4cbb56 commit edda2ec

File tree

5 files changed

+117
-6
lines changed

5 files changed

+117
-6
lines changed

options/locale/locale_en-US.ini

+1
Original file line numberDiff line numberDiff line change
@@ -3626,6 +3626,7 @@ runs.scheduled = Scheduled
36263626
runs.pushed_by = pushed by
36273627
runs.invalid_workflow_helper = Workflow config file is invalid. Please check your config file: %s
36283628
runs.no_matching_online_runner_helper = No matching online runner with label: %s
3629+
runs.no_job_without_needs = The workflow must contain at least one job without dependencies.
36293630
runs.actor = Actor
36303631
runs.status = Status
36313632
runs.actors_no_select = All actors

routers/web/repo/actions/actions.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,13 @@ func List(ctx *context.Context) {
104104
workflows = append(workflows, workflow)
105105
continue
106106
}
107-
// Check whether have matching runner
107+
// The workflow must contain at least one job without "needs". Otherwise, a deadlock will occur and no jobs will be able to run.
108+
hasJobWithoutNeeds := false
109+
// Check whether have matching runner and a job without "needs"
108110
for _, j := range wf.Jobs {
111+
if !hasJobWithoutNeeds && len(j.Needs()) == 0 {
112+
hasJobWithoutNeeds = true
113+
}
109114
runsOnList := j.RunsOn()
110115
for _, ro := range runsOnList {
111116
if strings.Contains(ro, "${{") {
@@ -123,6 +128,9 @@ func List(ctx *context.Context) {
123128
break
124129
}
125130
}
131+
if !hasJobWithoutNeeds {
132+
workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs")
133+
}
126134
workflows = append(workflows, workflow)
127135
}
128136
}

routers/web/repo/actions/view.go

+21-5
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,25 @@ func Rerun(ctx *context_module.Context) {
303303
return
304304
}
305305

306-
if jobIndexStr != "" {
307-
jobs = []*actions_model.ActionRunJob{job}
306+
if jobIndexStr == "" { // rerun all jobs
307+
for _, j := range jobs {
308+
// if the job has needs, it should be set to "blocked" status to wait for other jobs
309+
shouldBlock := len(j.Needs) > 0
310+
if err := rerunJob(ctx, j, shouldBlock); err != nil {
311+
ctx.Error(http.StatusInternalServerError, err.Error())
312+
return
313+
}
314+
}
315+
ctx.JSON(http.StatusOK, struct{}{})
316+
return
308317
}
309318

310-
for _, j := range jobs {
311-
if err := rerunJob(ctx, j); err != nil {
319+
rerunJobs := actions_service.GetAllRerunJobs(job, jobs)
320+
321+
for _, j := range rerunJobs {
322+
// jobs other than the specified one should be set to "blocked" status
323+
shouldBlock := j.JobID != job.JobID
324+
if err := rerunJob(ctx, j, shouldBlock); err != nil {
312325
ctx.Error(http.StatusInternalServerError, err.Error())
313326
return
314327
}
@@ -317,14 +330,17 @@ func Rerun(ctx *context_module.Context) {
317330
ctx.JSON(http.StatusOK, struct{}{})
318331
}
319332

320-
func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob) error {
333+
func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob, shouldBlock bool) error {
321334
status := job.Status
322335
if !status.IsDone() {
323336
return nil
324337
}
325338

326339
job.TaskID = 0
327340
job.Status = actions_model.StatusWaiting
341+
if shouldBlock {
342+
job.Status = actions_model.StatusBlocked
343+
}
328344
job.Started = 0
329345
job.Stopped = 0
330346

services/actions/rerun.go

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package actions
5+
6+
import (
7+
actions_model "code.gitea.io/gitea/models/actions"
8+
"code.gitea.io/gitea/modules/container"
9+
)
10+
11+
// GetAllRerunJobs get all jobs that need to be rerun when job should be rerun
12+
func GetAllRerunJobs(job *actions_model.ActionRunJob, allJobs []*actions_model.ActionRunJob) []*actions_model.ActionRunJob {
13+
rerunJobs := []*actions_model.ActionRunJob{job}
14+
rerunJobsIDSet := make(container.Set[string])
15+
rerunJobsIDSet.Add(job.JobID)
16+
17+
for {
18+
found := false
19+
for _, j := range allJobs {
20+
if rerunJobsIDSet.Contains(j.JobID) {
21+
continue
22+
}
23+
for _, need := range j.Needs {
24+
if rerunJobsIDSet.Contains(need) {
25+
found = true
26+
rerunJobs = append(rerunJobs, j)
27+
rerunJobsIDSet.Add(j.JobID)
28+
break
29+
}
30+
}
31+
}
32+
if !found {
33+
break
34+
}
35+
}
36+
37+
return rerunJobs
38+
}

services/actions/rerun_test.go

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package actions
5+
6+
import (
7+
"testing"
8+
9+
actions_model "code.gitea.io/gitea/models/actions"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestGetAllRerunJobs(t *testing.T) {
15+
job1 := &actions_model.ActionRunJob{JobID: "job1"}
16+
job2 := &actions_model.ActionRunJob{JobID: "job2", Needs: []string{"job1"}}
17+
job3 := &actions_model.ActionRunJob{JobID: "job3", Needs: []string{"job2"}}
18+
job4 := &actions_model.ActionRunJob{JobID: "job4", Needs: []string{"job2", "job3"}}
19+
20+
jobs := []*actions_model.ActionRunJob{job1, job2, job3, job4}
21+
22+
testCases := []struct {
23+
job *actions_model.ActionRunJob
24+
rerunJobs []*actions_model.ActionRunJob
25+
}{
26+
{
27+
job1,
28+
[]*actions_model.ActionRunJob{job1, job2, job3, job4},
29+
},
30+
{
31+
job2,
32+
[]*actions_model.ActionRunJob{job2, job3, job4},
33+
},
34+
{
35+
job3,
36+
[]*actions_model.ActionRunJob{job3, job4},
37+
},
38+
{
39+
job4,
40+
[]*actions_model.ActionRunJob{job4},
41+
},
42+
}
43+
44+
for _, tc := range testCases {
45+
rerunJobs := GetAllRerunJobs(tc.job, jobs)
46+
assert.ElementsMatch(t, tc.rerunJobs, rerunJobs)
47+
}
48+
}

0 commit comments

Comments
 (0)