Fix ephemeral runner deletion (#34447) (#34513)

Author: NorthRealm

models/actions/runner.go

@@ -5,6 +5,7 @@ package actions
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -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 {

models/actions/task.go

@@ -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
 }
 

models/fixtures/action_runner.yml

@@ -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"]'

models/fixtures/action_task.yml

@@ -117,3 +117,23 @@
   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

services/actions/cleanup.go

@@ -148,3 +148,19 @@ func CleanupEphemeralRunners(ctx context.Context) error {
 	log.Info("Removed %d runners", affected)
 	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
+}

services/repository/delete.go

@@ -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"
@@ -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},

tests/integration/api_actions_runner_test.go

@@ -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]
@@ -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{}
@@ -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)
 	})
 

tests/integration/ephemeral_actions_runner_deletion_test.go

@@ -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)
+}