diff --git a/hcloud/cloud.go b/hcloud/cloud.go index a7685fca4..35f595b24 100644 --- a/hcloud/cloud.go +++ b/hcloud/cloud.go @@ -43,8 +43,10 @@ import ( ) const ( - providerName = "hcloud" - apiClientTimeout = 15 * time.Second + providerName = "hcloud" + apiClientTimeout = 15 * time.Second + lbTypeCacheMaxAge = time.Hour + lbTypeCacheDefaultMode = cache.ModeAll ) // providerVersion is set by the build process using -ldflags -X. @@ -54,6 +56,7 @@ type cloud struct { client *hcloud.Client robotClient hrobot.RobotClient serverCache *cache.Cache[hcloud.Server] + lbTypeCache *cache.Cache[hcloud.LoadBalancerType] cfg config.HCCMConfiguration recorder record.EventRecorder networkID int64 @@ -150,11 +153,13 @@ func NewCloud(cidr string, nodeLister corelisters.NodeLister) (cloudprovider.Int klog.Infof("Hetzner Cloud k8s cloud controller %s started\n", providerVersion) serverCache := cache.NewServerCache(client, cfg.ServerCache.Mode, cfg.ServerCache.MaxAge) + lbTypeCache := cache.NewLoadBalancerTypeCache(client, lbTypeCacheDefaultMode, lbTypeCacheMaxAge) return &cloud{ client: client, robotClient: robotClient, serverCache: serverCache, + lbTypeCache: lbTypeCache, cfg: cfg, networkID: networkID, cidr: cidr, @@ -202,6 +207,7 @@ func (c *cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) { CertOps: &hcops.CertificateOps{ActionClient: &c.client.Action, CertClient: &c.client.Certificate}, ActionClient: &c.client.Action, NetworkClient: &c.client.Network, + LBTypeCache: c.lbTypeCache, NetworkID: c.networkID, Cfg: c.cfg, Recorder: c.recorder, diff --git a/internal/cache/lbtypecache.go b/internal/cache/lbtypecache.go new file mode 100644 index 000000000..ff21a8ab3 --- /dev/null +++ b/internal/cache/lbtypecache.go @@ -0,0 +1,42 @@ +package cache + +import ( + "context" + "time" + + "github.com/prometheus/client_golang/prometheus" + + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" + "github.com/hetznercloud/hcloud-go/v2/hcloud" +) + +var lbTypeCacheRequests = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "cloud_controller_manager_load_balancer_type_cache_requests_total", + Help: "Total cache requests to the Load Balancer Types API partitioned by subsystem, mode and result.", +}, []string{"subsystem", "mode", "result"}) + +func init() { + metrics.GetRegistry().MustRegister(lbTypeCacheRequests) +} + +func NewLoadBalancerTypeCache(client *hcloud.Client, defaultMode Mode, defaultMaxAge time.Duration) *Cache[hcloud.LoadBalancerType] { + return newCache[hcloud.LoadBalancerType]( + func(ctx context.Context, id int64) (*hcloud.LoadBalancerType, error) { + value, _, err := client.LoadBalancerType.GetByID(ctx, id) + return value, err + }, + func(ctx context.Context, name string) (*hcloud.LoadBalancerType, error) { + value, _, err := client.LoadBalancerType.GetByName(ctx, name) + return value, err + }, + func(ctx context.Context) ([]*hcloud.LoadBalancerType, error) { + values, err := client.LoadBalancerType.All(ctx) + return values, err + }, + func(value *hcloud.LoadBalancerType) int64 { return value.ID }, + func(value *hcloud.LoadBalancerType) string { return value.Name }, + lbTypeCacheRequests, + defaultMode, + defaultMaxAge, + ) +} diff --git a/internal/cache/lbtypecache_test.go b/internal/cache/lbtypecache_test.go new file mode 100644 index 000000000..2b5b9e1bb --- /dev/null +++ b/internal/cache/lbtypecache_test.go @@ -0,0 +1,65 @@ +package cache + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hetznercloud/hcloud-go/v2/hcloud" + "github.com/hetznercloud/hcloud-go/v2/hcloud/exp/mockutil" +) + +func TestNewLBTypeCache(t *testing.T) { + testCases := []struct { + name string + mode Mode + requests []mockutil.Request + }{ + { + mode: ModeAll, + requests: []mockutil.Request{ + {Method: "GET", Path: "/load_balancer_types?page=1&per_page=50", Status: 200, JSONRaw: `{ "load_balancer_types": [{ "id": 1, "name": "lb11" }]}`}, + }, + }, + { + mode: ModeOne, + requests: []mockutil.Request{ + {Method: "GET", Path: "/load_balancer_types/1", Status: 200, JSONRaw: `{ "load_balancer_type": { "id": 1, "name": "lb11" }}`}, + }, + }, + { + mode: ModeOff, + requests: []mockutil.Request{ + {Method: "GET", Path: "/load_balancer_types/1", Status: 200, JSONRaw: `{ "load_balancer_type": { "id": 1, "name": "lb11" }}`}, + {Method: "GET", Path: "/load_balancer_types?name=lb11", Status: 200, JSONRaw: `{ "load_balancer_types": [{ "id": 1, "name": "lb11" }]}`}, + }, + }, + } + + for _, tt := range testCases { + t.Run(string(tt.mode), func(t *testing.T) { + server := mockutil.NewServer(t, tt.requests) + client := hcloud.NewClient(hcloud.WithEndpoint(server.Server.URL)) + + cache := NewLoadBalancerTypeCache(client, tt.mode, 10*time.Second) + require.NotNil(t, cache) + require.NotNil(t, cache.fetchOneByID) + require.NotNil(t, cache.fetchOneByName) + require.NotNil(t, cache.fetchAll) + require.NotNil(t, cache.getID) + require.NotNil(t, cache.getName) + + ctx := t.Context() + + srv, err := cache.ByID(ctx, int64(1)) + require.NoError(t, err) + assert.NotNil(t, srv) + + srv, err = cache.ByName(ctx, "lb11") + require.NoError(t, err) + assert.NotNil(t, srv) + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 4211e6e62..30891e03f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -273,7 +273,7 @@ func Read() (HCCMConfiguration, error) { errs = append(errs, err) } - cfg.LoadBalancer.Type = os.Getenv(hcloudLoadBalancersType) + cfg.LoadBalancer.Type = os.Getenv(HcloudLoadBalancersType) cfg.Network.NameOrID = os.Getenv(hcloudNetwork) disableAttachedCheck, err := getEnvBool(hcloudNetworkDisableAttachedCheck, false) diff --git a/internal/config/load_balancer_envs.go b/internal/config/load_balancer_envs.go index 37c64141b..62f8f870e 100644 --- a/internal/config/load_balancer_envs.go +++ b/internal/config/load_balancer_envs.go @@ -81,11 +81,11 @@ const ( // Type: string hcloudLoadBalancersPrivateSubnetIPRange = "HCLOUD_LOAD_BALANCERS_PRIVATE_SUBNET_IP_RANGE" - // hcloudLoadBalancersType configures the default Load Balancer type this Load Balancer should be created with. + // HcloudLoadBalancersType configures the default Load Balancer type this Load Balancer should be created with. // // Type: string // Default: lb11 - hcloudLoadBalancersType = "HCLOUD_LOAD_BALANCERS_TYPE" + HcloudLoadBalancersType = "HCLOUD_LOAD_BALANCERS_TYPE" // hcloudLoadBalancersUsesProxyProtocol enables the proxyprotocol for a Load Balancer service by default. // diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index 177f567f4..b43b478e6 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -15,15 +15,23 @@ import ( "k8s.io/klog/v2" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/config" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/metrics" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/providerid" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/utils" "github.com/hetznercloud/hcloud-go/v2/hcloud" + "github.com/hetznercloud/hcloud-go/v2/hcloud/exp/deprecationutil" ) -// LabelServiceUID is a label added to the Hetzner Cloud backend to uniquely -// identify a load balancer managed by Hetzner Cloud Cloud Controller Manager. -const LabelServiceUID = "hcloud-ccm/service-uid" +const ( + // LabelServiceUID is a label added to the Hetzner Cloud backend to uniquely + // identify a load balancer managed by Hetzner Cloud Cloud Controller Manager. + LabelServiceUID = "hcloud-ccm/service-uid" + + defaultLoadBalancerType = "lb11" + loadBalancerSubsystem = "load_balancer" +) // LoadBalancerOps implements all operations regarding Hetzner Cloud Load Balancers. type LoadBalancerOps struct { @@ -31,6 +39,7 @@ type LoadBalancerOps struct { ActionClient hcloud.IActionClient NetworkClient hcloud.INetworkClient RobotClient hrobot.RobotClient + LBTypeCache *cache.Cache[hcloud.LoadBalancerType] CertOps *CertificateOps RetryDelay time.Duration NetworkID int64 @@ -109,6 +118,57 @@ func (l *LoadBalancerOps) GetByID(ctx context.Context, id int64) (*hcloud.LoadBa return lb, nil } +func (l *LoadBalancerOps) getType(ctx context.Context, svc *corev1.Service) (*hcloud.LoadBalancerType, bool, error) { + ctx = cache.SetSubsystem(ctx, loadBalancerSubsystem) + var lbTypeName string + var unset bool + + if l.Cfg.LoadBalancer.Type != "" { + lbTypeName = l.Cfg.LoadBalancer.Type + } + + if v, ok := annotation.LBType.StringFromService(svc); ok { + lbTypeName = v + } + + if lbTypeName == "" { + lbTypeName = defaultLoadBalancerType + unset = true + utils.WarnEventLogf( + l.Recorder, + svc, + "LoadBalancerTypeUnconfigured", + "Load Balancer Type unconfigured: this will be required in the future, set it with the annotation %q or cluster-wide with the environment variable %q", + annotation.LBType, + config.HcloudLoadBalancersType, + ) + } + + lbType, err := l.LBTypeCache.ByName(ctx, lbTypeName) + if err != nil { + return nil, unset, err + } + + if lbType == nil { + return nil, unset, fmt.Errorf("load balancer type not found: %s", lbTypeName) + } + + msg, unavailable := deprecationutil.LoadBalancerTypeMessage(lbType) + if unavailable { + return nil, false, errors.New(msg) + } + if msg != "" { + utils.WarnEventLogf( + l.Recorder, + svc, + "LoadBalancerTypeDeprecated", + "%s", msg, + ) + } + + return lbType, unset, nil +} + // Create creates a new Load Balancer using the Hetzner Cloud API. // // It adds annotations identifying the HC Load Balancer to svc. @@ -125,11 +185,13 @@ func (l *LoadBalancerOps) Create( LabelServiceUID: string(svc.ObjectMeta.UID), }, } - if v, ok := annotation.LBType.StringFromService(svc); ok { - opts.LoadBalancerType.Name = v - } else if l.Cfg.LoadBalancer.Type != "" { - opts.LoadBalancerType.Name = l.Cfg.LoadBalancer.Type + + lbType, _, err := l.getType(ctx, svc) + if err != nil { + return nil, fmt.Errorf("error getting load balancer type: %w", err) } + opts.LoadBalancerType = lbType + if l.Cfg.LoadBalancer.Location != "" { opts.Location = &hcloud.Location{Name: l.Cfg.LoadBalancer.Location} } @@ -395,18 +457,25 @@ func (l *LoadBalancerOps) changeType(ctx context.Context, lb *hcloud.LoadBalance const op = "hcops/LoadBalancerOps.changeType" metrics.OperationCalled.WithLabelValues(op).Inc() - lt, ok := annotation.LBType.StringFromService(svc) - if !ok { - if l.Cfg.LoadBalancer.Type == "" { - return false, nil - } - lt = l.Cfg.LoadBalancer.Type + opts := hcloud.LoadBalancerChangeTypeOpts{} + + lbType, unset, err := l.getType(ctx, svc) + if err != nil { + return false, fmt.Errorf("error getting load balancer type: %w", err) + } + + // If the user removes the annotation, we do not downgrade the Load Balancer + // back to its default value. This could be changed in a next major release. + if unset { + return false, nil } - if lt == lb.LoadBalancerType.Name { + + opts.LoadBalancerType = lbType + + if lb.LoadBalancerType.Name == lbType.Name { return false, nil } - opts := hcloud.LoadBalancerChangeTypeOpts{LoadBalancerType: &hcloud.LoadBalancerType{Name: lt}} action, _, err := l.LBClient.ChangeType(ctx, lb, opts) if err != nil { return false, fmt.Errorf("%s: %w", op, withInvalidInputFields(err)) diff --git a/internal/hcops/load_balancer_test.go b/internal/hcops/load_balancer_test.go index ca1d52e4f..b21d179f6 100644 --- a/internal/hcops/load_balancer_test.go +++ b/internal/hcops/load_balancer_test.go @@ -262,7 +262,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "some-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{ Name: "fsn1", }, @@ -284,7 +284,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ hcops.LabelServiceUID: "another-lb-uid", @@ -301,7 +301,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "some-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{ Name: "fsn1", }, @@ -320,7 +320,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "some-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ hcops.LabelServiceUID: "some-lb-uid", @@ -341,7 +341,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, NetworkZone: hcloud.NetworkZoneEUCentral, Labels: map[string]string{ hcops.LabelServiceUID: "another-lb-uid", @@ -362,7 +362,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{ Name: "fsn1", }, @@ -386,7 +386,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ hcops.LabelServiceUID: "another-lb-uid", @@ -402,7 +402,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb21"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ hcops.LabelServiceUID: "another-lb-uid", @@ -418,7 +418,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "another-lb", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, Algorithm: &hcloud.LoadBalancerAlgorithm{Type: hcloud.LoadBalancerAlgorithmTypeLeastConnections}, Labels: map[string]string{ @@ -437,7 +437,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "lb-default-type", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb21"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, Location: &hcloud.Location{Name: "nbg1"}, Labels: map[string]string{ hcops.LabelServiceUID: "lb-default-type-uid", @@ -455,7 +455,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "lb-disable-public", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, PublicInterface: new(false), Labels: map[string]string{ @@ -480,7 +480,7 @@ func TestLoadBalancerOps_Create(t *testing.T) { }, createOpts: hcloud.LoadBalancerCreateOpts{ Name: "lb-with-priv", - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 1, Name: "lb11"}, Location: &hcloud.Location{Name: "nbg1"}, PublicInterface: new(false), Labels: map[string]string{ @@ -704,7 +704,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, mock: func(_ *testing.T, tt *LBReconcilementTestCase) { opts := hcloud.LoadBalancerChangeTypeOpts{ - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb21"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, } action := &hcloud.Action{ID: 4711} @@ -737,7 +737,7 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) { }, mock: func(_ *testing.T, tt *LBReconcilementTestCase) { opts := hcloud.LoadBalancerChangeTypeOpts{ - LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb21"}, + LoadBalancerType: &hcloud.LoadBalancerType{ID: 2, Name: "lb21"}, } action := &hcloud.Action{ID: 5811} diff --git a/internal/hcops/testing.go b/internal/hcops/testing.go index 952fcb997..b95d9cefb 100644 --- a/internal/hcops/testing.go +++ b/internal/hcops/testing.go @@ -4,15 +4,30 @@ import ( "context" "math/rand" "net" + "net/http" + "net/http/httptest" "testing" + "time" hrobotmodels "github.com/syself/hrobot-go/models" "k8s.io/client-go/tools/record" + "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/cache" "github.com/hetznercloud/hcloud-cloud-controller-manager/internal/mocks" "github.com/hetznercloud/hcloud-go/v2/hcloud" ) +// LBTypesResponse is served by the fixtures Load Balancer type API. Tests that expect a +// Load Balancer type to be looked up have to use one of these types. +const LBTypesResponse = `{ + "load_balancer_types": [ + {"id": 1, "name": "lb11"}, + {"id": 2, "name": "lb21"}, + {"id": 3, "name": "lb31"} + ], + "meta": {"pagination": {"page": 1, "per_page": 50, "previous_page": null, "next_page": null, "last_page": 1, "total_entries": 3}} +}` + type LoadBalancerOpsFixture struct { Name string Ctx context.Context @@ -50,12 +65,28 @@ func NewLoadBalancerOpsFixture(t *testing.T) *LoadBalancerOpsFixture { ActionClient: fx.ActionClient, NetworkClient: fx.NetworkClient, RobotClient: fx.RobotClient, + LBTypeCache: newLBTypeCacheFixture(t), Recorder: &record.FakeRecorder{}, } return fx } +// newLBTypeCacheFixture returns a Load Balancer type cache backed by a test server that +// always serves [LBTypesResponse]. +func newLBTypeCacheFixture(t *testing.T) *cache.Cache[hcloud.LoadBalancerType] { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + if _, err := w.Write([]byte(LBTypesResponse)); err != nil { + t.Error(err) + } + })) + t.Cleanup(server.Close) + + client := hcloud.NewClient(hcloud.WithEndpoint(server.URL)) + return cache.NewLoadBalancerTypeCache(client, cache.ModeAll, time.Minute) +} + func (fx *LoadBalancerOpsFixture) MockGetByID(lb *hcloud.LoadBalancer, err error) { fx.LBClient.On("GetByID", fx.Ctx, lb.ID).Return(lb, nil, err) } diff --git a/internal/utils/eventlog.go b/internal/utils/eventlog.go new file mode 100644 index 000000000..088ef3558 --- /dev/null +++ b/internal/utils/eventlog.go @@ -0,0 +1,16 @@ +package utils + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" +) + +func WarnEventLogf(recorder record.EventRecorder, obj runtime.Object, reason string, msg string, args ...any) { + msgf := fmt.Sprintf(msg, args...) + recorder.Event(obj, corev1.EventTypeWarning, reason, msgf) + klog.Warning(msgf) +}