From 3df26ed79c151d5975029d682b04aba79c19fbbc Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Tue, 10 Sep 2024 10:04:08 +0200
Subject: [PATCH] osbuild-worker: fix "crashing" on worker registration issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the osbuild worker cannot register itself with the server
on startup the worker will "crash". This is inconsistent with the
existing behavior in `workerHeartbeat()` which deals with connectivity
or other server issue gracefully and retries periodically.

To unify the behavior this commit changes the behavior and only
issues a `logrus.Warnf` instead of the previous `Falalf` when
the registration fails.

Co-authored-by: Florian Schüller <florian.schueller@redhat.com>
---
 internal/worker/client.go      |  3 ++-
 internal/worker/client_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/internal/worker/client.go b/internal/worker/client.go
index 8f572bccc..77fb17ef8 100644
--- a/internal/worker/client.go
+++ b/internal/worker/client.go
@@ -123,7 +123,8 @@ func NewClient(conf ClientConfig) (*Client, error) {
 	}
 	err = client.registerWorker()
 	if err != nil {
-		return client, err
+		// workerHeartbeat below will periodically retry to register
+		logrus.Warnf("Error registering worker on startup, %v. Trying again later…", err)
 	}
 	go client.workerHeartbeat()
 	return client, nil
diff --git a/internal/worker/client_test.go b/internal/worker/client_test.go
index 78268e69d..dd5bf6c26 100644
--- a/internal/worker/client_test.go
+++ b/internal/worker/client_test.go
@@ -1,12 +1,14 @@
 package worker_test
 
 import (
+	"bytes"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 
 	"github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue"
@@ -137,3 +139,28 @@ func TestProxy(t *testing.T) {
 	// - cancel
 	require.Equal(t, 6, proxy.calls)
 }
+
+func TestNewClientWorkerNoErrorOnRegisterWorkerFailure(t *testing.T) {
+	logrusOutput := bytes.NewBuffer(nil)
+	logrus.SetOutput(logrusOutput)
+
+	apiCalls := 0
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		apiCalls++
+
+		require.Equal(t, "/api/image-builder-worker/v1/workers", req.URL.Path)
+		w.WriteHeader(400)
+		_, err := w.Write([]byte(`{"reason":"reason", "details": "details"}`))
+		require.NoError(t, err)
+	}))
+	defer srv.Close()
+
+	client, err := worker.NewClient(worker.ClientConfig{
+		BaseURL:  srv.URL,
+		BasePath: "/api/image-builder-worker/v1",
+	})
+	require.NoError(t, err)
+	require.NotNil(t, client)
+	require.Equal(t, 1, apiCalls)
+	require.Contains(t, logrusOutput.String(), `Error registering worker on startup, error registering worker: 400`)
+}