Skip to content

Commit

Permalink
feat(robot): handle ratelimiting with constant backoff
Browse files Browse the repository at this point in the history
Add a constant backoff in case we are being rate limited by the Robot API.

This is done through an opaque wrapping of the robot.Client interface,
so callers do not need to do this manually.
  • Loading branch information
janiskemper authored and apricote committed Nov 30, 2023
1 parent 8bb131f commit 76a0366
Show file tree
Hide file tree
Showing 6 changed files with 191 additions and 17 deletions.
6 changes: 5 additions & 1 deletion hcloud/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
var robotClient robot.Client
if cfg.Robot.Enabled {
c := hrobot.NewBasicAuthClient(cfg.Robot.User, cfg.Robot.Password)
robotClient = robot.NewClient(c, cfg.Robot.CacheTimeout)

robotClient = robot.WithRateLimit(
cfg.Robot.RateLimitWaitTime,
robot.WithCache(cfg.Robot.CacheTimeout, c),
)
}

var networkID int64
Expand Down
22 changes: 14 additions & 8 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ const (
hcloudNetwork = "HCLOUD_NETWORK"
hcloudDebug = "HCLOUD_DEBUG"

robotEnabled = "ROBOT_ENABLED"
robotUser = "ROBOT_USER"
robotPassword = "ROBOT_PASSWORD"
robotCacheTimeout = "ROBOT_CACHE_TIMEOUT"
robotEnabled = "ROBOT_ENABLED"
robotUser = "ROBOT_USER"
robotPassword = "ROBOT_PASSWORD"
robotCacheTimeout = "ROBOT_CACHE_TIMEOUT"
robotRateLimitWaitTime = "ROBOT_RATE_LIMIT_WAIT_TIME"

hcloudInstancesAddressFamily = "HCLOUD_INSTANCES_ADDRESS_FAMILY"

Expand All @@ -43,10 +44,11 @@ type HCloudClientConfiguration struct {
}

type RobotConfiguration struct {
Enabled bool
User string
Password string
CacheTimeout time.Duration
Enabled bool
User string
Password string
CacheTimeout time.Duration
RateLimitWaitTime time.Duration
}

type MetricsConfiguration struct {
Expand Down Expand Up @@ -124,6 +126,10 @@ func Read() (HCCMConfiguration, error) {
if cfg.Robot.CacheTimeout == 0 {
cfg.Robot.CacheTimeout = 5 * time.Minute
}
cfg.Robot.RateLimitWaitTime, err = getEnvDuration(robotRateLimitWaitTime)
if err != nil {
errs = append(errs, err)
}

cfg.Metrics.Enabled, err = getEnvBool(hcloudMetricsEnabled, true)
if err != nil {
Expand Down
14 changes: 9 additions & 5 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,16 @@ func TestRead(t *testing.T) {
"ROBOT_ENABLED", "true",
"ROBOT_USER", "foobar",
"ROBOT_PASSWORD", "secret-password",
"ROBOT_RATE_LIMIT_WAIT_TIME", "5m",
"ROBOT_CACHE_TIMEOUT", "1m",
},
want: HCCMConfiguration{
Robot: RobotConfiguration{
Enabled: true,
User: "foobar",
Password: "secret-password",
CacheTimeout: 1 * time.Minute,
Enabled: true,
User: "foobar",
Password: "secret-password",
CacheTimeout: 1 * time.Minute,
RateLimitWaitTime: 5 * time.Minute,
},
Metrics: MetricsConfiguration{Enabled: true, Address: ":8233"},
Instance: InstanceConfiguration{AddressFamily: AddressFamilyIPv4},
Expand Down Expand Up @@ -194,8 +196,10 @@ failed to parse HCLOUD_NETWORK_ROUTES_ENABLED: strconv.ParseBool: parsing "si":
name: "error parsing duration values",
env: []string{
"ROBOT_CACHE_TIMEOUT", "biweekly",
"ROBOT_RATE_LIMIT_WAIT_TIME", "42fortnights",
},
wantErr: errors.New(`failed to parse ROBOT_CACHE_TIMEOUT: time: invalid duration "biweekly"`),
wantErr: errors.New(`failed to parse ROBOT_CACHE_TIMEOUT: time: invalid duration "biweekly"
failed to parse ROBOT_RATE_LIMIT_WAIT_TIME: time: unknown unit "fortnights" in duration "42fortnights"`),
},
}

Expand Down
5 changes: 2 additions & 3 deletions internal/robot/client.go → internal/robot/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import (
"sync"
"time"

hrobot "github.com/syself/hrobot-go"
hrobotmodels "github.com/syself/hrobot-go/models"
)

type cacheRobotClient struct {
robotClient hrobot.RobotClient
robotClient Client
timeout time.Duration

lastUpdate time.Time
Expand All @@ -21,7 +20,7 @@ type cacheRobotClient struct {
serversByID map[int]*hrobotmodels.Server
}

func NewClient(robotClient hrobot.RobotClient, cacheTimeout time.Duration) Client {
func WithCache(cacheTimeout time.Duration, robotClient Client) Client {
return &cacheRobotClient{
timeout: cacheTimeout,
robotClient: robotClient,
Expand Down
93 changes: 93 additions & 0 deletions internal/robot/ratelimit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package robot

import (
"fmt"
"strings"
"time"

hrobotmodels "github.com/syself/hrobot-go/models"
)

type rateLimitClient struct {
robotClient Client

waitTime time.Duration
exceeded bool
lastChecked time.Time
}

func WithRateLimit(rateLimitWaitTime time.Duration, robotClient Client) Client {
return &rateLimitClient{
robotClient: robotClient,

waitTime: rateLimitWaitTime,
}
}

func (c *rateLimitClient) ServerGet(id int) (*hrobotmodels.Server, error) {
if c.isExceeded() {
return nil, c.getRateLimitError()
}

server, err := c.robotClient.ServerGet(id)
c.handleError(err)
return server, err
}

func (c *rateLimitClient) ServerGetList() ([]hrobotmodels.Server, error) {
if c.isExceeded() {
return nil, c.getRateLimitError()
}

servers, err := c.robotClient.ServerGetList()
c.handleError(err)
return servers, err
}

func (c *rateLimitClient) ResetGet(id int) (*hrobotmodels.Reset, error) {
if c.isExceeded() {
return nil, c.getRateLimitError()
}

reset, err := c.robotClient.ResetGet(id)
c.handleError(err)
return reset, err
}

func (c *rateLimitClient) set() {
c.exceeded = true
c.lastChecked = time.Now()
}

func (c *rateLimitClient) isExceeded() bool {
if !c.exceeded {
return false
}

if time.Now().Before(c.lastChecked.Add(c.waitTime)) {
return true
}
// Waiting time is over. Should try again
c.exceeded = false
c.lastChecked = time.Time{}
return false
}

func (c *rateLimitClient) handleError(err error) {
if err == nil {
return
}

if hrobotmodels.IsError(err, hrobotmodels.ErrorCodeRateLimitExceeded) || strings.Contains(err.Error(), "server responded with status code 403") {
c.set()
}
}

func (c *rateLimitClient) getRateLimitError() error {
if !c.isExceeded() {
return nil
}

nextPossibleCall := c.lastChecked.Add(c.waitTime)
return fmt.Errorf("rate limit exceeded, next try at %q", nextPossibleCall.String())
}
68 changes: 68 additions & 0 deletions internal/robot/ratelimit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package robot

import (
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
hrobotmodels "github.com/syself/hrobot-go/models"

"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks"
)

func TestRateLimit(t *testing.T) {
mock := mocks.RobotClient{}
mock.On("ServerGetList").Return([]hrobotmodels.Server{}, nil).Once()

client := WithRateLimit(5*time.Minute, &mock)

servers, err := client.ServerGetList()
assert.NoError(t, err)
assert.Len(t, servers, 0)
mock.AssertNumberOfCalls(t, "ServerGetList", 1)

mock.On("ServerGetList").Return(nil, hrobotmodels.Error{Code: hrobotmodels.ErrorCodeRateLimitExceeded, Message: "Rate limit exceeded"}).Once()
_, err = client.ServerGetList()
assert.Error(t, err)
mock.AssertNumberOfCalls(t, "ServerGetList", 2)

// No further call should be made
_, err = client.ServerGetList()
assert.Error(t, err)
mock.AssertNumberOfCalls(t, "ServerGetList", 2)
}

func TestRateLimitIsExceeded(t *testing.T) {
client := rateLimitClient{
waitTime: 5 * time.Minute,
exceeded: true,
lastChecked: time.Now(),
}
// Just exceeded
assert.True(t, client.isExceeded())

// Exceeded longer than wait time ago
client.lastChecked = time.Now().Add(-6 * time.Minute)
assert.False(t, client.isExceeded())

// Not exceeded ever
client.exceeded = false
client.lastChecked = time.Time{}
assert.False(t, client.isExceeded())
}

func TestRateLimitGetRateLimitError(t *testing.T) {
client := rateLimitClient{
waitTime: 5 * time.Minute,
}
err := client.getRateLimitError()
assert.NoError(t, err)

client.exceeded = true
client.lastChecked = time.Now()

err = client.getRateLimitError()
assert.Error(t, err)
assert.True(t, strings.HasPrefix(err.Error(), "rate limit exceeded, next try at "))
}

0 comments on commit 76a0366

Please sign in to comment.