From 1f8e3b6fb8aba01b89b25009f1197bf4cfbcc6e1 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 3 Nov 2025 23:36:01 +0100 Subject: [PATCH 1/4] Fix allow cancelling runs without running jobs --- models/actions/run.go | 12 ++++++ models/actions/run_job.go | 47 +++++++++++++----------- models/fixtures/action_run.yml | 20 ++++++++++ models/fixtures/action_run_job.yml | 14 +++++++ models/fixtures/action_task.yml | 19 ++++++++++ models/fixtures/repo_unit.yml | 7 ++++ tests/integration/actions_cancel_test.go | 37 +++++++++++++++++++ 7 files changed, 135 insertions(+), 21 deletions(-) create mode 100644 tests/integration/actions_cancel_test.go diff --git a/models/actions/run.go b/models/actions/run.go index be332d6857d89..dadfe1c8cae57 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -265,6 +265,13 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin func CancelJobs(ctx context.Context, jobs []*ActionRunJob) ([]*ActionRunJob, error) { cancelledJobs := make([]*ActionRunJob, 0, len(jobs)) + + // List of runIDs that have no cancelled jobs + runsToUpdate := map[int64]*ActionRunJob{} + for _, job := range jobs { + runsToUpdate[job.RunID] = job + } + // 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.). @@ -304,6 +311,11 @@ func CancelJobs(ctx context.Context, jobs []*ActionRunJob) ([]*ActionRunJob, err return cancelledJobs, fmt.Errorf("get job: %w", err) } cancelledJobs = append(cancelledJobs, updatedJob) + delete(runsToUpdate, job.RunID) + } + + for runID, job := range runsToUpdate { + UpdateRunStatus(ctx, job.RepoID, runID) } // Return nil to indicate successful cancellation of all running and waiting jobs. diff --git a/models/actions/run_job.go b/models/actions/run_job.go index f72a7040e3359..a2d0ff68590a1 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -138,6 +138,30 @@ func GetRunJobsByRunID(ctx context.Context, runID int64) (ActionJobList, error) return jobs, nil } +func UpdateRunStatus(ctx context.Context, repoID, runID int64) error { + // Other goroutines may aggregate the status of the run and update it too. + // So we need load the run and its jobs before updating the run. + run, err := GetRunByRepoAndID(ctx, repoID, runID) + if err != nil { + return err + } + jobs, err := GetRunJobsByRunID(ctx, runID) + if err != nil { + return err + } + run.Status = AggregateJobStatus(jobs) + if run.Started.IsZero() && run.Status.IsRunning() { + run.Started = timeutil.TimeStampNow() + } + if run.Stopped.IsZero() && run.Status.IsDone() { + run.Stopped = timeutil.TimeStampNow() + } + if err := UpdateRun(ctx, run, "status", "started", "stopped"); err != nil { + return fmt.Errorf("update run %d: %w", run.ID, err) + } + return nil +} + func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, cols ...string) (int64, error) { e := db.GetEngine(ctx) @@ -173,27 +197,8 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col } } - { - // Other goroutines may aggregate the status of the run and update it too. - // So we need load the run and its jobs before updating the run. - run, err := GetRunByRepoAndID(ctx, job.RepoID, job.RunID) - if err != nil { - return 0, err - } - jobs, err := GetRunJobsByRunID(ctx, job.RunID) - if err != nil { - return 0, err - } - run.Status = AggregateJobStatus(jobs) - if run.Started.IsZero() && run.Status.IsRunning() { - run.Started = timeutil.TimeStampNow() - } - if run.Stopped.IsZero() && run.Status.IsDone() { - run.Stopped = timeutil.TimeStampNow() - } - if err := UpdateRun(ctx, run, "status", "started", "stopped"); err != nil { - return 0, fmt.Errorf("update run %d: %w", run.ID, err) - } + if err := UpdateRunStatus(ctx, job.RepoID, job.RunID); err != nil { + return 0, err } return affected, nil diff --git a/models/fixtures/action_run.yml b/models/fixtures/action_run.yml index b9688dd5f5006..44b131c961272 100644 --- a/models/fixtures/action_run.yml +++ b/models/fixtures/action_run.yml @@ -159,3 +159,23 @@ updated: 1683636626 need_approval: 0 approved_by: 0 +- + id: 805 + title: "update actions" + repo_id: 4 + owner_id: 1 + workflow_id: "artifact.yaml" + index: 191 + trigger_user_id: 1 + ref: "refs/heads/master" + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + event: "push" + trigger_event: "push" + is_fork_pull_request: 0 + status: 5 + started: 1683636528 + stopped: 1683636626 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index 337e83605a086..c5aeb4931cf17 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -143,3 +143,17 @@ status: 1 started: 1683636528 stopped: 1683636626 +- + id: 206 + run_id: 805 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + name: job_2 + attempt: 1 + job_id: job_2 + task_id: 56 + status: 3 + started: 1683636528 + stopped: 1683636626 diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index e09fd6f2ec6d0..a28ddd0add797 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -197,3 +197,22 @@ log_length: 707 log_size: 90179 log_expired: 0 +- + id: 56 + attempt: 1 + runner_id: 1 + status: 3 # 3 is the status code for "cancelled" + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: 6d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4240c64a69a2cc1508825121b7b8394e48e00b1bf3718b2aaaab + token_salt: eeeeeeee + token_last_eight: eeeeeeee + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index f8bb8ef0d3282..4c3e37500f094 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -740,3 +740,10 @@ type: 10 config: "{}" created_unix: 946684810 + +- + id: 112 + repo_id: 4 + type: 10 + config: "{}" + created_unix: 946684810 diff --git a/tests/integration/actions_cancel_test.go b/tests/integration/actions_cancel_test.go new file mode 100644 index 0000000000000..5d4a18ab43bb7 --- /dev/null +++ b/tests/integration/actions_cancel_test.go @@ -0,0 +1,37 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "github.com/stretchr/testify/assert" +) + +// This verifies that cancelling a run without running jobs (stuck in waiting) is updated to cancelled status +func TestActionsCancelStuckWaitingRun(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + session := loginUser(t, user5.Name) + + // cancel the run by run index + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/actions/runs/%d/cancel", user5.Name, "repo4", 191), map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + }) + + session.MakeRequest(t, req, http.StatusOK) + + // check if the run is cancelled by id + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ + ID: 805, + }) + assert.Equal(t, actions_model.StatusCancelled, run.Status) + }) +} From 1a2e85020aa8c7370a70c3d424b56693a0834956 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 4 Nov 2025 00:03:57 +0100 Subject: [PATCH 2/4] lint --- models/actions/run.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/actions/run.go b/models/actions/run.go index dadfe1c8cae57..f06cb87b235bf 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -315,7 +315,9 @@ func CancelJobs(ctx context.Context, jobs []*ActionRunJob) ([]*ActionRunJob, err } for runID, job := range runsToUpdate { - UpdateRunStatus(ctx, job.RepoID, runID) + if err := UpdateRunStatus(ctx, job.RepoID, runID); err != nil { + return cancelledJobs, err + } } // Return nil to indicate successful cancellation of all running and waiting jobs. From cb9988ad5633d81b7f6a1863d446683f3bbb5c8e Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 4 Nov 2025 00:35:58 +0100 Subject: [PATCH 3/4] Add test for cancelling a run with waiting jobs This test checks the cancellation of a run with waiting jobs. --- tests/integration/actions_cancel_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/actions_cancel_test.go b/tests/integration/actions_cancel_test.go index 5d4a18ab43bb7..a2b6996fa3685 100644 --- a/tests/integration/actions_cancel_test.go +++ b/tests/integration/actions_cancel_test.go @@ -12,6 +12,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "github.com/stretchr/testify/assert" ) From a673e381a292a6adc3980939761ae50c7e5fbf1a Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 4 Nov 2025 00:38:32 +0100 Subject: [PATCH 4/4] Update run_test.go --- models/actions/run_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/actions/run_test.go b/models/actions/run_test.go index 0986f87516e42..bd2b92f4f66de 100644 --- a/models/actions/run_test.go +++ b/models/actions/run_test.go @@ -30,6 +30,6 @@ func TestUpdateRepoRunsNumbers(t *testing.T) { err = UpdateRepoRunsNumbers(t.Context(), repo) assert.NoError(t, err) repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - assert.Equal(t, 4, repo.NumActionRuns) + assert.Equal(t, 5, repo.NumActionRuns) assert.Equal(t, 3, repo.NumClosedActionRuns) }