Skip to content

Commit a92d5f6

Browse files
Fix auto concurrency cancellation skips commit status updates (go-gitea#33764)
* add missing commit status * conflicts with concurrency support Closes go-gitea#33763 Co-authored-by: Giteabot <[email protected]>
1 parent e47bba0 commit a92d5f6

File tree

10 files changed

+51
-28
lines changed

10 files changed

+51
-28
lines changed

models/actions/run.go

+12-8
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err
194194

195195
// CancelPreviousJobs cancels all previous jobs of the same repository, reference, workflow, and event.
196196
// It's useful when a new run is triggered, and all previous runs needn't be continued anymore.
197-
func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error {
197+
func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) ([]*ActionRunJob, error) {
198198
// Find all runs in the specified repository, reference, and workflow with non-final status
199199
runs, total, err := db.FindAndCount[ActionRun](ctx, FindRunOptions{
200200
RepoID: repoID,
@@ -204,22 +204,24 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin
204204
Status: []Status{StatusRunning, StatusWaiting, StatusBlocked},
205205
})
206206
if err != nil {
207-
return err
207+
return nil, err
208208
}
209209

210210
// If there are no runs found, there's no need to proceed with cancellation, so return nil.
211211
if total == 0 {
212-
return nil
212+
return nil, nil
213213
}
214214

215+
cancelledJobs := make([]*ActionRunJob, 0, total)
216+
215217
// Iterate over each found run and cancel its associated jobs.
216218
for _, run := range runs {
217219
// Find all jobs associated with the current run.
218220
jobs, err := db.Find[ActionRunJob](ctx, FindRunJobOptions{
219221
RunID: run.ID,
220222
})
221223
if err != nil {
222-
return err
224+
return cancelledJobs, err
223225
}
224226

225227
// Iterate over each job and attempt to cancel it.
@@ -238,27 +240,29 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin
238240
// Update the job's status and stopped time in the database.
239241
n, err := UpdateRunJob(ctx, job, builder.Eq{"task_id": 0}, "status", "stopped")
240242
if err != nil {
241-
return err
243+
return cancelledJobs, err
242244
}
243245

244246
// If the update affected 0 rows, it means the job has changed in the meantime, so we need to try again.
245247
if n == 0 {
246-
return fmt.Errorf("job has changed, try again")
248+
return cancelledJobs, fmt.Errorf("job has changed, try again")
247249
}
248250

251+
cancelledJobs = append(cancelledJobs, job)
249252
// Continue with the next job.
250253
continue
251254
}
252255

253256
// If the job has an associated task, try to stop the task, effectively cancelling the job.
254257
if err := StopTask(ctx, job.TaskID, StatusCancelled); err != nil {
255-
return err
258+
return cancelledJobs, err
256259
}
260+
cancelledJobs = append(cancelledJobs, job)
257261
}
258262
}
259263

260264
// Return nil to indicate successful cancellation of all running and waiting jobs.
261-
return nil
265+
return cancelledJobs, nil
262266
}
263267

264268
// InsertRun inserts a run

models/actions/schedule.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -117,21 +117,22 @@ func DeleteScheduleTaskByRepo(ctx context.Context, id int64) error {
117117
return committer.Commit()
118118
}
119119

120-
func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) error {
120+
func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) ([]*ActionRunJob, error) {
121121
// If actions disabled when there is schedule task, this will remove the outdated schedule tasks
122122
// There is no other place we can do this because the app.ini will be changed manually
123123
if err := DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil {
124-
return fmt.Errorf("DeleteCronTaskByRepo: %v", err)
124+
return nil, fmt.Errorf("DeleteCronTaskByRepo: %v", err)
125125
}
126126
// cancel running cron jobs of this repository and delete old schedules
127-
if err := CancelPreviousJobs(
127+
jobs, err := CancelPreviousJobs(
128128
ctx,
129129
repo.ID,
130130
repo.DefaultBranch,
131131
"",
132132
webhook_module.HookEventSchedule,
133-
); err != nil {
134-
return fmt.Errorf("CancelPreviousJobs: %v", err)
133+
)
134+
if err != nil {
135+
return jobs, fmt.Errorf("CancelPreviousJobs: %v", err)
135136
}
136-
return nil
137+
return jobs, nil
137138
}

routers/api/v1/repo/repo.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"strings"
1313
"time"
1414

15-
actions_model "code.gitea.io/gitea/models/actions"
1615
activities_model "code.gitea.io/gitea/models/activities"
1716
"code.gitea.io/gitea/models/db"
1817
"code.gitea.io/gitea/models/organization"
@@ -1049,7 +1048,7 @@ func updateRepoArchivedState(ctx *context.APIContext, opts api.EditRepoOption) e
10491048
ctx.APIErrorInternal(err)
10501049
return err
10511050
}
1052-
if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil {
1051+
if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil {
10531052
log.Error("CleanRepoScheduleTasks for archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
10541053
}
10551054
log.Trace("Repository was archived: %s/%s", ctx.Repo.Owner.Name, repo.Name)

routers/web/repo/setting/setting.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"strings"
1212
"time"
1313

14-
actions_model "code.gitea.io/gitea/models/actions"
1514
"code.gitea.io/gitea/models/db"
1615
"code.gitea.io/gitea/models/organization"
1716
"code.gitea.io/gitea/models/perm"
@@ -906,7 +905,7 @@ func SettingsPost(ctx *context.Context) {
906905
return
907906
}
908907

909-
if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil {
908+
if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil {
910909
log.Error("CleanRepoScheduleTasks for archived repo %s/%s: %v", ctx.Repo.Owner.Name, repo.Name, err)
911910
}
912911

services/actions/clear_tasks.go

+21-1
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import (
1010

1111
actions_model "code.gitea.io/gitea/models/actions"
1212
"code.gitea.io/gitea/models/db"
13+
repo_model "code.gitea.io/gitea/models/repo"
1314
"code.gitea.io/gitea/modules/actions"
1415
"code.gitea.io/gitea/modules/log"
1516
"code.gitea.io/gitea/modules/setting"
1617
"code.gitea.io/gitea/modules/timeutil"
18+
webhook_module "code.gitea.io/gitea/modules/webhook"
1719
)
1820

1921
// StopZombieTasks stops the task which have running status, but haven't been updated for a long time
@@ -32,6 +34,24 @@ func StopEndlessTasks(ctx context.Context) error {
3234
})
3335
}
3436

37+
func notifyWorkflowJobStatusUpdate(ctx context.Context, jobs []*actions_model.ActionRunJob) {
38+
if len(jobs) > 0 {
39+
CreateCommitStatus(ctx, jobs...)
40+
}
41+
}
42+
43+
func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error {
44+
jobs, err := actions_model.CancelPreviousJobs(ctx, repoID, ref, workflowID, event)
45+
notifyWorkflowJobStatusUpdate(ctx, jobs)
46+
return err
47+
}
48+
49+
func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) error {
50+
jobs, err := actions_model.CleanRepoScheduleTasks(ctx, repo)
51+
notifyWorkflowJobStatusUpdate(ctx, jobs)
52+
return err
53+
}
54+
3555
func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
3656
tasks, err := db.Find[actions_model.ActionTask](ctx, opts)
3757
if err != nil {
@@ -67,7 +87,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
6787
remove()
6888
}
6989

