diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 79679203489..9047155c859 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -11,6 +11,7 @@ import ( "gitea.dev/models/db" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" webhook_module "gitea.dev/modules/webhook" @@ -110,7 +111,7 @@ func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) ([ jobs, err := CancelPreviousJobs( ctx, repo.ID, - repo.DefaultBranch, + git.RefNameFromBranch(repo.DefaultBranch).String(), "", webhook_module.HookEventSchedule, ) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 871b3582088..73f93a9c3cc 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -717,7 +717,9 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err // Default branch only updated if changed and exist or the repository is empty updateRepoLicense := false + oldDefaultBranch := "" if opts.DefaultBranch != nil && repo.DefaultBranch != *opts.DefaultBranch && (repo.IsEmpty || git.IsBranchExist(ctx, ctx.Repo.Repository, *opts.DefaultBranch)) { + oldDefaultBranch = repo.DefaultBranch repo.DefaultBranch = *opts.DefaultBranch if !repo.IsEmpty { if err := git.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { @@ -733,6 +735,10 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err return err } + if oldDefaultBranch != "" { + actions_service.SwitchSchedulesToDefaultBranch(ctx, repo, oldDefaultBranch) + } + if visibilityChanged { audit.Record(ctx, audit_model.RepositoryVisibility, repo, "visibility", repo.IsPrivate) } diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index a2fe4bc1b76..a1389a54957 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -12,7 +12,9 @@ import ( actions_model "gitea.dev/models/actions" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" + unit_model "gitea.dev/models/unit" "gitea.dev/modules/actions" + "gitea.dev/modules/git" "gitea.dev/modules/log" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" @@ -62,6 +64,22 @@ func CleanRepoScheduleTasks(ctx context.Context, repo *repo_model.Repository) er return err } +// SwitchSchedulesToDefaultBranch cancels the running schedule runs of the old default branch +// and replaces the schedules with the ones on the current default branch +func SwitchSchedulesToDefaultBranch(ctx context.Context, repo *repo_model.Repository, oldDefaultBranch string) { + if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + log.Error("DeleteScheduleTaskByRepo: %v", err) + } + if err := CancelPreviousJobs(ctx, repo.ID, git.RefNameFromBranch(oldDefaultBranch).String(), "", webhook_module.HookEventSchedule); err != nil { + log.Error("CancelPreviousJobs: %v", err) + } + if repo.UnitEnabled(ctx, unit_model.TypeActions) { + if err := DetectAndHandleSchedules(ctx, repo); err != nil { + log.Error("DetectAndHandleSchedules: %v", err) + } + } +} + func shouldBlockJobByConcurrency(ctx context.Context, job *actions_model.ActionRunJob) (bool, error) { if job.RawConcurrency != "" && !job.IsConcurrencyEvaluated { // when the job depends on other jobs, we cannot evaluate its concurrency, so it should be blocked and will be evaluated again when its dependencies are done diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 6b8ac63814e..7bfee941963 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -577,8 +577,9 @@ func handleSchedules( log.Error("CountSchedules: %v", err) return err } else if count > 0 { - if err := CleanRepoScheduleTasks(ctx, input.Repo); err != nil { - log.Error("CleanRepoScheduleTasks: %v", err) + // replacing the schedules leaves their in-progress runs alone, as GitHub does + if err := actions_model.DeleteScheduleTaskByRepo(ctx, input.Repo.ID); err != nil { + log.Error("DeleteScheduleTaskByRepo: %v", err) } } diff --git a/services/repository/branch.go b/services/repository/branch.go index 0598e7c83a4..73988a093ca 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -9,7 +9,6 @@ import ( "fmt" "strings" - actions_model "gitea.dev/models/actions" "gitea.dev/models/db" git_model "gitea.dev/models/git" issues_model "gitea.dev/models/issues" @@ -28,7 +27,6 @@ import ( "gitea.dev/modules/reqctx" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" - webhook_module "gitea.dev/modules/webhook" actions_service "gitea.dev/services/actions" notify_service "gitea.dev/services/notify" release_service "gitea.dev/services/release" @@ -458,21 +456,6 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m } if isDefault { - // if default branch changed, we need to delete all schedules and cron jobs - if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { - log.Error("DeleteCronTaskByRepo: %v", err) - } - // cancel running cron jobs of this repository and delete old schedules - if err := actions_service.CancelPreviousJobs( - ctx, - repo.ID, - from, - "", - webhook_module.HookEventSchedule, - ); err != nil { - log.Error("CancelPreviousJobs: %v", err) - } - err2 = git.SetDefaultBranch(ctx, repo, to) if err2 != nil { return err2 @@ -484,6 +467,10 @@ func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_m return "", err } + if isDefault { + actions_service.SwitchSchedulesToDefaultBranch(ctx, repo, from) + } + notify_service.DeleteRef(ctx, doer, repo, git.RefNameFromBranch(from)) notify_service.CreateRef(ctx, doer, repo, git.RefNameFromBranch(to), fromBranch.CommitID) @@ -709,25 +696,13 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, newB return err } - if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { - log.Error("DeleteCronTaskByRepo: %v", err) - } - // cancel running cron jobs of this repository and delete old schedules - if err := actions_service.CancelPreviousJobs( - ctx, - repo.ID, - oldDefaultBranchName, - "", - webhook_module.HookEventSchedule, - ); err != nil { - log.Error("CancelPreviousJobs: %v", err) - } - return git.SetDefaultBranch(ctx, repo, newBranchName) }); err != nil { return err } + actions_service.SwitchSchedulesToDefaultBranch(ctx, repo, oldDefaultBranchName) + if !repo.IsEmpty { if err := AddRepoToLicenseUpdaterQueue(&LicenseUpdaterOptions{ RepoID: repo.ID, diff --git a/services/repository/setting.go b/services/repository/setting.go index 5182c9b20ec..da828c77a42 100644 --- a/services/repository/setting.go +++ b/services/repository/setting.go @@ -7,6 +7,7 @@ import ( "context" "slices" + actions_model "gitea.dev/models/actions" "gitea.dev/models/db" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" @@ -16,20 +17,19 @@ import ( // UpdateRepositoryUnits updates a repository's units func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, units []repo_model.RepoUnit, deleteUnitTypes []unit.Type) (err error) { - return db.WithTx(ctx, func(ctx context.Context) error { + disableActions := slices.Contains(deleteUnitTypes, unit.TypeActions) + + err = db.WithTx(ctx, func(ctx context.Context) error { // Delete existing settings of units before adding again for _, u := range units { deleteUnitTypes = append(deleteUnitTypes, u.Type) } - if slices.Contains(deleteUnitTypes, unit.TypeActions) { - if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil { - log.Error("CleanRepoScheduleTasks: %v", err) - } - } - for _, u := range units { if u.Type == unit.TypeActions { + if err := actions_model.DeleteScheduleTaskByRepo(ctx, repo.ID); err != nil { + log.Error("DeleteScheduleTaskByRepo: %v", err) + } if err := actions_service.DetectAndHandleSchedules(ctx, repo); err != nil { log.Error("DetectAndHandleSchedules: %v", err) } @@ -49,4 +49,15 @@ func UpdateRepositoryUnits(ctx context.Context, repo *repo_model.Repository, uni return nil }) + if err != nil { + return err + } + + // only disabling stops in-progress schedule runs + if disableActions { + if err := actions_service.CleanRepoScheduleTasks(ctx, repo); err != nil { + log.Error("CleanRepoScheduleTasks: %v", err) + } + } + return nil } diff --git a/tests/integration/actions_schedule_test.go b/tests/integration/actions_schedule_test.go index 230a8a41175..c6aa79cf34a 100644 --- a/tests/integration/actions_schedule_test.go +++ b/tests/integration/actions_schedule_test.go @@ -4,6 +4,7 @@ package integration import ( + "net/http" "net/url" "strconv" "strings" @@ -17,13 +18,16 @@ import ( unit_model "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/git" "gitea.dev/modules/migration" api "gitea.dev/modules/structs" + actions_service "gitea.dev/services/actions" mirror_service "gitea.dev/services/mirror" repo_service "gitea.dev/services/repository" files_service "gitea.dev/services/repository/files" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestScheduleUpdate(t *testing.T) { @@ -32,15 +36,17 @@ func TestScheduleUpdate(t *testing.T) { t.Run("DisableAndEnableActionsUnit", testScheduleUpdateDisableAndEnableActionsUnit) t.Run("ArchiveAndUnarchive", testScheduleUpdateArchiveAndUnarchive) t.Run("MirrorSync", testScheduleUpdateMirrorSync) + t.Run("ChangeDefaultBranch", testScheduleUpdateChangeDefaultBranch) + t.Run("RenameDefaultBranch", testScheduleUpdateRenameDefaultBranch) } func testScheduleUpdatePush(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { + run := createScheduleRun(t, repo) newCron := "30 5 * * 1,3" pushScheduleChange(t, u, repo, newCron) - branch, err := git_model.GetBranchExisting(t.Context(), repo.ID, repo.DefaultBranch) - assert.NoError(t, err) - return branch.CommitID, newCron + assertRunStatus(t, run, actions_model.StatusWaiting) + return newCron }) } @@ -68,7 +74,7 @@ jobs: for _, mergeStyle := range mergeStyles { t.Run(string(mergeStyle), func(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { // update workflow file _, err := files_service.ChangeRepoFiles(t.Context(), repo, user, &files_service.ChangeRepoFilesOptions{ NewBranch: newBranchName, @@ -91,15 +97,13 @@ jobs: testPullMerge(t, testContext.Session, repo.OwnerName, repo.Name, strconv.FormatInt(apiPull.Index, 10), MergeOptions{ Style: mergeStyle, }) - - pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: apiPull.ID}) - return pull.MergedCommitID, "@every 2m" + return "@every 2m" }) }) } t.Run(string(repo_model.MergeStyleManuallyMerged), func(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { // enable manual-merge doAPIEditRepository(testContext, &api.EditRepoOption{ HasPullRequests: new(true), @@ -137,13 +141,13 @@ jobs: pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: apiPull.ID}) assert.Equal(t, issues_model.PullRequestStatusManuallyMerged, pull.Status) - return pull.MergedCommitID, "@every 2m" + return "@every 2m" }) }) } func testScheduleUpdateMirrorSync(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { // create mirror repo opts := migration.MigrateOptions{ RepoName: "actions-schedule-mirror", @@ -193,41 +197,79 @@ func testScheduleUpdateMirrorSync(t *testing.T) { scheduleSpec = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionScheduleSpec{RepoID: mirrorRepo.ID, ScheduleID: actionSchedule.ID}) assert.Equal(t, newCron, scheduleSpec.Spec) - return repoDefaultBranch.CommitID, newCron + return newCron }) } func testScheduleUpdateArchiveAndUnarchive(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { + run := createScheduleRun(t, repo) doAPIEditRepository(testContext, &api.EditRepoOption{ Archived: new(true), })(t) assert.Zero(t, unittest.GetCount(t, &actions_model.ActionSchedule{RepoID: repo.ID})) + assertRunStatus(t, run, actions_model.StatusCancelled) doAPIEditRepository(testContext, &api.EditRepoOption{ Archived: new(false), })(t) - branch, err := git_model.GetBranchExisting(t.Context(), repo.ID, repo.DefaultBranch) - assert.NoError(t, err) - return branch.CommitID, "@every 1m" + return "@every 1m" }) } func testScheduleUpdateDisableAndEnableActionsUnit(t *testing.T) { - doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { + run := createScheduleRun(t, repo) + doAPIEditRepository(testContext, &api.EditRepoOption{ + HasActions: new(true), + })(t) + assertRunStatus(t, run, actions_model.StatusWaiting) + doAPIEditRepository(testContext, &api.EditRepoOption{ HasActions: new(false), })(t) assert.Zero(t, unittest.GetCount(t, &actions_model.ActionSchedule{RepoID: repo.ID})) + assertRunStatus(t, run, actions_model.StatusCancelled) doAPIEditRepository(testContext, &api.EditRepoOption{ HasActions: new(true), })(t) - branch, err := git_model.GetBranchExisting(t.Context(), repo.ID, repo.DefaultBranch) - assert.NoError(t, err) - return branch.CommitID, "@every 1m" + return "@every 1m" }) } -type scheduleUpdateTrigger func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (commitID, expectedSpec string) +func testScheduleUpdateChangeDefaultBranch(t *testing.T) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { + oldBranch, newBranch := repo.DefaultBranch, "release" + testAPICreateBranch(t, testContext.Session, repo.OwnerName, repo.Name, oldBranch, newBranch, http.StatusCreated) + + run := createScheduleRun(t, repo) + req := NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/settings/branches", map[string]string{ + "action": "default_branch", + "branch": newBranch, + }) + testContext.Session.MakeRequest(t, req, http.StatusSeeOther) + assertRunStatus(t, run, actions_model.StatusCancelled) + unittest.AssertExistsAndLoadBean(t, &actions_model.ActionSchedule{RepoID: repo.ID, Ref: git.RefNameFromBranch(newBranch).String()}) + + run = createScheduleRun(t, repo) + doAPIEditRepository(testContext, &api.EditRepoOption{ + DefaultBranch: new(oldBranch), + })(t) + assertRunStatus(t, run, actions_model.StatusCancelled) + return "@every 1m" + }) +} + +func testScheduleUpdateRenameDefaultBranch(t *testing.T) { + doTestScheduleUpdate(t, func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) { + newName := repo.DefaultBranch + "-renamed" + run := createScheduleRun(t, repo) + testAPIRenameBranch(t, user.Name, repo.OwnerName, repo.Name, repo.DefaultBranch, newName, http.StatusNoContent) + assertRunStatus(t, run, actions_model.StatusCancelled) + return "@every 1m" + }) +} + +type scheduleUpdateTrigger func(t *testing.T, u *url.URL, testContext APITestContext, user *user_model.User, repo *repo_model.Repository) (expectedSpec string) func doTestScheduleUpdate(t *testing.T, updateTrigger scheduleUpdateTrigger) { onGiteaRun(t, func(t *testing.T, u *url.URL) { @@ -260,14 +302,31 @@ jobs: scheduleSpec := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionScheduleSpec{RepoID: repo.ID, ScheduleID: actionSchedule.ID}) assert.Equal(t, "@every 1m", scheduleSpec.Spec) - commitID, expectedSpec := updateTrigger(t, u, httpContext, user2, repo) + expectedSpec := updateTrigger(t, u, httpContext, user2, repo) - actionSchedule = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionSchedule{RepoID: repo.ID, CommitSHA: commitID}) + defaultBranch := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo.ID}).DefaultBranch + branch, err := git_model.GetBranchExisting(t.Context(), repo.ID, defaultBranch) + require.NoError(t, err) + actionSchedule = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionSchedule{RepoID: repo.ID, Ref: git.RefNameFromBranch(defaultBranch).String(), CommitSHA: branch.CommitID}) scheduleSpec = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionScheduleSpec{RepoID: repo.ID, ScheduleID: actionSchedule.ID}) assert.Equal(t, expectedSpec, scheduleSpec.Spec) }) } +func createScheduleRun(t *testing.T, repo *repo_model.Repository) *actions_model.ActionRun { + spec := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionScheduleSpec{RepoID: repo.ID}) + spec.Repo = repo + spec.Schedule = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionSchedule{ID: spec.ScheduleID}) + require.NoError(t, actions_service.CreateScheduleTaskBySpec(t.Context(), spec)) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: repo.ID, ScheduleID: spec.ScheduleID}) + assert.Equal(t, actions_model.StatusWaiting, run.Status) + return run +} + +func assertRunStatus(t *testing.T, run *actions_model.ActionRun, status actions_model.Status) { + assert.Equal(t, status, unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID}).Status) +} + func pushScheduleChange(t *testing.T, u *url.URL, repo *repo_model.Repository, newCron string) { workflowTreePath := ".gitea/workflows/actions-schedule.yml" workflowContent := `name: actions-schedule