From 51829d1db3fd4d361386471a2172169cddc0364f Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 15:13:24 +0800 Subject: [PATCH 1/7] fix: StatusSkipped as CommitStatusSuccess --- services/actions/commit_status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 6604a149a5c9..89d6bde42f9f 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -109,9 +109,9 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er func toCommitStatus(status actions_model.Status) api.CommitStatusState { switch status { - case actions_model.StatusSuccess: + case actions_model.StatusSuccess, actions_model.StatusSkipped: return api.CommitStatusSuccess - case actions_model.StatusFailure, actions_model.StatusCancelled, actions_model.StatusSkipped: + case actions_model.StatusFailure, actions_model.StatusCancelled: return api.CommitStatusFailure case actions_model.StatusWaiting, actions_model.StatusBlocked: return api.CommitStatusPending From 41874c3c4d2ef23dbbd3d3cc26a43a3a0dff99db Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 15:23:32 +0800 Subject: [PATCH 2/7] fix: gitea-actions as creater --- services/actions/commit_status.go | 36 +++++++------------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 89d6bde42f9f..0d3b228906c6 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -21,49 +21,31 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } run := job.Run + var ( - sha string - creatorID int64 + sha string ) - switch run.Event { case webhook_module.HookEventPush: payload, err := run.GetPushEventPayload() if err != nil { return fmt.Errorf("GetPushEventPayload: %w", err) } - - // Since the payload comes from json data, we should check if it's broken, or it will cause panic - switch { - case payload.Repo == nil: - return fmt.Errorf("repo is missing in event payload") - case payload.Pusher == nil: - return fmt.Errorf("pusher is missing in event payload") - case payload.HeadCommit == nil: + if payload.HeadCommit == nil { return fmt.Errorf("head commit is missing in event payload") } - sha = payload.HeadCommit.ID - creatorID = payload.Pusher.ID case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync: payload, err := run.GetPullRequestEventPayload() if err != nil { return fmt.Errorf("GetPullRequestEventPayload: %w", err) } - - switch { - case payload.PullRequest == nil: + if payload.PullRequest == nil { return fmt.Errorf("pull request is missing in event payload") - case payload.PullRequest.Head == nil: + } else if payload.PullRequest.Head == nil { return fmt.Errorf("head of pull request is missing in event payload") - case payload.PullRequest.Head.Repository == nil: - return fmt.Errorf("head repository of pull request is missing in event payload") - case payload.PullRequest.Head.Repository.Owner == nil: - return fmt.Errorf("owner of head repository of pull request is missing in evnt payload") } - sha = payload.PullRequest.Head.Sha - creatorID = payload.PullRequest.Head.Repository.Owner.ID default: return nil } @@ -71,14 +53,11 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er repo := run.Repo ctxname := job.Name state := toCommitStatus(job.Status) - creator, err := user_model.GetUserByID(ctx, creatorID) - if err != nil { - return fmt.Errorf("GetUserByID: %w", err) - } if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}); err == nil { for _, v := range statuses { if v.Context == ctxname { if v.State == state { + // no need to update return nil } break @@ -88,6 +67,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return fmt.Errorf("GetLatestCommitStatus: %w", err) } + creator := user_model.NewActionsUser() if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{ Repo: repo, SHA: sha, @@ -97,7 +77,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er TargetURL: run.Link(), Description: "", Context: ctxname, - CreatorID: creatorID, + CreatorID: creator.ID, State: state, }, }); err != nil { From 7d34054f95f58a4cb74808acd4df0352f04204cf Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 15:39:49 +0800 Subject: [PATCH 3/7] fix: new context --- services/actions/commit_status.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 0d3b228906c6..96a31f8483dd 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -6,6 +6,7 @@ package actions import ( "context" "fmt" + "path" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" @@ -13,6 +14,8 @@ import ( user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" + + "github.com/nektos/act/pkg/jobparser" ) func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error { @@ -23,10 +26,12 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er run := job.Run var ( - sha string + sha string + event string ) switch run.Event { case webhook_module.HookEventPush: + event = "push" payload, err := run.GetPushEventPayload() if err != nil { return fmt.Errorf("GetPushEventPayload: %w", err) @@ -36,6 +41,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } sha = payload.HeadCommit.ID case webhook_module.HookEventPullRequest, webhook_module.HookEventPullRequestSync: + event = "pull_request" payload, err := run.GetPullRequestEventPayload() if err != nil { return fmt.Errorf("GetPullRequestEventPayload: %w", err) @@ -51,7 +57,12 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } repo := run.Repo - ctxname := job.Name + // TODO: store workflow name as a field in ActionRun to avoid parsing + runName := path.Base(run.WorkflowID) + if wfs, err := jobparser.Parse(job.WorkflowPayload); err == nil && len(wfs) > 0 { + runName = wfs[0].Name + } + ctxname := fmt.Sprintf("%s / %s (%s)", runName, job.Name, event) state := toCommitStatus(job.Status) if statuses, _, err := git_model.GetLatestCommitStatus(ctx, repo.ID, sha, db.ListOptions{}); err == nil { for _, v := range statuses { From ad4e21b102df2aa485d7c2fe52aaa567241aaff3 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 15:50:44 +0800 Subject: [PATCH 4/7] fix: description --- services/actions/commit_status.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 96a31f8483dd..ae58c5e060bc 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -78,6 +78,25 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return fmt.Errorf("GetLatestCommitStatus: %w", err) } + description := "" + switch run.Status { + // TODO: if we want support description in different languages, we need to support i18n placeholders in it + case actions_model.StatusSuccess: + description = fmt.Sprintf("Successful in %s", job.Duration()) + case actions_model.StatusFailure: + description = fmt.Sprintf("Failing after %s", job.Duration()) + case actions_model.StatusCancelled: + description = "Has been cancelled" + case actions_model.StatusSkipped: + description = "Has been skipped" + case actions_model.StatusRunning: + description = "Has started running" + case actions_model.StatusWaiting: + description = "Waiting to run" + case actions_model.StatusBlocked: + description = "Blocked by required conditions" + } + creator := user_model.NewActionsUser() if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{ Repo: repo, @@ -86,7 +105,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er CommitStatus: &git_model.CommitStatus{ SHA: sha, TargetURL: run.Link(), - Description: "", + Description: description, Context: ctxname, CreatorID: creator.ID, State: state, From 26de65d82b3c0b1093bfbd9625d50844a7c77b87 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 15:56:38 +0800 Subject: [PATCH 5/7] fix: use job status --- services/actions/commit_status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index ae58c5e060bc..b8c2eaacca8a 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -79,7 +79,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er } description := "" - switch run.Status { + switch job.Status { // TODO: if we want support description in different languages, we need to support i18n placeholders in it case actions_model.StatusSuccess: description = fmt.Sprintf("Successful in %s", job.Duration()) From 458c4c59844386e5e15a4b0c790382c23a1b001d Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 16:24:24 +0800 Subject: [PATCH 6/7] fix: update commit status --- routers/api/actions/runner/utils.go | 6 ++++++ routers/web/repo/actions/view.go | 7 +++++++ services/actions/job_emitter.go | 22 ++++++++++++++++------ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index 80e71941f21c..49e5e73c56c1 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/log" secret_module "code.gitea.io/gitea/modules/secret" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/actions" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "google.golang.org/protobuf/types/known/structpb" @@ -27,6 +28,11 @@ func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv return nil, false, nil } + if err := actions.CreateCommitStatus(ctx, t.Job); err != nil { + log.Error("Update commit status for job %v failed: %v", t.Job.ID, err) + // go on + } + task := &runnerv1.Task{ Id: t.ID, WorkflowPayload: t.Job.WorkflowPayload, diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 0fa255b7e65c..c025f6a32403 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -349,6 +349,13 @@ func Approve(ctx *context_module.Context) { return } + for _, job := range jobs { + if err := actions_service.CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + } + ctx.JSON(http.StatusOK, struct{}{}) } diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index cb2cc8d1ac06..7e6272052520 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -11,6 +11,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/graceful" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/queue" "xorm.io/builder" @@ -45,11 +46,11 @@ func jobEmitterQueueHandle(data ...queue.Data) []queue.Data { } func checkJobsOfRun(ctx context.Context, runID int64) error { - return db.WithTx(ctx, func(ctx context.Context) error { - jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: runID}) - if err != nil { - return err - } + jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: runID}) + if err != nil { + return err + } + if err := db.WithTx(ctx, func(ctx context.Context) error { idToJobs := make(map[string][]*actions_model.ActionRunJob, len(jobs)) for _, job := range jobs { idToJobs[job.JobID] = append(idToJobs[job.JobID], job) @@ -67,7 +68,16 @@ func checkJobsOfRun(ctx context.Context, runID int64) error { } } return nil - }) + }); err != nil { + return err + } + for _, job := range jobs { + if err := CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + } + return nil } type jobStatusResolver struct { From ae167c795b3d264f6b7c2cb9f0087c3bf41c2707 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 29 Mar 2023 16:33:27 +0800 Subject: [PATCH 7/7] refactor: CreateCommitStatus --- routers/api/actions/runner/runner.go | 5 +---- routers/api/actions/runner/utils.go | 5 +---- routers/web/repo/actions/view.go | 20 +++----------------- services/actions/clear_tasks.go | 12 ++---------- services/actions/commit_status.go | 13 ++++++++++++- services/actions/job_emitter.go | 8 +------- services/actions/notifier_helper.go | 7 +------ 7 files changed, 21 insertions(+), 49 deletions(-) diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 07657c91207e..a44586485893 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -149,10 +149,7 @@ func (s *Service) UpdateTask( return nil, status.Errorf(codes.Internal, "load job: %v", err) } - if err := actions_service.CreateCommitStatus(ctx, task.Job); err != nil { - log.Error("Update commit status for job %v failed: %v", task.Job.ID, err) - // go on - } + actions_service.CreateCommitStatus(ctx, task.Job) if req.Msg.State.Result != runnerv1.Result_RESULT_UNSPECIFIED { if err := actions_service.EmitJobsIfReady(task.Job.RunID); err != nil { diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index 49e5e73c56c1..a022445ff38a 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -28,10 +28,7 @@ func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv return nil, false, nil } - if err := actions.CreateCommitStatus(ctx, t.Job); err != nil { - log.Error("Update commit status for job %v failed: %v", t.Job.ID, err) - // go on - } + actions.CreateCommitStatus(ctx, t.Job) task := &runnerv1.Task{ Id: t.ID, diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index c025f6a32403..b2b625ea23ee 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -16,7 +16,6 @@ import ( "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/base" context_module "code.gitea.io/gitea/modules/context" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -264,10 +263,7 @@ func Rerun(ctx *context_module.Context) { return } - if err := actions_service.CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } + actions_service.CreateCommitStatus(ctx, job) ctx.JSON(http.StatusOK, struct{}{}) } @@ -308,12 +304,7 @@ func Cancel(ctx *context_module.Context) { return } - for _, job := range jobs { - if err := actions_service.CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } - } + actions_service.CreateCommitStatus(ctx, jobs...) ctx.JSON(http.StatusOK, struct{}{}) } @@ -349,12 +340,7 @@ func Approve(ctx *context_module.Context) { return } - for _, job := range jobs { - if err := actions_service.CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } - } + actions_service.CreateCommitStatus(ctx, jobs...) ctx.JSON(http.StatusOK, struct{}{}) } diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index 6f8e95218d67..0616a5fc0dcb 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -64,12 +64,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { } } - for _, job := range jobs { - if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } - } + CreateCommitStatus(ctx, jobs...) return nil } @@ -96,10 +91,7 @@ func CancelAbandonedJobs(ctx context.Context) error { log.Warn("cancel abandoned job %v: %v", job.ID, err) // go on } - if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } + CreateCommitStatus(ctx, job) } return nil diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index b8c2eaacca8a..984c41295677 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -12,13 +12,24 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" "github.com/nektos/act/pkg/jobparser" ) -func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error { +// CreateCommitStatus creates a commit status for the given job. +// It won't return an error failed, but will log it, because it's not critical. +func CreateCommitStatus(ctx context.Context, jobs ...*actions_model.ActionRunJob) { + for _, job := range jobs { + if err := createCommitStatus(ctx, job); err != nil { + log.Error("Failed to create commit status for job %d: %v", job.ID, err) + } + } +} + +func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) error { if err := job.LoadAttributes(ctx); err != nil { return fmt.Errorf("load run: %w", err) } diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index 7e6272052520..c6b6fc551e00 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -11,7 +11,6 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/graceful" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/queue" "xorm.io/builder" @@ -71,12 +70,7 @@ func checkJobsOfRun(ctx context.Context, runID int64) error { }); err != nil { return err } - for _, job := range jobs { - if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } - } + CreateCommitStatus(ctx, jobs...) return nil } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 574a37e9ab16..b0e199fc6bd4 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -185,12 +185,7 @@ func notify(ctx context.Context, input *notifyInput) error { if jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: run.ID}); err != nil { log.Error("FindRunJobs: %v", err) } else { - for _, job := range jobs { - if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("Update commit status for job %v failed: %v", job.ID, err) - // go on - } - } + CreateCommitStatus(ctx, jobs...) } }