Skip to content

Commit 29724f3

Browse files
denyskonwolfogre
denyskon
and
wolfogre
authored
Refactor commit status for Actions jobs (#23786) (#24060)
Backport #23786 Refactor commit status for Actions jobs (#23786) Highlights: - Treat `StatusSkipped` as `CommitStatusSuccess` instead of `CommitStatusFailure`, so it fixed #23599. - Use the bot user `gitea-actions` instead of the trigger as the creator of commit status. - New format `<run_name> / <job_name> / (<event>)` for the context of commit status to avoid conflicts. - Add descriptions for commit status. - Add the missing calls to `CreateCommitStatus`. - Refactor `CreateCommitStatus` to make it easier to use. Co-authored-by: Jason Song <[email protected]>
1 parent 580da8f commit 29724f3

File tree

7 files changed

+73
-70
lines changed

7 files changed

+73
-70
lines changed

‎routers/api/actions/runner/runner.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,7 @@ func (s *Service) UpdateTask(
149149
return nil, status.Errorf(codes.Internal, "load job: %v", err)
150150
}
151151

152-
if err := actions_service.CreateCommitStatus(ctx, task.Job); err != nil {
153-
log.Error("Update commit status for job %v failed: %v", task.Job.ID, err)
154-
// go on
155-
}
152+
actions_service.CreateCommitStatus(ctx, task.Job)
156153

157154
if req.Msg.State.Result != runnerv1.Result_RESULT_UNSPECIFIED {
158155
if err := actions_service.EmitJobsIfReady(task.Job.RunID); err != nil {

‎routers/api/actions/runner/utils.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/modules/log"
1515
secret_module "code.gitea.io/gitea/modules/secret"
1616
"code.gitea.io/gitea/modules/setting"
17+
"code.gitea.io/gitea/services/actions"
1718

1819
runnerv1 "code.gitea.io/actions-proto-go/runner/v1"
1920
"google.golang.org/protobuf/types/known/structpb"
@@ -28,6 +29,8 @@ func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv
2829
return nil, false, nil
2930
}
3031

32+
actions.CreateCommitStatus(ctx, t.Job)
33+
3134
task := &runnerv1.Task{
3235
Id: t.ID,
3336
WorkflowPayload: t.Job.WorkflowPayload,

‎routers/web/repo/actions/view.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"code.gitea.io/gitea/models/unit"
1616
"code.gitea.io/gitea/modules/actions"
1717
context_module "code.gitea.io/gitea/modules/context"
18-
"code.gitea.io/gitea/modules/log"
1918
"code.gitea.io/gitea/modules/timeutil"
2019
"code.gitea.io/gitea/modules/util"
2120
"code.gitea.io/gitea/modules/web"
@@ -215,10 +214,7 @@ func Rerun(ctx *context_module.Context) {
215214
return
216215
}
217216

218-
if err := actions_service.CreateCommitStatus(ctx, job); err != nil {
219-
log.Error("Update commit status for job %v failed: %v", job.ID, err)
220-
// go on
221-
}
217+
actions_service.CreateCommitStatus(ctx, job)
222218

223219
ctx.JSON(http.StatusOK, struct{}{})
224220
}
@@ -259,12 +255,7 @@ func Cancel(ctx *context_module.Context) {
259255
return
260256
}
261257

262-
for _, job := range jobs {
263-
if err := actions_service.CreateCommitStatus(ctx, job); err != nil {
264-
log.Error("Update commit status for job %v failed: %v", job.ID, err)
265-
// go on
266-
}
267-
}
258+
actions_service.CreateCommitStatus(ctx, jobs...)
268259

269260
ctx.JSON(http.StatusOK, struct{}{})
270261
}

‎services/actions/clear_tasks.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error {
6464
}
6565
}
6666

67-
for _, job := range jobs {
68-
if err := CreateCommitStatus(ctx, job); err != nil {
69-
log.Error("Update commit status for job %v failed: %v", job.ID, err)
70-
// go on
71-
}
72-
}
67+
CreateCommitStatus(ctx, jobs...)
7368

