From 73e9552f06652ff195a5086c5d2d2dca029b6e89 Mon Sep 17 00:00:00 2001 From: bircni Date: Thu, 1 Oct 2026 16:01:14 +0000 Subject: [PATCH] fix: fail fast in register and honor GITEA_MAX_REG_ATTEMPTS (#1197) `gitea-runner register` pinged the instance in an unbounded loop, so a permanent failure like an untrusted certificate or a wrong URL never returned. It now pings once and returns the error, like other one-shot commands. The image entrypoint keeps retrying registration indefinitely by default, like the daemon, so a container without a restart policy still recovers once Gitea is reachable. `GITEA_MAX_REG_ATTEMPTS` had no effect because its counter was incremented outside the loop. When set, it now caps the attempts, after which the entrypoint exits non-zero. Fixes: https://gitea.com/gitea/runner/issues/1195 Co-authored-by: silverwind Reviewed-on: https://gitea.com/gitea/runner/pulls/1197 Reviewed-by: Lunny Xiao Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Co-authored-by: bircni --- internal/app/cmd/register.go | 27 +++++---------------------- internal/app/cmd/register_test.go | 19 +++++++++++++++++++ scripts/run.sh | 23 +++++++++-------------- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/internal/app/cmd/register.go b/internal/app/cmd/register.go index 16408ff7..f7866c95 100644 --- a/internal/app/cmd/register.go +++ b/internal/app/cmd/register.go @@ -12,7 +12,6 @@ import ( "os/signal" goruntime "runtime" "strings" - "time" "gitea.com/gitea/runner/internal/app/run" "gitea.com/gitea/runner/internal/pkg/client" @@ -370,28 +369,12 @@ func doRegister(ctx context.Context, cfg *config.Config, inputs *registerInputs) config.RequestTimeout, ) - for { - _, err := cli.Ping(ctx, connect.NewRequest(&pingv1.PingRequest{ - Data: inputs.RunnerName, - })) - select { - case <-ctx.Done(): - return ctx.Err() - default: - } - if ctx.Err() != nil { - break - } - if err != nil { - log.WithError(err). - Errorln("Cannot ping the Gitea instance server") - // TODO: if ping failed, retry or exit - time.Sleep(time.Second) - } else { - log.Debugln("Successfully pinged the Gitea instance server") - break - } + if _, err := cli.Ping(ctx, connect.NewRequest(&pingv1.PingRequest{ + Data: inputs.RunnerName, + })); err != nil { + return fmt.Errorf("cannot ping the Gitea instance server: %w", err) } + log.Debugln("Successfully pinged the Gitea instance server") reg := &config.Registration{ Name: inputs.RunnerName, diff --git a/internal/app/cmd/register_test.go b/internal/app/cmd/register_test.go index 72b22495..aecb33ce 100644 --- a/internal/app/cmd/register_test.go +++ b/internal/app/cmd/register_test.go @@ -4,7 +4,11 @@ package cmd import ( + "net/http" + "net/http/httptest" "os" + "path/filepath" + "sync/atomic" "testing" "gitea.com/gitea/runner/internal/pkg/config" @@ -246,3 +250,18 @@ func TestInitInputs(t *testing.T) { require.Nil(t, inputs.Labels) }) } + +func TestDoRegisterReturnsPingErrorWithoutRetrying(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusServiceUnavailable) + })) + defer server.Close() + + cfg := &config.Config{} + cfg.Runner.File = filepath.Join(t.TempDir(), ".runner") + err := doRegister(t.Context(), cfg, ®isterInputs{InstanceAddr: server.URL, RunnerName: "runner"}) + assert.ErrorContains(t, err, "cannot ping the Gitea instance server") + assert.Equal(t, int32(1), requests.Load()) +} diff --git a/scripts/run.sh b/scripts/run.sh index 13deb89b..2f315bf8 100755 --- a/scripts/run.sh +++ b/scripts/run.sh @@ -36,28 +36,23 @@ fi test -f "$RUNNER_STATE_FILE" || echo "$RUNNER_STATE_FILE is missing or not a regular file" if [[ ! -s "$RUNNER_STATE_FILE" ]]; then - try=$((try + 1)) - success=0 - - # The point of this loop is to make it simple, when running both runner and gitea in docker, - # for the runner to wait a moment for gitea to become available before erroring out. Within - # the context of a single docker-compose, something similar could be done via healthchecks, but - # this is more flexible. - while [[ $success -eq 0 ]] && [[ $try -lt ${GITEA_MAX_REG_ATTEMPTS:-10} ]]; do + for ((try = 1; ; try++)); do # waits for Gitea like the daemon does, a container may have no restart policy gitea-runner register \ --instance "${GITEA_INSTANCE_URL}" \ --token "${GITEA_RUNNER_REGISTRATION_TOKEN}" \ --name "${GITEA_RUNNER_NAME:-`hostname`}" \ ${CONFIG_ARG} ${EXTRA_ARGS} --no-interactive 2>&1 | tee /tmp/reg.log - cat /tmp/reg.log | grep 'Runner registered successfully' > /dev/null - if [[ $? -eq 0 ]]; then + if grep -q 'Runner registered successfully' /tmp/reg.log; then echo "SUCCESS" - success=1 - else - echo "Waiting to retry ..." - sleep 5 + break fi + if [[ ${GITEA_MAX_REG_ATTEMPTS:-0} -gt 0 ]] && [[ $try -ge $GITEA_MAX_REG_ATTEMPTS ]]; then + echo "Registration failed after ${try} attempts" + exit 1 + fi + echo "Waiting to retry ..." + sleep 5 done fi # Prevent reading the token from the gitea-runner process