Handle too many requests error from service-manager (#378)

---------

Co-authored-by: Evyatar Yaffe <[email protected]>

Author: kerenlahav

controllers/base_controller.go

@@ -349,7 +349,7 @@ func (r *BaseReconciler) handleError(ctx context.Context, operationType smClient
 		return r.markAsNonTransientError(ctx, operationType, err.Error(), resource)
 	}
 	if r.isTransientError(smError, log) || shouldIgnoreNonTransient(log, resource, r.Config.IgnoreNonTransientTimeout) {
-		return r.markAsTransientError(ctx, operationType, smError.Error(), resource)
+		return r.markAsTransientError(ctx, operationType, smError, resource)
 	}
 
 	return r.markAsNonTransientError(ctx, operationType, smError.Error(), resource)
@@ -372,15 +372,18 @@ func (r *BaseReconciler) markAsNonTransientError(ctx context.Context, operationT
 	return ctrl.Result{}, nil
 }
 
-func (r *BaseReconciler) markAsTransientError(ctx context.Context, operationType smClientTypes.OperationCategory, errMsg string, object api.SAPBTPResource) (ctrl.Result, error) {
+func (r *BaseReconciler) markAsTransientError(ctx context.Context, operationType smClientTypes.OperationCategory, err error, object api.SAPBTPResource) (ctrl.Result, error) {
 	log := GetLogger(ctx)
-	setInProgressConditions(ctx, operationType, errMsg, object)
-	log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), errMsg))
-	if err := r.updateStatus(ctx, object); err != nil {
-		return ctrl.Result{}, err
+	//DO NOT REMOVE - 429 error is not reflected to the status
+	if smError, ok := err.(*sm.ServiceManagerError); !ok || smError.StatusCode != http.StatusTooManyRequests {
+		setInProgressConditions(ctx, operationType, err.Error(), object)
+		log.Info(fmt.Sprintf("operation %s of %s encountered a transient error %s, retrying operation :)", operationType, object.GetControllerName(), err.Error()))
+		if updateErr := r.updateStatus(ctx, object); updateErr != nil {
+			return ctrl.Result{}, updateErr
+		}
 	}
 
-	return ctrl.Result{}, fmt.Errorf(errMsg)
+	return ctrl.Result{}, err
 }
 
 func (r *BaseReconciler) removeAnnotation(ctx context.Context, object api.SAPBTPResource, keys ...string) error {

controllers/servicebinding_controller.go

@@ -180,13 +180,13 @@ func (r *ServiceBindingReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 		smClient, err := r.getSMClient(ctx, serviceBinding, serviceInstance.Spec.BTPAccessCredentialsSecret)
 		if err != nil {
-			return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding)
+			return r.markAsTransientError(ctx, Unknown, err, serviceBinding)
 		}
 
 		smBinding, err := r.getBindingForRecovery(ctx, smClient, serviceBinding)
 		if err != nil {
 			log.Error(err, "failed to check binding recovery")
-			return r.markAsTransientError(ctx, smClientTypes.CREATE, err.Error(), serviceBinding)
+			return r.markAsTransientError(ctx, smClientTypes.CREATE, err, serviceBinding)
 		}
 		if smBinding != nil {
 			return r.recover(ctx, serviceBinding, smBinding)
@@ -275,7 +275,7 @@ func (r *ServiceBindingReconciler) delete(ctx context.Context, serviceBinding *s
 	if controllerutil.ContainsFinalizer(serviceBinding, api.FinalizerName) {
 		smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret)
 		if err != nil {
-			return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding)
+			return r.markAsTransientError(ctx, Unknown, err, serviceBinding)
 		}
 
 		if len(serviceBinding.Status.BindingID) == 0 {
@@ -338,7 +338,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser
 
 	smClient, err := r.getSMClient(ctx, serviceBinding, btpAccessCredentialsSecret)
 	if err != nil {
-		return r.markAsTransientError(ctx, Unknown, err.Error(), serviceBinding)
+		return r.markAsTransientError(ctx, Unknown, err, serviceBinding)
 	}
 
 	status, statusErr := smClient.Status(serviceBinding.Status.OperationURL, nil)
@@ -359,7 +359,7 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, serviceBinding *ser
 	}
 
 	if status == nil {
-		return r.markAsTransientError(ctx, serviceBinding.Status.OperationType, fmt.Sprintf("failed to get last operation status of %s", serviceBinding.Name), serviceBinding)
+		return r.markAsTransientError(ctx, serviceBinding.Status.OperationType, fmt.Errorf("failed to get last operation status of %s", serviceBinding.Name), serviceBinding)
 	}
 	switch status.State {
 	case smClientTypes.INPROGRESS:
@@ -708,7 +708,7 @@ func (r *ServiceBindingReconciler) handleSecretError(ctx context.Context, op smC
 	if apierrors.ReasonForError(err) == metav1.StatusReasonUnknown {
 		return r.markAsNonTransientError(ctx, op, err.Error(), binding)
 	}
-	return r.markAsTransientError(ctx, op, err.Error(), binding)
+	return r.markAsTransientError(ctx, op, err, binding)
 }
 
 func (r *ServiceBindingReconciler) addInstanceInfo(ctx context.Context, binding *servicesv1.ServiceBinding, credentialsMap map[string][]byte) ([]SecretMetadataProperty, error) {

controllers/serviceinstance_controller.go

@@ -122,15 +122,15 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret)
 	if err != nil {
 		log.Error(err, "failed to get sm client")
-		return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance)
+		return r.markAsTransientError(ctx, Unknown, err, serviceInstance)
 	}
 
 	if serviceInstance.Status.InstanceID == "" {
 		log.Info("Instance ID is empty, checking if instance exist in SM")
 		smInstance, err := r.getInstanceForRecovery(ctx, smClient, serviceInstance)
 		if err != nil {
 			log.Error(err, "failed to check instance recovery")
-			return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance)
+			return r.markAsTransientError(ctx, Unknown, err, serviceInstance)
 		}
 		if smInstance != nil {
 			return r.recover(ctx, smClient, serviceInstance, smInstance)
@@ -266,7 +266,7 @@ func (r *ServiceInstanceReconciler) deleteInstance(ctx context.Context, serviceI
 		smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret)
 		if err != nil {
 			log.Error(err, "failed to get sm client")
-			return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance)
+			return r.markAsTransientError(ctx, Unknown, err, serviceInstance)
 		}
 		if len(serviceInstance.Status.InstanceID) == 0 {
 			log.Info("No instance id found validating instance does not exists in SM before removing finalizer")
@@ -348,7 +348,7 @@ func (r *ServiceInstanceReconciler) poll(ctx context.Context, serviceInstance *s
 	smClient, err := r.getSMClient(ctx, serviceInstance, serviceInstance.Spec.BTPAccessCredentialsSecret)
 	if err != nil {
 		log.Error(err, "failed to get sm client")
-		return r.markAsTransientError(ctx, Unknown, err.Error(), serviceInstance)
+		return r.markAsTransientError(ctx, Unknown, err, serviceInstance)
 	}
 
 	status, statusErr := smClient.Status(serviceInstance.Status.OperationURL, nil)