feat: auto cleanup of offline runners (#7803)

Fixes #7646

Adds a cron job to cleanup action runners that have been offline or inactive for X amount of time.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7803
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Julian Schlarb <julian.schlarb@denktmit.de>
Co-committed-by: Julian Schlarb <julian.schlarb@denktmit.de>

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

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

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: ~

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 <kbd>Tab</kbd> again or <kbd>Escape</kbd> to leave the editor.",
     "editor.textarea.shift_tab_hint": "No indentation on this line. Press <kbd>Shift</kbd> + <kbd>Tab</kbd> again or <kbd>Escape</kbd> 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."
 }

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

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

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

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) {