diff --git a/models/actions/runner.go b/models/actions/runner.go index 4d5056b425..bece1ae301 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -16,6 +16,7 @@ import ( repo_model "forgejo.org/models/repo" "forgejo.org/models/shared/types" user_model "forgejo.org/models/user" + "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/timeutil" "forgejo.org/modules/translation" @@ -353,3 +354,53 @@ func FixRunnersWithoutBelongingRepo(ctx context.Context) (int64, error) { } return res.RowsAffected() } + +func DeleteOfflineRunners(ctx context.Context, olderThan timeutil.TimeStamp, globalOnly bool) error { + log.Info("Doing: DeleteOfflineRunners") + + if olderThan.AsTime().After(timeutil.TimeStampNow().AddDuration(-RunnerOfflineTime).AsTime()) { + return fmt.Errorf("invalid `cron.cleanup_offline_runners.older_than`value: must be at least %q", RunnerOfflineTime) + } + + cond := builder.Or( + // never online + builder.And(builder.Eq{"last_online": 0}, builder.Lt{"created": olderThan}), + // was online but offline + builder.And(builder.Gt{"last_online": 0}, builder.Lt{"last_online": olderThan}), + ) + + if globalOnly { + cond = builder.And(cond, builder.Eq{"owner_id": 0}, builder.Eq{"repo_id": 0}) + } + + if err := db.Iterate( + ctx, + cond, + func(ctx context.Context, r *ActionRunner) error { + if err := DeleteRunner(ctx, r); err != nil { + return fmt.Errorf("DeleteOfflineRunners: %w", err) + } + lastOnline := r.LastOnline.AsTime() + olderThanTime := olderThan.AsTime() + if !lastOnline.IsZero() && lastOnline.Before(olderThanTime) { + log.Info( + "Deleted runner [ID: %d, Name: %s], last online %s ago", + r.ID, r.Name, olderThanTime.Sub(lastOnline).String(), + ) + } else { + log.Info( + "Deleted runner [ID: %d, Name: %s], unused since %s ago", + r.ID, r.Name, olderThanTime.Sub(r.Created.AsTime()).String(), + ) + } + + return nil + }, + ); err != nil { + return err + } + + log.Info("Finished: DeleteOfflineRunners") + + return nil +} diff --git a/models/actions/runner_test.go b/models/actions/runner_test.go index 0623e66046..1916c35a76 100644 --- a/models/actions/runner_test.go +++ b/models/actions/runner_test.go @@ -6,10 +6,12 @@ import ( "encoding/binary" "fmt" "testing" + "time" auth_model "forgejo.org/models/auth" "forgejo.org/models/db" "forgejo.org/models/unittest" + "forgejo.org/modules/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -73,3 +75,68 @@ func TestDeleteRunner(t *testing.T) { idAsBinary[6], idAsBinary[7]) assert.Equal(t, idAsHexadecimal, after.UUID[19:]) } + +func TestDeleteOfflineRunnersRunnerGlobalOnly(t *testing.T) { + baseTime := time.Date(2024, 5, 19, 7, 40, 32, 0, time.UTC) + timeutil.MockSet(baseTime) + defer timeutil.MockUnset() + + require.NoError(t, unittest.PrepareTestDatabase()) + + olderThan := timeutil.TimeStampNow().Add(-timeutil.Hour) + + require.NoError(t, DeleteOfflineRunners(db.DefaultContext, olderThan, true)) + + // create at test base time + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 12345678}) + // last_online test base time + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000001}) + // created one month ago but a repo + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000002}) + // last online one hour ago + unittest.AssertNotExistsBean(t, &ActionRunner{ID: 10000003}) + // last online 10 seconds ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000004}) + // created 1 month ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000005}) + // created 1 hour ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000006}) + // last online 1 hour ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000007}) +} + +func TestDeleteOfflineRunnersAll(t *testing.T) { + baseTime := time.Date(2024, 5, 19, 7, 40, 32, 0, time.UTC) + timeutil.MockSet(baseTime) + defer timeutil.MockUnset() + + require.NoError(t, unittest.PrepareTestDatabase()) + + olderThan := timeutil.TimeStampNow().Add(-timeutil.Hour) + + require.NoError(t, DeleteOfflineRunners(db.DefaultContext, olderThan, false)) + + // create at test base time + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 12345678}) + // last_online test base time + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000001}) + // created one month ago + unittest.AssertNotExistsBean(t, &ActionRunner{ID: 10000002}) + // last online one hour ago + unittest.AssertNotExistsBean(t, &ActionRunner{ID: 10000003}) + // last online 10 seconds ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000004}) + // created 1 month ago + unittest.AssertNotExistsBean(t, &ActionRunner{ID: 10000005}) + // created 1 hour ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000006}) + // last online 1 hour ago + unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: 10000007}) +} + +func TestDeleteOfflineRunnersErrorOnInvalidOlderThanValue(t *testing.T) { + baseTime := time.Date(2024, 5, 19, 7, 40, 32, 0, time.UTC) + timeutil.MockSet(baseTime) + defer timeutil.MockUnset() + require.Error(t, DeleteOfflineRunners(db.DefaultContext, timeutil.TimeStampNow(), false)) +} diff --git a/models/fixtures/action_runner.yml b/models/fixtures/action_runner.yml index 94deac998e..fcf26d49b6 100644 --- a/models/fixtures/action_runner.yml +++ b/models/fixtures/action_runner.yml @@ -18,3 +18,122 @@ created: 1716104432 updated: 1716104432 deleted: ~ +- id: 10000001 + uuid: 10d3b248-6460-4bf5-b819-1f5b3109e10f + name: global-online + version: v6.3.1+7-gc4c0ca0 + owner_id: 0 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: 7e9ed71f64e98ce1f70e94c63f3cb6c41a8cb0b90de3e1daf7ec5c35361d60ed44da67c5ac393b7aaf443dcfc766007dc828 + token_salt: WUcgZWl7mW + last_online: 1716104422 + last_active: 0 + agent_labels: '["docker"]' + created: 1716104431 + updated: 1716104422 + deleted: ~ +- id: 10000002 + uuid: 1d188484-dd97-4a70-b707-5e87b578ab6b + name: repo-never-used + version: v6.3.1+7-gc4c0ca0 + owner_id: 0 + repo_id: 1 + description: "" + base: 0 + repo_range: "" + token_hash: 51e88c17ac8b54dd101dc2e4f530a71643c703adba7170f4b1a28f1cb483b4cfb107798c521e0532ef3c6480b64518a5c6a5 + token_salt: 4rh8ncXYIO + last_online: 0 + last_active: 0 + agent_labels: '["docker"]' + created: 1713512432 + updated: 1713512432 + deleted: ~ +- id: 10000003 + uuid: 7a039c6b-b0b2-4cf5-a93d-715d617f99e2 + name: global-offline + version: v6.3.1+7-gc4c0ca0 + owner_id: 0 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: c76960c56bc6069f0d1648991ec626500abe8c15286f5c355d565c3b5ba945d7d6f1272a6c77849e592528179511b94f5d69 + token_salt: TFMe2jhOkB + last_online: 1715499632 + last_active: 0 + agent_labels: '["docker"]' + created: 1715499632 + updated: 1715499632 + deleted: ~ +- id: 10000004 + uuid: 93ca7fdd-faca-4df6-a474-8345263ef10b + name: user-online + version: v6.3.1+7-gc4c0ca0 + owner_id: 1 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: 6ddf7f0f2301d2b3f66418145dc497a6d09fa6586e659afcb5ae2a0c5b639561d795aff8062537db9df73b396842ea826134 + token_salt: QcdGuReAp4 + last_online: 1716104422 + last_active: 0 + agent_labels: '["docker"]' + created: 1716104431 + updated: 1716104422 + deleted: ~ +- id: 10000005 + uuid: a8534df6-c4be-40f4-9714-903b69d973d9 + name: user-never-used + version: v6.3.1+7-gc4c0ca0 + owner_id: 1 + repo_id: 0 + description: desc + base: 0 + repo_range: "" + token_hash: 4441de7defcfc3d21baa608dec66a562cf23307abddaabdbb836907ac5f48c8780c354891916c525b79ec7af8e95be7a09b4 + token_salt: ONNqIOnj3t + last_online: 0 + last_active: 0 + agent_labels: '["docker"]' + created: 1713512433 + updated: 1713512433 + deleted: ~ +- id: 10000006 + uuid: e1c5bb6c-de68-4335-8955-5192f76708ac + name: orga-fresh-created + version: v6.3.1+7-gc4c0ca0 + owner_id: 35 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: a61f9ee48c6847d243ace0a8936efe80af9277c7bc46d6da6e03d1d406608b8023ee66600ad24f0effaa8e3338f92ac97ac9 + token_salt: fZJKjrFGWA + last_online: 0 + last_active: 0 + agent_labels: '["docker"]' + created: 1716100832 + updated: 1716100832 + deleted: ~ +- id: 10000007 + uuid: ff755f06-948e-479b-8031-5b3e9f123e32 + name: orga-offline + version: v6.3.1+7-gc4c0ca0 + owner_id: 35 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: 9372efb38f9b64efe65065380abe2f24ef34a59d9619f4cdc08f1151e9849f0b6e722aa10538e8730288de6e2f09acdac695 + token_salt: TnU7iiIdCb + last_online: 1716100832 + last_active: 0 + agent_labels: '["docker"]' + created: 1736085520 + updated: 1716100832 + deleted: ~ diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 3015be3ecd..96d0c4bc63 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -92,5 +92,6 @@ "discussion.locked": "This discussion has been locked. Commenting is limited to contributors.", "editor.textarea.tab_hint": "Line already indented. Press Tab again or Escape to leave the editor.", "editor.textarea.shift_tab_hint": "No indentation on this line. Press Shift + Tab again or Escape to leave the editor.", + "admin.dashboard.cleanup_offline_runners": "Cleanup offline runners", "meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it." } diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index fde5286e60..918be0f185 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -126,3 +126,9 @@ func CleanupLogs(ctx context.Context) error { log.Info("Removed %d logs", count) return nil } + +// CleanupOfflineRunners removes offline runners +func CleanupOfflineRunners(ctx context.Context, duration time.Duration, globalOnly bool) error { + olderThan := timeutil.TimeStampNow().AddDuration(-duration) + return actions_model.DeleteOfflineRunners(ctx, olderThan, globalOnly) +} diff --git a/services/cron/setting.go b/services/cron/setting.go index 7fd4c4e1d8..2db6c15370 100644 --- a/services/cron/setting.go +++ b/services/cron/setting.go @@ -46,6 +46,13 @@ type CleanupHookTaskConfig struct { NumberToKeep int } +// CleanupOfflineRunnersConfig represents a cron task with settings to clean up offline-runner +type CleanupOfflineRunnersConfig struct { + BaseConfig + OlderThan time.Duration + GlobalScopeOnly bool +} + // GetSchedule returns the schedule for the base config func (b *BaseConfig) GetSchedule() string { return b.Schedule diff --git a/services/cron/tasks_actions.go b/services/cron/tasks_actions.go index a7fd3cd0bc..2cd484fa69 100644 --- a/services/cron/tasks_actions.go +++ b/services/cron/tasks_actions.go @@ -5,6 +5,7 @@ package cron import ( "context" + "time" user_model "forgejo.org/models/user" "forgejo.org/modules/setting" @@ -20,6 +21,7 @@ func initActionsTasks() { registerCancelAbandonedJobs() registerScheduleTasks() registerActionsCleanup() + registerOfflineRunnersCleanup() } func registerStopZombieTasks() { @@ -74,3 +76,22 @@ func registerActionsCleanup() { return actions_service.Cleanup(ctx) }) } + +func registerOfflineRunnersCleanup() { + RegisterTaskFatal("cleanup_offline_runners", &CleanupOfflineRunnersConfig{ + BaseConfig: BaseConfig{ + Enabled: false, + RunAtStart: false, + Schedule: "@midnight", + }, + GlobalScopeOnly: true, + OlderThan: time.Hour * 24, + }, func(ctx context.Context, _ *user_model.User, cfg Config) error { + c := cfg.(*CleanupOfflineRunnersConfig) + return actions_service.CleanupOfflineRunners( + ctx, + c.OlderThan, + c.GlobalScopeOnly, + ) + }) +} diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 104fdf4f67..9351dd9c20 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -333,11 +333,11 @@ func TestAPICron(t *testing.T) { AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "28", resp.Header().Get("X-Total-Count")) + assert.Equal(t, "29", resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 28) + assert.Len(t, crons, 29) }) t.Run("Execute", func(t *testing.T) {