Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed May 12, 2023
1 parent a8d3a23 commit 9d9d1cb
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 5 deletions.
File renamed without changes.
15 changes: 13 additions & 2 deletions internal/adminapi/backoff_strategy_konnect.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ type SystemClock struct{}
func (SystemClock) Now() time.Time { return time.Now() }

// KonnectBackoffStrategy keeps track of Konnect config push backoffs.
//
// It takes into account:
// - a regular exponential backoff that is incremented on every Update failure,
// - a last failed configuration hash (where we skip Update until a config changes).
//
// It's important to note that KonnectBackoffStrategy can use the latter (config hash)
// because of the nature of the one-directional integration where KIC is the only
// component responsible for populating configuration of Konnect's Runtime Group.
// In case that changes in the future (e.g. manual modifications to parts of the
// configuration are allowed on Konnect side for some reason), we might have to
// drop this part of the backoff strategy.
type KonnectBackoffStrategy struct {
b *backoff.Backoff
nextAttempt time.Time
Expand Down Expand Up @@ -59,7 +70,7 @@ func (s *KonnectBackoffStrategy) CanUpdate(configHash []byte) (bool, string) {
// The exponential backoff duration is satisfied.
// In case of the first attempt it will be satisfied as s.nextAttempt will be a zero value which is always in the past.
timeLeft := s.nextAttempt.Sub(s.clock.Now())
exponentialBackoffSatisfied := timeLeft.Seconds() <= 0
exponentialBackoffSatisfied := timeLeft <= 0

// The configuration we're attempting to update is not the same faulty config we've already tried pushing.
isTheSameFaultyConfig := s.lastFailedConfigHash != nil && bytes.Equal(s.lastFailedConfigHash, configHash)
Expand Down Expand Up @@ -123,7 +134,7 @@ func (s *KonnectBackoffStrategy) whyCannotUpdate(
))
}

if timeLeft.Seconds() > 0 {
if timeLeft > 0 {
reasons = append(reasons, fmt.Sprintf("next attempt allowed in %s", timeLeft))
}

Expand Down
9 changes: 8 additions & 1 deletion internal/dataplane/kong_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package dataplane

import (
"context"
"errors"
"fmt"
"reflect"
"sort"
Expand Down Expand Up @@ -430,7 +431,13 @@ func (c *KongClient) trySendOutToKonnectClient(ctx context.Context, s *kongstate
if _, err := c.sendToClient(ctx, konnectClient, s, config); err != nil {
// In case of an error, we only log it since we don't want make Konnect affect the basic functionality
// of the controller.
c.logger.WithError(err).Error("Failed pushing configuration to Konnect")

if errors.Is(err, sendconfig.ErrUpdateSkippedDueToBackoffStrategy{}) {
c.logger.WithError(err).Warn("Skipped pushing configuration to Konnect")
return
}

c.logger.WithError(err).Warn("Failed pushing configuration to Konnect")
}
}

Expand Down
3 changes: 1 addition & 2 deletions internal/dataplane/sendconfig/backoff_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,12 @@ func (s UpdateStrategyWithBackoff) Update(ctx context.Context, targetContent Con
resourceErrorsParseErr error,
) {
if canUpdate, whyNot := s.backoffStrategy.CanUpdate(targetContent.Hash); !canUpdate {
s.log.Debug("Skipping update due to a backoff strategy")
return NewErrUpdateSkippedDueToBackoffStrategy(whyNot), nil, nil
}

err, resourceErrors, resourceErrorsParseErr = s.decorated.Update(ctx, targetContent)
if err != nil {
s.log.WithError(err).Error("Update failed, registering it for backoff strategy")
s.log.WithError(err).Debug("Update failed, registering it for backoff strategy")
s.backoffStrategy.RegisterUpdateFailure(err, targetContent.Hash)
} else {
s.backoffStrategy.RegisterUpdateSuccess()
Expand Down

0 comments on commit 9d9d1cb

Please sign in to comment.