[SAPBTPCFS-7876] Optimize handling of non-transient errors

SAP · Dec 11, 2023 · cb31706 · cb31706
1 parent 6adea66
commit cb31706
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 22 deletions.
diff --git a/api/v1/serviceinstance_types.go b/api/v1/serviceinstance_types.go
@@ -217,40 +217,43 @@ func (si *ServiceInstance) ShouldBeShared() bool {
 }
 
 func (si *ServiceInstance) ValidateNonTransientTimestampAnnotation(log logr.Logger) error {
-	if si.Annotations != nil {
-		if _, ok := si.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation]; ok {
-			log.Info("ignoreNonTransientErrorAnnotation annotation exist- checking timeout")
-			annotationTime, err := time.Parse(time.RFC3339, si.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation])
-			if err != nil {
-				return fmt.Errorf("annotation %s is not a valid timestamp", api.IgnoreNonTransientErrorTimestampAnnotation)
-			}
-			sinceAnnotation := time.Since(annotationTime)
-			if sinceAnnotation < 0 {
-				return fmt.Errorf("annotation %s cannot be a future timestamp", api.IgnoreNonTransientErrorTimestampAnnotation)
-			}
-		}
-	}
 
+	sinceAnnotation, exist, err := si.GetTimeSinceIgnoreNonTransientAnnotationTimestamp(log)
+	if err != nil {
+		return err
+	}
+	if exist && sinceAnnotation < 0 {
+		return fmt.Errorf("annotation %s cannot be a future timestamp", api.IgnoreNonTransientErrorTimestampAnnotation)
+	}
 	return nil
 }
 
 func (si *ServiceInstance) IsIgnoreNonTransientAnnotationExistAndValid(log logr.Logger, timeout time.Duration) bool {
+
+	sinceAnnotation, exist, _ := si.GetTimeSinceIgnoreNonTransientAnnotationTimestamp(log)
+	if !exist {
+		return false
+	}
+	if sinceAnnotation > timeout {
+		log.Info(fmt.Sprintf("timeout reached- consider error to be non transient. sinceCreate %s, IgnoreNonTransientTimeout %s", sinceAnnotation, timeout))
+		return false
+	}
+	log.Info("timeout didn't reached- consider error to be transient")
+	return true
+
+}
+
+func (si *ServiceInstance) GetTimeSinceIgnoreNonTransientAnnotationTimestamp(log logr.Logger) (time.Duration, bool, error) {
 	if si.Annotations != nil {
 		if _, ok := si.Annotations[api.IgnoreNonTransientErrorAnnotation]; ok {
 			log.Info("ignoreNonTransientErrorAnnotation annotation exist- checking timeout")
 			annotationTime, err := time.Parse(time.RFC3339, si.Annotations[api.IgnoreNonTransientErrorTimestampAnnotation])
 			if err != nil {
 				log.Error(err, fmt.Sprintf("failed to parse %s", api.IgnoreNonTransientErrorTimestampAnnotation))
-				return false
-			}
-			sinceAnnotation := time.Since(annotationTime)
-			if sinceAnnotation > timeout {
-				log.Info(fmt.Sprintf("timeout reached- consider error to be non transient. sinceCreate %s, IgnoreNonTransientTimeout %s", sinceAnnotation, timeout))
-				return false
+				return time.Since(time.Now()), false, fmt.Errorf("annotation %s is not a valid timestamp", api.IgnoreNonTransientErrorTimestampAnnotation)
 			}
-			log.Info("timeout didn't reached- consider error to be transient")
-			return true
+			return time.Since(annotationTime), true, nil
 		}
 	}
-	return false
+	return time.Since(time.Now()), false, nil
 }
diff --git a/api/v1/serviceinstance_types_test.go b/api/v1/serviceinstance_types_test.go
@@ -134,6 +134,7 @@ var _ = Describe("Service Instance Type Test", func() {
 	It("validate timestamp annotation- not date", func() {
 
 		annotation := map[string]string{
+			api.IgnoreNonTransientErrorAnnotation:          "true",
 			api.IgnoreNonTransientErrorTimestampAnnotation: "true",
 		}
 		instance.SetAnnotations(annotation)
@@ -144,6 +145,7 @@ var _ = Describe("Service Instance Type Test", func() {
 	It("validate timestamp annotation- future date", func() {
 
 		annotation := map[string]string{
+			api.IgnoreNonTransientErrorAnnotation:          "true",
 			api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339),
 		}
 		instance.SetAnnotations(annotation)

diff --git a/api/v1/serviceinstance_validating_webhook_test.go b/api/v1/serviceinstance_validating_webhook_test.go
@@ -28,6 +28,7 @@ var _ = Describe("Service Instance Webhook Test", func() {
 		When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with not a date", func() {
 			It("should not return error from webhook", func() {
 				instance.Annotations = map[string]string{
+					api.IgnoreNonTransientErrorAnnotation:          "true",
 					api.IgnoreNonTransientErrorTimestampAnnotation: "true",
 				}
 				_, err := instance.ValidateUpdate(instance)
@@ -38,6 +39,7 @@ var _ = Describe("Service Instance Webhook Test", func() {
 		When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with future date", func() {
 			It("should not return error from webhook", func() {
 				instance.Annotations = map[string]string{
+					api.IgnoreNonTransientErrorAnnotation:          "true",
 					api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339),
 				}
 				_, err := instance.ValidateUpdate(instance)
@@ -77,6 +79,7 @@ var _ = Describe("Service Instance Webhook Test", func() {
 			When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with not a date", func() {
 				It("should not return error from webhook", func() {
 					instance.Annotations = map[string]string{
+						api.IgnoreNonTransientErrorAnnotation:          "true",
 						api.IgnoreNonTransientErrorTimestampAnnotation: "true",
 					}
 					_, err := instance.ValidateCreate()
@@ -87,6 +90,7 @@ var _ = Describe("Service Instance Webhook Test", func() {
 			When("service instance IgnoreNonTransientErrorTimestampAnnotation annotation is not set with future date", func() {
 				It("should not return error from webhook", func() {
 					instance.Annotations = map[string]string{
+						api.IgnoreNonTransientErrorAnnotation:          "true",
 						api.IgnoreNonTransientErrorTimestampAnnotation: time.Now().Add(48 * time.Hour).Format(time.RFC3339),
 					}
 					_, err := instance.ValidateCreate()