]> source.dussan.org Git - gitea.git/commitdiff
Refactor commit status for Actions jobs (#23786)
authorJason Song <i@wolfogre.com>
Wed, 29 Mar 2023 15:27:37 +0000 (23:27 +0800)
committerGitHub <noreply@github.com>
Wed, 29 Mar 2023 15:27:37 +0000 (11:27 -0400)
Before:
<img width="353" alt="xnip_230329_163852"
src="https://user-images.githubusercontent.com/9418365/228479807-424452df-10fa-45cf-ae4b-09939c0ed54c.png">
After:
<img width="508" alt="xnip_230329_163358"
src="https://user-images.githubusercontent.com/9418365/228479923-537b54fe-9564-4105-a068-bcc75fa2a7ea.png">

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.

routers/api/actions/runner/runner.go
routers/api/actions/runner/utils.go
routers/web/repo/actions/view.go
services/actions/clear_tasks.go
services/actions/commit_status.go
services/actions/job_emitter.go
services/actions/notifier_helper.go

index 07657c91207e9f27e13fe106dce298c71328bbc4..a445864858931db1429ebfe1d3d1601a79a0a3f7 100644 (file)
@@ -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 {
index 80e71941f21c0830fbcd729e037f850413a83810..a022445ff38a93ff94cacea700036185d1002aa8 100644 (file)
@@ -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,8 @@ func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv
                return nil, false, nil
        }
 
+       actions.CreateCommitStatus(ctx, t.Job)
+
        task := &runnerv1.Task{
                Id:              t.ID,
                WorkflowPayload: t.Job.WorkflowPayload,
index 0fa255b7e65c97d52d135c9b2c0ca3f4e92d640e..b2b625ea23ee809a658ee22f8635295ec60e4e5e 100644 (file)
@@ -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,6 +340,8 @@ func Approve(ctx *context_module.Context) {
                return
        }
 
+       actions_service.CreateCommitStatus(ctx, jobs...)
+
        ctx.JSON(http.StatusOK, struct{}{})
 }
 
index 6f8e95218d678c8c8b67beac2c72d32d998c30a2..0616a5fc0dcb2bfabbdad4e8fde2fdd69bab85be 100644 (file)
@@ -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
index 6604a149a5c933f57f417dbe2408e4234b696b4c..984c41295677e839303cb5ea2db5be866f72c04e 100644 (file)
@@ -6,79 +6,80 @@ package actions
 import (
        "context"
        "fmt"
+       "path"
 
        actions_model "code.gitea.io/gitea/models/actions"
        "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)
        }
 
        run := job.Run
+
        var (
-               sha       string
-               creatorID int64
+               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)
                }
-
-               // 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:
+               event = "pull_request"
                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
        }
 
        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)
+       // 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 {
                        if v.Context == ctxname {
                                if v.State == state {
+                                       // no need to update
                                        return nil
                                }
                                break
@@ -88,6 +89,26 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
                return fmt.Errorf("GetLatestCommitStatus: %w", err)
        }
 
+       description := ""
+       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())
+       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,
                SHA:     sha,
@@ -95,9 +116,9 @@ 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:   creatorID,
+                       CreatorID:   creator.ID,
                        State:       state,
                },
        }); err != nil {
@@ -109,9 +130,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
index cb2cc8d1ac0641c2ca269d7403fab38c4e0dd217..c6b6fc551e00bf9f28757a5267517ba1c03cb429 100644 (file)
@@ -45,11 +45,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 +67,11 @@ func checkJobsOfRun(ctx context.Context, runID int64) error {
                        }
                }
                return nil
-       })
+       }); err != nil {
+               return err
+       }
+       CreateCommitStatus(ctx, jobs...)
+       return nil
 }
 
 type jobStatusResolver struct {
index 574a37e9ab168bcca3aabbcaab8d6683d10c3db2..b0e199fc6bd4e59283c6c8a279bfada9b28f516c 100644 (file)
@@ -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...)
                }
 
        }