Skip to content

Fix ephemeral runner deletion #34447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions models/actions/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package actions

import (
"context"
"errors"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -298,6 +299,23 @@ func DeleteRunner(ctx context.Context, id int64) error {
return err
}

// DeleteEphemeralRunner deletes a ephemeral runner by given ID.
func DeleteEphemeralRunner(ctx context.Context, id int64) error {
runner, err := GetRunnerByID(ctx, id)
if err != nil {
if errors.Is(err, util.ErrNotExist) {
return nil
}
return err
}
if !runner.Ephemeral {
return nil
}

_, err = db.DeleteByID[ActionRunner](ctx, id)
return err
}

// CreateRunner creates new runner.
func CreateRunner(ctx context.Context, t *ActionRunner) error {
if t.OwnerID != 0 && t.RepoID != 0 {
Expand Down
5 changes: 5 additions & 0 deletions models/actions/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,11 @@ func UpdateTask(ctx context.Context, task *ActionTask, cols ...string) error {
sess.Cols(cols...)
}
_, err := sess.Update(task)

// Automatically delete the ephemeral runner if the task is done
if err == nil && task.Status.IsDone() && util.SliceContainsString(cols, "status") {
return DeleteEphemeralRunner(ctx, task.RunnerID)
}
return err
}

Expand Down
11 changes: 11 additions & 0 deletions models/fixtures/action_runner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,14 @@
repo_id: 0
description: "This runner is going to be deleted"
agent_labels: '["runner_to_be_deleted","linux"]'
-
id: 34350
name: runner_to_be_deleted-org-ephemeral
uuid: 3FF231BD-FBB7-4E4B-9602-E6F28363EF20
token_hash: 3FF231BD-FBB7-4E4B-9602-E6F28363EF20
ephemeral: true
version: "1.0.0"
owner_id: 3
repo_id: 0
description: "This runner is going to be deleted"
agent_labels: '["runner_to_be_deleted","linux"]'
20 changes: 20 additions & 0 deletions models/fixtures/action_task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,26 @@
log_length: 707
log_size: 90179
log_expired: 0
-
id: 52
job_id: 196
attempt: 1
runner_id: 34350
status: 6 # running
started: 1683636528
stopped: 1683636626
repo_id: 4
owner_id: 1
commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
is_fork_pull_request: 0
token_hash: f8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784222
token_salt: ffffffffff
token_last_eight: ffffffff
log_filename: artifact-test2/2f/47.log
log_in_storage: 1
log_length: 707
log_size: 90179
log_expired: 0
-
id: 53
job_id: 198
Expand Down
16 changes: 16 additions & 0 deletions services/actions/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,22 @@ func CleanupEphemeralRunners(ctx context.Context) error {
return nil
}

// CleanupEphemeralRunnersByPickedTaskOfRepo removes all ephemeral runners that have active/finished tasks on the given repository
func CleanupEphemeralRunnersByPickedTaskOfRepo(ctx context.Context, repoID int64) error {
subQuery := builder.Select("`action_runner`.id").
From(builder.Select("*").From("`action_runner`"), "`action_runner`"). // mysql needs this redundant subquery
Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`").
Where(builder.And(builder.Eq{"`action_runner`.`ephemeral`": true}, builder.Eq{"`action_task`.`repo_id`": repoID}))
b := builder.Delete(builder.In("id", subQuery)).From("`action_runner`")
res, err := db.GetEngine(ctx).Exec(b)
if err != nil {
return fmt.Errorf("find runners: %w", err)
}
affected, _ := res.RowsAffected()
log.Info("Removed %d runners", affected)
return nil
}

// DeleteRun deletes workflow run, including all logs and artifacts.
func DeleteRun(ctx context.Context, run *actions_model.ActionRun) error {
if !run.Status.IsDone() {
Expand Down
9 changes: 9 additions & 0 deletions services/repository/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"code.gitea.io/gitea/modules/lfs"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/storage"
actions_service "code.gitea.io/gitea/services/actions"
asymkey_service "code.gitea.io/gitea/services/asymkey"

"xorm.io/builder"
Expand Down Expand Up @@ -133,6 +134,14 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
return err
}

// CleanupEphemeralRunnersByPickedTaskOfRepo deletes ephemeral global/org/user that have started any task of this repo
// The cannot pick a second task hardening for ephemeral runners expect that task objects remain available until runner deletion
// This method will delete affected ephemeral global/org/user runners
// &actions_model.ActionRunner{RepoID: repoID} does only handle ephemeral repository runners
if err := actions_service.CleanupEphemeralRunnersByPickedTaskOfRepo(ctx, repoID); err != nil {
return fmt.Errorf("cleanupEphemeralRunners: %w", err)
}

if err := db.DeleteBeans(ctx,
&access_model.Access{RepoID: repo.ID},
&activities_model.Action{RepoID: repo.ID},
Expand Down
26 changes: 14 additions & 12 deletions tests/integration/api_actions_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ func testActionsRunnerAdmin(t *testing.T) {
runnerList := api.ActionRunnersResponse{}
DecodeJSON(t, runnerListResp, &runnerList)

assert.Len(t, runnerList.Entries, 4)

idx := slices.IndexFunc(runnerList.Entries, func(e *api.ActionRunner) bool { return e.ID == 34349 })
require.NotEqual(t, -1, idx)
expectedRunner := runnerList.Entries[idx]
Expand Down Expand Up @@ -160,16 +158,20 @@ func testActionsRunnerOwner(t *testing.T) {
runnerList := api.ActionRunnersResponse{}
DecodeJSON(t, runnerListResp, &runnerList)

assert.Len(t, runnerList.Entries, 1)
assert.Equal(t, "runner_to_be_deleted-org", runnerList.Entries[0].Name)
assert.Equal(t, int64(34347), runnerList.Entries[0].ID)
assert.False(t, runnerList.Entries[0].Ephemeral)
assert.Len(t, runnerList.Entries[0].Labels, 2)
assert.Equal(t, "runner_to_be_deleted", runnerList.Entries[0].Labels[0].Name)
assert.Equal(t, "linux", runnerList.Entries[0].Labels[1].Name)
idx := slices.IndexFunc(runnerList.Entries, func(e *api.ActionRunner) bool { return e.ID == 34347 })
require.NotEqual(t, -1, idx)
expectedRunner := runnerList.Entries[idx]

require.NotNil(t, expectedRunner)
assert.Equal(t, "runner_to_be_deleted-org", expectedRunner.Name)
assert.Equal(t, int64(34347), expectedRunner.ID)
assert.False(t, expectedRunner.Ephemeral)
assert.Len(t, expectedRunner.Labels, 2)
assert.Equal(t, "runner_to_be_deleted", expectedRunner.Labels[0].Name)
assert.Equal(t, "linux", expectedRunner.Labels[1].Name)

// Verify get the runner by id
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", runnerList.Entries[0].ID)).AddTokenAuth(token)
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", expectedRunner.ID)).AddTokenAuth(token)
runnerResp := MakeRequest(t, req, http.StatusOK)

runner := api.ActionRunner{}
Expand All @@ -183,11 +185,11 @@ func testActionsRunnerOwner(t *testing.T) {
assert.Equal(t, "linux", runner.Labels[1].Name)

// Verify delete the runner by id
req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", runnerList.Entries[0].ID)).AddTokenAuth(token)
req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", expectedRunner.ID)).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNoContent)

// Verify runner deletion
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", runnerList.Entries[0].ID)).AddTokenAuth(token)
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/orgs/org3/actions/runners/%d", expectedRunner.ID)).AddTokenAuth(token)
MakeRequest(t, req, http.StatusNotFound)
})

Expand Down
79 changes: 79 additions & 0 deletions tests/integration/ephemeral_actions_runner_deletion_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package integration

import (
"testing"

actions_model "code.gitea.io/gitea/models/actions"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/util"
repo_service "code.gitea.io/gitea/services/repository"
user_service "code.gitea.io/gitea/services/user"
"code.gitea.io/gitea/tests"

"github.com/stretchr/testify/assert"
)

func TestEphemeralActionsRunnerDeletion(t *testing.T) {
t.Run("ByTaskCompletion", testEphemeralActionsRunnerDeletionByTaskCompletion)
t.Run("ByRepository", testEphemeralActionsRunnerDeletionByRepository)
t.Run("ByUser", testEphemeralActionsRunnerDeletionByUser)
}

// Test that the ephemeral runner is deleted when the task is finished
func testEphemeralActionsRunnerDeletionByTaskCompletion(t *testing.T) {
defer tests.PrepareTestEnv(t)()

_, err := actions_model.GetRunnerByID(t.Context(), 34350)
assert.NoError(t, err)

task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 52})
assert.Equal(t, actions_model.StatusRunning, task.Status)

task.Status = actions_model.StatusSuccess
err = actions_model.UpdateTask(t.Context(), task, "status")
assert.NoError(t, err)

_, err = actions_model.GetRunnerByID(t.Context(), 34350)
assert.ErrorIs(t, err, util.ErrNotExist)
}

func testEphemeralActionsRunnerDeletionByRepository(t *testing.T) {
defer tests.PrepareTestEnv(t)()

_, err := actions_model.GetRunnerByID(t.Context(), 34350)
assert.NoError(t, err)

task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 52})
assert.Equal(t, actions_model.StatusRunning, task.Status)

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})

err = repo_service.DeleteRepositoryDirectly(t.Context(), user, task.RepoID, true)
assert.NoError(t, err)

_, err = actions_model.GetRunnerByID(t.Context(), 34350)
assert.ErrorIs(t, err, util.ErrNotExist)
}

// Test that the ephemeral runner is deleted when a user is deleted
func testEphemeralActionsRunnerDeletionByUser(t *testing.T) {
defer tests.PrepareTestEnv(t)()

_, err := actions_model.GetRunnerByID(t.Context(), 34350)
assert.NoError(t, err)

task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 52})
assert.Equal(t, actions_model.StatusRunning, task.Status)

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})

err = user_service.DeleteUser(t.Context(), user, true)
assert.NoError(t, err)

_, err = actions_model.GetRunnerByID(t.Context(), 34350)
assert.ErrorIs(t, err, util.ErrNotExist)
}