Skip to content

Commit

Permalink
osbuild-worker: fix "crashing" on worker registration issues
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mvo5 and schuellerf committed Sep 10, 2024
1 parent d6031ae commit 7d47d5b
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
3 changes: 2 additions & 1 deletion internal/worker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions internal/worker/client_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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`)
}

0 comments on commit 7d47d5b

Please sign in to comment.