Repository: syself/cluster-api-provider-hetzner
Pull request #881 (2023-08-31 15:31:25)

:seedling: Refactor rate limit condition
**What this PR does / why we need it**:
Refactoring rate limit condition so that it shows positive result if things are positive and negative results otherwise.

**TODOs**:
- [x] squash commits
- [ ] include documentation
- [ ] add unit tests




Patch:
diff --git a/api/v1beta1/conditions_const.go b/api/v1beta1/conditions_const.go
index dd7793c9..d93cef7c 100644
--- a/api/v1beta1/conditions_const.go
+++ b/api/v1beta1/conditions_const.go
@@ -89,10 +89,10 @@ const (
 )
 
 const (
-	// RateLimitExceeded reports whether the rate limit has been reached.
-	RateLimitExceeded clusterv1.ConditionType = "RateLimitExceeded"
-	// RateLimitNotReachedReason indicates that the rate limit is not reached yet.
-	RateLimitNotReachedReason = "RateLimitNotReached"
+	// HetznerAPIReachableCondition reports whether the Hetzner APIs are reachable.
+	HetznerAPIReachableCondition clusterv1.ConditionType = "HetznerAPIReachable"
+	// RateLimitExceededReason indicates that a rate limit has been exceeded.
+	RateLimitExceededReason = "RateLimitExceeded"
 )
 
 const (
diff --git a/controllers/hetznercluster_controller.go b/controllers/hetznercluster_controller.go
index cbaf6b02..e0329d81 100644
--- a/controllers/hetznercluster_controller.go
+++ b/controllers/hetznercluster_controller.go
@@ -321,8 +321,8 @@ func (r *HetznerClusterReconciler) reconcileDelete(ctx context.Context, clusterS
 // reconcileRateLimit checks whether a rate limit has been reached and returns whether
 // the controller should wait a bit more.
 func reconcileRateLimit(setter conditions.Setter) bool {
-	condition := conditions.Get(setter, infrav1.RateLimitExceeded)
-	if condition != nil && condition.Status == corev1.ConditionTrue {
+	condition := conditions.Get(setter, infrav1.HetznerAPIReachableCondition)
+	if condition != nil && condition.Status == corev1.ConditionFalse {
 		if time.Now().Before(condition.LastTransitionTime.Time.Add(rateLimitWaitTime)) {
 			// Not yet timed out, reconcile again after timeout
 			// Don't give a more precise requeueAfter value to not reconcile too many
@@ -330,13 +330,7 @@ func reconcileRateLimit(setter conditions.Setter) bool {
 			return true
 		}
 		// Wait time is over, we continue
-		conditions.MarkFalse(
-			setter,
-			infrav1.RateLimitExceeded,
-			infrav1.RateLimitNotReachedReason,
-			clusterv1.ConditionSeverityInfo,
-			"wait time is over. Try reconciling again",
-		)
+		conditions.MarkTrue(setter, infrav1.HetznerAPIReachableCondition)
 	}
 	return false
 }
diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go
index c06a75ac..22da9be5 100644
--- a/pkg/services/baremetal/host/host.go
+++ b/pkg/services/baremetal/host/host.go
@@ -1364,7 +1364,14 @@ func (s *Service) actionDeleting() actionResult {
 
 func (s *Service) handleRateLimitExceeded(err error, functionName string) {
 	if models.IsError(err, models.ErrorCodeRateLimitExceeded) {
-		conditions.MarkTrue(s.scope.HetznerBareMetalHost, infrav1.RateLimitExceeded)
-		record.Warnf(s.scope.HetznerBareMetalHost, "RateLimitExceeded", "exceeded rate limit with calling function %q", functionName)
+		msg := fmt.Sprintf("exceeded rate limit with calling function %q", functionName)
+		conditions.MarkFalse(
+			s.scope.HetznerBareMetalHost,
+			infrav1.HetznerAPIReachableCondition,
+			infrav1.RateLimitExceededReason,
+			clusterv1.ConditionSeverityWarning,
+			msg,
+		)
+		record.Warnf(s.scope.HetznerBareMetalHost, "RateLimitExceeded", msg)
 	}
 }
diff --git a/pkg/services/hcloud/util/utils.go b/pkg/services/hcloud/util/utils.go
index e37c14b9..e0c2ff6d 100644
--- a/pkg/services/hcloud/util/utils.go
+++ b/pkg/services/hcloud/util/utils.go
@@ -23,11 +23,11 @@ import (
 	"strings"
 
 	"github.com/hetznercloud/hcloud-go/hcloud"
+	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/record"
-
-	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
 )
 
 const providerIDPrefix = "hcloud://"
@@ -70,7 +70,14 @@ type runtimeObjectWithConditions interface {
 // HandleRateLimitExceeded handles rate limit exceeded errors.
 func HandleRateLimitExceeded(obj runtimeObjectWithConditions, err error, functionName string) {
 	if hcloud.IsError(err, hcloud.ErrorCodeRateLimitExceeded) {
-		conditions.MarkTrue(obj, infrav1.RateLimitExceeded)
-		record.Warnf(obj, "RateLimitExceeded", "exceeded rate limit with calling function %q", functionName)
+		msg := fmt.Sprintf("exceeded rate limit with calling function %q", functionName)
+		conditions.MarkFalse(
+			obj,
+			infrav1.HetznerAPIReachableCondition,
+			infrav1.RateLimitExceededReason,
+			clusterv1.ConditionSeverityWarning,
+			msg,
+		)
+		record.Warnf(obj, "RateLimitExceeded", msg)
 	}
 }


Test patch:
diff --git a/controllers/hetznercluster_controller_test.go b/controllers/hetznercluster_controller_test.go
index 34049d8d..d5947bc1 100644
--- a/controllers/hetznercluster_controller_test.go
+++ b/controllers/hetznercluster_controller_test.go
@@ -985,20 +985,20 @@ var _ = Describe("reconcileRateLimit", func() {
 		}
 	})
 
-	It("returns wait== true if rate limit condition is set and time is not over", func() {
-		conditions.MarkTrue(hetznerCluster, infrav1.RateLimitExceeded)
+	It("returns wait== true if rate limit exceeded is set and time is not over", func() {
+		conditions.MarkFalse(hetznerCluster, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason, clusterv1.ConditionSeverityWarning, "")
 		Expect(reconcileRateLimit(hetznerCluster)).To(BeTrue())
 	})
 
-	It("returns wait== false if rate limit condition is set and time is over", func() {
-		conditions.MarkTrue(hetznerCluster, infrav1.RateLimitExceeded)
+	It("returns wait== false if rate limit exceeded is set and time is over", func() {
+		conditions.MarkFalse(hetznerCluster, infrav1.HetznerAPIReachableCondition, infrav1.RateLimitExceededReason, clusterv1.ConditionSeverityWarning, "")
 		conditionList := hetznerCluster.GetConditions()
 		conditionList[0].LastTransitionTime = metav1.NewTime(time.Now().Add(-time.Hour))
 		Expect(reconcileRateLimit(hetznerCluster)).To(BeFalse())
 	})
 
-	It("returns wait== false if rate limit condition is set to false", func() {
-		conditions.MarkFalse(hetznerCluster, infrav1.RateLimitExceeded, "", clusterv1.ConditionSeverityInfo, "")
+	It("returns wait== false if rate limit condition is set to true", func() {
+		conditions.MarkTrue(hetznerCluster, infrav1.HetznerAPIReachableCondition)
 		Expect(reconcileRateLimit(hetznerCluster)).To(BeFalse())
 	})