Skip to content

Commit ddcac0a

Browse files
whyrasswhyrass
authored
🌱Add handleRateLimit method (#1387)
1 parent bd87b33 commit ddcac0a

File tree

2 files changed

+140
-30
lines changed

2 files changed

+140
-30
lines changed

‎pkg/services/hcloud/server/server.go

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,23 @@ func (s *Service) Reconcile(ctx context.Context) (res reconcile.Result, err erro
189189
return res, nil
190190
}
191191

192+
// implements setting rate limit on hcloudmachine.
193+
func handleRateLimit(hm *infrav1.HCloudMachine, err error, functionName string, errMsg string) error {
194+
// returns error if not a rate limit exceeded error
195+
if !hcloud.IsError(err, hcloud.ErrorCodeRateLimitExceeded) {
196+
return fmt.Errorf("%s: %w", errMsg, err)
197+
}
198+
199+
// does not return error if machine is running and does not have a deletion timestamp
200+
if hm.Status.Ready && hm.DeletionTimestamp.IsZero() {
201+
return nil
202+
}
203+
204+
// check for a rate limit exceeded error if the machine is not running or if machine has a deletion timestamp
205+
hcloudutil.HandleRateLimitExceeded(hm, err, functionName)
206+
return fmt.Errorf("%s: %w", errMsg, err)
207+
}
208+
192209
// Delete implements delete method of server.
193210
func (s *Service) Delete(ctx context.Context) (res reconcile.Result, err error) {
194211
server, err := s.findServer(ctx)
@@ -246,12 +263,11 @@ func (s *Service) reconcileNetworkAttachment(ctx context.Context, server *hcloud
246263
ID: s.scope.HetznerCluster.Status.Network.ID,
247264
},
248265
}); err != nil {
249-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "AttachServerToNetwork")
250266
// check if network status is old and server is in fact already attached
251267
if hcloud.IsError(err, hcloud.ErrorCodeServerAlreadyAttached) {
252268
return nil
253269
}
254-
return fmt.Errorf("failed to attach server to network: %w", err)
270+
return handleRateLimit(s.scope.HCloudMachine, err, "AttachServerToNetwork", "failed to attach server to network")
255271
}
256272

257273
return nil
@@ -308,11 +324,11 @@ func (s *Service) reconcileLoadBalancerAttachment(ctx context.Context, server *h
308324
}
309325

310326
if err := s.scope.HCloudClient.AddTargetServerToLoadBalancer(ctx, opts, loadBalancer); err != nil {
311-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "AddTargetServerToLoadBalancer")
312327
if hcloud.IsError(err, hcloud.ErrorCodeTargetAlreadyDefined) {
313328
return reconcile.Result{}, nil
314329
}
315-
return reconcile.Result{}, fmt.Errorf("failed to add server %s with ID %d as target to load balancer: %w", server.Name, server.ID, err)
330+
errMsg := fmt.Sprintf("failed to add server %s with ID %d as target to load balancer", server.Name, server.ID)
331+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "AddTargetServerToLoadBalancer", errMsg)
316332
}
317333

318334
record.Eventf(
@@ -429,8 +445,7 @@ func (s *Service) createServer(ctx context.Context) (*hcloud.Server, error) {
429445
// get all ssh keys that are stored in HCloud API
430446
sshKeysAPI, err := s.scope.HCloudClient.ListSSHKeys(ctx, hcloud.SSHKeyListOpts{})
431447
if err != nil {
432-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "ListSSHKeys")
433-
return nil, fmt.Errorf("failed listing ssh heys from hcloud: %w", err)
448+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListSSHKeys", "failed listing ssh keys from hcloud")
434449
}
435450

436451
// find matching keys and store them
@@ -479,8 +494,8 @@ func (s *Service) createServer(ctx context.Context) (*hcloud.Server, error) {
479494
s.scope.Name(),
480495
err,
481496
)
482-
err = errors.Join(errServerCreateNotPossible, err)
483-
return nil, fmt.Errorf("failed to create HCloud server %s: %w", s.scope.HCloudMachine.Name, err)
497+
errMsg := fmt.Sprintf("failed to create HCloud server %s", s.scope.HCloudMachine.Name)
498+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "CreateServer", errMsg)
484499
}
485500

