From a20c5c3681fa95d22a6c73c3a55b814aa3d10c90 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 6 Jul 2023 11:35:46 +0800 Subject: [PATCH 01/18] feat: implement auto-cancellation of concurrent jobs - Add a new function `CancelRunningJobs` to cancel all running jobs of a run - Update `FindRunOptions` struct to include `Ref` field and update its condition in `toConds` function - Implement auto cancellation of running jobs in the same workflow in `notify` function Signed-off-by: Bo-Yi Wu --- models/actions/run.go | 54 +++++++++++++++++++++++++++++ models/actions/run_list.go | 4 +++ services/actions/notifier_helper.go | 6 ++++ 3 files changed, 64 insertions(+) diff --git a/models/actions/run.go b/models/actions/run.go index 7b62ff884f4ca..5563fa1f6c832 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -164,6 +164,60 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err return err } +// CancelRunningJobs cancels all running jobs of a run +func CancelRunningJobs(ctx context.Context, run *ActionRun) error { + runs, _, err := FindRuns(ctx, FindRunOptions{ + RepoID: run.RepoID, + Ref: run.Ref, + Status: StatusRunning, + }) + if err != nil { + return err + } + + if len(runs) == 0 { + return nil + } + + for _, run := range runs { + run.Status = StatusCancelled + run.Stopped = timeutil.TimeStampNow() + if err := UpdateRun(ctx, run, []string{"status", "stopped"}...); err != nil { + return err + } + + // cancel all running jobs + jobs, _, err := FindRunJobs(ctx, FindRunJobOptions{ + RunID: run.ID, + }) + if err != nil { + return err + } + for _, job := range jobs { + status := job.Status + if status.IsDone() { + continue + } + if job.TaskID == 0 { + job.Status = StatusCancelled + job.Stopped = timeutil.TimeStampNow() + n, err := UpdateRunJob(ctx, job, builder.Eq{"task_id": 0}, "status", "stopped") + if err != nil { + return err + } + if n == 0 { + return fmt.Errorf("job has changed, try again") + } + continue + } + if err := StopTask(ctx, job.TaskID, StatusCancelled); err != nil { + return err + } + } + } + return nil +} + // InsertRun inserts a run func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWorkflow) error { ctx, commiter, err := db.TxContext(ctx) diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 29ab193d57f3e..2b1d3958175a1 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -69,6 +69,7 @@ type FindRunOptions struct { RepoID int64 OwnerID int64 WorkflowFileName string + Ref string TriggerUserID int64 Approved bool // not util.OptionalBool, it works only when it's true Status Status @@ -94,6 +95,9 @@ func (opts FindRunOptions) toConds() builder.Cond { if opts.Status > StatusUnknown { cond = cond.And(builder.Eq{"status": opts.Status}) } + if opts.Ref != "" { + cond = cond.And(builder.Eq{"ref": opts.Ref}) + } return cond } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 8e6cdcf680d1b..f1d73e5a51608 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -232,6 +232,12 @@ func notify(ctx context.Context, input *notifyInput) error { log.Error("jobparser.Parse: %v", err) continue } + + // auto cancel running jobs in the same workflow + if err := actions_model.CancelRunningJobs(ctx, run); err != nil { + log.Error("CancelRunningJobs: %v", err) + } + if err := actions_model.InsertRun(ctx, run, jobs); err != nil { log.Error("InsertRun: %v", err) continue From a1b22e22bc1b316a791fbfa87a558f7958044ce8 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Thu, 6 Jul 2023 11:43:15 +0800 Subject: [PATCH 02/18] refactor: refactor status field to support multiple values - Change the `Status` field in `CancelRunningJobs`, `FindRunOptions`, and `List` functions from a single `Status` to a slice of `Status`. - Update the condition in `toConds` function to check the length of `Status` slice instead of comparing a single `Status` value with `StatusUnknown`. - Modify the condition to use `builder.In` for matching multiple `Status` values instead of `builder.Eq` for a single `Status` value. Signed-off-by: Bo-Yi Wu --- models/actions/run.go | 2 +- models/actions/run_list.go | 6 +++--- routers/web/repo/actions/actions.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 5563fa1f6c832..389cddd77e855 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -169,7 +169,7 @@ func CancelRunningJobs(ctx context.Context, run *ActionRun) error { runs, _, err := FindRuns(ctx, FindRunOptions{ RepoID: run.RepoID, Ref: run.Ref, - Status: StatusRunning, + Status: []Status{StatusRunning, StatusWaiting}, }) if err != nil { return err diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 2b1d3958175a1..b90788152bc91 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -72,7 +72,7 @@ type FindRunOptions struct { Ref string TriggerUserID int64 Approved bool // not util.OptionalBool, it works only when it's true - Status Status + Status []Status } func (opts FindRunOptions) toConds() builder.Cond { @@ -92,8 +92,8 @@ func (opts FindRunOptions) toConds() builder.Cond { if opts.Approved { cond = cond.And(builder.Gt{"approved_by": 0}) } - if opts.Status > StatusUnknown { - cond = cond.And(builder.Eq{"status": opts.Status}) + if len(opts.Status) > 0 { + cond = cond.And(builder.In("status", opts.Status)) } if opts.Ref != "" { cond = cond.And(builder.Eq{"ref": opts.Ref}) diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index e1e07b5a72966..7de14af853ee1 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -146,7 +146,7 @@ func List(ctx *context.Context) { RepoID: ctx.Repo.Repository.ID, WorkflowFileName: workflow, TriggerUserID: actorID, - Status: actions_model.Status(status), + Status: []actions_model.Status{actions_model.Status(status)}, } runs, total, err := actions_model.FindRuns(ctx, opts) From f066eb06d3c4624c9f26d1bc428b9f15649ea229 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 7 Jul 2023 08:41:49 +0800 Subject: [PATCH 03/18] refactor: optimize job cancellation and action run indexing - Add an index to the `Ref` field in the `ActionRun` struct - Modify the `CancelRunningJobs` function to use `total` instead of `len(runs)` for checking if there are no runs. Signed-off-by: Bo-Yi Wu --- models/actions/run.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 389cddd77e855..f0d92b58d009b 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -34,7 +34,7 @@ type ActionRun struct { Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository TriggerUserID int64 `xorm:"index"` TriggerUser *user_model.User `xorm:"-"` - Ref string + Ref string `xorm:"index"` // the ref of the run CommitSHA string IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. NeedApproval bool // may need approval if it's a fork pull request @@ -166,7 +166,7 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err // CancelRunningJobs cancels all running jobs of a run func CancelRunningJobs(ctx context.Context, run *ActionRun) error { - runs, _, err := FindRuns(ctx, FindRunOptions{ + runs, total, err := FindRuns(ctx, FindRunOptions{ RepoID: run.RepoID, Ref: run.Ref, Status: []Status{StatusRunning, StatusWaiting}, @@ -175,7 +175,7 @@ func CancelRunningJobs(ctx context.Context, run *ActionRun) error { return err } - if len(runs) == 0 { + if total == 0 { return nil } From b64ed173414151681d3d17641ba3c109b9cc9a70 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 7 Jul 2023 09:24:58 +0800 Subject: [PATCH 04/18] refactor: refactor job cancellation process - Remove the code that sets the status of a run to cancelled and updates the run in `CancelRunningJobs` function. Signed-off-by: Bo-Yi Wu --- models/actions/run.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index f0d92b58d009b..47b6a922788a1 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -180,12 +180,6 @@ func CancelRunningJobs(ctx context.Context, run *ActionRun) error { } for _, run := range runs { - run.Status = StatusCancelled - run.Stopped = timeutil.TimeStampNow() - if err := UpdateRun(ctx, run, []string{"status", "stopped"}...); err != nil { - return err - } - // cancel all running jobs jobs, _, err := FindRunJobs(ctx, FindRunJobOptions{ RunID: run.ID, From 2db7378c1d81584018c297bbfa1109612b973e9d Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 12 Jul 2023 14:52:44 +0800 Subject: [PATCH 05/18] Update models/actions/run.go Co-authored-by: Jason Song --- models/actions/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run.go b/models/actions/run.go index 47b6a922788a1..755fdd270abc5 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -164,7 +164,7 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err return err } -// CancelRunningJobs cancels all running jobs of a run +// CancelRunningJobs cancels all running jobs of the runs which share the same repo and ref with the inputted run. func CancelRunningJobs(ctx context.Context, run *ActionRun) error { runs, total, err := FindRuns(ctx, FindRunOptions{ RepoID: run.RepoID, From a7ed00811b643777c273205614f4b81ad5bf7dea Mon Sep 17 00:00:00 2001 From: appleboy Date: Fri, 21 Jul 2023 21:02:26 +0800 Subject: [PATCH 06/18] refactor: refactor code to use `WorkflowID` instead of `WorkflowFileName` - Add `WorkflowID` to the `CancelRunningJobs` function in `run.go` - Replace `WorkflowFileName` with `WorkflowID` in `FindRunOptions` struct in `run_list.go` - Update conditions in `toConds` function to use `WorkflowID` instead of `WorkflowFileName` in `run_list.go` - Replace `WorkflowFileName` with `WorkflowID` in `List` function in `actions.go` Signed-off-by: appleboy --- models/actions/run.go | 7 ++++--- models/actions/run_list.go | 18 +++++++++--------- routers/web/repo/actions/actions.go | 8 ++++---- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 755fdd270abc5..d294a1f4b15fe 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -167,9 +167,10 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err // CancelRunningJobs cancels all running jobs of the runs which share the same repo and ref with the inputted run. func CancelRunningJobs(ctx context.Context, run *ActionRun) error { runs, total, err := FindRuns(ctx, FindRunOptions{ - RepoID: run.RepoID, - Ref: run.Ref, - Status: []Status{StatusRunning, StatusWaiting}, + RepoID: run.RepoID, + Ref: run.Ref, + WorkflowID: run.WorkflowID, + Status: []Status{StatusRunning, StatusWaiting}, }) if err != nil { return err diff --git a/models/actions/run_list.go b/models/actions/run_list.go index b90788152bc91..347970c931eff 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -66,13 +66,13 @@ func (runs RunList) LoadRepos() error { type FindRunOptions struct { db.ListOptions - RepoID int64 - OwnerID int64 - WorkflowFileName string - Ref string - TriggerUserID int64 - Approved bool // not util.OptionalBool, it works only when it's true - Status []Status + RepoID int64 + OwnerID int64 + WorkflowID string + Ref string + TriggerUserID int64 + Approved bool // not util.OptionalBool, it works only when it's true + Status []Status } func (opts FindRunOptions) toConds() builder.Cond { @@ -83,8 +83,8 @@ func (opts FindRunOptions) toConds() builder.Cond { if opts.OwnerID > 0 { cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } - if opts.WorkflowFileName != "" { - cond = cond.And(builder.Eq{"workflow_id": opts.WorkflowFileName}) + if opts.WorkflowID != "" { + cond = cond.And(builder.Eq{"workflow_id": opts.WorkflowID}) } if opts.TriggerUserID > 0 { cond = cond.And(builder.Eq{"trigger_user_id": opts.TriggerUserID}) diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 7de14af853ee1..24624575a4026 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -143,10 +143,10 @@ func List(ctx *context.Context) { Page: page, PageSize: convert.ToCorrectPageSize(ctx.FormInt("limit")), }, - RepoID: ctx.Repo.Repository.ID, - WorkflowFileName: workflow, - TriggerUserID: actorID, - Status: []actions_model.Status{actions_model.Status(status)}, + RepoID: ctx.Repo.Repository.ID, + WorkflowID: workflow, + TriggerUserID: actorID, + Status: []actions_model.Status{actions_model.Status(status)}, } runs, total, err := actions_model.FindRuns(ctx, opts) From 18cd66f4c0d574661e4a8dbf1b476de9e5400351 Mon Sep 17 00:00:00 2001 From: appleboy Date: Fri, 21 Jul 2023 21:08:10 +0800 Subject: [PATCH 07/18] refactor: refine job cancellation and notification logic - Modify the comment for the `CancelRunningJobs` function to specify that it also marks unstarted jobs as cancelled - Change the comment in `notify` function to specify that running jobs are cancelled only if the event is a push - Add a condition in `notify` function to cancel running jobs of the same workflow only if the event is a push Signed-off-by: appleboy --- models/actions/run.go | 3 ++- services/actions/notifier_helper.go | 9 ++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index d294a1f4b15fe..cc8df492035d8 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -164,7 +164,8 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err return err } -// CancelRunningJobs cancels all running jobs of the runs which share the same repo and ref with the inputted run. +// CancelRunningJobs cancels all running jobs of a run +// if the job is not started, it will be marked as cancelled func CancelRunningJobs(ctx context.Context, run *ActionRun) error { runs, total, err := FindRuns(ctx, FindRunOptions{ RepoID: run.RepoID, diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 29633fb622841..2b88ea69ddd99 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -231,9 +231,12 @@ func notify(ctx context.Context, input *notifyInput) error { continue } - // auto cancel running jobs in the same workflow - if err := actions_model.CancelRunningJobs(ctx, run); err != nil { - log.Error("CancelRunningJobs: %v", err) + // cancel running jobs if the event is push + if run.Event == webhook_module.HookEventPush { + // cancel running jobs of the same workflow + if err := actions_model.CancelRunningJobs(ctx, run); err != nil { + log.Error("CancelRunningJobs: %v", err) + } } if err := actions_model.InsertRun(ctx, run, jobs); err != nil { From 05f078d62a5649c7ce4adb219800b7ab6cdb6f55 Mon Sep 17 00:00:00 2001 From: appleboy Date: Fri, 21 Jul 2023 21:14:29 +0800 Subject: [PATCH 08/18] refactor: refactor job cancellation in workflows - Update the `CancelRunningJobs` function to cancel all running and waiting jobs associated with a specific workflow - Modify the function parameters to include `repoID`, `ref`, and `workflowID` - Add checks and conditions to handle different job states during cancellation - Update the call to `CancelRunningJobs` in `notifier_helper.go` to match the new function signature Signed-off-by: appleboy --- models/actions/run.go | 31 ++++++++++++++++++++++------- services/actions/notifier_helper.go | 7 ++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index cc8df492035d8..eb973b2e5dc45 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -164,53 +164,70 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err return err } -// CancelRunningJobs cancels all running jobs of a run -// if the job is not started, it will be marked as cancelled -func CancelRunningJobs(ctx context.Context, run *ActionRun) error { +// CancelRunningJobs cancels all running and waiting jobs associated with a specific workflow. +func CancelRunningJobs(ctx context.Context, repoID int64, ref string, workflowID string) error { + // Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'. runs, total, err := FindRuns(ctx, FindRunOptions{ - RepoID: run.RepoID, - Ref: run.Ref, - WorkflowID: run.WorkflowID, + RepoID: repoID, + Ref: ref, + WorkflowID: workflowID, Status: []Status{StatusRunning, StatusWaiting}, }) if err != nil { return err } + // If there are no runs found, there's no need to proceed with cancellation, so return nil. if total == 0 { return nil } + // Iterate over each found run and cancel its associated jobs. for _, run := range runs { - // cancel all running jobs + // Find all jobs associated with the current run. jobs, _, err := FindRunJobs(ctx, FindRunJobOptions{ RunID: run.ID, }) if err != nil { return err } + + // Iterate over each job and attempt to cancel it. for _, job := range jobs { + // Skip jobs that are already in a terminal state (completed, cancelled, etc.). status := job.Status if status.IsDone() { continue } + + // If the job has no associated task (probably an error), set its status to 'Cancelled' and stop it. if job.TaskID == 0 { job.Status = StatusCancelled job.Stopped = timeutil.TimeStampNow() + + // Update the job's status and stopped time in the database. n, err := UpdateRunJob(ctx, job, builder.Eq{"task_id": 0}, "status", "stopped") if err != nil { return err } + + // If the update affected 0 rows, it means the job has changed in the meantime, so we need to try again. if n == 0 { return fmt.Errorf("job has changed, try again") } + + // Continue with the next job. continue } + + // If the job has an associated task, try to stop the task, effectively cancelling the job. if err := StopTask(ctx, job.TaskID, StatusCancelled); err != nil { return err } } } + + // Return nil to indicate successful cancellation of all running and waiting jobs. return nil } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 2b88ea69ddd99..e08ef5382171a 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -234,7 +234,12 @@ func notify(ctx context.Context, input *notifyInput) error { // cancel running jobs if the event is push if run.Event == webhook_module.HookEventPush { // cancel running jobs of the same workflow - if err := actions_model.CancelRunningJobs(ctx, run); err != nil { + if err := actions_model.CancelRunningJobs( + ctx, + run.ID, + run.Ref, + run.WorkflowID, + ); err != nil { log.Error("CancelRunningJobs: %v", err) } } From 231e2372effcb9fe5dea92cc0f0fbd75f6f391d8 Mon Sep 17 00:00:00 2001 From: appleboy Date: Fri, 21 Jul 2023 21:21:33 +0800 Subject: [PATCH 09/18] refactor: improve code readability and test robustness - Refactor the `CancelRunningJobs` function parameters for better readability. Signed-off-by: appleboy --- models/actions/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run.go b/models/actions/run.go index eb973b2e5dc45..4a0e1cb2058e5 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -165,7 +165,7 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err } // CancelRunningJobs cancels all running and waiting jobs associated with a specific workflow. -func CancelRunningJobs(ctx context.Context, repoID int64, ref string, workflowID string) error { +func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string) error { // Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'. runs, total, err := FindRuns(ctx, FindRunOptions{ RepoID: repoID, From c542d1bf980891d20a82b98476a5075fe9a9be9d Mon Sep 17 00:00:00 2001 From: appleboy Date: Fri, 21 Jul 2023 23:08:09 +0800 Subject: [PATCH 10/18] feat: implement migration to update actions ref index - Add a new migration "Update Action Ref" to the migrations list - Create a new file `v267.go` in `models/migrations/v1_21` directory - Define a new function `UpdateActionsRefIndex` in `v267.go` to update the index of actions ref field - The `UpdateActionsRefIndex` function syncs a new instance of `ActionRun` struct which includes fields like `ID`, `Title`, `RepoID`, `OwnerID`, `WorkflowID`, `Index`, `TriggerUserID`, `Ref`, `CommitSHA`, `IsForkPullRequest`, `NeedApproval`, `ApprovedBy`, `Event`, `EventPayload`, `TriggerEvent`, `Status`, `Started`, `Stopped`, `Created`, `Updated`. Signed-off-by: appleboy --- models/migrations/migrations.go | 2 ++ models/migrations/v1_21/v267.go | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 models/migrations/v1_21/v267.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6599cb9cda3eb..d4b6fb0eb3c68 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -515,6 +515,8 @@ var migrations = []Migration{ NewMigration("Alter Actions Artifact table", v1_21.AlterActionArtifactTable), // v266 -> v267 NewMigration("Reduce commit status", v1_21.ReduceCommitStatus), + // v267 -> v268 + NewMigration("Update Action Ref", v1_21.UpdateActionsRefIndex), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v267.go b/models/migrations/v1_21/v267.go new file mode 100644 index 0000000000000..2927268f6c453 --- /dev/null +++ b/models/migrations/v1_21/v267.go @@ -0,0 +1,39 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint + +import ( + "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/timeutil" + webhook_module "code.gitea.io/gitea/modules/webhook" + + "xorm.io/xorm" +) + +// UpdateActionsRefIndex updates the index of actions ref field +func UpdateActionsRefIndex(x *xorm.Engine) error { + type ActionRun struct { + ID int64 + Title string + RepoID int64 `xorm:"index unique(repo_index)"` + OwnerID int64 `xorm:"index"` + WorkflowID string `xorm:"index"` // the name of workflow file + Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository + TriggerUserID int64 `xorm:"index"` + Ref string `xorm:"index"` // the ref of the run + CommitSHA string + IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. + NeedApproval bool // may need approval if it's a fork pull request + ApprovedBy int64 `xorm:"index"` // who approved + Event webhook_module.HookEventType // the webhook event that causes the workflow to run + EventPayload string `xorm:"LONGTEXT"` + TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow + Status actions.Status `xorm:"index"` + Started timeutil.TimeStamp + Stopped timeutil.TimeStamp + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` + } + return x.Sync(new(ActionRun)) +} From 2412e84755a93bba1043bb3d7ccc785e11ba35c2 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sat, 22 Jul 2023 09:29:13 +0800 Subject: [PATCH 11/18] refactor: refactor notifier helper and improve job handling - Refactor the `notify` function in `notifier_helper.go` to separate the job finding and error handling steps. - Replace the `jobs` variable with `alljobs` to better reflect its purpose. - Remove the else clause and use `continue` for better readability and flow control. - Call `CreateCommitStatus` with `alljobs` instead of `jobs`. Signed-off-by: appleboy --- services/actions/notifier_helper.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index e08ef5382171a..49d62813b94ad 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -248,12 +248,13 @@ func notify(ctx context.Context, input *notifyInput) error { log.Error("InsertRun: %v", err) continue } - if jobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: run.ID}); err != nil { + + alljobs, _, err := actions_model.FindRunJobs(ctx, actions_model.FindRunJobOptions{RunID: run.ID}) + if err != nil { log.Error("FindRunJobs: %v", err) - } else { - CreateCommitStatus(ctx, jobs...) + continue } - + CreateCommitStatus(ctx, alljobs...) } return nil } From 5d785961a96e2cfbe144b1437cc176b1c6366523 Mon Sep 17 00:00:00 2001 From: appleboy Date: Sat, 22 Jul 2023 11:20:54 +0800 Subject: [PATCH 12/18] refactor: refactor status filter handling in `actions.go` - Remove the status filter from the list function in `actions.go` - Add a check for `StatusUnknown` before applying the status filter in `actions.go` Signed-off-by: appleboy --- routers/web/repo/actions/actions.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 5d1505c390fbb..5a12f52dcd142 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -153,7 +153,11 @@ func List(ctx *context.Context) { RepoID: ctx.Repo.Repository.ID, WorkflowID: workflow, TriggerUserID: actorID, - Status: []actions_model.Status{actions_model.Status(status)}, + } + + // if status is not StatusUnknown, it means user has selected a status filter + if actions_model.Status(status) != actions_model.StatusUnknown { + opts.Status = []actions_model.Status{actions_model.Status(status)} } runs, total, err := actions_model.FindRuns(ctx, opts) From d7b577fc9cc3d540da108c1f277005215231f50c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Mon, 24 Jul 2023 12:17:25 +0800 Subject: [PATCH 13/18] Apply suggestions from code review --- models/migrations/v1_21/v267.go | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/models/migrations/v1_21/v267.go b/models/migrations/v1_21/v267.go index 2927268f6c453..1cd056eb17e03 100644 --- a/models/migrations/v1_21/v267.go +++ b/models/migrations/v1_21/v267.go @@ -4,36 +4,13 @@ package v1_21 //nolint import ( - "code.gitea.io/gitea/models/actions" - "code.gitea.io/gitea/modules/timeutil" - webhook_module "code.gitea.io/gitea/modules/webhook" - "xorm.io/xorm" ) // UpdateActionsRefIndex updates the index of actions ref field func UpdateActionsRefIndex(x *xorm.Engine) error { type ActionRun struct { - ID int64 - Title string - RepoID int64 `xorm:"index unique(repo_index)"` - OwnerID int64 `xorm:"index"` - WorkflowID string `xorm:"index"` // the name of workflow file - Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository - TriggerUserID int64 `xorm:"index"` - Ref string `xorm:"index"` // the ref of the run - CommitSHA string - IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. - NeedApproval bool // may need approval if it's a fork pull request - ApprovedBy int64 `xorm:"index"` // who approved - Event webhook_module.HookEventType // the webhook event that causes the workflow to run - EventPayload string `xorm:"LONGTEXT"` - TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow - Status actions.Status `xorm:"index"` - Started timeutil.TimeStamp - Stopped timeutil.TimeStamp - Created timeutil.TimeStamp `xorm:"created"` - Updated timeutil.TimeStamp `xorm:"updated"` + Ref string `xorm:"index"` // the ref of the run } return x.Sync(new(ActionRun)) } From e767742d83708f626bd40c033059a60b9fb96631 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Tue, 25 Jul 2023 07:23:16 +0800 Subject: [PATCH 14/18] Update models/actions/run.go Co-authored-by: delvh --- models/actions/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run.go b/models/actions/run.go index 4a0e1cb2058e5..3cb806e8c3e79 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -34,7 +34,7 @@ type ActionRun struct { Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository TriggerUserID int64 `xorm:"index"` TriggerUser *user_model.User `xorm:"-"` - Ref string `xorm:"index"` // the ref of the run + Ref string `xorm:"index"` // the commit/tag/… that caused the run CommitSHA string IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. NeedApproval bool // may need approval if it's a fork pull request From 9ab6ec4248f558603b6a177046546aa5c3b5e77f Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Tue, 25 Jul 2023 07:23:28 +0800 Subject: [PATCH 15/18] Update models/actions/run_list.go Co-authored-by: delvh --- models/actions/run_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 347970c931eff..db36f6df981d3 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -69,7 +69,7 @@ type FindRunOptions struct { RepoID int64 OwnerID int64 WorkflowID string - Ref string + Ref string // the commit/tag/… that caused this workflow TriggerUserID int64 Approved bool // not util.OptionalBool, it works only when it's true Status []Status From d649e95e1e72ea132e24af23a0b133e8268cb284 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Tue, 25 Jul 2023 07:23:54 +0800 Subject: [PATCH 16/18] Update models/migrations/v1_21/v267.go Co-authored-by: delvh --- models/migrations/v1_21/v267.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/v1_21/v267.go b/models/migrations/v1_21/v267.go index 1cd056eb17e03..332793ff073b8 100644 --- a/models/migrations/v1_21/v267.go +++ b/models/migrations/v1_21/v267.go @@ -10,7 +10,7 @@ import ( // UpdateActionsRefIndex updates the index of actions ref field func UpdateActionsRefIndex(x *xorm.Engine) error { type ActionRun struct { - Ref string `xorm:"index"` // the ref of the run + Ref string `xorm:"index"` // the commit/tag/… causing the run } return x.Sync(new(ActionRun)) } From face5383c7111d3201c03e506ecd92bc85dfd847 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Tue, 25 Jul 2023 10:07:43 +0800 Subject: [PATCH 17/18] Update services/actions/notifier_helper.go --- services/actions/notifier_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 49d62813b94ad..764d24a7dbc8b 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -236,7 +236,7 @@ func notify(ctx context.Context, input *notifyInput) error { // cancel running jobs of the same workflow if err := actions_model.CancelRunningJobs( ctx, - run.ID, + run.RepoID, run.Ref, run.WorkflowID, ); err != nil { From 8a05c675b8894dbf6f3748d15615ad0f5073e40a Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Tue, 25 Jul 2023 10:39:30 +0800 Subject: [PATCH 18/18] chore: update migration version and rename file - Update migration version from `v267` to `v269` - Rename migration file from `v267.go` to `v268.go` Signed-off-by: Bo-Yi Wu --- models/migrations/migrations.go | 2 +- models/migrations/v1_21/{v267.go => v268.go} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename models/migrations/v1_21/{v267.go => v268.go} (100%) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index d4b6fb0eb3c68..bb6d97d8583a6 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -515,7 +515,7 @@ var migrations = []Migration{ NewMigration("Alter Actions Artifact table", v1_21.AlterActionArtifactTable), // v266 -> v267 NewMigration("Reduce commit status", v1_21.ReduceCommitStatus), - // v267 -> v268 + // v268 -> v269 NewMigration("Update Action Ref", v1_21.UpdateActionsRefIndex), } diff --git a/models/migrations/v1_21/v267.go b/models/migrations/v1_21/v268.go similarity index 100% rename from models/migrations/v1_21/v267.go rename to models/migrations/v1_21/v268.go