Skip to content

Commit

Permalink
Fix schedule tasks bugs (go-gitea#28691)
Browse files Browse the repository at this point in the history
Fix go-gitea#28157 

This PR fix the possible bugs about actions schedule.

## The Changes

- Move `UpdateRepositoryUnit` and `SetRepoDefaultBranch` from models to
service layer
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when actions unit has been disabled
or global disabled.
- Remove schedules plan from database and cancel waiting & running
schedules tasks in this repository when default branch changed.
  • Loading branch information
lunny authored Jan 12, 2024
1 parent 6c68239 commit 97292da
Show file tree
Hide file tree
Showing 19 changed files with 204 additions and 88 deletions.
11 changes: 6 additions & 5 deletions models/actions/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,14 @@ 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, workflowID string) error {
func CancelRunningJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error {
// Find all runs in the specified repository, reference, and workflow with statuses 'Running' or 'Waiting'.
runs, total, err := db.FindAndCount[ActionRun](ctx, FindRunOptions{
RepoID: repoID,
Ref: ref,
WorkflowID: workflowID,
Status: []Status{StatusRunning, StatusWaiting},
RepoID: repoID,
Ref: ref,
WorkflowID: workflowID,
TriggerEvent: event,
Status: []Status{StatusRunning, StatusWaiting},
})
if err != nil {
return err
Expand Down
5 changes: 5 additions & 0 deletions models/actions/run_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/container"
webhook_module "code.gitea.io/gitea/modules/webhook"

"xorm.io/builder"
)
Expand Down Expand Up @@ -71,6 +72,7 @@ type FindRunOptions struct {
WorkflowID string
Ref string // the commit/tag/… that caused this workflow
TriggerUserID int64
TriggerEvent webhook_module.HookEventType
Approved bool // not util.OptionalBool, it works only when it's true
Status []Status
}
Expand Down Expand Up @@ -98,6 +100,9 @@ func (opts FindRunOptions) ToConds() builder.Cond {
if opts.Ref != "" {
cond = cond.And(builder.Eq{"ref": opts.Ref})
}
if opts.TriggerEvent != "" {
cond = cond.And(builder.Eq{"trigger_event": opts.TriggerEvent})
}
return cond
}

Expand Down
20 changes: 20 additions & 0 deletions models/actions/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package actions

import (
"context"
"fmt"
"time"

"code.gitea.io/gitea/models/db"
Expand Down Expand Up @@ -118,3 +119,22 @@ func DeleteScheduleTaskByRepo(ctx context.Context, id int64) error {

return committer.Commit()
}

func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) error {
// If actions disabled when there is schedule task, this will remove the outdated schedule tasks
// There is no other place we can do this because the app.ini will be changed manually
if err := DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil {
return fmt.Errorf("DeleteCronTaskByRepo: %v", err)
}
// cancel running cron jobs of this repository and delete old schedules
if err := CancelRunningJobs(
ctx,
repo.ID,
repo.DefaultBranch,
"",
webhook_module.HookEventSchedule,
); err != nil {
return fmt.Errorf("CancelRunningJobs: %v", err)
}
return nil
}
4 changes: 2 additions & 2 deletions models/git/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch *
}

// RenameBranch rename a branch
func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(isDefault bool) error) (err error) {
func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to string, gitAction func(ctx context.Context, isDefault bool) error) (err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return err
Expand Down Expand Up @@ -358,7 +358,7 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, from, to str
}

// 5. do git action
if err = gitAction(isDefault); err != nil {
if err = gitAction(ctx, isDefault); err != nil {
return err
}

Expand Down
3 changes: 2 additions & 1 deletion models/git/branch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package git_test

import (
"context"
"testing"

"code.gitea.io/gitea/models/db"
Expand Down Expand Up @@ -132,7 +133,7 @@ func TestRenameBranch(t *testing.T) {
}, git_model.WhitelistOptions{}))
assert.NoError(t, committer.Commit())

assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(isDefault bool) error {
assert.NoError(t, git_model.RenameBranch(db.DefaultContext, repo1, "master", "main", func(ctx context.Context, isDefault bool) error {
_isDefault = isDefault
return nil
}))
Expand Down
26 changes: 0 additions & 26 deletions models/repo/repo_unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,29 +283,3 @@ func UpdateRepoUnit(ctx context.Context, unit *RepoUnit) error {
_, err := db.GetEngine(ctx).ID(unit.ID).Update(unit)
return err
}

// UpdateRepositoryUnits updates a repository's units
func UpdateRepositoryUnits(ctx context.Context, repo *Repository, units []RepoUnit, deleteUnitTypes []unit.Type) (err error) {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return err
}
defer committer.Close()

// Delete existing settings of units before adding again
for _, u := range units {
deleteUnitTypes = append(deleteUnitTypes, u.Type)
}

if _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).In("type", deleteUnitTypes).Delete(new(RepoUnit)); err != nil {
return err
}

if len(units) > 0 {
if err = db.Insert(ctx, units); err != nil {
return err
}
}

return committer.Commit()
}
4 changes: 4 additions & 0 deletions modules/actions/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const (
GithubEventRelease = "release"
GithubEventPullRequestComment = "pull_request_comment"
GithubEventGollum = "gollum"
GithubEventSchedule = "schedule"
)

