Skip to content

Feature: Add per-runner “Disable/Pause” #36776

Open
bircni wants to merge 9 commits intogo-gitea:mainfrom
bircni:feature/per-runner-disable
Open

Feature: Add per-runner “Disable/Pause” #36776
bircni wants to merge 9 commits intogo-gitea:mainfrom
bircni:feature/per-runner-disable

Conversation

@bircni
Copy link
Contributor

@bircni bircni commented Feb 27, 2026

This PR adds per-runner disable/enable support for Gitea Actions so a registered runner can be paused from picking up new jobs without unregistering.

Disabled runners stay registered and online but are excluded from new task assignment; running tasks are allowed to finish. Re-enabling restores pickup, and runner list/get responses now expose disabled state.

Also added an endpoint for testing http://localhost:3000/devtest/runner-edit/enable

Bildschirmfoto 2026-02-27 um 22 13 24

Fixes: #36767

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 27, 2026
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/migrations labels Feb 27, 2026
@bircni
Copy link
Contributor Author

bircni commented Feb 28, 2026

Authorship: Reviewed by Codex (GPT-5.3)

I reviewed PR #36776 end-to-end (models, migrations, API/web routes, conversion layer, templates, and integration tests) and I did not find any blocking issues.

What I validated:

  • is_disabled persistence + migration path (models/migrations/v1_26/v326.go, test included)
  • API enable/disable coverage across admin/user/org/repo routes
  • Web enable/disable handlers and permission checks in shared runner settings
  • Scheduler behavior change to prevent disabled runners from receiving new work (services/actions/task.go, models/actions/task.go)
  • API response surface for disabled state (modules/structs/repo_actions.go, services/convert/convert.go)
  • Integration coverage for toggle behavior and scope checks

Local checks run:

  • go test -tags 'sqlite sqlite_unlock_notify' ./models/migrations/v1_26 -run Test_AddDisabledToActionRunner -count=1
  • go test -tags 'sqlite sqlite_unlock_notify' ./services/actions -run TestNonExistent -count=1 (compile check)

@silverwind - first review with codex 5.3 😄

@bircni bircni marked this pull request as ready for review February 28, 2026 11:42
@silverwind silverwind requested a review from Copilot February 28, 2026 13:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a per-runner disable/pause feature for Gitea Actions, allowing admins to stop a registered runner from picking up new jobs without unregistering it. Disabled runners remain registered and online but are excluded from new task assignment; running tasks finish normally. Re-enabling a runner restores task pickup. The feature is exposed via REST API endpoints for all runner scopes (global/admin, org, user, repo) and through the existing web UI runner edit page.

Changes:

  • DB migration (#326) adding is_disabled column to action_runner table, with model field IsDisabled, and SetRunnerDisabled() function that updates the field and bumps the task version inside a transaction
  • New DisableRunner/EnableRunner API endpoints at all four scopes (admin/org/user/repo), with updated ActionRunner API struct exposing a disabled field and corresponding swagger documentation
  • Web UI changes adding an enable/disable toggle button to the runner edit page, with a new actions.runners.availability status indicator and locale strings

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
models/actions/runner.go Adds IsDisabled field and SetRunnerDisabled with transactional update + task version bump
models/actions/task.go Adds IsDisabled guard in CreateTaskForRunner (defense-in-depth)
models/migrations/v1_26/v326.go + v326_test.go DB migration adds is_disabled NOT NULL DEFAULT false column
models/migrations/migrations.go Registers migration 326
modules/structs/repo_actions.go Adds Disabled bool to ActionRunner API struct
services/actions/task.go Adds IsDisabled early-exit guard in PickTask
services/actions/interface.go Adds DisableRunner/EnableRunner to the API interface
services/convert/convert.go Maps runner.IsDisabled to Disabled in API response
routers/api/v1/shared/runners.go Implements shared updateRunnerDisabled, DisableRunner, EnableRunner
routers/api/v1/admin/runners.go + org/action.go + repo/action.go + user/runners.go Scope-specific handler wrappers with swagger comments
routers/api/v1/api.go Registers PUT /{runner_id}/disable and /{runner_id}/enable routes
routers/web/shared/actions/runners.go Implements web RunnerDisablePost/RunnerEnablePost handlers
routers/web/web.go Registers POST disable/enable web routes; adds devtest runner-edit routes
routers/web/devtest/runner_edit.go Devtest handler for previewing enable/disable UI states
templates/shared/actions/runner_edit.tmpl Adds availability status indicator and enable/disable toggle button
templates/devtest/runner-edit.tmpl Devtest page for UI verification
templates/swagger/v1_json.tmpl Adds swagger definitions for all new endpoints and disabled field in ActionRunner definition; fixes "an" → "a" article in repo/user endpoint summaries
options/locale/locale_en-US.json Adds locale strings for availability label and enable/disable actions
tests/integration/api_actions_runner_test.go Integration tests for disable/enable API at all scopes, including idempotency and scope-enforcement
tests/integration/actions_runner_modify_test.go Web integration tests for disable/enable in runner modify flow
tests/integration/actions_job_test.go End-to-end test verifying disabled runner does not pick up tasks but re-enabled runner does

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bircni bircni force-pushed the feature/per-runner-disable branch from 9d903da to 678ae44 Compare February 28, 2026 13:59
@bircni
Copy link
Contributor Author

bircni commented Mar 3, 2026

@silverwind fixed copilots comments

@lunny lunny added the type/enhancement An improvement of existing functionality label Mar 3, 2026
@bircni
Copy link
Contributor Author

bircni commented Mar 5, 2026

anyone who can review? :-)

