From f40f079d79f508567dde0b0108c127cadf4cb63a Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sat, 9 Nov 2024 14:57:41 +0100 Subject: [PATCH 01/37] wip: ephemeral runners --- models/actions/runner.go | 2 ++ models/actions/task.go | 4 +++- routers/api/actions/runner/runner.go | 8 +++++++- routers/api/actions/runner/utils.go | 11 +++++++++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/models/actions/runner.go b/models/actions/runner.go index 2023ba4f995af..f445f388967d3 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -61,6 +61,8 @@ type ActionRunner struct { Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` Deleted timeutil.TimeStamp `xorm:"deleted"` + + Ephemeral bool `xorm:"ephemeral"` } const ( diff --git a/models/actions/task.go b/models/actions/task.go index b62a0c351b99b..af74faf937e5f 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -341,7 +341,7 @@ func UpdateTask(ctx context.Context, task *ActionTask, cols ...string) error { // UpdateTaskByState updates the task by the state. // It will always update the task if the state is not final, even there is no change. // So it will update ActionTask.Updated to avoid the task being judged as a zombie task. -func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionTask, error) { +func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.TaskState) (*ActionTask, error) { stepStates := map[int64]*runnerv1.StepState{} for _, v := range state.Steps { stepStates[v.Id] = v @@ -360,6 +360,8 @@ func UpdateTaskByState(ctx context.Context, state *runnerv1.TaskState) (*ActionT return nil, err } else if !has { return nil, util.ErrNotExist + } else if runnerID != task.RunnerID { + return nil, fmt.Errorf("invalid runner for task") } if task.Status.IsDone() { diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index d4078d8af22cf..502214ff26dc9 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -175,7 +175,9 @@ func (s *Service) UpdateTask( ctx context.Context, req *connect.Request[runnerv1.UpdateTaskRequest], ) (*connect.Response[runnerv1.UpdateTaskResponse], error) { - task, err := actions_model.UpdateTaskByState(ctx, req.Msg.State) + runner := GetRunner(ctx) + + task, err := actions_model.UpdateTaskByState(ctx, runner.ID, req.Msg.State) if err != nil { return nil, status.Errorf(codes.Internal, "update task: %v", err) } @@ -237,11 +239,15 @@ func (s *Service) UpdateLog( ctx context.Context, req *connect.Request[runnerv1.UpdateLogRequest], ) (*connect.Response[runnerv1.UpdateLogResponse], error) { + runner := GetRunner(ctx) + res := connect.NewResponse(&runnerv1.UpdateLogResponse{}) task, err := actions_model.GetTaskByID(ctx, req.Msg.TaskId) if err != nil { return nil, status.Errorf(codes.Internal, "get task: %v", err) + } else if runner.RunnerID != task.RunnerID { + return nil, status.Errorf(codes.Internal, "invalid runner for task") } ack := task.LogLength diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index ff6ec5bd54c62..a2ebf3f019d6e 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -23,6 +23,17 @@ import ( ) func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv1.Task, bool, error) { + if runner.Ephemaral { + var task actions_model.ActionTask + has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) + if err == nil && has { + if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked + return nil, false, nil + } + return nil, false, fmt.Errorf("runner has been removed") + } + } + t, ok, err := actions_model.CreateTaskForRunner(ctx, runner) if err != nil { return nil, false, fmt.Errorf("CreateTaskForRunner: %w", err) From 088ede6082c06581f507522ea4934cbfc39d0175 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sat, 9 Nov 2024 16:34:59 +0100 Subject: [PATCH 02/37] fix --- routers/api/actions/runner/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 502214ff26dc9..8f365cc92670a 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -246,7 +246,7 @@ func (s *Service) UpdateLog( task, err := actions_model.GetTaskByID(ctx, req.Msg.TaskId) if err != nil { return nil, status.Errorf(codes.Internal, "get task: %v", err) - } else if runner.RunnerID != task.RunnerID { + } else if runner.ID != task.RunnerID { return nil, status.Errorf(codes.Internal, "invalid runner for task") } ack := task.LogLength From 959957ad252405b4e6e1ef5acd802dcb667469fa Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 8 Feb 2025 16:54:02 +0100 Subject: [PATCH 03/37] fix scetch --- routers/api/actions/runner/utils.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index 995c7bdcc7fa1..18b172f00092e 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -8,6 +8,7 @@ import ( "fmt" actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/services/actions" @@ -17,17 +18,17 @@ import ( ) func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv1.Task, bool, error) { - if runner.Ephemaral { + if runner.Ephemeral { var task actions_model.ActionTask has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) if err == nil && has { - if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked + if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { return nil, false, nil } return nil, false, fmt.Errorf("runner has been removed") } } - + t, ok, err := actions_model.CreateTaskForRunner(ctx, runner) if err != nil { return nil, false, fmt.Errorf("CreateTaskForRunner: %w", err) From 6300bb40d62339b866b10b0decb9b3a7a448017e Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 8 Feb 2025 17:01:38 +0100 Subject: [PATCH 04/37] add migration --- models/actions/runner.go | 4 ++-- models/migrations/v1_24/v313.go | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 models/migrations/v1_24/v313.go diff --git a/models/actions/runner.go b/models/actions/runner.go index ce9dc3231d93e..1670fba4e2849 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -57,12 +57,12 @@ type ActionRunner struct { // Store labels defined in state file (default: .runner file) of `act_runner` AgentLabels []string `xorm:"TEXT"` + // Store if this is a runner that only ever get one single job assigned + Ephemeral bool `xorm:"ephemeral"` Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` Deleted timeutil.TimeStamp `xorm:"deleted"` - - Ephemeral bool `xorm:"ephemeral"` } const ( diff --git a/models/migrations/v1_24/v313.go b/models/migrations/v1_24/v313.go new file mode 100644 index 0000000000000..adac125041541 --- /dev/null +++ b/models/migrations/v1_24/v313.go @@ -0,0 +1,16 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_24 //nolint + +import ( + "xorm.io/xorm" +) + +func AddEphemeralToActionRunner(x *xorm.Engine) error { + type ActionRunner struct { + Ephemeral bool `xorm:"ephemeral"` + } + + return x.Sync(new(ActionRunner)) +} From 4e2f4193dcd1446d595d63f43263679302a6f168 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 8 Feb 2025 17:03:03 +0100 Subject: [PATCH 05/37] fixup --- models/migrations/migrations.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 95364ab705751..c94765e36fca2 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -373,6 +373,7 @@ func prepareMigrationTasks() []*migration { // Gitea 1.23.0-rc0 ends at migration ID number 311 (database version 312) newMigration(312, "Add DeleteBranchAfterMerge to AutoMerge", v1_24.AddDeleteBranchAfterMergeForAutoMerge), + newMigration(313, "Add Ephemeral to ActionRunner", v1_24.AddEphemeralToActionRunner), } return preparedMigrations } From 546de66e6a8f422efa651c521ce7c35a46defa7f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 8 Feb 2025 20:15:56 +0100 Subject: [PATCH 06/37] wip --- services/actions/cleanup.go | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index ee1d1677133e2..0220df6e34708 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -9,6 +9,7 @@ import ( "time" actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" actions_module "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -16,7 +17,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" ) -// Cleanup removes expired actions logs, data and artifacts +// Cleanup removes expired actions logs, data, artifacts and used ephemeral runners func Cleanup(ctx context.Context) error { // clean up expired artifacts if err := CleanupArtifacts(ctx); err != nil { @@ -28,6 +29,11 @@ func Cleanup(ctx context.Context) error { return fmt.Errorf("cleanup logs: %w", err) } + // clean up old ephemeral runners + if err := CleanupEphemeralRunners(ctx); err != nil { + return fmt.Errorf("cleanup old ephemeral runners: %w", err) + } + return nil } @@ -123,3 +129,21 @@ func CleanupLogs(ctx context.Context) error { log.Info("Removed %d logs", count) return nil } + +const deleteEphemeralRunnerBatchSize = 100 + +// CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs +func CleanupEphemeralRunners(ctx context.Context) error { + + runners := []*actions_model.ActionRunner{} + err := db.GetEngine(ctx).Join("INNER", "action_task", "action_task.runner_id = action_runner.id").Where("action_runner.ephemeral").Limit(deleteEphemeralRunnerBatchSize).Find(&runners) + if err != nil { + return fmt.Errorf("find runners: %w", err) + } + count, err := db.GetEngine(ctx).Delete(&runners) + if err != nil { + return fmt.Errorf("delete runners: %w", err) + } + log.Info("Removed %d runners", count) + return nil +} From 52a96be237ad8a1e7eb1aebc210c782a5d4aaf24 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 01:08:19 +0100 Subject: [PATCH 07/37] Ephemeral proto hack --- go.mod | 2 ++ routers/api/actions/runner/runner.go | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index e2ccdd5ae65c5..723a5dfbbd487 100644 --- a/go.mod +++ b/go.mod @@ -330,3 +330,5 @@ exclude github.com/gofrs/uuid v4.0.0+incompatible exclude github.com/goccy/go-json v0.4.11 exclude github.com/satori/go.uuid v1.2.0 + +replace code.gitea.io/actions-proto-go v0.4.0 => ../actions-proto-def/gen diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index c55b30f7ebb78..fc1b6226b28c8 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -77,6 +77,7 @@ func (s *Service) Register( RepoID: runnerToken.RepoID, Version: req.Msg.Version, AgentLabels: labels, + Ephemeral: req.Msg.Ephemeral, } if err := runner.GenerateToken(); err != nil { return nil, errors.New("can't generate token") @@ -95,12 +96,13 @@ func (s *Service) Register( res := connect.NewResponse(&runnerv1.RegisterResponse{ Runner: &runnerv1.Runner{ - Id: runner.ID, - Uuid: runner.UUID, - Token: runner.Token, - Name: runner.Name, - Version: runner.Version, - Labels: runner.AgentLabels, + Id: runner.ID, + Uuid: runner.UUID, + Token: runner.Token, + Name: runner.Name, + Version: runner.Version, + Labels: runner.AgentLabels, + Ephemeral: runner.Ephemeral, }, }) From 971f9b798df7b468bdfd1f2a61ff6f938d1a1763 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 12:27:12 +0100 Subject: [PATCH 08/37] enhance --- routers/api/actions/runner/utils.go | 2 + tests/integration/actions_job_test.go | 120 ++++++++++++++++++++++- tests/integration/actions_log_test.go | 2 +- tests/integration/actions_runner_test.go | 15 +-- 4 files changed, 128 insertions(+), 11 deletions(-) diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index 18b172f00092e..3e639ebe684fc 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -25,6 +25,8 @@ func pickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { return nil, false, nil } + // task has been finished, remove it + _, _ = db.GetEngine(ctx).Delete(runner) return nil, false, fmt.Errorf("runner has been removed") } } diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index a0c06b06fd205..fe4e71f5ac322 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "connectrpc.com/connect" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "github.com/stretchr/testify/assert" @@ -133,7 +134,7 @@ jobs: apiRepo := createActionsTestRepo(t, token, "actions-jobs-with-needs", false) runner := newMockRunner() - runner.registerAsRepoRunner(t, user2.Name, apiRepo.Name, "mock-runner", []string{"ubuntu-latest"}) + runner.registerAsRepoRunner(t, user2.Name, apiRepo.Name, "mock-runner", []string{"ubuntu-latest"}, false) for _, tc := range testCases { t.Run(fmt.Sprintf("test %s", tc.treePath), func(t *testing.T) { @@ -319,7 +320,7 @@ jobs: apiRepo := createActionsTestRepo(t, token, "actions-jobs-outputs-with-matrix", false) runner := newMockRunner() - runner.registerAsRepoRunner(t, user2.Name, apiRepo.Name, "mock-runner", []string{"ubuntu-latest"}) + runner.registerAsRepoRunner(t, user2.Name, apiRepo.Name, "mock-runner", []string{"ubuntu-latest"}, false) for _, tc := range testCases { t.Run(fmt.Sprintf("test %s", tc.treePath), func(t *testing.T) { @@ -364,7 +365,7 @@ func TestActionsGiteaContext(t *testing.T) { user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository) runner := newMockRunner() - runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}) + runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, false) // init the workflow wfTreePath := ".gitea/workflows/pull.yml" @@ -438,6 +439,119 @@ jobs: }) } +// Ephemeral +func TestActionsGiteaContextEphemeral(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + user2Session := loginUser(t, user2.Name) + user2Token := getTokenForLoggedInUser(t, user2Session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + + apiBaseRepo := createActionsTestRepo(t, user2Token, "actions-gitea-context", false) + baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiBaseRepo.ID}) + user2APICtx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository) + + runner := newMockRunner() + runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, true) + + // init the workflow + wfTreePath := ".gitea/workflows/pull.yml" + wfFileContent := `name: Pull Request +on: pull_request +jobs: + wf1-job: + runs-on: ubuntu-latest + steps: + - run: echo 'test the pull' + wf2-job: + runs-on: ubuntu-latest + steps: + - run: echo 'test the pull' +` + opts := getWorkflowCreateFileOptions(user2, baseRepo.DefaultBranch, fmt.Sprintf("create %s", wfTreePath), wfFileContent) + createWorkflowFile(t, user2Token, baseRepo.OwnerName, baseRepo.Name, wfTreePath, opts) + // user2 creates a pull request + doAPICreateFile(user2APICtx, "user2-patch.txt", &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + NewBranchName: "user2/patch-1", + Message: "create user2-patch.txt", + Author: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Committer: api.Identity{ + Name: user2.Name, + Email: user2.Email, + }, + Dates: api.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte("user2-fix")), + })(t) + apiPull, err := doAPICreatePullRequest(user2APICtx, baseRepo.OwnerName, baseRepo.Name, baseRepo.DefaultBranch, "user2/patch-1")(t) + assert.NoError(t, err) + task := runner.fetchTask(t) + gtCtx := task.Context.GetFields() + actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: task.Id}) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: actionTask.JobID}) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: actionRunJob.RunID}) + assert.NoError(t, actionRun.LoadAttributes(context.Background())) + + assert.Equal(t, user2.Name, gtCtx["actor"].GetStringValue()) + assert.Equal(t, setting.AppURL+"api/v1", gtCtx["api_url"].GetStringValue()) + assert.Equal(t, apiPull.Base.Ref, gtCtx["base_ref"].GetStringValue()) + runEvent := map[string]any{} + assert.NoError(t, json.Unmarshal([]byte(actionRun.EventPayload), &runEvent)) + assert.True(t, reflect.DeepEqual(gtCtx["event"].GetStructValue().AsMap(), runEvent)) + assert.Equal(t, actionRun.TriggerEvent, gtCtx["event_name"].GetStringValue()) + assert.Equal(t, apiPull.Head.Ref, gtCtx["head_ref"].GetStringValue()) + assert.Equal(t, actionRunJob.JobID, gtCtx["job"].GetStringValue()) + assert.Equal(t, actionRun.Ref, gtCtx["ref"].GetStringValue()) + assert.Equal(t, (git.RefName(actionRun.Ref)).ShortName(), gtCtx["ref_name"].GetStringValue()) + assert.False(t, gtCtx["ref_protected"].GetBoolValue()) + assert.Equal(t, string((git.RefName(actionRun.Ref)).RefType()), gtCtx["ref_type"].GetStringValue()) + assert.Equal(t, actionRun.Repo.OwnerName+"/"+actionRun.Repo.Name, gtCtx["repository"].GetStringValue()) + assert.Equal(t, actionRun.Repo.OwnerName, gtCtx["repository_owner"].GetStringValue()) + assert.Equal(t, actionRun.Repo.HTMLURL(), gtCtx["repositoryUrl"].GetStringValue()) + assert.Equal(t, fmt.Sprint(actionRunJob.RunID), gtCtx["run_id"].GetStringValue()) + assert.Equal(t, fmt.Sprint(actionRun.Index), gtCtx["run_number"].GetStringValue()) + assert.Equal(t, fmt.Sprint(actionRunJob.Attempt), gtCtx["run_attempt"].GetStringValue()) + assert.Equal(t, "Actions", gtCtx["secret_source"].GetStringValue()) + assert.Equal(t, setting.AppURL, gtCtx["server_url"].GetStringValue()) + assert.Equal(t, actionRun.CommitSHA, gtCtx["sha"].GetStringValue()) + assert.Equal(t, actionRun.WorkflowID, gtCtx["workflow"].GetStringValue()) + assert.Equal(t, setting.Actions.DefaultActionsURL.URL(), gtCtx["gitea_default_actions_url"].GetStringValue()) + token := gtCtx["token"].GetStringValue() + assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) + + resp, err := runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + TasksVersion: 0, + })) + assert.NoError(t, err) + assert.Nil(t, resp.Msg.Task) + runner.client.runnerServiceClient.UpdateTask(context.Background(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ + State: &runnerv1.TaskState{ + Id: actionTask.ID, + Result: runnerv1.Result_RESULT_SUCCESS, + }, + })) + resp, err = runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + TasksVersion: 0, + })) + assert.Error(t, err) + assert.Nil(t, resp) + + resp, err = runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + TasksVersion: 0, + })) + assert.Error(t, err) + assert.Nil(t, resp) + + doAPIDeleteRepository(user2APICtx)(t) + }) +} + func createActionsTestRepo(t *testing.T, authToken, repoName string, isPrivate bool) *api.Repository { req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos", &api.CreateRepoOption{ Name: repoName, diff --git a/tests/integration/actions_log_test.go b/tests/integration/actions_log_test.go index fd055fc4c4f9e..8f1022672122f 100644 --- a/tests/integration/actions_log_test.go +++ b/tests/integration/actions_log_test.go @@ -105,7 +105,7 @@ jobs: apiRepo := createActionsTestRepo(t, token, "actions-download-task-logs", false) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) runner := newMockRunner() - runner.registerAsRepoRunner(t, user2.Name, repo.Name, "mock-runner", []string{"ubuntu-latest"}) + runner.registerAsRepoRunner(t, user2.Name, repo.Name, "mock-runner", []string{"ubuntu-latest"}, false) for _, tc := range testCases { t.Run(fmt.Sprintf("test %s", tc.treePath), func(t *testing.T) { diff --git a/tests/integration/actions_runner_test.go b/tests/integration/actions_runner_test.go index 355ea1705e23c..aa3ac7e574b91 100644 --- a/tests/integration/actions_runner_test.go +++ b/tests/integration/actions_runner_test.go @@ -67,19 +67,20 @@ func (r *mockRunner) doPing(t *testing.T) { assert.Equal(t, "Hello, mock-runner!", resp.Msg.Data) } -func (r *mockRunner) doRegister(t *testing.T, name, token string, labels []string) { +func (r *mockRunner) doRegister(t *testing.T, name, token string, labels []string, ephemeral bool) { r.doPing(t) resp, err := r.client.runnerServiceClient.Register(context.Background(), connect.NewRequest(&runnerv1.RegisterRequest{ - Name: name, - Token: token, - Version: "mock-runner-version", - Labels: labels, + Name: name, + Token: token, + Version: "mock-runner-version", + Labels: labels, + Ephemeral: ephemeral, })) assert.NoError(t, err) r.client = newMockRunnerClient(resp.Msg.Runner.Uuid, resp.Msg.Runner.Token) } -func (r *mockRunner) registerAsRepoRunner(t *testing.T, ownerName, repoName, runnerName string, labels []string) { +func (r *mockRunner) registerAsRepoRunner(t *testing.T, ownerName, repoName, runnerName string, labels []string, ephemeral bool) { session := loginUser(t, ownerName) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/actions/runners/registration-token", ownerName, repoName)).AddTokenAuth(token) @@ -88,7 +89,7 @@ func (r *mockRunner) registerAsRepoRunner(t *testing.T, ownerName, repoName, run Token string `json:"token"` } DecodeJSON(t, resp, ®istrationToken) - r.doRegister(t, runnerName, registrationToken.Token, labels) + r.doRegister(t, runnerName, registrationToken.Token, labels, ephemeral) } func (r *mockRunner) fetchTask(t *testing.T, timeout ...time.Duration) *runnerv1.Task { From 84e02aeb727ec6544c80e6a9a9e7cd690d36c3ca Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 12:37:11 +0100 Subject: [PATCH 09/37] add temporary proto replacement --- go.mod | 3 ++- go.sum | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 723a5dfbbd487..d0a2613454cd6 100644 --- a/go.mod +++ b/go.mod @@ -331,4 +331,5 @@ exclude github.com/goccy/go-json v0.4.11 exclude github.com/satori/go.uuid v1.2.0 -replace code.gitea.io/actions-proto-go v0.4.0 => ../actions-proto-def/gen +// TODO remove before merge +replace code.gitea.io/actions-proto-go v0.4.0 => gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a diff --git a/go.sum b/go.sum index bc0265c51fb26..8b81899497bba 100644 --- a/go.sum +++ b/go.sum @@ -16,6 +16,8 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078 h1:cliQ4HHsCo6xi2oWZYKWW4bly/Ory9FuTpFPRxj/mAg= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078/go.mod h1:g/V2Hjas6Z1UHUp4yIx6bATpNzJ7DYtD0FG3+xARWxs= +gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a h1:n8ieXo1R58An44ehxHvhP44OIvo37J3YMr46cmejHRY= +gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= gitea.com/gitea/act v0.261.3 h1:BhiYpGJQKGq0XMYYICCYAN4KnsEWHyLbA6dxhZwFcV4= gitea.com/gitea/act v0.261.3/go.mod h1:Pg5C9kQY1CEA3QjthjhlrqOC/QOT5NyWNjOjRHw23Ok= gitea.com/gitea/git-lfs-transfer v0.2.0 h1:baHaNoBSRaeq/xKayEXwiDQtlIjps4Ac/Ll4KqLMB40= From 0b17e10426f849033a4cbcfb42c9b90c53f632d4 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 13:06:21 +0100 Subject: [PATCH 10/37] migrate change --- services/actions/task.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/services/actions/task.go b/services/actions/task.go index bc54ade3478e6..d154578760242 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -22,6 +22,18 @@ func PickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv ) if err := db.WithTx(ctx, func(ctx context.Context) error { + if runner.Ephemeral { + var task actions_model.ActionTask + has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) + if err == nil && has { + if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { + return nil + } + // task has been finished, remove it + _, _ = db.GetEngine(ctx).Delete(runner) + return fmt.Errorf("runner has been removed") + } + } t, ok, err := actions_model.CreateTaskForRunner(ctx, runner) if err != nil { return fmt.Errorf("CreateTaskForRunner: %w", err) From 662055b3eb8fdf9f1f5b25c8a18c38a278567b83 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 13:52:46 +0100 Subject: [PATCH 11/37] fix lint --- go.sum | 2 -- services/actions/cleanup.go | 1 - tests/integration/actions_job_test.go | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/go.sum b/go.sum index 8b81899497bba..7ae163111ea9e 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,5 @@ cloud.google.com/go/compute/metadata v0.5.2 h1:UxK4uu/Tn+I3p2dYWTfiX4wva7aYlKixAHn3fyqngqo= cloud.google.com/go/compute/metadata v0.5.2/go.mod h1:C66sj2AluDcIqakBq/M8lw8/ybHgOZqin2obFxa/E5k= -code.gitea.io/actions-proto-go v0.4.0 h1:OsPBPhodXuQnsspG1sQ4eRE1PeoZyofd7+i73zCwnsU= -code.gitea.io/actions-proto-go v0.4.0/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= code.gitea.io/gitea-vet v0.2.3 h1:gdFmm6WOTM65rE8FUBTRzeQZYzXePKSSB1+r574hWwI= code.gitea.io/gitea-vet v0.2.3/go.mod h1:zcNbT/aJEmivCAhfmkHOlT645KNOf9W2KnkLgFjGGfE= code.gitea.io/sdk/gitea v0.19.0 h1:8I6s1s4RHgzxiPHhOQdgim1RWIRcr0LVMbHBjBFXq4Y= diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 0220df6e34708..4553305bea123 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -134,7 +134,6 @@ const deleteEphemeralRunnerBatchSize = 100 // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { - runners := []*actions_model.ActionRunner{} err := db.GetEngine(ctx).Join("INNER", "action_task", "action_task.runner_id = action_runner.id").Where("action_runner.ephemeral").Limit(deleteEphemeralRunnerBatchSize).Find(&runners) if err != nil { diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index fe4e71f5ac322..279c345ce490e 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -22,9 +22,9 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - "connectrpc.com/connect" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" + "connectrpc.com/connect" "github.com/stretchr/testify/assert" ) From e50873fcb0b1697a3418ea078b4b779c4cf0aab3 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Feb 2025 14:16:16 +0100 Subject: [PATCH 12/37] move ephemeral runner check out of transaction --- services/actions/task.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/services/actions/task.go b/services/actions/task.go index d154578760242..32f301b97ec10 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -21,19 +21,23 @@ func PickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv job *actions_model.ActionRunJob ) - if err := db.WithTx(ctx, func(ctx context.Context) error { - if runner.Ephemeral { - var task actions_model.ActionTask - has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) - if err == nil && has { - if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { - return nil - } - // task has been finished, remove it - _, _ = db.GetEngine(ctx).Delete(runner) - return fmt.Errorf("runner has been removed") + if runner.Ephemeral { + var task actions_model.ActionTask + has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) + if err == nil && has { + if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { + return nil, false, nil + } + // task has been finished, remove it + _, err = db.GetEngine(ctx).Delete(runner) + if err != nil { + return nil, false, err } + return nil, false, fmt.Errorf("runner has been removed") } + } + + if err := db.WithTx(ctx, func(ctx context.Context) error { t, ok, err := actions_model.CreateTaskForRunner(ctx, runner) if err != nil { return fmt.Errorf("CreateTaskForRunner: %w", err) From 2fe409146307f7fee9bf4c1e4744b14263e06618 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Thu, 13 Feb 2025 12:02:50 +0100 Subject: [PATCH 13/37] cleanup don't remove every ephemeral runner that has a job assigned --- services/actions/cleanup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 4553305bea123..cb48f5315a0d0 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -135,7 +135,7 @@ const deleteEphemeralRunnerBatchSize = 100 // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { runners := []*actions_model.ActionRunner{} - err := db.GetEngine(ctx).Join("INNER", "action_task", "action_task.runner_id = action_runner.id").Where("action_runner.ephemeral").Limit(deleteEphemeralRunnerBatchSize).Find(&runners) + err := db.GetEngine(ctx).Join("INNER", "action_task", "action_task.runner_id = action_runner.id").Where("action_runner.ephemeral and action_task.status != ? and action_task.status != ? and action_task.status != ?)", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) if err != nil { return fmt.Errorf("find runners: %w", err) } From 8319c9fc1a78abdaf86379bb2687a0f2f526c2fe Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 23 Feb 2025 13:52:53 +0100 Subject: [PATCH 14/37] fix merge --- tests/integration/actions_job_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 487e905dfe4f2..455796fad4f02 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -495,7 +495,7 @@ jobs: actionTask := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: task.Id}) actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: actionTask.JobID}) actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: actionRunJob.RunID}) - assert.NoError(t, actionRun.LoadAttributes(context.Background())) + assert.NoError(t, actionRun.LoadAttributes(t.Context())) assert.Equal(t, user2.Name, gtCtx["actor"].GetStringValue()) assert.Equal(t, setting.AppURL+"api/v1", gtCtx["api_url"].GetStringValue()) @@ -524,24 +524,24 @@ jobs: token := gtCtx["token"].GetStringValue() assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) - resp, err := runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + resp, err := runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.NoError(t, err) assert.Nil(t, resp.Msg.Task) - runner.client.runnerServiceClient.UpdateTask(context.Background(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ + runner.client.runnerServiceClient.UpdateTask(t.Context(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ State: &runnerv1.TaskState{ Id: actionTask.ID, Result: runnerv1.Result_RESULT_SUCCESS, }, })) - resp, err = runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + resp, err = runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.Error(t, err) assert.Nil(t, resp) - resp, err = runner.client.runnerServiceClient.FetchTask(context.Background(), connect.NewRequest(&runnerv1.FetchTaskRequest{ + resp, err = runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.Error(t, err) From b69a4f18638720c5f3ae5baca42c1deed49b3369 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 23 Feb 2025 17:09:31 +0100 Subject: [PATCH 15/37] cleanup --- go.sum | 4 ---- 1 file changed, 4 deletions(-) diff --git a/go.sum b/go.sum index 15aea5a07d620..2afb8344a81b3 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,5 @@ -cloud.google.com/go/compute/metadata v0.5.2 h1:UxK4uu/Tn+I3p2dYWTfiX4wva7aYlKixAHn3fyqngqo= -cloud.google.com/go/compute/metadata v0.5.2/go.mod h1:C66sj2AluDcIqakBq/M8lw8/ybHgOZqin2obFxa/E5k= cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I= cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg= -code.gitea.io/actions-proto-go v0.4.0 h1:OsPBPhodXuQnsspG1sQ4eRE1PeoZyofd7+i73zCwnsU= -code.gitea.io/actions-proto-go v0.4.0/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= code.gitea.io/gitea-vet v0.2.3 h1:gdFmm6WOTM65rE8FUBTRzeQZYzXePKSSB1+r574hWwI= code.gitea.io/gitea-vet v0.2.3/go.mod h1:zcNbT/aJEmivCAhfmkHOlT645KNOf9W2KnkLgFjGGfE= code.gitea.io/sdk/gitea v0.20.0 h1:Zm/QDwwZK1awoM4AxdjeAQbxolzx2rIP8dDfmKu+KoU= From e21e91db6508039963f35f1500d923279172ef58 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 25 Feb 2025 12:40:16 +0100 Subject: [PATCH 16/37] remove actions-proto-go fork --- go.mod | 5 +---- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index e4fb92dff665e..3d0cb125cc9f6 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ go 1.24 godebug x509negativeserial=1 require ( - code.gitea.io/actions-proto-go v0.4.0 + code.gitea.io/actions-proto-go v0.4.1-0.20250224031915-32cdcf1b5f71 code.gitea.io/gitea-vet v0.2.3 code.gitea.io/sdk/gitea v0.20.0 codeberg.org/gusted/mcaptcha v0.0.0-20220723083913-4f3072e1d570 @@ -333,6 +333,3 @@ exclude github.com/gofrs/uuid v4.0.0+incompatible exclude github.com/goccy/go-json v0.4.11 exclude github.com/satori/go.uuid v1.2.0 - -// TODO remove before merge -replace code.gitea.io/actions-proto-go v0.4.0 => gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a diff --git a/go.sum b/go.sum index 2afb8344a81b3..bb224b0c1a89b 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ cloud.google.com/go/compute/metadata v0.6.0 h1:A6hENjEsCDtC1k8byVsgwvVcioamEHvZ4j01OwKxG9I= cloud.google.com/go/compute/metadata v0.6.0/go.mod h1:FjyFAW1MW0C203CEOMDTu3Dk1FlqW3Rga40jzHL4hfg= +code.gitea.io/actions-proto-go v0.4.1-0.20250224031915-32cdcf1b5f71 h1:tpcCWq2GLmRCY4zUqXNzQpZlh3AId4jXVsItRppzfuU= +code.gitea.io/actions-proto-go v0.4.1-0.20250224031915-32cdcf1b5f71/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= code.gitea.io/gitea-vet v0.2.3 h1:gdFmm6WOTM65rE8FUBTRzeQZYzXePKSSB1+r574hWwI= code.gitea.io/gitea-vet v0.2.3/go.mod h1:zcNbT/aJEmivCAhfmkHOlT645KNOf9W2KnkLgFjGGfE= code.gitea.io/sdk/gitea v0.20.0 h1:Zm/QDwwZK1awoM4AxdjeAQbxolzx2rIP8dDfmKu+KoU= @@ -14,8 +16,6 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078 h1:cliQ4HHsCo6xi2oWZYKWW4bly/Ory9FuTpFPRxj/mAg= git.sr.ht/~mariusor/go-xsd-duration v0.0.0-20220703122237-02e73435a078/go.mod h1:g/V2Hjas6Z1UHUp4yIx6bATpNzJ7DYtD0FG3+xARWxs= -gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a h1:n8ieXo1R58An44ehxHvhP44OIvo37J3YMr46cmejHRY= -gitea.com/ChristopherHX/actions-proto-go v0.4.1-0.20250212113254-35d54b458c4a/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= gitea.com/gitea/act v0.261.3 h1:BhiYpGJQKGq0XMYYICCYAN4KnsEWHyLbA6dxhZwFcV4= gitea.com/gitea/act v0.261.3/go.mod h1:Pg5C9kQY1CEA3QjthjhlrqOC/QOT5NyWNjOjRHw23Ok= gitea.com/gitea/git-lfs-transfer v0.2.0 h1:baHaNoBSRaeq/xKayEXwiDQtlIjps4Ac/Ll4KqLMB40= From c87aa86c055381e4c667bd16d757d712c1c3a8b8 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 25 Feb 2025 12:42:57 +0100 Subject: [PATCH 17/37] do not allow to bypass ephemeral check --- services/actions/task.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/actions/task.go b/services/actions/task.go index 32f301b97ec10..1bc1e04504fa2 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -24,7 +24,11 @@ func PickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv if runner.Ephemeral { var task actions_model.ActionTask has, err := db.GetEngine(ctx).Where("runner_id = ?", runner.ID).Get(&task) - if err == nil && has { + // Let the runner retry the request, do not allow to proceed + if err != nil { + return nil, false, err + } + if has { if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { return nil, false, nil } From 10300812120300fef853692d910ac9a7547851d2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 4 Mar 2025 23:21:02 +0100 Subject: [PATCH 18/37] set ephemeral to false by sql statement --- models/migrations/v1_24/v314.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/models/migrations/v1_24/v314.go b/models/migrations/v1_24/v314.go index d429cef4ee110..52003c5daebd2 100644 --- a/models/migrations/v1_24/v314.go +++ b/models/migrations/v1_24/v314.go @@ -12,5 +12,11 @@ func AddEphemeralToActionRunner(x *xorm.Engine) error { Ephemeral bool `xorm:"ephemeral"` } - return x.Sync(new(ActionRunner)) + if err := x.Sync(new(ActionRunner)); err != nil { + return err + } + + // update all records to set ephemeral to false + _, err := x.Exec("UPDATE `action_runner` SET `ephemeral` = false WHERE `ephemeral` IS NULL") + return err } From 17ce36aee13bc6c406b6930bf6b1207c2f624f6f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 4 Mar 2025 23:26:23 +0100 Subject: [PATCH 19/37] quote some sql --- services/actions/cleanup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index cb48f5315a0d0..36259318f57de 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -135,7 +135,7 @@ const deleteEphemeralRunnerBatchSize = 100 // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { runners := []*actions_model.ActionRunner{} - err := db.GetEngine(ctx).Join("INNER", "action_task", "action_task.runner_id = action_runner.id").Where("action_runner.ephemeral and action_task.status != ? and action_task.status != ? and action_task.status != ?)", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) + err := db.GetEngine(ctx).Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`").Where("`action_runner`.`ephemeral` and `action_task`.`status` != ? and `action_task`.`status` != ? and `action_task`.`status` != ?)", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) if err != nil { return fmt.Errorf("find runners: %w", err) } From 9f546abc3851c711587a693790a22bf64307027e Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Wed, 5 Mar 2025 14:00:39 +0100 Subject: [PATCH 20/37] fix merge error --- services/actions/workflow.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/actions/workflow.go b/services/actions/workflow.go index 9aecad171b4c1..ccdefa802daf0 100644 --- a/services/actions/workflow.go +++ b/services/actions/workflow.go @@ -141,14 +141,14 @@ func GetActionWorkflow(ctx *context.APIContext, workflowID string) (*api.ActionW func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, workflowID, ref string, processInputs func(model *model.WorkflowDispatch, inputs map[string]any) error) error { if workflowID == "" { - return util.ErrWrapLocale( + return util.ErrorWrapLocale( util.NewNotExistErrorf("workflowID is empty"), "actions.workflow.not_found", workflowID, ) } if ref == "" { - return util.ErrWrapLocale( + return util.ErrorWrapLocale( util.NewNotExistErrorf("ref is empty"), "form.target_ref_not_exist", ref, ) @@ -158,7 +158,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re cfgUnit := repo.MustGetUnit(ctx, unit.TypeActions) cfg := cfgUnit.ActionsConfig() if cfg.IsWorkflowDisabled(workflowID) { - return util.ErrWrapLocale( + return util.ErrorWrapLocale( util.NewPermissionDeniedErrorf("workflow is disabled"), "actions.workflow.disabled", ) @@ -177,7 +177,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re runTargetCommit, err = gitRepo.GetBranchCommit(ref) } if err != nil { - return util.ErrWrapLocale( + return util.ErrorWrapLocale( util.NewNotExistErrorf("ref %q doesn't exist", ref), "form.target_ref_not_exist", ref, ) @@ -208,7 +208,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re } if len(workflows) == 0 { - return util.ErrWrapLocale( + return util.ErrorWrapLocale( util.NewNotExistErrorf("workflow %q doesn't exist", workflowID), "actions.workflow.not_found", workflowID, ) From b7a31515ccbc20dda7ffd64151fb227971da013e Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Mar 2025 14:40:55 +0100 Subject: [PATCH 21/37] check ci if default false works in mssql --- models/actions/runner.go | 2 +- models/migrations/v1_24/v315.go | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/models/actions/runner.go b/models/actions/runner.go index d50d0946aaf95..9ddf346aa6e82 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -58,7 +58,7 @@ type ActionRunner struct { // Store labels defined in state file (default: .runner file) of `act_runner` AgentLabels []string `xorm:"TEXT"` // Store if this is a runner that only ever get one single job assigned - Ephemeral bool `xorm:"ephemeral"` + Ephemeral bool `xorm:"ephemeral NOT NULL DEFAULT false"` Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` diff --git a/models/migrations/v1_24/v315.go b/models/migrations/v1_24/v315.go index 52003c5daebd2..aefb872d0fb30 100644 --- a/models/migrations/v1_24/v315.go +++ b/models/migrations/v1_24/v315.go @@ -9,14 +9,8 @@ import ( func AddEphemeralToActionRunner(x *xorm.Engine) error { type ActionRunner struct { - Ephemeral bool `xorm:"ephemeral"` + Ephemeral bool `xorm:"ephemeral NOT NULL DEFAULT false"` } - if err := x.Sync(new(ActionRunner)); err != nil { - return err - } - - // update all records to set ephemeral to false - _, err := x.Exec("UPDATE `action_runner` SET `ephemeral` = false WHERE `ephemeral` IS NULL") - return err + return x.Sync(new(ActionRunner)) } From 8e2085a8329423d624ce967b0dc3a99f04ac44d8 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 18:02:46 +0100 Subject: [PATCH 22/37] remove doAPIDeleteRepository --- tests/integration/actions_job_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 455796fad4f02..4d75958544ad1 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -546,8 +546,6 @@ jobs: })) assert.Error(t, err) assert.Nil(t, resp) - - doAPIDeleteRepository(user2APICtx)(t) }) } From 905ec6ef65d4307d09f9aa931790a7ba241ddbcb Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 18:07:10 +0100 Subject: [PATCH 23/37] improve sql statement --- services/actions/cleanup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 36259318f57de..14416e007fea6 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -135,7 +135,7 @@ const deleteEphemeralRunnerBatchSize = 100 // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { runners := []*actions_model.ActionRunner{} - err := db.GetEngine(ctx).Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`").Where("`action_runner`.`ephemeral` and `action_task`.`status` != ? and `action_task`.`status` != ? and `action_task`.`status` != ?)", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) + err := db.GetEngine(ctx).Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`").Where("`action_runner`.`ephemeral` = ? and `action_task`.`status` NOT IN (?, ?, ?)", true, actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) if err != nil { return fmt.Errorf("find runners: %w", err) } From 2d555c8ceee1b883cf3c5db4bda5bc3d0673e7d5 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Wed, 12 Mar 2025 19:24:20 +0000 Subject: [PATCH 24/37] migrate api --- tests/integration/repo_webhook_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index effeff111dd0b..7e85c10d4b709 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -639,7 +639,7 @@ func Test_WebhookWorkflowJob(t *testing.T) { assert.NoError(t, err) runner := newMockRunner() - runner.registerAsRepoRunner(t, "user2", "repo1", "mock-runner", []string{"ubuntu-latest"}) + runner.registerAsRepoRunner(t, "user2", "repo1", "mock-runner", []string{"ubuntu-latest"}, false) // 2. trigger the webhooks From 974c1f2c2684d08f0925b7d6f6262d39be25aaa1 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 21:36:08 +0100 Subject: [PATCH 25/37] ensure CleanupEphemeralRunners does work correctly --- services/actions/cleanup.go | 17 ++++++++------ tests/integration/actions_job_test.go | 32 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 14416e007fea6..052a1995b9faf 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" ) // Cleanup removes expired actions logs, data, artifacts and used ephemeral runners @@ -134,15 +135,17 @@ const deleteEphemeralRunnerBatchSize = 100 // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { - runners := []*actions_model.ActionRunner{} - err := db.GetEngine(ctx).Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`").Where("`action_runner`.`ephemeral` = ? and `action_task`.`status` NOT IN (?, ?, ?)", true, actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked).Limit(deleteEphemeralRunnerBatchSize).Find(&runners) + subQuery := builder.Select("`action_runner`.id"). + From("`action_runner`"). + Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`"). + Where(builder.Eq{"`action_runner`.`ephemeral`": true}). + And(builder.NotIn("`action_task`.`status`", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked)) + 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) } - count, err := db.GetEngine(ctx).Delete(&runners) - if err != nil { - return fmt.Errorf("delete runners: %w", err) - } - log.Info("Removed %d runners", count) + affected, err := res.RowsAffected() + log.Info("Removed %d runners", affected) return nil } diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 4d75958544ad1..ba11c66d9e8ca 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + actions_service "code.gitea.io/gitea/services/actions" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "connectrpc.com/connect" @@ -452,6 +453,9 @@ func TestActionsGiteaContextEphemeral(t *testing.T) { runner := newMockRunner() runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, true) + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + // init the workflow wfTreePath := ".gitea/workflows/pull.yml" wfFileContent := `name: Pull Request @@ -524,11 +528,18 @@ jobs: token := gtCtx["token"].GetStringValue() assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + resp, err := runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.NoError(t, err) assert.Nil(t, resp.Msg.Task) + + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + runner.client.runnerServiceClient.UpdateTask(t.Context(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ State: &runnerv1.TaskState{ Id: actionTask.ID, @@ -546,6 +557,27 @@ jobs: })) assert.Error(t, err) assert.Nil(t, resp) + + // create an runner that picks a job and get force cancelled + runnerToBeRemoved := newMockRunner() + runnerToBeRemoved.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner-to-be-removed", []string{"ubuntu-latest"}, true) + + taskToStopApiObj := runnerToBeRemoved.fetchTask(t) + + taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopApiObj.Id}) + + // verify CleanupEphemeralRunners does not remove the custom crafted runner + actions_service.CleanupEphemeralRunners(t.Context()) + + runnerToRemove := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: taskToStop.RunnerID}) + + err = actions_model.StopTask(t.Context(), taskToStop.ID, actions_model.StatusFailure) + assert.NoError(t, err) + + // verify CleanupEphemeralRunners does remove the custom crafted runner + actions_service.CleanupEphemeralRunners(t.Context()) + + unittest.AssertNotExistsBean(t, runnerToRemove) }) } From ff3dddda3f608b773b5a1dec2e018b6e8e4a0788 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 21:47:50 +0100 Subject: [PATCH 26/37] format --- services/actions/cleanup.go | 1 + 1 file changed, 1 insertion(+) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 052a1995b9faf..7787ae376dabe 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" ) From 01b6f4afa3d6ea3807390968c74359be49cb2679 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 21:50:14 +0100 Subject: [PATCH 27/37] fix test for mssql --- tests/integration/actions_job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index ba11c66d9e8ca..8abeca3ffea83 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -577,7 +577,7 @@ jobs: // verify CleanupEphemeralRunners does remove the custom crafted runner actions_service.CleanupEphemeralRunners(t.Context()) - unittest.AssertNotExistsBean(t, runnerToRemove) + unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID}) }) } From 3362bbb7a03f0823aab376e9b6dfbc1ec40167a5 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 22:10:21 +0100 Subject: [PATCH 28/37] fix lint --- services/actions/cleanup.go | 2 +- tests/integration/actions_job_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 7787ae376dabe..6c549e82ab1bf 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -146,7 +146,7 @@ func CleanupEphemeralRunners(ctx context.Context) error { if err != nil { return fmt.Errorf("find runners: %w", err) } - affected, err := res.RowsAffected() + affected, _ := res.RowsAffected() log.Info("Removed %d runners", affected) return nil } diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 8abeca3ffea83..fb6c73091d7fe 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -562,9 +562,9 @@ jobs: runnerToBeRemoved := newMockRunner() runnerToBeRemoved.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner-to-be-removed", []string{"ubuntu-latest"}, true) - taskToStopApiObj := runnerToBeRemoved.fetchTask(t) + taskToStopAPIObj := runnerToBeRemoved.fetchTask(t) - taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopApiObj.Id}) + taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopAPIObj.Id}) // verify CleanupEphemeralRunners does not remove the custom crafted runner actions_service.CleanupEphemeralRunners(t.Context()) From 3cad61b0240eb5bcda2c5b1184787ccd2c6ef65b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 22:19:09 +0100 Subject: [PATCH 29/37] remove unused deleteEphemeralRunnerBatchSize --- services/actions/cleanup.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 6c549e82ab1bf..c4ec0216d1a5a 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -132,8 +132,6 @@ func CleanupLogs(ctx context.Context) error { return nil } -const deleteEphemeralRunnerBatchSize = 100 - // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { subQuery := builder.Select("`action_runner`.id"). From 8f79a8fb64c0def8602301fddfc4e9c45d60ff12 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 22:29:53 +0100 Subject: [PATCH 30/37] remove broken test (pgsql failure) --- tests/integration/actions_job_test.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index fb6c73091d7fe..eb05063abc2ce 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -557,27 +557,6 @@ jobs: })) assert.Error(t, err) assert.Nil(t, resp) - - // create an runner that picks a job and get force cancelled - runnerToBeRemoved := newMockRunner() - runnerToBeRemoved.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner-to-be-removed", []string{"ubuntu-latest"}, true) - - taskToStopAPIObj := runnerToBeRemoved.fetchTask(t) - - taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopAPIObj.Id}) - - // verify CleanupEphemeralRunners does not remove the custom crafted runner - actions_service.CleanupEphemeralRunners(t.Context()) - - runnerToRemove := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: taskToStop.RunnerID}) - - err = actions_model.StopTask(t.Context(), taskToStop.ID, actions_model.StatusFailure) - assert.NoError(t, err) - - // verify CleanupEphemeralRunners does remove the custom crafted runner - actions_service.CleanupEphemeralRunners(t.Context()) - - unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID}) }) } From f4b6e604de2d990992693f25917bc662f69cd705 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 22:41:08 +0100 Subject: [PATCH 31/37] remove calling CleanupEphemeralRunners in test * pgsql does not work --- tests/integration/actions_job_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index eb05063abc2ce..4ca3396c7da6c 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -21,7 +21,6 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" - actions_service "code.gitea.io/gitea/services/actions" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "connectrpc.com/connect" @@ -453,9 +452,6 @@ func TestActionsGiteaContextEphemeral(t *testing.T) { runner := newMockRunner() runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, true) - // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) - // init the workflow wfTreePath := ".gitea/workflows/pull.yml" wfFileContent := `name: Pull Request @@ -528,18 +524,12 @@ jobs: token := gtCtx["token"].GetStringValue() assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) - // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) - resp, err := runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.NoError(t, err) assert.Nil(t, resp.Msg.Task) - // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) - runner.client.runnerServiceClient.UpdateTask(t.Context(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ State: &runnerv1.TaskState{ Id: actionTask.ID, From e4908f9f8557f4f00fc92ea3b4280c4447b4c980 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 22:55:00 +0100 Subject: [PATCH 32/37] revert remove doAPIDeleteRepository to test for problem --- tests/integration/actions_job_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 4ca3396c7da6c..93462db8e154e 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -547,6 +547,8 @@ jobs: })) assert.Error(t, err) assert.Nil(t, resp) + + doAPIDeleteRepository(user2APICtx)(t) }) } From 41213e9bcf3f92608fecc760913ca4e6820a93c4 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 23:03:56 +0100 Subject: [PATCH 33/37] Revert "remove calling CleanupEphemeralRunners in test" This reverts commit f4b6e604de2d990992693f25917bc662f69cd705. --- tests/integration/actions_job_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 93462db8e154e..cb74c6f5d99df 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + actions_service "code.gitea.io/gitea/services/actions" runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "connectrpc.com/connect" @@ -452,6 +453,9 @@ func TestActionsGiteaContextEphemeral(t *testing.T) { runner := newMockRunner() runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, true) + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + // init the workflow wfTreePath := ".gitea/workflows/pull.yml" wfFileContent := `name: Pull Request @@ -524,12 +528,18 @@ jobs: token := gtCtx["token"].GetStringValue() assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + resp, err := runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, })) assert.NoError(t, err) assert.Nil(t, resp.Msg.Task) + // verify CleanupEphemeralRunners does not remove this runner + actions_service.CleanupEphemeralRunners(t.Context()) + runner.client.runnerServiceClient.UpdateTask(t.Context(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ State: &runnerv1.TaskState{ Id: actionTask.ID, From 3f56e44b84bb8f108f7516be92ec4a8fdb44e36a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 23:05:24 +0100 Subject: [PATCH 34/37] Revert "remove broken test (pgsql failure)" This reverts commit 8f79a8fb64c0def8602301fddfc4e9c45d60ff12. --- tests/integration/actions_job_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index cb74c6f5d99df..9b69b946887bd 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -558,6 +558,28 @@ jobs: assert.Error(t, err) assert.Nil(t, resp) + // create an runner that picks a job and get force cancelled + runnerToBeRemoved := newMockRunner() + runnerToBeRemoved.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner-to-be-removed", []string{"ubuntu-latest"}, true) + + taskToStopAPIObj := runnerToBeRemoved.fetchTask(t) + + taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopAPIObj.Id}) + + // verify CleanupEphemeralRunners does not remove the custom crafted runner + actions_service.CleanupEphemeralRunners(t.Context()) + + runnerToRemove := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: taskToStop.RunnerID}) + + err = actions_model.StopTask(t.Context(), taskToStop.ID, actions_model.StatusFailure) + assert.NoError(t, err) + + // verify CleanupEphemeralRunners does remove the custom crafted runner + actions_service.CleanupEphemeralRunners(t.Context()) + + unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID})# + + // this cleanup is required to allow further tests to pass doAPIDeleteRepository(user2APICtx)(t) }) } From 7d30577b22f505dcd67348cbccddf031351cf9b5 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 12 Mar 2025 23:11:03 +0100 Subject: [PATCH 35/37] cleanup --- tests/integration/actions_job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 9b69b946887bd..4b915b7288696 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -577,7 +577,7 @@ jobs: // verify CleanupEphemeralRunners does remove the custom crafted runner actions_service.CleanupEphemeralRunners(t.Context()) - unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID})# + unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID}) // this cleanup is required to allow further tests to pass doAPIDeleteRepository(user2APICtx)(t) From d66292befe3d7d463d18c042b12069896a211c50 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Thu, 13 Mar 2025 17:23:13 +0100 Subject: [PATCH 36/37] fix CleanupEphemeralRunners mysql --- services/actions/cleanup.go | 2 +- tests/integration/actions_job_test.go | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index c4ec0216d1a5a..23d6e3a49d954 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -135,7 +135,7 @@ func CleanupLogs(ctx context.Context) error { // CleanupEphemeralRunners removes used ephemeral runners which are no longer able to process jobs func CleanupEphemeralRunners(ctx context.Context) error { subQuery := builder.Select("`action_runner`.id"). - From("`action_runner`"). + 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.Eq{"`action_runner`.`ephemeral`": true}). And(builder.NotIn("`action_task`.`status`", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked)) diff --git a/tests/integration/actions_job_test.go b/tests/integration/actions_job_test.go index 4b915b7288696..caab215cee92c 100644 --- a/tests/integration/actions_job_test.go +++ b/tests/integration/actions_job_test.go @@ -454,7 +454,8 @@ func TestActionsGiteaContextEphemeral(t *testing.T) { runner.registerAsRepoRunner(t, baseRepo.OwnerName, baseRepo.Name, "mock-runner", []string{"ubuntu-latest"}, true) // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) + err := actions_service.CleanupEphemeralRunners(t.Context()) + assert.NoError(t, err) // init the workflow wfTreePath := ".gitea/workflows/pull.yml" @@ -529,7 +530,8 @@ jobs: assert.Equal(t, actionTask.TokenLastEight, token[len(token)-8:]) // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) + err = actions_service.CleanupEphemeralRunners(t.Context()) + assert.NoError(t, err) resp, err := runner.client.runnerServiceClient.FetchTask(t.Context(), connect.NewRequest(&runnerv1.FetchTaskRequest{ TasksVersion: 0, @@ -538,7 +540,8 @@ jobs: assert.Nil(t, resp.Msg.Task) // verify CleanupEphemeralRunners does not remove this runner - actions_service.CleanupEphemeralRunners(t.Context()) + err = actions_service.CleanupEphemeralRunners(t.Context()) + assert.NoError(t, err) runner.client.runnerServiceClient.UpdateTask(t.Context(), connect.NewRequest(&runnerv1.UpdateTaskRequest{ State: &runnerv1.TaskState{ @@ -567,7 +570,8 @@ jobs: taskToStop := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskToStopAPIObj.Id}) // verify CleanupEphemeralRunners does not remove the custom crafted runner - actions_service.CleanupEphemeralRunners(t.Context()) + err = actions_service.CleanupEphemeralRunners(t.Context()) + assert.NoError(t, err) runnerToRemove := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunner{ID: taskToStop.RunnerID}) @@ -575,7 +579,8 @@ jobs: assert.NoError(t, err) // verify CleanupEphemeralRunners does remove the custom crafted runner - actions_service.CleanupEphemeralRunners(t.Context()) + err = actions_service.CleanupEphemeralRunners(t.Context()) + assert.NoError(t, err) unittest.AssertNotExistsBean(t, &actions_model.ActionRunner{ID: runnerToRemove.ID}) From 79146cfa020485ae9c210aa1683b07cdefacf30f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Thu, 13 Mar 2025 17:26:13 +0100 Subject: [PATCH 37/37] feedback deletebyid --- services/actions/task.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/actions/task.go b/services/actions/task.go index a514ce2c5069c..9c8198206afc1 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -35,7 +35,7 @@ func PickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv return nil, false, nil } // task has been finished, remove it - _, err = db.GetEngine(ctx).Delete(runner) + _, err = db.DeleteByID[actions_model.ActionRunner](ctx, runner.ID) if err != nil { return nil, false, err }