Clarify labels #69

Merged
wolfogre merged 6 commits from wolfogre/act_runner:feature/host_mode into main 2023-03-23 12:48:33 +00:00
5 changed files with 30 additions and 26 deletions
Showing only changes of commit 96ce0b83d7 - Show all commits

View File

@ -45,7 +45,7 @@ INFO Enter the runner token:
fe884e8027dc292970d4e0303fe82b14xxxxxxxx
INFO Enter the runner name (if set empty, use hostname: Test.local):
INFO Enter the runner labels, leave blank to use the default labels (comma-separated, for example, self-hosted,ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster):
INFO Enter the runner labels, leave blank to use the default labels (comma-separated, for example, ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster,linux_arm:host):
wolfogre marked this conversation as resolved
Review

I guess it could have some more explanations about each part of the "label" .

And it looks strange that linux_arm:host becomes -self-hosted

I guess it could have some more explanations about each part of the "label" . And it looks strange that `linux_arm:host` becomes `-self-hosted`
INFO Registering runner, name=Test.local, instance=http://192.168.8.8:3000/, labels=[ubuntu-latest:docker://node:16-bullseye ubuntu-22.04:docker://node:16-bullseye ubuntu-20.04:docker://node:16-bullseye ubuntu-18.04:docker://node:16-buster].
DEBU Successfully pinged the Gitea instance server

View File

@ -8,18 +8,18 @@ import (
"os"
"strings"
"github.com/joho/godotenv"
"github.com/mattn/go-isatty"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
"gitea.com/gitea/act_runner/artifactcache"
"gitea.com/gitea/act_runner/client"
"gitea.com/gitea/act_runner/config"
"gitea.com/gitea/act_runner/engine"
"gitea.com/gitea/act_runner/poller"
"gitea.com/gitea/act_runner/runtime"
"github.com/joho/godotenv"
"github.com/mattn/go-isatty"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"golang.org/x/sync/errgroup"
)
func runDaemon(ctx context.Context, envFile string) func(cmd *cobra.Command, args []string) error {
@ -39,7 +39,7 @@ func runDaemon(ctx context.Context, envFile string) func(cmd *cobra.Command, arg
needsDocker := false
for _, l := range cfg.Runner.Labels {
splits := strings.SplitN(l, ":", 2)
if len(splits) == 2 && strings.HasPrefix(splits[1], "docker://") {
if len(splits) == 2 && strings.HasPrefix(splits[1], "docker:") {
needsDocker = true
break
}

View File

@ -14,15 +14,15 @@ import (
"time"
pingv1 "code.gitea.io/actions-proto-go/ping/v1"
"gitea.com/gitea/act_runner/client"
"gitea.com/gitea/act_runner/config"
"gitea.com/gitea/act_runner/register"
"github.com/bufbuild/connect-go"
"github.com/joho/godotenv"
"github.com/mattn/go-isatty"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"gitea.com/gitea/act_runner/client"
"gitea.com/gitea/act_runner/config"
"gitea.com/gitea/act_runner/register"
)
// runRegister registers a runner to the server
@ -122,10 +122,9 @@ func (r *registerInputs) validate() error {
func validateLabels(labels []string) error {
for _, label := range labels {
values := strings.SplitN(label, ":", 2)
if len(values) > 2 {
if len(values) != 2 {
return fmt.Errorf("Invalid label: %s", label)
}
// len(values) == 1, label for non docker execution environment
// TODO: validate value format, like docker://node:16-buster
}
return nil
@ -167,7 +166,7 @@ func (r *registerInputs) assignToNext(stage registerStage, value string) registe
}
if validateLabels(r.CustomLabels) != nil {
log.Infoln("Invalid labels, please input again, leave blank to use the default labels (for example, ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster)")
log.Infoln("Invalid labels, please input again, leave blank to use the default labels (for example, ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster,linux_arm:host)")
return StageInputCustomLabels
}
return StageWaitingForRegistration
@ -231,7 +230,7 @@ func printStageHelp(stage registerStage) {
hostname, _ := os.Hostname()
log.Infof("Enter the runner name (if set empty, use hostname: %s):\n", hostname)
case StageInputCustomLabels:
log.Infoln("Enter the runner labels, leave blank to use the default labels (comma-separated, for example, self-hosted,ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster):")
log.Infoln("Enter the runner labels, leave blank to use the default labels (comma-separated, for example, ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster,linux_arm:host):")
case StageWaitingForRegistration:
log.Infoln("Waiting for registration...")
}

View File

@ -6,7 +6,7 @@ package cmd
import "testing"
func TestValidateLabels(t *testing.T) {
labels := []string{"ubuntu-latest:docker://node:16-buster", "self-hosted"}
labels := []string{"ubuntu-latest:docker://node:16-buster", "linux_arm:host"}
if err := validateLabels(labels); err != nil {
t.Errorf("validateLabels() error = %v", err)
}

View File

@ -36,12 +36,15 @@ func (s *Runner) Run(ctx context.Context, task *runnerv1.Task) error {
func (s *Runner) platformPicker(labels []string) string {
// "ubuntu-18.04:docker://node:16-buster"
// "self-hosted"
// "linux_arm:host"
platforms := make(map[string]string, len(labels))
wolfogre marked this conversation as resolved Outdated

should be make(map[string]string, len(s.Labels)) ?

should be `make(map[string]string, len(s.Labels))` ?
for _, l := range s.Labels {
// "ubuntu-18.04:docker://node:16-buster"
splits := strings.SplitN(l, ":", 2)
if len(splits) != 1 {
continue
}
if len(splits) == 1 {
wolfogre marked this conversation as resolved Outdated

There is a if len(splits) != 1 { above?

There is a `if len(splits) != 1 {` above?

Typo. 😂

Typo. 😂
// identifier for non docker execution environment
platforms[splits[0]] = "-self-hosted"
@ -50,13 +53,13 @@ func (s *Runner) platformPicker(labels []string) string {
// ["ubuntu-18.04", "docker://node:16-buster"]
k, v := splits[0], splits[1]
if prefix := "docker://"; !strings.HasPrefix(v, prefix) {
continue
} else {
v = strings.TrimPrefix(v, prefix)
switch {
wolfogre marked this conversation as resolved Outdated

Does it need a default case? At least, show some error logs.

Does it need a `default` case? At least, show some error logs.
case strings.HasPrefix(v, "docker:"):
// TODO "//" will be ignored, maybe we should use 'ubuntu-18.04:docker:node:16-buster' instead
platforms[k] = strings.TrimPrefix(strings.TrimPrefix(v, "docker:"), "//")
case v == "host":
platforms[k] = "-self-hosted"
}
// ubuntu-18.04 => node:16-buster
platforms[k] = v
}
for _, label := range labels {
@ -71,6 +74,8 @@ func (s *Runner) platformPicker(labels []string) string {
// ["with-gpu"] => "linux:with-gpu"
// ["ubuntu-22.04", "with-gpu"] => "ubuntu:22.04_with-gpu"
// return default
return "node:16-bullseye"
// return default.
// So the runner receives a task with a label that the runner doesn't have,
// it happens when the user have edited the label of the runner in the web UI.
return "node:16-bullseye" // TODO: it may be not correct, what if the runner is used as host mode only?
}