Merge pull request #19 from syself/cache-client

✨ Add cache client for Hetzner robot

.golangci.yaml

@@ -8,7 +8,6 @@ linters-settings:
     sections:
       - standard
       - default
-      - prefix(github.com/syself)
 
   importas:
     no-unaliased: true

hcloud/cloud.go

@@ -24,11 +24,15 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/metadata"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/hcops"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
+	robotclient "github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/robot/client/cache"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/util"
 	hrobot "github.com/syself/hrobot-go"
 	cloudprovider "k8s.io/cloud-provider"
 	"k8s.io/klog/v2"
@@ -47,6 +51,9 @@ const (
 	// Default is 5 minutes.
 	RateLimitWaitTimeRobot = "RATE_LIMIT_WAIT_TIME_ROBOT"
 
+	// default is 3 minutes.
+	CacheTimeout = "CACHE_TIMEOUT"
+
 	// Disable the "master/server is attached to the network" check against the metadata service.
 	hcloudNetworkDisableAttachedCheckENVVar  = "HCLOUD_NETWORK_DISABLE_ATTACHED_CHECK"
 	hcloudNetworkRoutesEnabledENVVar         = "HCLOUD_NETWORK_ROUTES_ENABLED"
@@ -71,7 +78,7 @@ var providerVersion = "unknown"
 
 type cloud struct {
 	client       *hcloud.Client
-	robotClient  hrobot.RobotClient
+	robotClient  robotclient.Client
 	instances    *instances
 	routes       *routes
 	loadBalancer *loadBalancers
@@ -118,9 +125,19 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
 	robotUserName, foundRobotUserName := os.LookupEnv(robotUserNameENVVar)
 	robotPassword, foundRobotPassword := os.LookupEnv(robotPasswordENVVar)
 
-	var robotClient hrobot.RobotClient
+	cacheTimeout, err := util.GetEnvDuration(CacheTimeout)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
+
+	if cacheTimeout == 0 {
+		cacheTimeout = 3 * time.Minute
+	}
+
+	var robotClient robotclient.Client
 	if foundRobotUserName && foundRobotPassword {
-		robotClient = hrobot.NewBasicAuthClient(robotUserName, robotPassword)
+		c := hrobot.NewBasicAuthClient(robotUserName, robotPassword)
+		robotClient = cache.NewClient(c, cacheTimeout)
 	}
 
 	var networkID int64
@@ -153,7 +170,7 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
 	}
 
 	// Validate that the provided token works, and we have network connectivity to the Hetzner Cloud API
-	_, _, err := client.Server.List(context.Background(), hcloud.ServerListOpts{})
+	_, _, err = client.Server.List(context.Background(), hcloud.ServerListOpts{})
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}

hcloud/instances.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
-	hrobot "github.com/syself/hrobot-go"
+	robotclient "github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
 	"github.com/syself/hrobot-go/models"
 	corev1 "k8s.io/api/core/v1"
 	cloudprovider "k8s.io/cloud-provider"
