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 751ad6a commit 3df26ed
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 3df26ed

Please sign in to comment.