Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/decode gitlab error #246

Merged
merged 3 commits into from
Jan 20, 2025
Merged

Feat/decode gitlab error #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
066efdc
feat: decode bad post Gitlab requests
puffitos Dec 25, 2024
451928a
chore: logs with context in target manager
puffitos Dec 25, 2024
88466dc
refactor: generic gitlab API error message
puffitos Dec 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions pkg/sparrow/targets/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,31 +106,31 @@ func (t *manager) Reconcile(ctx context.Context) error {
defer registrationTimer.Stop()
defer updateTimer.Stop()

log.Info("Starting target manager reconciliation")
log.InfoContext(ctx, "Starting target manager reconciliation")
for {
select {
case <-ctx.Done():
log.Error("Error while reconciling targets", "err", ctx.Err())
log.ErrorContext(ctx, "Error while reconciling targets", "error", ctx.Err())
return ctx.Err()
case <-t.done:
log.Info("Target manager reconciliation stopped")
log.InfoContext(ctx, "Target manager reconciliation stopped")
return nil
case <-checkTimer.C:
err := t.refreshTargets(ctx)
if err != nil {
log.Warn("Failed to get global targets", "error", err)
log.WarnContext(ctx, "Failed to get global targets", "error", err)
}
checkTimer.Reset(t.cfg.CheckInterval)
case <-registrationTimer.C:
err := t.register(ctx)
if err != nil {
log.Warn("Failed to register self as global target", "error", err)
log.WarnContext(ctx, "Failed to register self as global target", "error", err)
}
registrationTimer.Reset(t.cfg.RegistrationInterval)
case <-updateTimer.C:
err := t.update(ctx)
if err != nil {
log.Warn("Failed to update registration", "error", err)
log.WarnContext(ctx, "Failed to update registration", "error", err)
}
updateTimer.Reset(t.cfg.UpdateInterval)
}
Expand All @@ -151,7 +151,7 @@ func (t *manager) Shutdown(ctx context.Context) error {

errC := ctx.Err()
log := logger.FromContext(ctx)
log.Debug("Shut down signal received")
log.DebugContext(ctx, "Shut down signal received")
ctxS, cancel := context.WithTimeout(context.Background(), shutdownTimeout)
defer cancel()

Expand All @@ -164,17 +164,17 @@ func (t *manager) Shutdown(ctx context.Context) error {
f.SetFileName(fmt.Sprintf("%s.json", t.name))
err := t.interactor.DeleteFile(ctxS, f)
if err != nil {
log.Error("Failed to shutdown gracefully", "error", err)
log.ErrorContext(ctx, "Failed to shutdown gracefully", "error", err)
return fmt.Errorf("failed to shutdown gracefully: %w", errors.Join(errC, err))
}
t.registered = false
t.metrics.registered.Set(0)
log.Info("Successfully unregistered as global target")
log.InfoContext(ctx, "Successfully unregistered as global target")
}

select {
case t.done <- struct{}{}:
log.Debug("Stopping gitlab reconciliation routine")
log.DebugContext(ctx, "Stopping gitlab reconciliation routine")
default:
}

Expand All @@ -189,7 +189,7 @@ func (t *manager) register(ctx context.Context) error {
defer t.mu.Unlock()

if t.registered {
log.Debug("Already registered as global target")
log.DebugContext(ctx, "Already registered as global target")
return nil
}

Expand All @@ -201,13 +201,13 @@ func (t *manager) register(ctx context.Context) error {
}
f.SetFileName(fmt.Sprintf("%s.json", t.name))

log.Debug("Registering as global target")
log.DebugContext(ctx, "Registering as global target")
err := t.interactor.PostFile(ctx, f)
if err != nil {
log.Error("Failed to register global gitlabTargetManager", "error", err)
log.ErrorContext(ctx, "Failed to register global gitlabTargetManager", "error", err)
return err
}
log.Info("Successfully registered")
log.InfoContext(ctx, "Successfully registered")
t.registered = true
t.metrics.registered.Set(1)

Expand All @@ -221,7 +221,7 @@ func (t *manager) update(ctx context.Context) error {
t.mu.Lock()
defer t.mu.Unlock()
if !t.registered {
log.Debug("Not registered as global target, no update done.")
log.DebugContext(ctx, "Not registered as global target, no update done.")
return nil
}

Expand All @@ -233,13 +233,13 @@ func (t *manager) update(ctx context.Context) error {
}
f.SetFileName(fmt.Sprintf("%s.json", t.name))

log.Debug("Updating instance registration")
log.DebugContext(ctx, "Updating instance registration")
err := t.interactor.PutFile(ctx, f)
if err != nil {
log.Error("Failed to update registration", "error", err)
log.ErrorContext(ctx, "Failed to update registration", "error", err)
return err
}
log.Debug("Successfully updated registration")
log.DebugContext(ctx, "Successfully updated registration")
return nil
}

Expand All @@ -251,14 +251,14 @@ func (t *manager) refreshTargets(ctx context.Context) error {
var healthyTargets []checks.GlobalTarget
targets, err := t.interactor.FetchFiles(ctx)
if err != nil {
log.Error("Failed to update global targets", "error", err)
log.ErrorContext(ctx, "Failed to update global targets", "error", err)
return err
}

// filter unhealthy targets - this may be removed in the future
for _, target := range targets {
if !t.registered && target.Url == fmt.Sprintf("%s://%s", t.cfg.Scheme, t.name) {
log.Debug("Found self as global target", "lastSeenMin", time.Since(target.LastSeen).Minutes())
log.DebugContext(ctx, "Found self as global target", "lastSeenMinsAgo", time.Since(target.LastSeen).Minutes())
t.registered = true
t.metrics.registered.Set(1)
}
Expand All @@ -269,14 +269,14 @@ func (t *manager) refreshTargets(ctx context.Context) error {
}

if time.Now().Add(-t.cfg.UnhealthyThreshold).After(target.LastSeen) {
log.Debug("Skipping unhealthy target", "target", target)
log.DebugContext(ctx, "Skipping unhealthy target", "target", target)
continue
}
healthyTargets = append(healthyTargets, target)
}

t.targets = healthyTargets
log.Debug("Updated global targets", "targets", len(t.targets))
log.DebugContext(ctx, "Updated global targets", "targets", len(t.targets))
return nil
}

Expand Down
44 changes: 29 additions & 15 deletions pkg/sparrow/targets/remote/gitlab/gitlab.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ type Config struct {
Branch string `yaml:"branch" mapstructure:"branch"`
}

// apiError wraps non-expected API errors & status codes
// during the interaction with the gitlab API
type apiError struct {
message string
code int
}

func (e apiError) Error() string {
return fmt.Sprintf("gitlab API sent an unexpected status code (%d) with the following error message: %s", e.code, e.message)
}

// New creates a new gitlab client
func New(cfg Config) remote.Interactor {
c := &client{
Expand Down Expand Up @@ -130,7 +141,7 @@ func (c *client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget,

if resp.StatusCode != http.StatusOK {
log.ErrorContext(ctx, "Failed to fetch file", "status", resp.Status)
return res, fmt.Errorf("request failed, status is %s", resp.Status)
return res, toError(resp)
}

err = json.NewDecoder(resp.Body).Decode(&res)
Expand Down Expand Up @@ -196,7 +207,7 @@ func (c *client) fetchNextFileList(ctx context.Context, reqUrl string) ([]string

if resp.StatusCode != http.StatusOK {
log.ErrorContext(ctx, "Failed to fetch file list", "status", resp.Status)
return nil, fmt.Errorf("request failed, status is %s", resp.Status)
return nil, toError(resp)
}

var fl []file
Expand Down Expand Up @@ -262,14 +273,9 @@ func (c *client) PutFile(ctx context.Context, file remote.File) error { //nolint
err = errors.Join(err, resp.Body.Close())
}()

// Unfortunately the Gitlab API does not give any information about what went wrong and just returns a 400 Bad Request in case of
// an error while committing the file. A probable cause for this is that the branch does not exist or is protected.
// The documentation documents some more possible errors, but not all of them: https://docs.gitlab.com/ee/api/repository_files.html#update-existing-file-in-repository
// Therefore we just check for the status code and return an error if it is not 200 OK.
// This is not ideal, but the best we can do with the current API without implementing a full blown error handling mechanism.
if resp.StatusCode != http.StatusOK {
log.ErrorContext(ctx, "Failed to push registration file", "status", resp.Status)
return fmt.Errorf("request failed, status is %s", resp.Status)
return toError(resp)
}

return nil
Expand Down Expand Up @@ -311,14 +317,9 @@ func (c *client) PostFile(ctx context.Context, file remote.File) error { //nolin
err = errors.Join(err, resp.Body.Close())
}()

// Unfortunately the Gitlab API does not give any information about what went wrong and just returns a 400 Bad Request in case of
// an error while committing the file. A probable cause for this is that the branch does not exist or is protected.
// The documentation documents some more possible errors, but not all of them: https://docs.gitlab.com/ee/api/repository_files.html#update-existing-file-in-repository
// Therefore we just check for the status code and return an error if it is not 200 OK.
// This is not ideal, but the best we can do with the current API without implementing a full blown error handling mechanism.
if resp.StatusCode != http.StatusCreated {
log.ErrorContext(ctx, "Failed to post file", "status", resp.Status)
return fmt.Errorf("request failed, status is %s", resp.Status)
return toError(resp)
}

return nil
Expand Down Expand Up @@ -365,7 +366,7 @@ func (c *client) DeleteFile(ctx context.Context, file remote.File) error { //nol

if resp.StatusCode != http.StatusNoContent {
log.ErrorContext(ctx, "Failed to delete file", "status", resp.Status)
return fmt.Errorf("request failed, status is %s", resp.Status)
return toError(resp)
}

return nil
Expand Down Expand Up @@ -433,3 +434,16 @@ func (c *client) fetchDefaultBranch() string {
log.WarnContext(ctx, "No default branch found, using fallback", "fallback", fallbackBranch)
return fallbackBranch
}

// toError reads the error response from the gitlab API
func toError(resp *http.Response) error {
buf := new(bytes.Buffer)
_, err := buf.ReadFrom(resp.Body)
if err != nil {
return fmt.Errorf("failed to read response body from API: %w", err)
}
return apiError{
message: buf.String(),
code: resp.StatusCode,
}
}
Loading