From 8848b0ea2b34cca5f9a2d1f54e77b609579d4d97 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Fri, 22 Mar 2024 11:19:17 +0800 Subject: [PATCH] Fix bugs in rerunning jobs (#29955) Fix #28761 Fix #27884 Fix #28093 ## Changes ### Rerun all jobs When rerun all jobs, status of the jobs with `needs` will be set to `blocked` instead of `waiting`. Therefore, these jobs will not run until the required jobs are completed. ### Rerun a single job When a single job is rerun, its dependents should also be rerun, just like GitHub does (https://github.com/go-gitea/gitea/issues/28761#issuecomment-2008620820). In this case, only the specified job will be set to `waiting`, its dependents will be set to `blocked` to wait the job. ### Show warning if every job has `needs` If every job in a workflow has `needs`, all jobs will be blocked and no job can be run. So I add a warning message. (cherry picked from commit 2f060c5834d81f0317c795fc281f9a07e03e5962) --- options/locale/locale_en-US.ini | 1 + routers/web/repo/actions/actions.go | 10 +++++- routers/web/repo/actions/view.go | 26 +++++++++++++--- services/actions/rerun.go | 38 +++++++++++++++++++++++ services/actions/rerun_test.go | 48 +++++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 services/actions/rerun.go create mode 100644 services/actions/rerun_test.go diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index aaa30033f..1e32cb86f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3673,6 +3673,7 @@ runs.pushed_by = pushed by runs.workflow = Workflow runs.invalid_workflow_helper = Workflow config file is invalid. Please check your config file: %s runs.no_matching_online_runner_helper = No matching online runner with label: %s +runs.no_job_without_needs = The workflow must contain at least one job without dependencies. runs.actor = Actor runs.status = Status runs.actors_no_select = All actors diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index f27329aa0..6059ad141 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -104,8 +104,13 @@ func List(ctx *context.Context) { workflows = append(workflows, workflow) continue } - // Check whether have matching runner + // The workflow must contain at least one job without "needs". Otherwise, a deadlock will occur and no jobs will be able to run. + hasJobWithoutNeeds := false + // Check whether have matching runner and a job without "needs" for _, j := range wf.Jobs { + if !hasJobWithoutNeeds && len(j.Needs()) == 0 { + hasJobWithoutNeeds = true + } runsOnList := j.RunsOn() for _, ro := range runsOnList { if strings.Contains(ro, "${{") { @@ -123,6 +128,9 @@ func List(ctx *context.Context) { break } } + if !hasJobWithoutNeeds { + workflow.ErrMsg = ctx.Locale.TrString("actions.runs.no_job_without_needs") + } workflows = append(workflows, workflow) } } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 23ce70a15..fa687bbb9 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -353,12 +353,25 @@ func Rerun(ctx *context_module.Context) { return } - if jobIndexStr != "" { - jobs = []*actions_model.ActionRunJob{job} + if jobIndexStr == "" { // rerun all jobs + for _, j := range jobs { + // if the job has needs, it should be set to "blocked" status to wait for other jobs + shouldBlock := len(j.Needs) > 0 + if err := rerunJob(ctx, j, shouldBlock); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + } + ctx.JSON(http.StatusOK, struct{}{}) + return } - for _, j := range jobs { - if err := rerunJob(ctx, j); err != nil { + rerunJobs := actions_service.GetAllRerunJobs(job, jobs) + + for _, j := range rerunJobs { + // jobs other than the specified one should be set to "blocked" status + shouldBlock := j.JobID != job.JobID + if err := rerunJob(ctx, j, shouldBlock); err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return } @@ -367,7 +380,7 @@ func Rerun(ctx *context_module.Context) { ctx.JSON(http.StatusOK, struct{}{}) } -func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob) error { +func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob, shouldBlock bool) error { status := job.Status if !status.IsDone() { return nil @@ -375,6 +388,9 @@ func rerunJob(ctx *context_module.Context, job *actions_model.ActionRunJob) erro job.TaskID = 0 job.Status = actions_model.StatusWaiting + if shouldBlock { + job.Status = actions_model.StatusBlocked + } job.Started = 0 job.Stopped = 0 diff --git a/services/actions/rerun.go b/services/actions/rerun.go new file mode 100644 index 000000000..60f665090 --- /dev/null +++ b/services/actions/rerun.go @@ -0,0 +1,38 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/container" +) + +// GetAllRerunJobs get all jobs that need to be rerun when job should be rerun +func GetAllRerunJobs(job *actions_model.ActionRunJob, allJobs []*actions_model.ActionRunJob) []*actions_model.ActionRunJob { + rerunJobs := []*actions_model.ActionRunJob{job} + rerunJobsIDSet := make(container.Set[string]) + rerunJobsIDSet.Add(job.JobID) + + for { + found := false + for _, j := range allJobs { + if rerunJobsIDSet.Contains(j.JobID) { + continue + } + for _, need := range j.Needs { + if rerunJobsIDSet.Contains(need) { + found = true + rerunJobs = append(rerunJobs, j) + rerunJobsIDSet.Add(j.JobID) + break + } + } + } + if !found { + break + } + } + + return rerunJobs +} diff --git a/services/actions/rerun_test.go b/services/actions/rerun_test.go new file mode 100644 index 000000000..a98de7b78 --- /dev/null +++ b/services/actions/rerun_test.go @@ -0,0 +1,48 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + + "github.com/stretchr/testify/assert" +) + +func TestGetAllRerunJobs(t *testing.T) { + job1 := &actions_model.ActionRunJob{JobID: "job1"} + job2 := &actions_model.ActionRunJob{JobID: "job2", Needs: []string{"job1"}} + job3 := &actions_model.ActionRunJob{JobID: "job3", Needs: []string{"job2"}} + job4 := &actions_model.ActionRunJob{JobID: "job4", Needs: []string{"job2", "job3"}} + + jobs := []*actions_model.ActionRunJob{job1, job2, job3, job4} + + testCases := []struct { + job *actions_model.ActionRunJob + rerunJobs []*actions_model.ActionRunJob + }{ + { + job1, + []*actions_model.ActionRunJob{job1, job2, job3, job4}, + }, + { + job2, + []*actions_model.ActionRunJob{job2, job3, job4}, + }, + { + job3, + []*actions_model.ActionRunJob{job3, job4}, + }, + { + job4, + []*actions_model.ActionRunJob{job4}, + }, + } + + for _, tc := range testCases { + rerunJobs := GetAllRerunJobs(tc.job, jobs) + assert.ElementsMatch(t, tc.rerunJobs, rerunJobs) + } +}