From 58c5eb8d21d28d8bb2de7be028f2e3bb1f1d1c7c Mon Sep 17 00:00:00 2001 From: bircni Date: Wed, 15 Jul 2026 16:58:07 +0000 Subject: [PATCH] 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 Reviewed-on: https://gitea.com/gitea/runner/pulls/1085 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> --- README.md | 8 ++++++ internal/app/cmd/cmd.go | 1 + internal/app/cmd/daemon.go | 30 +++++++++++++++++---- internal/app/cmd/daemon_test.go | 26 ++++++++++++++++++ internal/app/cmd/register.go | 5 +++- internal/app/cmd/register_test.go | 25 +++++++++++------- internal/app/run/runner_test.go | 5 ++-- internal/pkg/labels/labels.go | 22 ++++++++++++---- internal/pkg/labels/labels_test.go | 42 +++++++++++++++++++++++++++--- scripts/run.sh | 10 ++++--- 10 files changed, 146 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index ee2c1566..e848a679 100644 --- a/README.md +++ b/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`) diff --git a/internal/app/cmd/cmd.go b/internal/app/cmd/cmd.go index d20d518c..18aaa373 100644 --- a/internal/app/cmd/cmd.go +++ b/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 diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index 486fc18f..a4b5a0a6 100644 --- a/internal/app/cmd/daemon.go +++ b/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. diff --git a/internal/app/cmd/daemon_test.go b/internal/app/cmd/daemon_test.go index e90a510a..215b6127 100644 --- a/internal/app/cmd/daemon_test.go +++ b/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) diff --git a/internal/app/cmd/register.go b/internal/app/cmd/register.go index bc2f3f63..6859522d 100644 --- a/internal/app/cmd/register.go +++ b/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. diff --git a/internal/app/cmd/register_test.go b/internal/app/cmd/register_test.go index 0c3234b5..5442d16d 100644 --- a/internal/app/cmd/register_test.go +++ b/internal/app/cmd/register_test.go @@ -15,11 +15,11 @@ import ( func TestRegisterNonInteractiveReturnsLabelValidationError(t *testing.T) { err := registerNoInteractive(t.Context(), "", ®isterArgs{ - 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 := ®isterInputs{} 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 := ®isterInputs{} - 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 := ®isterInputs{} + 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 := ®isterInputs{} require.Equal(t, StageInputInstance, inputs.assignToNext(StageOverwriteLocalConfig, "Y", emptyCfg)) diff --git a/internal/app/run/runner_test.go b/internal/app/run/runner_test.go index 43635549..6e0ff6a7 100644 --- a/internal/app/run/runner_test.go +++ b/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"]) diff --git a/internal/pkg/labels/labels.go b/internal/pkg/labels/labels.go index 542b52dc..342d5b4d 100644 --- a/internal/pkg/labels/labels.go +++ b/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 diff --git a/internal/pkg/labels/labels_test.go b/internal/pkg/labels/labels_test.go index 8b276d70..8d8eb598 100644 --- a/internal/pkg/labels/labels_test.go +++ b/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})) +} diff --git a/scripts/run.sh b/scripts/run.sh index 75f43b55..1914af65 100755 --- a/scripts/run.sh +++ b/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