7469
return nil
7570
}
@@ -96,10 +91,7 @@ func CancelAbandonedJobs(ctx context.Context) error {
9691
log.Warn("cancel abandoned job %v: %v", job.ID, err)
9792
// go on
9893
}
99-
if err := CreateCommitStatus(ctx, job); err != nil {
100-
log.Error("Update commit status for job %v failed: %v", job.ID, err)
101-
// go on
102-
}
94+
CreateCommitStatus(ctx, job)
10395
}
10496

10597
return nil

‎services/actions/commit_status.go‎

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,79 +6,80 @@ package actions
66
import (
77
"context"
88
"fmt"
9+
"path"
910

1011
actions_model "code.gitea.io/gitea/models/actions"
1112
"code.gitea.io/gitea/models/db"
1213
git_model "code.gitea.io/gitea/models/git"
1314
user_model "code.gitea.io/gitea/models/user"
15+
"code.gitea.io/gitea/modules/log"
1416
api "code.gitea.io/gitea/modules/structs"
1517
webhook_module "code.gitea.io/gitea/modules/webhook"
18+
19+
"github.com/nektos/act/pkg/jobparser"
1620
)
1721

18-
func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error {
22+
// CreateCommitStatus creates a commit status for the given job.
23+
// It won't return an error failed, but will log it, because it's not critical.
24+
func CreateCommitStatus(ctx context.Context, jobs ...*actions_model.ActionRunJob) {
25+
for _, job := range jobs {
26+
if err := createCommitStatus(ctx, job); err != nil {
27+
log.Error("Failed to create commit status for job %d: %v", job.ID, err)
28+
}
29+
}
30+
}
31+
32+
func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error {
1933
if err := job.LoadAttributes(ctx); err != nil {
2034
return fmt.Errorf("load run: %w", err)
2135
}
2236

2337
run := job.Run
38+
2439
var (
25-
sha string
26-
creatorID int64
40+
sha string
41+
event string
2742
)
28-
2943
switch run.Event {
3044
case webhook_module.HookEventPush:
45+
event = "push"
3146
payload, err := run.GetPushEventPayload()
3247
if err != nil {
3348
return fmt.Errorf("GetPushEventPayload: %w", err)
3449
}
35-
36-
// Since the payload comes from json data, we should check if it's broken, or it will cause panic
37-
switch {
38-
case payload.Repo == nil:
39-
return fmt.Errorf("repo is missing in event payload")
40-
case payload.Pusher == nil:
41-
return fmt.Errorf("pusher is missing in event payload")
42-
case payload.HeadCommit == nil:
50+
if payload.HeadCommit == nil {
4351
return fmt.Errorf("head commit is missing in event payload")
4452
}
45-
4653
sha = payload.HeadCommit.ID
47-
creatorID = payload.Pusher.ID
4854
case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync:
55+
event = "pull_request"
4956
payload, err := run.GetPullRequestEventPayload()
5057
if err != nil {
5158
return fmt.Errorf("GetPullRequestEventPayload: %w", err)
5259
}
53-
54-
switch {
55-
case payload.PullRequest == nil:
60+
if payload.PullRequest == nil {
5661
return fmt.Errorf("pull request is missing in event payload")
57-
case payload.PullRequest.Head == nil:
62+
} else if payload.PullRequest.Head == nil {
5863
return fmt.Errorf("head of pull request is missing in event payload")
59-
case payload.PullRequest.Head.Repository == nil:
60-
return fmt.Errorf("head repository of pull request is missing in event payload")
61-
case payload.PullRequest.Head.Repository.Owner == nil:
62-
return fmt.Errorf("owner of head repository of pull request is missing in evnt payload")
6364
}
64-
6565
sha = payload.PullRequest.Head.Sha
66-
creatorID = payload.PullRequest.Head.Repository.Owner.ID
6766
default:
6867
return nil
6968
}
7069

7170
repo := run.Repo
72-
ctxname := job.Name
73-
state := toCommitStatus(job.Status)
74-
creator, err := user_model.GetUserByID(ctx, creatorID)
75-
if err != nil {
76-
return fmt.Errorf("GetUserByID: %w", err)
71+
// TODO: store workflow name as a field in ActionRun to avoid parsing
72+
runName := path.Base(run.WorkflowID)
73+
if wfs, err := jobparser.Parse(job.WorkflowPayload); err == nil && len(wfs) > 0 {
74+
runName = wfs[0].Name
7775
}
76+
ctxname := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event)
77+
state := toCommitStatus(job.Status)
7878
if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}); err == nil {
7979
for _, v := range statuses {
8080
if v.Context == ctxname {
8181
if v.State == state {
82+
// no need to update
8283
return nil
8384
}
8485
break
@@ -93,16 +94,36 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
9394
return fmt.Errorf("getIndexOfJob: %w", err)
9495
}
9596

97+
description := ""
98+
switch job.Status {
99+
// TODO: if we want support description in different languages, we need to support i18n placeholders in it
100+
case actions_model.StatusSuccess:
101+
description = fmt.Sprintf("Successful in %s", job.Duration())
102+
case actions_model.StatusFailure:
103+
description = fmt.Sprintf("Failing after %s", job.Duration())
104+
case actions_model.StatusCancelled:
105+
description = "Has been cancelled"
106+
case actions_model.StatusSkipped:
107+
description = "Has been skipped"
108+
case actions_model.StatusRunning:
109+
description = "Has started running"
110+
case actions_model.StatusWaiting:
111+
description = "Waiting to run"
112+
case actions_model.StatusBlocked:
113+
description = "Blocked by required conditions"
114+
}
115+
116+
creator := user_model.NewActionsUser()
96117
if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{
97118
Repo: repo,
98119
SHA: sha,
99120
Creator: creator,
100121
CommitStatus: &git_model.CommitStatus{
101122
SHA: sha,
102123
TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), index),
103-
Description: "",
124+
Description: description,
104125
Context: ctxname,
105-
CreatorID: creatorID,
126+
CreatorID: creator.ID,
106127
State: state,
107128
},
108129
}); err != nil {
@@ -114,9 +135,9 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
114135

115136
func toCommitStatus(status actions_model.Status) api.CommitStatusState {
116137
switch status {
117-
case actions_model.StatusSuccess:
138+
case actions_model.StatusSuccess, actions_model.StatusSkipped:
118139
return api.CommitStatusSuccess
119-
case actions_model.StatusFailure, actions_model.StatusCancelled, actions_model.StatusSkipped:
140+
case actions_model.StatusFailure, actions_model.StatusCancelled:
120141
return api.CommitStatusFailure
121142
case actions_model.StatusWaiting, actions_model.StatusBlocked:
122143
return api.CommitStatusPending

‎services/actions/job_emitter.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ func jobEmitterQueueHandle(data ...queue.Data) []queue.Data {
4545
}
4646

4747
func checkJobsOfRun(ctx context.Context, runID int64) error {
48-
return db.WithTx(ctx, func(ctx context.Context) error {
49-
jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: runID})
50-
if err != nil {
51-
return err
52-
}
48+
jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: runID})
49+
if err != nil {
50+
return err
51+
}
52+
if err := db.WithTx(ctx, func(ctx context.Context) error {
5353
idToJobs := make(map[string][]*actions_model.ActionRunJob, len(jobs))
5454
for _, job := range jobs {
5555
idToJobs[job.JobID] = append(idToJobs[job.JobID], job)
@@ -67,7 +67,11 @@ func checkJobsOfRun(ctx context.Context, runID int64) error {
6767
}
6868
}
6969
return nil
70-
})
70+
}); err != nil {
71+
return err
72+
}
73+
CreateCommitStatus(ctx, jobs...)
74+
return nil
7175
}
7276

7377
type jobStatusResolver struct {

‎services/actions/notifier_helper.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,7 @@ func notify(ctx context.Context, input *notifyInput) error {
193193
if jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: run.ID}); err != nil {
194194
log.Error("FindRunJobs: %v", err)
195195
} else {
196-
for _, job := range jobs {
197-
if err := CreateCommitStatus(ctx, job); err != nil {
198-
log.Error("Update commit status for job %v failed: %v", job.ID, err)
199-
// go on
200-
}
201-
}
196+
CreateCommitStatus(ctx, jobs...)
202197
}
203198

204199
}

0 commit comments

Comments
 (0)