mirror of
https://github.com/go-gitea/gitea.git
synced 2026-07-26 02:31:52 +00:00
fix(actions): improve runner list status sorting, labels and task job links (#38586)
Several small fixes to the Actions runner management UI.
### Runner task list links to the job, not the workflow run
Relabeled the first column from "Run" to "Job"; it now shows the job ID
and links to the specific job (`/actions/runs/{runID}/jobs/{jobID}`).
Renamed locale key `task_list.run` to `task_list.job`.
### Missing "Disabled" translation
The runner list rendered a grey label via `actions.runners.disabled`,
but that key
did not exist in `locale_en-US.json`, so the raw key string leaked into
the UI.
Replaced `"actions.runners.disabled"` with `"disabled"`.
### Status column sorting ignored active vs idle
Sorting by status ordered purely on `last_online`, but the displayed
status is
computed from both `last_online` (offline) and `last_active` (idle vs
active).
As a result idle runners were interleaved with active ones. Sorting now
ranks by
the computed status (active → idle → offline). Disabled runners sink to
the bottom
of their status group (`is_disabled` as a secondary key), with
`last_online`/`id`
as stable tiebreakers so pagination stays deterministic.
### Status label colors
Active and idle both rendered green. Idle is now yellow, active green,
and
offline/unknown grey; the separate grey "Disabled" badge is unchanged.
This keeps
connectivity visible even for disabled runners (e.g. a disabled runner
still shows
whether it is idle or offline).
<img width="715" height="406" alt="image"
src="https://github.com/user-attachments/assets/9ef06aa8-a870-4de5-9d94-603a58186908"
/>
---------
Co-authored-by: Zettat123 <zettat123@gmail.com>
This commit is contained in:
@@ -270,15 +270,31 @@ func (opts FindRunnerOptions) ToConds() builder.Cond {
|
||||
return cond
|
||||
}
|
||||
|
||||
// runnerStatusOrderExpr builds an ORDER BY fragment that ranks runners by their
|
||||
// computed status (see ActionRunner.Status): active (0), idle (1), offline (2).
|
||||
// The thresholds are evaluated against the current time, mirroring ToConds, so
|
||||
// sorting by status groups active and idle runners instead of interleaving them
|
||||
// by raw last_online.
|
||||
func runnerStatusOrderExpr() string {
|
||||
now := time.Now()
|
||||
offlineThreshold := now.Add(-RunnerOfflineTime).Unix()
|
||||
idleThreshold := now.Add(-RunnerIdleTime).Unix()
|
||||
return fmt.Sprintf("CASE WHEN last_online <= %d THEN 2 WHEN last_active <= %d THEN 1 ELSE 0 END", offlineThreshold, idleThreshold)
|
||||
}
|
||||
|
||||
func (opts FindRunnerOptions) ToOrders() string {
|
||||
// A unique tiebreaker (id) is appended so that runners sharing the same
|
||||
// last_online or name keep a deterministic order across paginated queries,
|
||||
// otherwise the same runner may appear on more than one page.
|
||||
// status, last_online or name keep a deterministic order across paginated
|
||||
// queries, otherwise the same runner may appear on more than one page.
|
||||
statusRank := runnerStatusOrderExpr()
|
||||
switch opts.Sort {
|
||||
case "online":
|
||||
return "last_online DESC, id ASC"
|
||||
// Rank by computed status first so idle runners are not interleaved with
|
||||
// active ones; disabled runners sink to the bottom of their status group
|
||||
// (is_disabled ASC), then last_online breaks ties within a group.
|
||||
return statusRank + " ASC, is_disabled ASC, last_online DESC, id ASC"
|
||||
case "offline":
|
||||
return "last_online ASC, id ASC"
|
||||
return statusRank + " DESC, is_disabled ASC, last_online ASC, id ASC"
|
||||
case "alphabetically":
|
||||
return "name ASC, id ASC"
|
||||
case "reversealphabetically":
|
||||
@@ -288,7 +304,7 @@ func (opts FindRunnerOptions) ToOrders() string {
|
||||
case "oldest":
|
||||
return "id ASC"
|
||||
}
|
||||
return "last_online DESC, id ASC"
|
||||
return statusRank + " ASC, is_disabled ASC, last_online DESC, id ASC"
|
||||
}
|
||||
|
||||
// GetRunnerByUUID returns a runner via uuid
|
||||
|
||||
@@ -4,16 +4,12 @@
|
||||
package actions
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"gitea.dev/models/db"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/timeutil"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestShouldPersistLastOnline(t *testing.T) {
|
||||
@@ -85,65 +81,3 @@ func TestShouldPersistLastActive(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFindRunnerOptions_ToOrders_StableTiebreaker(t *testing.T) {
|
||||
// Sorts on a non-unique column must end with the unique id tiebreaker so
|
||||
// pagination is deterministic; without it, runners sharing the same
|
||||
// last_online or name can appear on more than one page. Sorts already on
|
||||
// the unique id need no tiebreaker.
|
||||
expected := map[string]string{
|
||||
"": "last_online DESC, id ASC",
|
||||
"online": "last_online DESC, id ASC",
|
||||
"offline": "last_online ASC, id ASC",
|
||||
"alphabetically": "name ASC, id ASC",
|
||||
"reversealphabetically": "name DESC, id ASC",
|
||||
"newest": "id DESC",
|
||||
"oldest": "id ASC",
|
||||
}
|
||||
for sort, want := range expected {
|
||||
assert.Equal(t, want, FindRunnerOptions{Sort: sort}.ToOrders(), "sort %q", sort)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFindRunners_PaginationNoDuplicates(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
ctx := t.Context()
|
||||
|
||||
// Create several runners that all share the same last_online value so the
|
||||
// primary sort key (last_online) is tied for all of them.
|
||||
const ownerID = 1000
|
||||
const count = 6
|
||||
for i := range count {
|
||||
runner := &ActionRunner{
|
||||
Name: "paginated-runner",
|
||||
UUID: fmt.Sprintf("PAGINATE-TEST-0000-0000-00000000000%d", i),
|
||||
TokenHash: fmt.Sprintf("paginate-test-token-hash-%d", i),
|
||||
OwnerID: ownerID,
|
||||
RepoID: 0,
|
||||
LastOnline: 42,
|
||||
}
|
||||
require.NoError(t, db.Insert(ctx, runner))
|
||||
}
|
||||
|
||||
// Page through the runners and ensure every id is returned exactly once.
|
||||
seen := make(map[int64]int)
|
||||
const pageSize = 2
|
||||
for page := 1; ; page++ {
|
||||
runners, err := db.Find[ActionRunner](ctx, FindRunnerOptions{
|
||||
ListOptions: db.ListOptions{Page: page, PageSize: pageSize},
|
||||
OwnerID: ownerID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
if len(runners) == 0 {
|
||||
break
|
||||
}
|
||||
for _, r := range runners {
|
||||
seen[r.ID]++
|
||||
}
|
||||
}
|
||||
|
||||
assert.Len(t, seen, count, "each runner should be returned exactly once across all pages")
|
||||
for id, n := range seen {
|
||||
assert.Equal(t, 1, n, "runner %d appeared on %d pages", id, n)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -85,11 +85,15 @@ func (task *ActionTask) IsStopped() bool {
|
||||
return task.Stopped > 0
|
||||
}
|
||||
|
||||
func (task *ActionTask) GetRunLink() string {
|
||||
if task.Job == nil || task.Job.Run == nil {
|
||||
func (task *ActionTask) GetRunJobLink() string {
|
||||
// Run.Repo can be nil when the repository was deleted while task/run rows remain
|
||||
// (TaskList.LoadAttributes copies job.Repo into run.Repo, leaving it nil on a miss).
|
||||
// Run.Link() already returns "" in that case, so guard here to avoid emitting a
|
||||
// broken relative "/jobs/N" link from the Sprintf below.
|
||||
if task.Job == nil || task.Job.Run == nil || task.Job.Run.Repo == nil {
|
||||
return ""
|
||||
}
|
||||
return task.Job.Run.Link()
|
||||
return fmt.Sprintf("%s/jobs/%d", task.Job.Run.Link(), task.Job.ID)
|
||||
}
|
||||
|
||||
func (task *ActionTask) GetCommitLink() string {
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
|
||||
runnerv1 "gitea.dev/actions-proto-go/runner/v1"
|
||||
"gitea.dev/models/db"
|
||||
repo_model "gitea.dev/models/repo"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/actions/jobparser"
|
||||
"gitea.dev/modules/timeutil"
|
||||
@@ -18,6 +19,21 @@ import (
|
||||
"google.golang.org/protobuf/types/known/timestamppb"
|
||||
)
|
||||
|
||||
func TestActionTask_GetRunJobLink(t *testing.T) {
|
||||
repo := &repo_model.Repository{OwnerName: "org", Name: "consumer"}
|
||||
run := &ActionRun{ID: 10, Repo: repo}
|
||||
job := &ActionRunJob{ID: 42, Run: run}
|
||||
|
||||
// a task with a loaded job links to that specific job, not just the run
|
||||
task := &ActionTask{Job: job}
|
||||
assert.Equal(t, run.Link()+"/jobs/42", task.GetRunJobLink())
|
||||
|
||||
// missing job, run or repo yields an empty link instead of a broken URL
|
||||
assert.Empty(t, (&ActionTask{}).GetRunJobLink())
|
||||
assert.Empty(t, (&ActionTask{Job: &ActionRunJob{ID: 42}}).GetRunJobLink())
|
||||
assert.Empty(t, (&ActionTask{Job: &ActionRunJob{ID: 42, Run: &ActionRun{ID: 10}}}).GetRunJobLink())
|
||||
}
|
||||
|
||||
func TestMakeTaskStepDisplayName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -3741,7 +3741,7 @@
|
||||
"actions.runners.runner_title": "Runner",
|
||||
"actions.runners.task_list": "Recent tasks on this runner",
|
||||
"actions.runners.task_list.no_tasks": "There is no task yet.",
|
||||
"actions.runners.task_list.run": "Run",
|
||||
"actions.runners.task_list.job": "Job",
|
||||
"actions.runners.task_list.status": "Status",
|
||||
"actions.runners.task_list.repository": "Repository",
|
||||
"actions.runners.task_list.commit": "Commit",
|
||||
|
||||
@@ -65,7 +65,7 @@
|
||||
<table class="ui very basic table unstackable">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.task_list.run"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.task_list.job"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.task_list.status"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.task_list.repository"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.task_list.commit"}}</th>
|
||||
@@ -75,7 +75,7 @@
|
||||
<tbody>
|
||||
{{range .Tasks}}
|
||||
<tr>
|
||||
<td><a href="{{.GetRunLink}}" target="_blank">{{.ID}}</a></td>
|
||||
<td>{{if .Job}}<a href="{{.GetRunJobLink}}" target="_blank">{{.Job.ID}}</a>{{else}}{{.ID}}{{end}}</td>
|
||||
<td><span class="ui label task-status-{{.Status.String}}">{{.Status.LocaleString ctx.Locale}}</span></td>
|
||||
<td><a href="{{.GetRepoLink}}" target="_blank">{{.GetRepoName}}</a></td>
|
||||
<td>
|
||||
|
||||
@@ -62,6 +62,10 @@
|
||||
{{if .AllowBulkActions}}
|
||||
<th class="tw-w-8"><div class="ui checkbox tw-flex"><input type="checkbox" class="runner-bulk-select-all" aria-label="{{ctx.Locale.Tr "admin.notices.select_all"}}"></div></th>
|
||||
{{end}}
|
||||
<th data-sortt-asc="alphabetically" data-sortt-desc="reversealphabetically">
|
||||
{{ctx.Locale.Tr "actions.runners.name"}}
|
||||
{{SortArrow "alphabetically" "reversealphabetically" .SortType false}}
|
||||
</th>
|
||||
<th data-sortt-asc="online" data-sortt-desc="offline">
|
||||
{{ctx.Locale.Tr "actions.runners.status"}}
|
||||
{{SortArrow "online" "offline" .SortType false}}
|
||||
@@ -70,10 +74,6 @@
|
||||
{{ctx.Locale.Tr "actions.runners.id"}}
|
||||
{{SortArrow "oldest" "newest" .SortType false}}
|
||||
</th>
|
||||
<th data-sortt-asc="alphabetically" data-sortt-desc="reversealphabetically">
|
||||
{{ctx.Locale.Tr "actions.runners.name"}}
|
||||
{{SortArrow "alphabetically" "reversealphabetically" .SortType false}}
|
||||
</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.version"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.owner_type"}}</th>
|
||||
<th>{{ctx.Locale.Tr "actions.runners.labels"}}</th>
|
||||
@@ -87,12 +87,12 @@
|
||||
{{if $.AllowBulkActions}}
|
||||
<td><div class="ui checkbox tw-flex"><input type="checkbox" class="runner-bulk-select" data-runner-id="{{.ID}}" aria-label="{{ctx.Locale.Tr "repo.issues.action_check"}}: {{.Name}}"></div></td>
|
||||
{{end}}
|
||||
<td><p data-tooltip-content="{{.Description}}">{{.Name}}</p></td>
|
||||
<td>
|
||||
<span class="ui label {{if .IsOnline}}green{{end}}">{{.StatusLocaleName ctx.Locale}}</span>
|
||||
{{if .IsDisabled}}<span class="ui grey label">{{ctx.Locale.Tr "actions.runners.disabled"}}</span>{{end}}
|
||||
<span class="ui label {{if eq .StatusName "active"}}green{{else if eq .StatusName "idle"}}yellow{{else if eq .StatusName "offline"}}red{{end}}">{{.StatusLocaleName ctx.Locale}}</span>
|
||||
{{if .IsDisabled}}<span class="ui grey label">{{ctx.Locale.Tr "disabled"}}</span>{{end}}
|
||||
</td>
|
||||
<td>{{.ID}}</td>
|
||||
<td><p data-tooltip-content="{{.Description}}">{{.Name}}</p></td>
|
||||
<td>{{if .Version}}{{.Version}}{{else}}{{ctx.Locale.Tr "unknown"}}{{end}}</td>
|
||||
<td><span data-tooltip-content="{{.BelongsToOwnerName}}">{{.BelongsToOwnerType.LocaleString ctx.Locale}}</span></td>
|
||||
<td>
|
||||
|
||||
130
tests/integration/actions_runner_sort_test.go
Normal file
130
tests/integration/actions_runner_sort_test.go
Normal file
@@ -0,0 +1,130 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
actions_model "gitea.dev/models/actions"
|
||||
"gitea.dev/models/db"
|
||||
"gitea.dev/modules/timeutil"
|
||||
"gitea.dev/tests"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestFindRunnersSortByStatus verifies that sorting by status ranks runners by
|
||||
// their computed status (active, then idle, then offline) instead of interleaving
|
||||
// idle runners with active ones by raw last_online, and that a disabled runner
|
||||
// sinks to the bottom of its status group rather than mixing with enabled runners.
|
||||
//
|
||||
// It lives in tests/integration rather than a unit test because the status rank is
|
||||
// a database-evaluated CASE expression; unit tests only run against SQLite, so the
|
||||
// expression must be exercised against MySQL/PostgreSQL/MSSQL in CI to catch dialect
|
||||
// differences.
|
||||
func TestFindRunnersSortByStatus(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
ctx := t.Context()
|
||||
|
||||
const ownerID = 1001
|
||||
now := time.Now()
|
||||
// An idle runner that went online most recently would sort before an active
|
||||
// runner when ordering by last_online alone; the status rank must override that.
|
||||
insert := func(name string, lastOnline, lastActive time.Time, disabled bool) {
|
||||
require.NoError(t, db.Insert(ctx, &actions_model.ActionRunner{
|
||||
Name: name,
|
||||
UUID: "STATUS-SORT-" + name,
|
||||
TokenHash: "status-sort-token-" + name,
|
||||
OwnerID: ownerID,
|
||||
LastOnline: timeutil.TimeStamp(lastOnline.Unix()),
|
||||
LastActive: timeutil.TimeStamp(lastActive.Unix()),
|
||||
IsDisabled: disabled,
|
||||
}))
|
||||
}
|
||||
// Each disabled runner has the most recent last_online within its status group,
|
||||
// so it would sort first in that group if the disabled flag were ignored; the
|
||||
// last_active value keeps it in the intended group (offline<idle<active).
|
||||
insert("active", now.Add(-8*time.Second), now.Add(-5*time.Second), false)
|
||||
insert("active-disabled", now, now.Add(-2*time.Second), true)
|
||||
insert("idle", now.Add(-30*time.Second), now.Add(-20*time.Second), false)
|
||||
insert("idle-disabled", now, now.Add(-20*time.Second), true)
|
||||
insert("offline", now.Add(-3*time.Minute), now.Add(-3*time.Minute), false)
|
||||
insert("offline-disabled", now.Add(-90*time.Second), now.Add(-90*time.Second), true)
|
||||
|
||||
names := func(runners []*actions_model.ActionRunner) []string {
|
||||
out := make([]string, len(runners))
|
||||
for i, r := range runners {
|
||||
out[i] = r.Name
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Active group first, then idle, then offline; within each group enabled before disabled.
|
||||
runners, err := db.Find[actions_model.ActionRunner](ctx, actions_model.FindRunnerOptions{OwnerID: ownerID, Sort: "online"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, []string{
|
||||
"active", "active-disabled",
|
||||
"idle", "idle-disabled",
|
||||
"offline", "offline-disabled",
|
||||
}, names(runners))
|
||||
|
||||
// The descending status sort reverses the group order but still keeps disabled
|
||||
// runners at the bottom of their group.
|
||||
runners, err = db.Find[actions_model.ActionRunner](ctx, actions_model.FindRunnerOptions{OwnerID: ownerID, Sort: "offline"})
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, []string{
|
||||
"offline", "offline-disabled",
|
||||
"idle", "idle-disabled",
|
||||
"active", "active-disabled",
|
||||
}, names(runners))
|
||||
}
|
||||
|
||||
// TestFindRunnersPaginationNoDuplicates verifies that the unique id tiebreaker in
|
||||
// FindRunnerOptions.ToOrders keeps pagination deterministic when the primary sort
|
||||
// key (last_online) is tied for every runner. It is an integration test so the
|
||||
// ordering is validated against every supported database, not only SQLite.
|
||||
func TestFindRunnersPaginationNoDuplicates(t *testing.T) {
|
||||
defer tests.PrepareTestEnv(t)()
|
||||
ctx := t.Context()
|
||||
|
||||
// Create several runners that all share the same last_online value so the
|
||||
// primary sort key (last_online) is tied for all of them.
|
||||
const ownerID = 1000
|
||||
const count = 6
|
||||
for i := range count {
|
||||
require.NoError(t, db.Insert(ctx, &actions_model.ActionRunner{
|
||||
Name: "paginated-runner",
|
||||
UUID: fmt.Sprintf("PAGINATE-TEST-0000-0000-00000000000%d", i),
|
||||
TokenHash: fmt.Sprintf("paginate-test-token-hash-%d", i),
|
||||
OwnerID: ownerID,
|
||||
RepoID: 0,
|
||||
LastOnline: 42,
|
||||
}))
|
||||
}
|
||||
|
||||
// Page through the runners and ensure every id is returned exactly once.
|
||||
seen := make(map[int64]int)
|
||||
const pageSize = 2
|
||||
for page := 1; ; page++ {
|
||||
runners, err := db.Find[actions_model.ActionRunner](ctx, actions_model.FindRunnerOptions{
|
||||
ListOptions: db.ListOptions{Page: page, PageSize: pageSize},
|
||||
OwnerID: ownerID,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
if len(runners) == 0 {
|
||||
break
|
||||
}
|
||||
for _, r := range runners {
|
||||
seen[r.ID]++
|
||||
}
|
||||
}
|
||||
|
||||
assert.Len(t, seen, count, "each runner should be returned exactly once across all pages")
|
||||
for id, n := range seen {
|
||||
assert.Equal(t, 1, n, "runner %d appeared on %d pages", id, n)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user