// canGithubEventMatch check if the input Github event can match any Gitea event.
Expand Down Expand Up @@ -69,6 +70,9 @@ func canGithubEventMatch(eventName string, triggedEvent webhook_module.HookEvent
return false
}

case GithubEventSchedule:
return triggedEvent == webhook_module.HookEventSchedule

default:
return eventName == string(triggedEvent)
}
Expand Down
24 changes: 14 additions & 10 deletions modules/actions/workflows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

type DetectedWorkflow struct {
EntryName string
TriggerEvent string
TriggerEvent *jobparser.Event
Content []byte
}

Expand Down Expand Up @@ -100,6 +100,7 @@ func DetectWorkflows(
commit *git.Commit,
triggedEvent webhook_module.HookEventType,
payload api.Payloader,
detectSchedule bool,
) ([]*DetectedWorkflow, []*DetectedWorkflow, error) {
entries, err := ListWorkflows(commit)
if err != nil {
Expand All @@ -114,6 +115,7 @@ func DetectWorkflows(
return nil, nil, err
}

// one workflow may have multiple events
events, err := GetEventsFromContent(content)
if err != nil {
log.Warn("ignore invalid workflow %q: %v", entry.Name(), err)
Expand All @@ -122,17 +124,18 @@ func DetectWorkflows(
for _, evt := range events {
log.Trace("detect workflow %q for event %#v matching %q", entry.Name(), evt, triggedEvent)
if evt.IsSchedule() {
dwf := &DetectedWorkflow{
EntryName: entry.Name(),
TriggerEvent: evt.Name,
Content: content,
if detectSchedule {
dwf := &DetectedWorkflow{
EntryName: entry.Name(),
TriggerEvent: evt,
Content: content,
}
schedules = append(schedules, dwf)
}
schedules = append(schedules, dwf)
}
if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
} else if detectMatched(gitRepo, commit, triggedEvent, payload, evt) {
dwf := &DetectedWorkflow{
EntryName: entry.Name(),
TriggerEvent: evt.Name,
TriggerEvent: evt,
Content: content,
}
workflows = append(workflows, dwf)
Expand All @@ -153,7 +156,8 @@ func detectMatched(gitRepo *git.Repository, commit *git.Commit, triggedEvent web
webhook_module.HookEventCreate,
webhook_module.HookEventDelete,
webhook_module.HookEventFork,
webhook_module.HookEventWiki:
webhook_module.HookEventWiki,
webhook_module.HookEventSchedule:
if len(evt.Acts()) != 0 {
log.Warn("Ignore unsupported %s event arguments %v", triggedEvent, evt.Acts())
}
Expand Down
7 changes: 7 additions & 0 deletions modules/actions/workflows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ func TestDetectMatched(t *testing.T) {
yamlOn: "on: gollum",
expected: true,
},
{
desc: "HookEventSchedue(schedule) matches GithubEventSchedule(schedule)",
triggedEvent: webhook_module.HookEventSchedule,
payload: nil,
yamlOn: "on: schedule",
expected: true,
},
}

for _, tc := range testCases {
Expand Down
1 change: 1 addition & 0 deletions modules/webhook/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
HookEventRepository HookEventType = "repository"
HookEventRelease HookEventType = "release"
HookEventPackage HookEventType = "package"
HookEventSchedule HookEventType = "schedule"
)

// Event returns the HookEventType as an event string
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
}

if len(units)+len(deleteUnitTypes) > 0 {
if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
return err
}
Expand Down
26 changes: 8 additions & 18 deletions routers/web/repo/setting/default_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ package setting
import (
"net/http"

repo_model "code.gitea.io/gitea/models/repo"
git_model "code.gitea.io/gitea/models/git"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/routers/web/repo"
notify_service "code.gitea.io/gitea/services/notify"
repo_service "code.gitea.io/gitea/services/repository"
)

// SetDefaultBranchPost set default branch
Expand All @@ -35,23 +34,14 @@ func SetDefaultBranchPost(ctx *context.Context) {
}

branch := ctx.FormString("branch")
if !ctx.Repo.GitRepo.IsBranchExist(branch) {
ctx.Status(http.StatusNotFound)
return
} else if repo.DefaultBranch != branch {
repo.DefaultBranch = branch
if err := ctx.Repo.GitRepo.SetDefaultBranch(branch); err != nil {
if !git.IsErrUnsupportedVersion(err) {
ctx.ServerError("SetDefaultBranch", err)
return
}
}
if err := repo_model.UpdateDefaultBranch(ctx, repo); err != nil {
if err := repo_service.SetRepoDefaultBranch(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, branch); err != nil {
switch {
case git_model.IsErrBranchNotExist(err):
ctx.Status(http.StatusNotFound)
default:
ctx.ServerError("SetDefaultBranch", err)
return
}

notify_service.ChangeDefaultBranch(ctx, repo)
return
}

log.Trace("Repository basic settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name)
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ func SettingsPost(ctx *context.Context) {
return
}

if err := repo_model.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
if err := repo_service.UpdateRepositoryUnits(ctx, repo, units, deleteUnitTypes); err != nil {
ctx.ServerError("UpdateRepositoryUnits", err)
return
}
Expand Down
Loading

0 comments on commit 97292da

Please sign in to comment.