feat: honour GITEA_RUNNER_LABELS on daemon start and accept labels containing a colon (#1085)

Fixes #648
Fixes #656
Fixes #664

---------

Co-authored-by: silverwind <me@silverwind.io>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1085
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>

README.md

@@ -199,6 +199,14 @@ Labels are chosen at registration time (`--labels`, or the interactive prompt) a
 
 If `runner.labels` is set in the YAML file, those labels are used during `register` and the `--labels` CLI flag is ignored.
 
+The `daemon` command also accepts `--labels` (which defaults to the `GITEA_RUNNER_LABELS` environment variable), so the labels of an already registered runner can be changed without deleting its registration file. The most explicit source wins:
+
+```
+--labels / GITEA_RUNNER_LABELS   >   runner.labels in the config file   >   labels in the .runner file
+```
+
+Whenever the resulting labels differ from the ones in the registration file, they are written back to it and re-declared to the Gitea instance on startup.
+
 > **Note:** A runner that only exposes `host` labels still needs access to a Docker daemon (e.g. a mounted `/var/run/docker.sock`) whenever a job uses a `docker://` action or a service container. `host` labels only change where the job's own steps run; container-based steps and actions are still executed with Docker.
 
 #### Caching (`actions/cache`)

internal/app/cmd/cmd.go

@@ -51,6 +51,7 @@ func Execute(ctx context.Context) {
 		RunE:  runDaemon(ctx, &daemArgs, &configFile),
 	}
 	daemonCmd.Flags().BoolVar(&daemArgs.Once, "once", false, "Run one job then exit")
+	daemonCmd.Flags().StringVar(&daemArgs.Labels, "labels", os.Getenv("GITEA_RUNNER_LABELS"), "Runner labels, comma separated. Overrides the labels of an already registered runner")
 	rootCmd.AddCommand(daemonCmd)
 
 	// ./gitea-runner exec

internal/app/cmd/daemon.go

@@ -49,10 +49,7 @@ func runDaemon(ctx context.Context, daemArgs *daemonArgs, configFile *string) fu
 			return fmt.Errorf("failed to load registration file: %w", err)
 		}
 
-		lbls := reg.Labels
-		if len(cfg.Runner.Labels) > 0 {
-			lbls = cfg.Runner.Labels
-		}
+		lbls := resolveLabels(daemArgs.Labels, cfg.Runner.Labels, reg.Labels)
 
 		ls := labels.Labels{}
 		for _, l := range lbls {
@@ -203,7 +200,30 @@ func runDaemon(ctx context.Context, daemArgs *daemonArgs, configFile *string) fu
 }
 
 type daemonArgs struct {
-	Once bool
+	Once   bool
+	Labels string
+}
+
+// resolveLabels picks the labels to run with: --labels/GITEA_RUNNER_LABELS > config > .runner.
+// The flag lets a registered runner change its labels without deleting the .runner file.
+func resolveLabels(argLabels string, cfgLabels, regLabels []string) []string {
+	if lbls := splitLabels(argLabels); len(lbls) > 0 {
+		return lbls
+	}
+	if len(cfgLabels) > 0 {
+		return cfgLabels
+	}
+	return regLabels
+}
+
+func splitLabels(s string) []string {
+	var lbls []string
+	for l := range strings.SplitSeq(s, ",") {
+		if l = strings.TrimSpace(l); l != "" {
+			lbls = append(lbls, l)
+		}
+	}
+	return lbls
 }
 
 // initLogging setup the global logrus logger.

internal/app/cmd/daemon_test.go