70-
CreateCommitStatus(ctx, jobs...)
90+
notifyWorkflowJobStatusUpdate(ctx, jobs)
7191

7292
return nil
7393
}

services/actions/notifier_helper.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func notify(ctx context.Context, input *notifyInput) error {
136136
return nil
137137
}
138138
if unit_model.TypeActions.UnitGlobalDisabled() {
139-
if err := actions_model.CleanRepoScheduleTasks(ctx, input.Repo); err != nil {
139+
if err := CleanRepoScheduleTasks(ctx, input.Repo); err != nil {
140140
log.Error("CleanRepoScheduleTasks: %v", err)
141141
}
142142
return nil
@@ -341,7 +341,7 @@ func handleWorkflows(
341341
// cancel running jobs if the event is push or pull_request_sync
342342
if run.Event == webhook_module.HookEventPush ||
343343
run.Event == webhook_module.HookEventPullRequestSync {
344-
if err := actions_model.CancelPreviousJobs(
344+
if err := CancelPreviousJobs(
345345
ctx,
346346
run.RepoID,
347347
run.Ref,
@@ -472,7 +472,7 @@ func handleSchedules(
472472
log.Error("CountSchedules: %v", err)
473473
return err
474474
} else if count > 0 {
475-
if err := actions_model.CleanRepoScheduleTasks(ctx, input.Repo); err != nil {
475+
if err := CleanRepoScheduleTasks(ctx, input.Repo); err != nil {
476476
log.Error("CleanRepoScheduleTasks: %v", err)
477477
}
478478
}

services/actions/schedule_tasks.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func startTasks(ctx context.Context) error {
5555
// cancel running jobs if the event is push
5656
if row.Schedule.Event == webhook_module.HookEventPush {
5757
// cancel running jobs of the same workflow
58-
if err := actions_model.CancelPreviousJobs(
58+
if err := CancelPreviousJobs(
5959
ctx,
6060
row.RepoID,
6161
row.Schedule.Ref,

services/actions/workflow.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
256256
}
257257

258258
// cancel running jobs of the same workflow
259-
if err := actions_model.CancelPreviousJobs(
259+
if err := CancelPreviousJobs(
260260
ctx,
261261
run.RepoID,
262262
run.Ref,

services/repository/branch.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"code.gitea.io/gitea/modules/timeutil"
3131
"code.gitea.io/gitea/modules/util"
3232
webhook_module "code.gitea.io/gitea/modules/webhook"
33+
actions_service "code.gitea.io/gitea/services/actions"
3334
notify_service "code.gitea.io/gitea/services/notify"
3435
release_service "code.gitea.io/gitea/services/release"
3536
files_service "code.gitea.io/gitea/services/repository/files"
@@ -452,7 +453,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m
452453
log.Error("DeleteCronTaskByRepo: %v", err)
453454
}
454455
// cancel running cron jobs of this repository and delete old schedules
455-
if err := actions_model.CancelPreviousJobs(
456+
if err := actions_service.CancelPreviousJobs(
456457
ctx,
457458
repo.ID,
458459
from,
@@ -639,7 +640,7 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR
639640
log.Error("DeleteCronTaskByRepo: %v", err)
640641
}
641642
// cancel running cron jobs of this repository and delete old schedules
642-
if err := actions_model.CancelPreviousJobs(
643+
if err := actions_service.CancelPreviousJobs(
643644
ctx,
644645
repo.ID,
645646
oldDefaultBranchName,

services/repository/setting.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"context"
88
"slices"
99

10-
actions_model "code.gitea.io/gitea/models/actions"
1110
"code.gitea.io/gitea/models/db"
1211
repo_model "code.gitea.io/gitea/models/repo"
1312
"code.gitea.io/gitea/models/unit"
@@ -29,7 +28,7 @@ func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, uni
2928
}
3029

3130
if slices.Contains(deleteUnitTypes, unit.TypeActions) {
32-
if err := actions_model.CleanRepoScheduleTasks(ctx, repo); err != nil {
31+
if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil {
3332
log.Error("CleanRepoScheduleTasks: %v", err)
3433
}
3534
}

0 commit comments

Comments
 (0)