486501
// set ssh keys to status
@@ -497,8 +512,7 @@ func (s *Service) getServerImage(ctx context.Context) (*hcloud.Image, error) {
497512
// Get server type so we can filter for images with correct architecture
498513
serverType, err := s.scope.HCloudClient.GetServerType(ctx, string(s.scope.HCloudMachine.Spec.Type))
499514
if err != nil {
500-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "GetServerType")
501-
return nil, fmt.Errorf("failed to get server type in HCloud: %w", err)
515+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "GetServerType", "failed to get server type in HCloud")
502516
}
503517
if serverType == nil {
504518
conditions.MarkFalse(
@@ -522,8 +536,7 @@ func (s *Service) getServerImage(ctx context.Context) (*hcloud.Image, error) {
522536

523537
images, err := s.scope.HCloudClient.ListImages(ctx, listOpts)
524538
if err != nil {
525-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "ListImages")
526-
return nil, fmt.Errorf("failed to list images by label in HCloud: %w", err)
539+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by label in HCloud")
527540
}
528541

529542
// query for an existing image by name.
@@ -533,8 +546,7 @@ func (s *Service) getServerImage(ctx context.Context) (*hcloud.Image, error) {
533546
}
534547
imagesByName, err := s.scope.HCloudClient.ListImages(ctx, listOpts)
535548
if err != nil {
536-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "ListImages")
537-
return nil, fmt.Errorf("failed to list images by name in HCloud: %w", err)
549+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListImages", "failed to list images by name in HCloud")
538550
}
539551

540552
images = append(images, imagesByName...)
@@ -577,12 +589,11 @@ func (s *Service) handleServerStatusOff(ctx context.Context, server *hcloud.Serv
577589
if time.Now().Before(serverAvailableCondition.LastTransitionTime.Time.Add(serverOffTimeout)) {
578590
// Not yet timed out, try again to power on
579591
if err := s.scope.HCloudClient.PowerOnServer(ctx, server); err != nil {
580-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "PowerOnServer")
581592
if hcloud.IsError(err, hcloud.ErrorCodeLocked) {
582593
// if server is locked, we just retry again
583594
return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
584595
}
585-
return reconcile.Result{}, fmt.Errorf("failed to power on server: %w", err)
596+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "PowerOnServer", "failed to power on server")
586597
}
587598
} else {
588599
// Timed out. Set failure reason
@@ -592,12 +603,11 @@ func (s *Service) handleServerStatusOff(ctx context.Context, server *hcloud.Serv
592603
} else {
593604
// No condition set yet. Try to power server on.
594605
if err := s.scope.HCloudClient.PowerOnServer(ctx, server); err != nil {
595-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "PowerOnServer")
596606
if hcloud.IsError(err, hcloud.ErrorCodeLocked) {
597607
// if server is locked, we just retry again
598608
return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
599609
}
600-
return reconcile.Result{}, fmt.Errorf("failed to power on server: %w", err)
610+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "PowerOnServer", "failed to power on server")
601611
}
602612
conditions.MarkFalse(
603613
s.scope.HCloudMachine,
@@ -619,8 +629,7 @@ func (s *Service) handleDeleteServerStatusRunning(ctx context.Context, server *h
619629

620630
if s.scope.HasServerAvailableCondition() {
621631
if err := s.scope.HCloudClient.ShutdownServer(ctx, server); err != nil {
622-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "ShutdownServer")
623-
return reconcile.Result{}, fmt.Errorf("failed to shutdown server: %w", err)
632+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "ShutdownServer", "failed to shutdown server")
624633
}
625634

626635
conditions.MarkFalse(s.scope.HCloudMachine,
@@ -635,9 +644,8 @@ func (s *Service) handleDeleteServerStatusRunning(ctx context.Context, server *h
635644

636645
// timeout for shutdown has been reached - delete server
637646
if err := s.scope.HCloudClient.DeleteServer(ctx, server); err != nil {
638-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "DeleteServer")
639647
record.Warnf(s.scope.HCloudMachine, "FailedDeleteHCloudServer", "Failed to delete HCloud server %s", s.scope.Name())
640-
return reconcile.Result{}, fmt.Errorf("failed to delete server: %w", err)
648+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "DeleteServer", "failed to delete server")
641649
}
642650

643651
record.Eventf(s.scope.HCloudMachine, "HCloudServerDeleted", "HCloud server %s deleted", s.scope.Name())
@@ -647,9 +655,8 @@ func (s *Service) handleDeleteServerStatusRunning(ctx context.Context, server *h
647655
func (s *Service) handleDeleteServerStatusOff(ctx context.Context, server *hcloud.Server) (res reconcile.Result, err error) {
648656
// server is off and can be deleted
649657
if err := s.scope.HCloudClient.DeleteServer(ctx, server); err != nil {
650-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "DeleteServer")
651658
record.Warnf(s.scope.HCloudMachine, "FailedDeleteHCloudServer", "Failed to delete HCloud server %s", s.scope.Name())
652-
return reconcile.Result{}, fmt.Errorf("failed to delete server: %w", err)
659+
return reconcile.Result{}, handleRateLimit(s.scope.HCloudMachine, err, "DeleteServer", "failed to delete server")
653660
}
654661

655662
record.Eventf(s.scope.HCloudMachine, "HCloudServerDeleted", "HCloud server %s deleted", s.scope.Name())
@@ -660,12 +667,12 @@ func (s *Service) deleteServerOfLoadBalancer(ctx context.Context, server *hcloud
660667
lb := &hcloud.LoadBalancer{ID: s.scope.HetznerCluster.Status.ControlPlaneLoadBalancer.ID}
661668

662669
if err := s.scope.HCloudClient.DeleteTargetServerOfLoadBalancer(ctx, lb, server); err != nil {
663-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "DeleteTargetServerOfLoadBalancer")
664670
// do not return an error in case the target was not found
665671
if strings.Contains(err.Error(), "load_balancer_target_not_found") {
666672
return nil
667673
}
668-
return fmt.Errorf("failed to delete server %s with ID %d as target of load balancer %s with ID %d: %w", server.Name, server.ID, lb.Name, lb.ID, err)
674+
errMsg := fmt.Sprintf("failed to delete server %s with ID %d as target of load balancer %s with ID %d", server.Name, server.ID, lb.Name, lb.ID)
675+
return handleRateLimit(s.scope.HCloudMachine, err, "DeleteTargetServerOfLoadBalancer", errMsg)
669676
}
670677
record.Eventf(
671678
s.scope.HetznerCluster,
@@ -685,8 +692,8 @@ func (s *Service) findServer(ctx context.Context) (*hcloud.Server, error) {
685692
if err == nil {
686693
server, err = s.scope.HCloudClient.GetServer(ctx, serverID)
687694
if err != nil {
688-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "GetServer")
689-
return nil, fmt.Errorf("failed to get server %d: %w", serverID, err)
695+
errMsg := fmt.Sprintf("failed to get server %d", serverID)
696+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "GetServer", errMsg)
690697
}
691698

692699
// if server has been found, return it
@@ -702,8 +709,7 @@ func (s *Service) findServer(ctx context.Context) (*hcloud.Server, error) {
702709

703710
servers, err := s.scope.HCloudClient.ListServers(ctx, opts)
704711
if err != nil {
705-
hcloudutil.HandleRateLimitExceeded(s.scope.HCloudMachine, err, "ListServers")
706-
return nil, fmt.Errorf("failed to list servers: %w", err)
712+
return nil, handleRateLimit(s.scope.HCloudMachine, err, "ListServers", "failed to list servers")
707713
}
708714

709715
if len(servers) > 1 {

‎pkg/services/hcloud/server/server_test.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ package server
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"time"
2223

2324
"github.com/hetznercloud/hcloud-go/v2/hcloud"
2425
. "github.com/onsi/ginkgo/v2"
2526
. "github.com/onsi/gomega"
27+
corev1 "k8s.io/api/core/v1"
2628
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2729
"k8s.io/utils/ptr"
2830
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -339,3 +341,105 @@ var _ = Describe("Test ValidateLabels", func() {
339341
}),
340342
)
341343
})
344+
345+
var _ = Describe("Test handleRateLimit", func() {
346+
type testCaseHandleRateLimit struct {
347+
hm *infrav1.HCloudMachine
348+
err error
349+
functionName string
350+
errMsg string
351+
expectError error
352+
expectCondition bool
353+
}
354+
355+
DescribeTable("Test handleRateLimit",
356+
func(tc testCaseHandleRateLimit) {
357+
err := handleRateLimit(tc.hm, tc.err, tc.functionName, tc.errMsg)
358+
if tc.expectError != nil {
359+
Expect(err).To(MatchError(tc.expectError))
360+
} else {
361+
Expect(err).To(BeNil())
362+
}
363+
if tc.expectCondition {
364+
Expect(isPresentAndFalseWithReason(tc.hm, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason)).To(BeTrue())
365+
} else {
366+
Expect(conditions.Get(tc.hm, infrav1.HetznerAPIReachableCondition)).To(BeNil())
367+
}
368+
},
369+
Entry("machine not ready, rate limit exceeded error", testCaseHandleRateLimit{
370+
hm: &infrav1.HCloudMachine{
371+
Status: infrav1.HCloudMachineStatus{Ready: false},
372+
},
373+
err: hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded},
374+
functionName: "TestFunction",
375+
errMsg: "Test error message",
376+
expectError: fmt.Errorf("Test error message: %w", hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded}),
377+
expectCondition: true,
378+
}),
379+
Entry("machine has deletion timestamp, rate limit exceeded error", testCaseHandleRateLimit{
380+
hm: &infrav1.HCloudMachine{
381+
ObjectMeta: metav1.ObjectMeta{
382+
DeletionTimestamp: &metav1.Time{Time: time.Now()},
383+
},
384+
Status: infrav1.HCloudMachineStatus{Ready: true},
385+
},
386+
err: hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded},
387+
functionName: "TestFunction",
388+
errMsg: "Test error message",
389+
expectError: fmt.Errorf("Test error message: %w", hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded}),
390+
expectCondition: true,
391+
}),
392+
Entry("machine not ready, has deletion timestamp, rate limit exceeded error", testCaseHandleRateLimit{
393+
hm: &infrav1.HCloudMachine{
394+
ObjectMeta: metav1.ObjectMeta{
395+
DeletionTimestamp: &metav1.Time{Time: time.Now()},
396+
},
397+
Status: infrav1.HCloudMachineStatus{Ready: false},
398+
},
399+
err: hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded},
400+
functionName: "TestFunction",
401+
errMsg: "Test error message",
402+
expectError: fmt.Errorf("Test error message: %w", hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded}),
403+
expectCondition: true,
404+
}),
405+
Entry("machine ready, rate limit exceeded error", testCaseHandleRateLimit{
406+
hm: &infrav1.HCloudMachine{
407+
Status: infrav1.HCloudMachineStatus{Ready: true},
408+
},
409+
err: hcloud.Error{Code: hcloud.ErrorCodeRateLimitExceeded},
410+
functionName: "TestFunction",
411+
errMsg: "Test error message",
412+
expectError: nil,
413+
expectCondition: false,
414+
}),
415+
Entry("machine ready, other error", testCaseHandleRateLimit{
416+
hm: &infrav1.HCloudMachine{
417+
Status: infrav1.HCloudMachineStatus{Ready: true},
418+
},
419+
err: hcloud.Error{Code: hcloud.ErrorCodeResourceUnavailable},
420+
functionName: "TestFunction",
421+
errMsg: "Test error message",
422+
expectError: fmt.Errorf("Test error message: %w", hcloud.Error{Code: hcloud.ErrorCodeResourceUnavailable}),
423+
expectCondition: false,
424+
}),
425+
Entry("machine not ready, other error", testCaseHandleRateLimit{
426+
hm: &infrav1.HCloudMachine{
427+
Status: infrav1.HCloudMachineStatus{Ready: false},
428+
},
429+
err: hcloud.Error{Code: hcloud.ErrorCodeConflict},
430+
functionName: "TestFunction",
431+
errMsg: "Test conflict error message",
432+
expectError: fmt.Errorf("Test conflict error message: %w", hcloud.Error{Code: hcloud.ErrorCodeConflict}),
433+
expectCondition: false,
434+
}),
435+
)
436+
})
437+
438+
func isPresentAndFalseWithReason(getter conditions.Getter, condition clusterv1.ConditionType, reason string) bool {
439+
if !conditions.Has(getter, condition) {
440+
return false
441+
}
442+
objectCondition := conditions.Get(getter, condition)
443+
return objectCondition.Status == corev1.ConditionFalse &&
444+
objectCondition.Reason == reason
445+
}

0 commit comments

Comments
 (0)