@@ -12,6 +12,32 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestResolveLabels(t *testing.T) {
+	var (
+		cfgLabels = []string{"cfg:host"}
+		regLabels = []string{"reg:host"}
+	)
+
+	tests := []struct {
+		name string
+		arg  string
+		cfg  []string
+		reg  []string
+		want []string
+	}{
+		{"flag wins", "flag:host,other", cfgLabels, regLabels, []string{"flag:host", "other"}},
+		{"config wins over registration", "", cfgLabels, regLabels, cfgLabels},
+		{"registration is the fallback", "", nil, regLabels, regLabels},
+		{"blank flag is ignored", " , ", cfgLabels, regLabels, cfgLabels},
+		{"nothing configured", "", nil, nil, nil},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, resolveLabels(tt.arg, tt.cfg, tt.reg))
+		})
+	}
+}
+
 func TestGetDockerSocketPathUsesConfigAndEnvironment(t *testing.T) {
 	got, err := getDockerSocketPath("tcp://docker.example:2376")
 	require.NoError(t, err)

internal/app/cmd/register.go

@@ -387,7 +387,10 @@ func doRegister(ctx context.Context, cfg *config.Config, inputs *registerInputs)
 
 	ls := make([]string, len(reg.Labels))
 	for i, v := range reg.Labels {
-		l, _ := labels.Parse(v)
+		l, err := labels.Parse(v)
+		if err != nil {
+			return fmt.Errorf("failed to parse label %q: %w", v, err)
+		}
 		ls[i] = l.Name
 	}
 	// register new runner.

internal/app/cmd/register_test.go

@@ -15,11 +15,11 @@ import (
 
 func TestRegisterNonInteractiveReturnsLabelValidationError(t *testing.T) {
 	err := registerNoInteractive(t.Context(), "", &registerArgs{
-		Labels:       "label:invalid",
+		Labels:       "ubuntu:host,,broken",
 		Token:        "token",
 		InstanceAddr: "http://localhost:3000",
 	})
-	assert.Error(t, err, "unsupported schema: invalid")
+	assert.ErrorContains(t, err, "empty label")
 }
 
 func TestRegisterInputsValidate(t *testing.T) {
@@ -40,8 +40,8 @@ func TestRegisterInputsValidate(t *testing.T) {
 		},
 		{
 			name:    "invalid label",
-			inputs:  registerInputs{InstanceAddr: "http://localhost:3000", Token: "token", Labels: []string{"ubuntu:vm:bad"}},
-			wantErr: "unsupported schema: vm",
+			inputs:  registerInputs{InstanceAddr: "http://localhost:3000", Token: "token", Labels: []string{""}},
+			wantErr: "empty label",
 		},
 		{
 			name:   "valid",
@@ -62,7 +62,9 @@ func TestRegisterInputsValidate(t *testing.T) {
 
 func TestValidateLabels(t *testing.T) {
 	require.NoError(t, validateLabels([]string{"ubuntu:host", "ubuntu:docker://node:18"}))
-	require.Error(t, validateLabels([]string{"ubuntu:host", "ubuntu:vm:bad"}))
+	// a colon that is not a supported schema is part of the label name
+	require.NoError(t, validateLabels([]string{"pool:e57e18d4-10d4-406f-93bf-60f127221bdd"}))
+	require.Error(t, validateLabels([]string{"ubuntu:host", ""}))
 }
 
 func TestRegisterInputsStageValue(t *testing.T) {
@@ -106,11 +108,10 @@ func TestRegisterInputsAssignToNext(t *testing.T) {
 
 	t.Run("labels from config skip the labels stage", func(t *testing.T) {
 		cfg := &config.Config{}
-		cfg.Runner.Labels = []string{"ubuntu:host", "ubuntu:vm:bad"}
+		cfg.Runner.Labels = []string{"ubuntu:host", "", "pool:e57e18d4"}
 		inputs := &registerInputs{}
 		require.Equal(t, StageWaitingForRegistration, inputs.assignToNext(StageInputRunnerName, "runner", cfg))
-		// only the valid label survives
-		require.Equal(t, []string{"ubuntu:host"}, inputs.Labels)
+		require.Equal(t, []string{"ubuntu:host", "pool:e57e18d4"}, inputs.Labels)
 	})
 
 	t.Run("blank labels input uses defaults", func(t *testing.T) {
@@ -121,10 +122,16 @@ func TestRegisterInputsAssignToNext(t *testing.T) {
 
 	t.Run("invalid labels input loops back", func(t *testing.T) {
 		inputs := &registerInputs{}
-		require.Equal(t, StageInputLabels, inputs.assignToNext(StageInputLabels, "ubuntu:vm:bad", emptyCfg))
+		require.Equal(t, StageInputLabels, inputs.assignToNext(StageInputLabels, "ubuntu:host,,bad", emptyCfg))
 		require.Nil(t, inputs.Labels)
 	})
 
+	t.Run("labels containing a colon are accepted", func(t *testing.T) {
+		inputs := &registerInputs{}
+		require.Equal(t, StageWaitingForRegistration, inputs.assignToNext(StageInputLabels, "pool:e57e18d4,ubuntu:host", emptyCfg))
+		require.Equal(t, []string{"pool:e57e18d4", "ubuntu:host"}, inputs.Labels)
+	})
+
 	t.Run("overwrite local config", func(t *testing.T) {
 		inputs := &registerInputs{}
 		require.Equal(t, StageInputInstance, inputs.assignToNext(StageOverwriteLocalConfig, "Y", emptyCfg))

internal/app/run/runner_test.go

@@ -75,7 +75,7 @@ func TestNewRunnerInitializesLabelsAndEnvironment(t *testing.T) {
 	cfg.Runner.Envs = map[string]string{"EXISTING": "value"}
 	reg := &config.Registration{
 		Name:   "runner",
-		Labels: []string{"ubuntu:host", "bad:vm:label"},
+		Labels: []string{"ubuntu:host", "", "pool:e57e18d4"},
 	}
 	cli := clientmocks.NewClient(t)
 	cli.On("Address").Return("https://gitea.example/").Maybe()
@@ -83,7 +83,8 @@ func TestNewRunnerInitializesLabelsAndEnvironment(t *testing.T) {
 	r := NewRunner(cfg, reg, cli)
 
 	require.Equal(t, "runner", r.name)
-	require.Len(t, r.labels, 1)
+	require.Len(t, r.labels, 2)
+	require.Equal(t, []string{"ubuntu", "pool:e57e18d4"}, r.labels.Names())
 	require.Equal(t, "value", r.envs["EXISTING"])
 	require.Equal(t, "https://gitea.example/api/actions_pipeline/", r.envs["ACTIONS_RUNTIME_URL"])
 	require.Equal(t, "https://gitea.example", r.envs["ACTIONS_RESULTS_URL"])

internal/pkg/labels/labels.go

@@ -4,7 +4,7 @@
 package labels
 
 import (
-	"fmt"
+	"errors"
 	"strings"
 )
 
@@ -17,13 +17,20 @@ type Label struct {
 	Name   string
 	Schema string
 	Arg    string
+	// Opaque marks a label whose name contains a colon but no supported schema,
+	// like "pool:e57e18d4-...". It is kept verbatim and behaves like a host label.
+	Opaque bool
 }
 
 func Parse(str string) (*Label, error) {
+	if str == "" {
+		return nil, errors.New("empty label")
+	}
+
 	splits := strings.SplitN(str, ":", 3)
 	label := &Label{
 		Name:   splits[0],
-		Schema: "host",
+		Schema: SchemeHost,
 		Arg:    "",
 	}
 	if len(splits) >= 2 {
@@ -33,7 +40,12 @@ func Parse(str string) (*Label, error) {
 		label.Arg = splits[2]
 	}
 	if label.Schema != SchemeHost && label.Schema != SchemeDocker {
-		return nil, fmt.Errorf("unsupported schema: %s", label.Schema)
+		// Not a schema we know: the colon belongs to the label name itself.
+		return &Label{
+			Name:   str,
+			Schema: SchemeHost,
+			Opaque: true,
+		}, nil
 	}
 	return label, nil
 }
@@ -59,7 +71,7 @@ func (l Labels) PickPlatform(runsOn []string) string {
 		case SchemeHost:
 			platforms[label.Name] = "-self-hosted"
 		default:
-			// It should not happen, because Parse has checked it.
+			// unreachable: Parse only produces host or docker schemas
 			continue
 		}
 	}
@@ -94,7 +106,7 @@ func (l Labels) ToStrings() []string {
 	ls := make([]string, 0, len(l))
 	for _, label := range l {
 		lbl := label.Name
-		if label.Schema != "" {
+		if !label.Opaque && label.Schema != "" {
 			lbl += ":" + label.Schema
 			if label.Arg != "" {
 				lbl += ":" + label.Arg

internal/pkg/labels/labels_test.go

@@ -44,7 +44,27 @@ func TestParse(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			args:    "ubuntu:vm:ubuntu-18.04",
+			args: "pool:e57e18d4-10d4-406f-93bf-60f127221bdd",
+			want: &Label{
+				Name:   "pool:e57e18d4-10d4-406f-93bf-60f127221bdd",
+				Schema: "host",
+				Arg:    "",
+				Opaque: true,
+			},
+			wantErr: false,
+		},
+		{
+			args: "ubuntu:vm:ubuntu-18.04",
+			want: &Label{
+				Name:   "ubuntu:vm:ubuntu-18.04",
+				Schema: "host",
+				Arg:    "",
+				Opaque: true,
+			},
+			wantErr: false,
+		},
+		{
+			args:    "",
 			want:    nil,
 			wantErr: true,
 		},
@@ -116,8 +136,8 @@ func TestPickPlatform(t *testing.T) {
 }
 
 func TestNames(t *testing.T) {
-	ls := mustParse(t, "ubuntu:docker://node:18", "self-hosted:host")
-	require.Equal(t, []string{"ubuntu", "self-hosted"}, ls.Names())
+	ls := mustParse(t, "ubuntu:docker://node:18", "self-hosted:host", "pool:e57e18d4")
+	require.Equal(t, []string{"ubuntu", "self-hosted", "pool:e57e18d4"}, ls.Names())
 	require.Empty(t, Labels{}.Names())
 }
 
@@ -126,10 +146,26 @@ func TestToStrings(t *testing.T) {
 		"ubuntu:docker://node:18",
 		"self-hosted:host",
 		"bare",
+		"pool:e57e18d4",
 	)
 	require.Equal(t, []string{
 		"ubuntu:docker://node:18",
 		"self-hosted:host",
 		"bare:host",
+		"pool:e57e18d4",
 	}, ls.ToStrings())
 }
+
+// a colon-containing name must survive a write to and read back from the .runner file
+func TestOpaqueLabelRoundTrip(t *testing.T) {
+	const raw = "pool:e57e18d4-10d4-406f-93bf-60f127221bdd"
+
+	ls := mustParse(t, raw)
+	require.Equal(t, []string{raw}, ls.ToStrings())
+
+	again := mustParse(t, ls.ToStrings()...)
+	require.Equal(t, ls, again)
+	require.Equal(t, []string{raw}, again.Names())
+	require.False(t, again.RequireDocker())
+	require.Equal(t, "-self-hosted", again.PickPlatform([]string{raw}))
+}

scripts/run.sh

@@ -12,14 +12,18 @@ CONFIG_ARG=""
 if [[ ! -z "${CONFIG_FILE}" ]]; then
   CONFIG_ARG="--config ${CONFIG_FILE}"
 fi
-EXTRA_ARGS=""
+LABEL_ARGS=""
 if [[ ! -z "${GITEA_RUNNER_LABELS}" ]]; then
-  EXTRA_ARGS="${EXTRA_ARGS} --labels ${GITEA_RUNNER_LABELS}"
+  LABEL_ARGS="--labels ${GITEA_RUNNER_LABELS}"
 fi
+EXTRA_ARGS="${LABEL_ARGS}"
 if [[ ! -z "${GITEA_RUNNER_EPHEMERAL}" ]]; then
   EXTRA_ARGS="${EXTRA_ARGS} --ephemeral"
 fi
-RUN_ARGS=""
+# Also pass the labels to the daemon, so that an already registered runner
+# picks up changes to GITEA_RUNNER_LABELS instead of keeping the labels it
+# was first registered with.
+RUN_ARGS="${LABEL_ARGS}"
 if [[ ! -z "${GITEA_RUNNER_ONCE}" ]]; then
   RUN_ARGS="${RUN_ARGS} --once"
 fi