@@ -38,14 +38,14 @@ const (
 
 type instances struct {
 	client        *hcloud.Client
-	robotClient   hrobot.RobotClient
+	robotClient   robotclient.Client
 	addressFamily addressFamily
 	networkID     int64
 }
 
 var errServerNotFound = fmt.Errorf("server not found")
 
-func newInstances(client *hcloud.Client, robotClient hrobot.RobotClient, addressFamily addressFamily, networkID int64) *instances {
+func newInstances(client *hcloud.Client, robotClient robotclient.Client, addressFamily addressFamily, networkID int64) *instances {
 	return &instances{client, robotClient, addressFamily, networkID}
 }
 

hcloud/instances_test.go

@@ -24,13 +24,12 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/hetznercloud/hcloud-go/v2/hcloud/schema"
 	"github.com/syself/hrobot-go/models"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	cloudprovider "k8s.io/cloud-provider"
-
-	"github.com/hetznercloud/hcloud-go/v2/hcloud"
-	"github.com/hetznercloud/hcloud-go/v2/hcloud/schema"
 )
 
 // TestInstances_InstanceExists also tests [lookupServer]. The other tests

hcloud/routes_test.go

@@ -6,10 +6,9 @@ import (
 	"net/http"
 	"testing"
 
-	cloudprovider "k8s.io/cloud-provider"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/schema"
+	cloudprovider "k8s.io/cloud-provider"
 )
 
 func TestRoutes_CreateRoute(t *testing.T) {

hcloud/testing.go

@@ -5,14 +5,13 @@ import (
 	"os"
 	"testing"
 
+	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/annotation"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/hcops"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/mocks"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-
-	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
 
 // Setenv prepares the environment for testing the

hcloud/util.go

@@ -25,7 +25,7 @@ import (
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/hcops"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
-	hrobot "github.com/syself/hrobot-go"
+	robotclient "github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
 	"github.com/syself/hrobot-go/models"
 	"k8s.io/klog/v2"
 )
@@ -53,7 +53,7 @@ func getHCloudServerByID(ctx context.Context, c *hcloud.Client, id int64) (*hclo
 	return server, nil
 }
 
-func getRobotServerByName(c hrobot.RobotClient, name string) (server *models.Server, err error) {
+func getRobotServerByName(c robotclient.Client, name string) (server *models.Server, err error) {
 	const op = "robot/getServerByName"
 
 	if c == nil {
@@ -80,7 +80,7 @@ func getRobotServerByName(c hrobot.RobotClient, name string) (server *models.Ser
 	return server, nil
 }
 
-func getRobotServerByID(c hrobot.RobotClient, id int) (*models.Server, error) {
+func getRobotServerByID(c robotclient.Client, id int) (*models.Server, error) {
 	const op = "robot/getServerByID"
 
 	if c == nil {

internal/annotation/testing.go

@@ -5,10 +5,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
-
-	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
 
 // AssertServiceAnnotated asserts that svc has been annotated with all

internal/hcops/load_balancer.go

@@ -13,7 +13,7 @@ import (
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/annotation"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
-	hrobot "github.com/syself/hrobot-go"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/klog/v2"
 )
@@ -72,7 +72,7 @@ type LoadBalancerOps struct {
 	LBClient      HCloudLoadBalancerClient
 	ActionClient  HCloudActionClient
 	NetworkClient HCloudNetworkClient
-	RobotClient   hrobot.RobotClient
+	RobotClient   client.Client
 	CertOps       *CertificateOps
 	RetryDelay    time.Duration
 	NetworkID     int64
@@ -724,7 +724,6 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 	// Assign the dedicated servers which are currently assigned as nodes
 	// to the K8S Load Balancer as IP targets to the HC Load Balancer.
 	for id := range k8sNodeIDsRobot {
-
 		var arr []string
 		if l.Defaults.DisableIPv6 {
 			arr = []string{

internal/hcops/ratelimit.go

@@ -1,16 +1,15 @@
 package hcops
 
 import (
-	"fmt"
-	"os"
 	"strings"
 	"time"
 
+	"github.com/syself/hetzner-cloud-controller-manager/internal/util"
 	"github.com/syself/hrobot-go/models"
 )
 
 func init() {
-	rateLimitWaitTimeRobot, err := getEnvDuration("RATE_LIMIT_WAIT_TIME_ROBOT")
+	rateLimitWaitTimeRobot, err := util.GetEnvDuration("RATE_LIMIT_WAIT_TIME_ROBOT")
 	if err != nil {
 		panic(err)
 	}
@@ -78,19 +77,3 @@ func HandleRateLimitExceededError(err error) {
 		SetRateLimit()
 	}
 }
-
-// getEnvDuration returns the duration parsed from the environment variable with the given key and a potential error
-// parsing the var. Returns false if the env var is unset.
-func getEnvDuration(key string) (time.Duration, error) {
-	v := os.Getenv(key)
-	if v == "" {
-		return 0, nil
-	}
-
-	b, err := time.ParseDuration(v)
-	if err != nil {
-		return 0, fmt.Errorf("%s: %v", key, err)
-	}
-
-	return b, nil
-}

internal/mocks/action.go

@@ -3,9 +3,8 @@ package mocks
 import (
 	"context"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/stretchr/testify/mock"
 )
 
 type ActionClient struct {

internal/mocks/certificates.go

@@ -3,9 +3,8 @@ package mocks
 import (
 	"context"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/stretchr/testify/mock"
 )
 
 type CertificateClient struct {

internal/mocks/loadbalancer.go

@@ -4,9 +4,8 @@ import (
 	"context"
 	"net"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/stretchr/testify/mock"
 )
 
 type LoadBalancerClient struct {

internal/mocks/network.go

@@ -3,9 +3,8 @@ package mocks
 import (
 	"context"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/stretchr/testify/mock"
 )
 
 type NetworkClient struct {

internal/mocks/server.go

@@ -4,9 +4,8 @@ import (
 	"context"
 	"testing"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/stretchr/testify/mock"
 )
 
 // ServerClient is a mock implementation of the hcloud.ServerClient.

internal/robot/client/cache/client.go

@@ -0,0 +1,85 @@
+package cache
+
+import (
+	"time"
+
+	"github.com/syself/hetzner-cloud-controller-manager/internal/robot/client"
+	hrobot "github.com/syself/hrobot-go"
+	"github.com/syself/hrobot-go/models"
+)
+
+var handler = &cacheRobotClient{}
+
+type cacheRobotClient struct {
+	robotClient hrobot.RobotClient
+	timeout     time.Duration
+
+	lastUpdate time.Time
+
+	// cache
+	l []models.Server
+	m map[int]*models.Server
+}
+
+func NewClient(robotClient hrobot.RobotClient, cacheTimeout time.Duration) client.Client {
+	handler.timeout = cacheTimeout
+	handler.robotClient = robotClient
+	return handler
+}
+
+func (c *cacheRobotClient) ServerGet(id int) (*models.Server, error) {
+	if c.shouldSync() {
+		server, err := c.robotClient.ServerGet(id)
+		if err != nil {
+			return server, err
+		}
+
+		// Add server to cache in case there is another Get call - but do not update time of last update.
+		// That only makes sense for list calls.
+		c.m[server.ServerNumber] = server
+		return server, nil
+	}
+
+	server, found := c.m[id]
+	if !found {
+		// return not found error
+		return nil, models.Error{Code: models.ErrorCodeServerNotFound, Message: "server not found"}
+	}
+
+	return server, nil
+}
+
+func (c *cacheRobotClient) ServerGetList() ([]models.Server, error) {
+	if c.shouldSync() {
+		list, err := c.robotClient.ServerGetList()
+		if err != nil {
+			return list, err
+		}
+
+		// populate list
+		c.l = list
+
+		// remove all entries from map and populate it freshly
+		c.m = make(map[int]*models.Server)
+		for i, server := range list {
+			c.m[server.ServerNumber] = &list[i]
+		}
+
+		// set time of last update
+		c.lastUpdate = time.Now()
+	}
+
+	return c.l, nil
+}
+
+func (c *cacheRobotClient) shouldSync() bool {
+	// map is nil means we have no cached value yet
+	if c.m == nil {
+		c.m = make(map[int]*models.Server)
+		return true
+	}
+	if time.Now().After(c.lastUpdate.Add(c.timeout)) {
+		return true
+	}
+	return false
+}

internal/robot/client/interface.go

@@ -0,0 +1,8 @@
+package client
+
+import "github.com/syself/hrobot-go/models"
+
+type Client interface {
+	ServerGet(id int) (*models.Server, error)
+	ServerGetList() ([]models.Server, error)
+}