feat(robot): handle ratelimiting with constant backoff (#572)

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.

Co-authored-by: Janis Kemper <janis.kemper@syself.com>

hcloud/cloud.go

@@ -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.NewRateLimitedClient(
+			cfg.Robot.RateLimitWaitTime,
+			robot.NewCachedClient(cfg.Robot.CacheTimeout, c),
+		)
 	}
 
 	var networkID int64

internal/config/config.go

@@ -18,6 +18,7 @@ const (
 	robotUser              = "ROBOT_USER"
 	robotPassword          = "ROBOT_PASSWORD"
 	robotCacheTimeout      = "ROBOT_CACHE_TIMEOUT"
+	robotRateLimitWaitTime = "ROBOT_RATE_LIMIT_WAIT_TIME"
 
 	hcloudInstancesAddressFamily = "HCLOUD_INSTANCES_ADDRESS_FAMILY"
 
@@ -47,6 +48,7 @@ type RobotConfiguration struct {
 	User              string
 	Password          string
 	CacheTimeout      time.Duration
+	RateLimitWaitTime time.Duration
 }
 
 type MetricsConfiguration struct {
@@ -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 {

internal/config/config_test.go

@@ -74,6 +74,7 @@ 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{
@@ -82,6 +83,7 @@ func TestRead(t *testing.T) {
 					User:              "foobar",
 					Password:          "secret-password",
 					CacheTimeout:      1 * time.Minute,
+					RateLimitWaitTime: 5 * time.Minute,
 				},
 				Metrics:      MetricsConfiguration{Enabled: true, Address: ":8233"},
 				Instance:     InstanceConfiguration{AddressFamily: AddressFamilyIPv4},
@@ -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"`),
 		},
 	}
 

internal/robot/client.go → internal/robot/cache.go

@@ -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
@@ -21,7 +20,7 @@ type cacheRobotClient struct {
 	serversByID map[int]*hrobotmodels.Server
 }
 
-func NewClient(robotClient hrobot.RobotClient, cacheTimeout time.Duration) Client {
+func NewCachedClient(cacheTimeout time.Duration, robotClient Client) Client {
 	return &cacheRobotClient{
 		timeout:     cacheTimeout,
 		robotClient: robotClient,

internal/robot/ratelimit.go

+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 NewRateLimitedClient(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())
+}

internal/robot/ratelimit_test.go

+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 := NewRateLimitedClient(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 "))
+}