@lunny
Copy link
Member

lunny commented Mar 5, 2026

anyone who can review? :-)

I will take care of that, but it may take some time since the task is relatively large.

@bircni
Copy link
Contributor Author

bircni commented Mar 6, 2026

Found some issues fixing them

@bircni bircni force-pushed the feature/per-runner-disable branch from d5f0054 to 5bf5c62 Compare March 7, 2026 21:25
@bircni
Copy link
Contributor Author

bircni commented Mar 7, 2026

@lunny should be ready for a review now - rebased and fixed some stuff 😄

@bircni bircni force-pushed the feature/per-runner-disable branch from 5bf5c62 to a53dde2 Compare March 7, 2026 21:35
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. fetchTaskOnce will panic on error (tests/integration/actions_runner_test.go): Uses assert.NoError which records the failure but continues execution. If FetchTask returns an error, resp is nil and resp.Msg.Task panics. Should use require.NoError to fail fast.

  2. Consider a single toggle endpoint: Separate /disable and /enable endpoints result in 8 route registrations + 8 swagger handler functions for a boolean toggle. A single PATCH .../runners/{runner_id} with {"disabled": true/false} would halve the API surface and be more conventional REST. Though I understand this may be intentional for simplicity.

  1. Redundant early-return in API handler: updateRunnerDisabled in routers/api/v1/shared/runners.go checks runner.IsDisabled == isDisabled before calling SetRunnerDisabled, but SetRunnerDisabled already has this exact same check. The API handler check just saves one function call — not worth the duplicated logic.

silverwind

This comment was marked as duplicate.

@bircni
Copy link
Contributor Author

bircni commented Mar 8, 2026

@silverwind which of these comments do you want fixed?

@silverwind
Copy link
Member

silverwind commented Mar 8, 2026

1 and 3 at least. And don't ask me questions like that, it's your job to judge these comments :)

@bircni
Copy link
Contributor Author

bircni commented Mar 8, 2026

@silverwind is there something wrong with CI?
I didn't touch the part which raises errors now :-(

@bircni bircni force-pushed the feature/per-runner-disable branch from dbda9f6 to c1d40c4 Compare March 8, 2026 20:58
@bircni
Copy link
Contributor Author

bircni commented Mar 8, 2026

ig I found it: #36858

@bircni
Copy link
Contributor Author

bircni commented Mar 8, 2026

It was a lie :-( whyyyyy

@silverwind
Copy link
Member

silverwind commented Mar 8, 2026

int64-related issues may be coming from #36858. Also I think you need to run make tidy.

@bircni
Copy link
Contributor Author

bircni commented Mar 9, 2026

@silverwind @lunny fixed and ready for review again

@bircni bircni requested review from lunny and silverwind March 9, 2026 19:00
- Use *bool for EditActionRunnerOption.Disabled (standard pattern),
  removing custom UnmarshalJSON and modules/json import from structs
- Fix enable/disable buttons to use link-action with JSONRedirect
  instead of formaction which does not work with JS-driven forms
- Remove redundant IsDisabled check in CreateTaskForRunner (already
  guarded in PickTask)
- Use grey label for disabled badge in runner list and edit page

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind
Copy link
Member

Some cleanups done and UI now uses "grey" label which looks better here imho:

Screenshot 2026-03-10 at 06 41 42

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 10, 2026
// the current TasksVersion (not 0), the server still re-evaluates after disable
// and does not assign a task to a disabled runner (version bump on disable).
func TestRunnerDisableEnableTasksVersionPath(t *testing.T) {
onGiteaRun(t, func(t *testing.T, _ *url.URL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean , is it possible to merge TestRunnerDisableEnable and TestRunnerDisableEnableTasksVersionPath, then only use one "onGiteaRun"?

The reason is that "onGiteaRun" is slow, keeping adding new "onGiteaRun" calls just slows down the testing more and more.

So if there is a chance, I would always suggest to merge test cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/need 1 This PR needs approval from one additional maintainer to be merged. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/migrations modifies/templates This PR modifies the template files type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add per-runner “Disable/Pause” so a registered runner can stop picking up jobs (without unregistering)

6 participants