mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-10 15:09:50 +02:00
fix(actions): settle cancelled reusable workflow runs stuck in cancelling (#39706)
Fixes #39702 **Root Cause:** Cancelling a reusable workflow caller wrote it `cancelled` right away, even while a child was still `cancelling`. When the runner acknowledged, the caller did not change, so the run was never refreshed and stayed `cancelling`. SQLite hid this because the child is cancelled a second time, which re-aggregates the caller, while MySQL reports no affected rows for that unchanged update. **Changes:** - A caller now takes its status only from its children: it stays `cancelling` until its last child finishes, and its final status then refreshes the run. - Cancelling a run whose jobs are all done only settles the run, which did not wake the runs waiting for its concurrency group. It now does, so force-cancelling an already stuck run also frees them. --------- Signed-off-by: BenV <165034+benv666@users.noreply.github.com> Co-authored-by: Zettat123 <zettat123@gmail.com>
This commit is contained in:
1 parent
251e697b0a
commit
c344004e53
4 files changed
+173
-11
No files matched your search
+20
-11
@@ -853,25 +853,34 @@ func cancelReusableCaller(ctx context.Context, caller *ActionRunJob, force bool)
|
||||
}
|
||||
|
||||
// Cancel descendants deepest-first, then the caller: a caller's status is aggregated from its children,
|
||||
// so each child must reach its final state before its parent caller is re-aggregated.
|
||||
// so each child must be cancelled before its parent caller is re-read.
|
||||
// A child's ID always exceeds its parent's, so descending ID is a valid deepest-first order.
|
||||
descendants := CollectAllDescendantJobs(caller, attemptJobs)
|
||||
slices.SortFunc(descendants, func(a, b *ActionRunJob) int { return cmp.Compare(b.ID, a.ID) })
|
||||
callersWithChildren := make(container.Set[int64])
|
||||
for _, d := range descendants {
|
||||
callersWithChildren.Add(d.ParentJobID)
|
||||
}
|
||||
|
||||
for _, c := range descendants {
|
||||
cancelled, err := cancelOneJob(ctx, c, force)
|
||||
for _, job := range append(descendants, caller) {
|
||||
if !callersWithChildren.Contains(job.ID) {
|
||||
cancelled, err := cancelOneJob(ctx, job, force)
|
||||
if err != nil {
|
||||
return cancelledJobs, err
|
||||
}
|
||||
if cancelled != nil {
|
||||
cancelledJobs = append(cancelledJobs, cancelled)
|
||||
}
|
||||
continue
|
||||
}
|
||||
// the job is a caller and its children's cascade already re-aggregated it
|
||||
reloaded, err := GetRunJobByRunAndID(ctx, job.RunID, job.ID)
|
||||
if err != nil {
|
||||
return cancelledJobs, err
|
||||
}
|
||||
if cancelled != nil {
|
||||
cancelledJobs = append(cancelledJobs, cancelled)
|
||||
if reloaded.Status != job.Status {
|
||||
cancelledJobs = append(cancelledJobs, reloaded)
|
||||
}
|
||||
}
|
||||
|
||||
if c, err := cancelOneJob(ctx, caller, force); err != nil {
|
||||
return cancelledJobs, err
|
||||
} else if c != nil {
|
||||
cancelledJobs = append(cancelledJobs, c)
|
||||
}
|
||||
return cancelledJobs, nil
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
runnerv1 "gitea.dev/actionslib/runner/v1"
|
||||
"gitea.dev/models/db"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/timeutil"
|
||||
@@ -397,3 +398,72 @@ func TestForceCancelJobs(t *testing.T) {
|
||||
assert.Equal(t, StatusCancelled, callerAfter.Status)
|
||||
})
|
||||
}
|
||||
|
||||
func TestCancelJobs_CallerWaitsForCancellingChild(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
ctx := t.Context()
|
||||
|
||||
run := &ActionRun{
|
||||
Title: "caller-cancelling-child",
|
||||
RepoID: 4,
|
||||
Index: 9811,
|
||||
OwnerID: 1,
|
||||
WorkflowID: "test.yaml",
|
||||
TriggerUserID: 1,
|
||||
Ref: "refs/heads/master",
|
||||
CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0",
|
||||
Event: "push",
|
||||
TriggerEvent: "push",
|
||||
EventPayload: "{}",
|
||||
Status: StatusRunning,
|
||||
}
|
||||
require.NoError(t, db.Insert(ctx, run))
|
||||
attempt := &ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: 1, Status: StatusRunning}
|
||||
require.NoError(t, db.Insert(ctx, attempt))
|
||||
run.LatestAttemptID = attempt.ID
|
||||
require.NoError(t, UpdateRun(ctx, run, "latest_attempt_id"))
|
||||
|
||||
newJob := func(name string, parentID int64, isCaller bool) *ActionRunJob {
|
||||
job := &ActionRunJob{
|
||||
RunID: run.ID,
|
||||
RunAttemptID: attempt.ID,
|
||||
RepoID: run.RepoID,
|
||||
OwnerID: run.OwnerID,
|
||||
CommitSHA: run.CommitSHA,
|
||||
Name: name,
|
||||
JobID: name,
|
||||
Attempt: 1,
|
||||
Status: StatusRunning,
|
||||
ParentJobID: parentID,
|
||||
IsReusableCaller: isCaller,
|
||||
IsExpanded: isCaller,
|
||||
}
|
||||
require.NoError(t, db.Insert(ctx, job))
|
||||
return job
|
||||
}
|
||||
outer := newJob("outer", 0, true)
|
||||
inner := newJob("inner", outer.ID, true)
|
||||
child := newJob("child", inner.ID, false)
|
||||
|
||||
runner := &ActionRunner{UUID: "caller-cancelling-child", Name: "caller-cancelling-child", HasCancellingSupport: true}
|
||||
require.NoError(t, db.Insert(ctx, runner))
|
||||
task := &ActionTask{JobID: child.ID, Attempt: 1, RunnerID: runner.ID, Status: StatusRunning, Started: timeutil.TimeStampNow(), RepoID: run.RepoID, OwnerID: run.OwnerID, CommitSHA: run.CommitSHA}
|
||||
require.NoError(t, db.Insert(ctx, task))
|
||||
child.TaskID = task.ID
|
||||
_, err := UpdateRunJob(ctx, child, nil, "task_id")
|
||||
require.NoError(t, err)
|
||||
|
||||
cancelled, err := CancelJobs(ctx, []*ActionRunJob{outer}, false)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, cancelled, 3)
|
||||
for _, job := range []*ActionRunJob{outer, inner, child} {
|
||||
assert.Equal(t, StatusCancelling, unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}).Status, job.Name)
|
||||
}
|
||||
|
||||
_, err = UpdateTaskByState(ctx, runner.ID, &runnerv1.TaskState{Id: task.ID, Result: runnerv1.Result_RESULT_CANCELLED})
|
||||
require.NoError(t, err)
|
||||
for _, job := range []*ActionRunJob{outer, inner, child} {
|
||||
assert.Equal(t, StatusCancelled, unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}).Status, job.Name)
|
||||
}
|
||||
assert.Equal(t, StatusCancelled, unittest.AssertExistsAndLoadBean(t, &ActionRun{ID: run.ID}).Status)
|
||||
}
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
actions_model "gitea.dev/models/actions"
|
||||
"gitea.dev/models/db"
|
||||
"gitea.dev/modules/actions/jobparser"
|
||||
"gitea.dev/modules/log"
|
||||
)
|
||||
|
||||
// CancelRun cancels a run's cancellable jobs and returns the run's post-cancellation state.
|
||||
@@ -52,6 +53,12 @@ func cancelRun(ctx context.Context, run *actions_model.ActionRun, jobs []*action
|
||||
if len(updatedJobs) > 0 || reloaded.Status != run.Status {
|
||||
NotifyWorkflowRunStatusUpdate(ctx, reloaded)
|
||||
}
|
||||
if len(updatedJobs) == 0 && reloaded.Status != run.Status {
|
||||
// the run's status was updated, so emit it for the emitter to check
|
||||
if err := EmitJobsIfReadyByRun(run.ID); err != nil {
|
||||
log.Error("Check jobs of run %d: %v", run.ID, err)
|
||||
}
|
||||
}
|
||||
return reloaded, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package actions
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
actions_model "gitea.dev/models/actions"
|
||||
"gitea.dev/models/db"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/test"
|
||||
"gitea.dev/modules/timeutil"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestForceCancelRun_SettledRunWakesConcurrencyWaiters(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
ctx := t.Context()
|
||||
|
||||
// a run left cancelling although its caller and child are all cancelled already
|
||||
run := &actions_model.ActionRun{
|
||||
Title: "settled-run",
|
||||
RepoID: 4,
|
||||
Index: 9821,
|
||||
OwnerID: 1,
|
||||
WorkflowID: "test.yaml",
|
||||
TriggerUserID: 1,
|
||||
Ref: "refs/heads/master",
|
||||
CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0",
|
||||
Event: "push",
|
||||
TriggerEvent: "push",
|
||||
EventPayload: "{}",
|
||||
Status: actions_model.StatusCancelling,
|
||||
}
|
||||
require.NoError(t, db.Insert(ctx, run))
|
||||
attempt := &actions_model.ActionRunAttempt{RepoID: run.RepoID, RunID: run.ID, Attempt: 1, TriggerUserID: 1, Status: actions_model.StatusCancelling}
|
||||
require.NoError(t, db.Insert(ctx, attempt))
|
||||
run.LatestAttemptID = attempt.ID
|
||||
require.NoError(t, actions_model.UpdateRun(ctx, run, "latest_attempt_id"))
|
||||
|
||||
newJob := func(name string, parentID int64, isCaller bool) *actions_model.ActionRunJob {
|
||||
job := &actions_model.ActionRunJob{
|
||||
RunID: run.ID,
|
||||
RunAttemptID: attempt.ID,
|
||||
RepoID: run.RepoID,
|
||||
OwnerID: run.OwnerID,
|
||||
CommitSHA: run.CommitSHA,
|
||||
Name: name,
|
||||
JobID: name,
|
||||
Attempt: 1,
|
||||
Status: actions_model.StatusCancelled,
|
||||
Stopped: timeutil.TimeStampNow(),
|
||||
ParentJobID: parentID,
|
||||
IsReusableCaller: isCaller,
|
||||
IsExpanded: isCaller,
|
||||
}
|
||||
require.NoError(t, db.Insert(ctx, job))
|
||||
return job
|
||||
}
|
||||
caller := newJob("caller", 0, true)
|
||||
child := newJob("child", caller.ID, false)
|
||||
|
||||
var emitted []int64
|
||||
defer test.MockVariableValue(&EmitJobsIfReadyByRun, func(runID int64) error {
|
||||
emitted = append(emitted, runID)
|
||||
return nil
|
||||
})()
|
||||
|
||||
got, err := ForceCancelRun(ctx, run, []*actions_model.ActionRunJob{caller, child})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, actions_model.StatusCancelled, got.Status)
|
||||
assert.Equal(t, []int64{run.ID}, emitted)
|
||||
}
|
||||
Reference in new issue
Block a user