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 <me@silverwind.io>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1197
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com>
Co-authored-by: bircni <bircni@icloud.com>

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,

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, &registerInputs{InstanceAddr: server.URL, RunnerName: "runner"})
+	assert.ErrorContains(t, err, "cannot ping the Gitea instance server")
+	assert.Equal(t, int32(1), requests.Load())
+}

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