Support changing labels of Actions runner without re-registration (#24806)

close #24540 related: - Protocol: https://gitea.com/gitea/actions-proto-def/pulls/9 - Runner side: https://gitea.com/gitea/act_runner/pulls/201 changes: - Add column of `labels` to table `action_runner`, and combine the value of `agent_labels` and `custom_labels` column to `labels` column. - Store `labels` when registering `act_runner`. - Update `labels` when `act_runner` starting and calling `Declare`. - Users cannot modify the `custom labels` in edit page any more. other changes: - Store `version` when registering `act_runner`. - If runner is latest version, parse version from `Declare`. But older version runner still parse version from request header.
2023-06-13 22:28:31 +08:00 · 2023-06-13 22:28:31 +08:00 · 8228751c55
parent 6bbccdd177
commit 8228751c55
15 changed files with 113 additions and 63 deletions
--- a/go.mod
+++ b/go.mod
@ -3,7 +3,7 @@ module code.gitea.io/gitea
 go 1.20

 require (
-	code.gitea.io/actions-proto-go v0.2.1
+	code.gitea.io/actions-proto-go v0.3.0
 	code.gitea.io/gitea-vet v0.2.2
 	code.gitea.io/sdk/gitea v0.15.1
 	codeberg.org/gusted/mcaptcha v0.0.0-20220723083913-4f3072e1d570
--- a/go.sum
+++ b/go.sum
@ -40,8 +40,8 @@ cloud.google.com/go/storage v1.6.0/go.mod h1:N7U0C8pVQ/+NIKOBQyamJIeKQKkZ+mxpohl
 cloud.google.com/go/storage v1.8.0/go.mod h1:Wv1Oy7z6Yz3DshWRJFhqM/UCfaWIRTdp0RXyy7KQOVs=
 cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9ullr3+Kg0=
 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo=
-code.gitea.io/actions-proto-go v0.2.1 h1:ToMN/8thz2q10TuCq8dL2d8mI+/pWpJcHCvG+TELwa0=
-code.gitea.io/actions-proto-go v0.2.1/go.mod h1:00ys5QDo1iHN1tHNvvddAcy2W/g+425hQya1cCSvq9A=
+code.gitea.io/actions-proto-go v0.3.0 h1:9Tvg8+TaaCXPKi6EnWl9vVgs2VZsj1Cs5afnsHa4AmM=
+code.gitea.io/actions-proto-go v0.3.0/go.mod h1:00ys5QDo1iHN1tHNvvddAcy2W/g+425hQya1cCSvq9A=
 code.gitea.io/gitea-vet v0.2.1/go.mod h1:zcNbT/aJEmivCAhfmkHOlT645KNOf9W2KnkLgFjGGfE=
 code.gitea.io/gitea-vet v0.2.2 h1:TEOV/Glf38iGmKzKP0EB++Z5OSL4zGg3RrAvlwaMuvk=
 code.gitea.io/gitea-vet v0.2.2/go.mod h1:zcNbT/aJEmivCAhfmkHOlT645KNOf9W2KnkLgFjGGfE=
--- a/models/actions/runner.go
+++ b/models/actions/runner.go
@ -43,10 +43,8 @@ type ActionRunner struct {
 	LastOnline timeutil.TimeStamp `xorm:"index"`
 	LastActive timeutil.TimeStamp `xorm:"index"`

-	// Store OS and Artch.
-	AgentLabels []string
-	// Store custom labes use defined.
-	CustomLabels []string
+	// Store labels defined in state file (default: .runner file) of `act_runner`
+	AgentLabels []string `xorm:"TEXT"`

 	Created timeutil.TimeStamp `xorm:"created"`
 	Updated timeutil.TimeStamp `xorm:"updated"`
@ -104,11 +102,6 @@ func (r *ActionRunner) IsOnline() bool {
 	return false
 }

-// AllLabels returns agent and custom labels
-func (r *ActionRunner) AllLabels() []string {
-	return append(r.AgentLabels, r.CustomLabels...)
-}
-
 // Editable checks if the runner is editable by the user
 func (r *ActionRunner) Editable(ownerID, repoID int64) bool {
 	if ownerID == 0 && repoID == 0 {
--- a/models/actions/task.go
+++ b/models/actions/task.go
@ -241,11 +241,9 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask

 	// TODO: a more efficient way to filter labels
 	var job *ActionRunJob
-	labels := runner.AgentLabels
-	labels = append(labels, runner.CustomLabels...)
-	log.Trace("runner labels: %v", labels)
+	log.Trace("runner labels: %v", runner.AgentLabels)
 	for _, v := range jobs {
-		if isSubset(labels, v.RunsOn) {
+		if isSubset(runner.AgentLabels, v.RunsOn) {
 			job = v
 			break
 		}
--- a/models/migrations/migrations.go
+++ b/models/migrations/migrations.go
@ -20,6 +20,7 @@ import (
 	"code.gitea.io/gitea/models/migrations/v1_18"
 	"code.gitea.io/gitea/models/migrations/v1_19"
 	"code.gitea.io/gitea/models/migrations/v1_20"
+	"code.gitea.io/gitea/models/migrations/v1_21"
 	"code.gitea.io/gitea/models/migrations/v1_6"
 	"code.gitea.io/gitea/models/migrations/v1_7"
 	"code.gitea.io/gitea/models/migrations/v1_8"
@ -497,6 +498,11 @@ var migrations = []Migration{
 	NewMigration("Add PinOrder Column", v1_20.AddPinOrderToIssue),
 	// v259 -> 260
 	NewMigration("Convert scoped access tokens", v1_20.ConvertScopedAccessTokens),
+
+	// Gitea 1.21.0 ends at 260
+
+	// v260 -> v261
+	NewMigration("Add label column to action_run table, and combine labels", v1_21.DropCustomLabelsColumnToActRunner),
 }

 // GetCurrentDBVersion returns the current db version
--- a/models/migrations/v1_21/main_test.go
+++ b/models/migrations/v1_21/main_test.go
@ -0,0 +1,14 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_21 //nolint
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/migrations/base"
+)
+
+func TestMain(m *testing.M) {
+	base.MainTest(m)
+}
--- a/models/migrations/v1_21/v260.go
+++ b/models/migrations/v1_21/v260.go
@ -0,0 +1,26 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_21 //nolint
+
+import (
+	"code.gitea.io/gitea/models/migrations/base"
+
+	"xorm.io/xorm"
+)
+
+func DropCustomLabelsColumnToActRunner(x *xorm.Engine) error {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	// drop "custom_labels" cols
+	if err := base.DropTableColumns(sess, "action_runner", "custom_labels"); err != nil {
+		return err
+	}
+
+	return sess.Commit()
+}
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@ -3426,11 +3426,9 @@ runners.owner_type = Type
 runners.description = Description
 runners.labels = Labels
 runners.last_online = Last Online Time
-runners.agent_labels = Agent Labels
-runners.custom_labels = Custom Labels
-runners.custom_labels_helper = Custom labels are labels that are added manually by an administrator. A comma separates labels, whitespace at the start and end of each label is ignored.
 runners.runner_title = Runner
 runners.task_list = Recent tasks on this runner
+runners.task_list.no_tasks = There is no task yet.
 runners.task_list.run = Run
 runners.task_list.status = Status
 runners.task_list.repository = Repository
--- a/routers/api/actions/runner/interceptor.go
+++ b/routers/api/actions/runner/interceptor.go
@ -23,9 +23,8 @@ import (
 const (
 	uuidHeaderKey  = "x-runner-uuid"
 	tokenHeaderKey = "x-runner-token"
+	// Deprecated: will be removed after Gitea 1.20 released.
 	versionHeaderKey = "x-runner-version"
-
-	versionUnknown = "Unknown"
 )

 var withRunner = connect.WithInterceptors(connect.UnaryInterceptorFunc(func(unaryFunc connect.UnaryFunc) connect.UnaryFunc {
@ -36,11 +35,9 @@ var withRunner = connect.WithInterceptors(connect.UnaryInterceptorFunc(func(unar
 		}
 		uuid := request.Header().Get(uuidHeaderKey)
 		token := request.Header().Get(tokenHeaderKey)
+		// TODO: version will be removed from request header after Gitea 1.20 released.
+		// And Gitea will not try to read version from reuqest header
 		version := request.Header().Get(versionHeaderKey)
-		if util.IsEmptyString(version) {
-			version = versionUnknown
-		}
-		version, _ = util.SplitStringAtByteN(version, 64)

 		runner, err := actions_model.GetRunnerByUUID(ctx, uuid)
 		if err != nil {
@ -54,7 +51,11 @@ var withRunner = connect.WithInterceptors(connect.UnaryInterceptorFunc(func(unar
 		}

 		cols := []string{"last_online"}
-		if runner.Version != version {
+
+		// TODO: version will be removed from request header after Gitea 1.20 released.
+		// And Gitea will not try to read version from reuqest header
+		version, _ = util.SplitStringAtByteN(version, 64)
+		if !util.IsEmptyString(version) && runner.Version != version {
 			runner.Version = version
 			cols = append(cols, "version")
 		}
--- a/routers/api/actions/runner/runner.go
+++ b/routers/api/actions/runner/runner.go
@ -54,6 +54,14 @@ func (s *Service) Register(
 		return nil, errors.New("runner token has already been activated")
 	}

+	labels := req.Msg.Labels
+	// TODO: agent_labels should be removed from pb after Gitea 1.20 released.
+	// Old version runner's agent_labels slice is not empty and labels slice is empty.
+	// And due to compatibility with older versions, it is temporarily marked as Deprecated in pb, so use `//nolint` here.
+	if len(req.Msg.AgentLabels) > 0 && len(req.Msg.Labels) == 0 { //nolint:staticcheck
+		labels = req.Msg.AgentLabels //nolint:staticcheck
+	}
+
 	// create new runner
 	name, _ := util.SplitStringAtByteN(req.Msg.Name, 255)
 	runner := &actions_model.ActionRunner{
@ -61,8 +69,8 @@ func (s *Service) Register(
 		Name:        name,
 		OwnerID:     runnerToken.OwnerID,
 		RepoID:      runnerToken.RepoID,
-		AgentLabels:  req.Msg.AgentLabels,
-		CustomLabels: req.Msg.CustomLabels,
+		Version:     req.Msg.Version,
+		AgentLabels: labels,
 	}
 	if err := runner.GenerateToken(); err != nil {
 		return nil, errors.New("can't generate token")
@ -85,14 +93,37 @@ func (s *Service) Register(
 			Uuid:    runner.UUID,
 			Token:   runner.Token,
 			Name:    runner.Name,
-			AgentLabels:  runner.AgentLabels,
-			CustomLabels: runner.CustomLabels,
+			Version: runner.Version,
+			Labels:  runner.AgentLabels,
 		},
 	})

 	return res, nil
 }

+func (s *Service) Declare(
+	ctx context.Context,
+	req *connect.Request[runnerv1.DeclareRequest],
+) (*connect.Response[runnerv1.DeclareResponse], error) {
+	runner := GetRunner(ctx)
+	runner.AgentLabels = req.Msg.Labels
+	runner.Version = req.Msg.Version
+	if err := actions_model.UpdateRunner(ctx, runner, "agent_labels", "version"); err != nil {
+		return nil, status.Errorf(codes.Internal, "update runner: %v", err)
+	}
+
+	return connect.NewResponse(&runnerv1.DeclareResponse{
+		Runner: &runnerv1.Runner{
+			Id:      runner.ID,
+			Uuid:    runner.UUID,
+			Token:   runner.Token,
+			Name:    runner.Name,
+			Version: runner.Version,
+			Labels:  runner.AgentLabels,
+		},
+	}), nil
+}
+
 // FetchTask assigns a task to the runner
 func (s *Service) FetchTask(
 	ctx context.Context,
--- a/routers/web/repo/actions/actions.go
+++ b/routers/web/repo/actions/actions.go
@ -84,7 +84,6 @@ func List(ctx *context.Context) {
 		allRunnerLabels := make(container.Set[string])
 		for _, r := range runners {
 			allRunnerLabels.AddMultiple(r.AgentLabels...)
-			allRunnerLabels.AddMultiple(r.CustomLabels...)
 		}

 		workflows = make([]Workflow, 0, len(entries))
--- a/routers/web/shared/actions/runners.go
+++ b/routers/web/shared/actions/runners.go
@ -6,7 +6,6 @@ package actions
 import (
 	"errors"
 	"net/http"
-	"strings"

 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/db"
@ -126,9 +125,8 @@ func RunnerDetailsEditPost(ctx *context.Context, runnerID, ownerID, repoID int64

 	form := web.GetForm(ctx).(*forms.EditRunnerForm)
 	runner.Description = form.Description
-	runner.CustomLabels = splitLabels(form.CustomLabels)

-	err = actions_model.UpdateRunner(ctx, runner, "description", "custom_labels")
+	err = actions_model.UpdateRunner(ctx, runner, "description")
 	if err != nil {
 		log.Warn("RunnerDetailsEditPost.UpdateRunner failed: %v, url: %s", err, ctx.Req.URL)
 		ctx.Flash.Warning(ctx.Tr("actions.runners.update_runner_failed"))
@ -176,11 +174,3 @@ func RunnerDeletePost(ctx *context.Context, runnerID int64,
 		"redirect": successRedirectTo,
 	})
 }
-
-func splitLabels(s string) []string {
-	labels := strings.Split(s, ",")
-	for i, v := range labels {
-		labels[i] = strings.TrimSpace(v)
-	}
-	return labels
-}
--- a/services/forms/runner.go
+++ b/services/forms/runner.go
@ -15,7 +15,6 @@ import (
 // EditRunnerForm form for admin to create runner
 type EditRunnerForm struct {
 	Description string
-	CustomLabels string // comma-separated
 }

 // Validate validates form fields
--- a/templates/shared/actions/runner_edit.tmpl
+++ b/templates/shared/actions/runner_edit.tmpl
@ -13,10 +13,10 @@
 				</div>
 				<div class="field gt-dib gt-mr-4">
 					<label>{{.locale.Tr "actions.runners.last_online"}}</label>
-					<span>{{if .LastOnline}}{{TimeSinceUnix .LastOnline $.locale}}{{else}}{{$.locale.Tr "never"}}{{end}}</span>
+					<span>{{if .Runner.LastOnline}}{{TimeSinceUnix .Runner.LastOnline $.locale}}{{else}}{{$.locale.Tr "never"}}{{end}}</span>
 				</div>
 				<div class="field gt-dib gt-mr-4">
-					<label>{{.locale.Tr "actions.runners.agent_labels"}}</label>
+					<label>{{.locale.Tr "actions.runners.labels"}}</label>
 					<span>
 						{{range .Runner.AgentLabels}}
 						<span>{{.}}</span>
@ -35,11 +35,6 @@
 				<label for="description">{{.locale.Tr "actions.runners.description"}}</label>
 				<input id="description" name="description" value="{{.Runner.Description}}">
 			</div>
-			<div class="field" data-tooltip-content="Labels are comma-separated. Whitespace at the beginning, end, and around the commas are ignored.">
-				<label for="custom_labels">{{.locale.Tr "actions.runners.custom_labels"}}</label>
-				<input id="custom_labels" name="custom_labels" value="{{StringUtils.Join .Runner.CustomLabels `,`}}">
-				<p class="help">{{.locale.Tr "actions.runners.custom_labels_helper"}}</p>
-			</div>

 			<div class="ui divider"></div>

@ -81,7 +76,7 @@
 				{{end}}
 				{{if not .Tasks}}
 				<tr>
-					<td colspan="5">{{.locale.Tr "runners.task_list.no_tasks"}}</td>
+					<td colspan="5">{{.locale.Tr "actions.runners.task_list.no_tasks"}}</td>
 				</tr>
 				{{end}}
 			</tbody>
--- a/templates/shared/actions/runner_list.tmpl
+++ b/templates/shared/actions/runner_list.tmpl
@ -67,7 +67,7 @@
 						<td>{{if .Version}}{{.Version}}{{else}}{{$.locale.Tr "unknown"}}{{end}}</td>
 						<td><span data-tooltip-content="{{.BelongsToOwnerName}}">{{.BelongsToOwnerType.LocaleString $.locale}}<span></td>
 						<td class="runner-tags">
-							{{range .AllLabels}}<span class="ui label">{{.}}</span>{{end}}
+							{{range .AgentLabels}}<span class="ui label">{{.}}</span>{{end}}
 						</td>
 						<td>{{if .LastOnline}}{{TimeSinceUnix .LastOnline $.locale}}{{else}}{{$.locale.Tr "never"}}{{end}}</td>
 						<td class="runner-ops">