diff --git a/controllers/base_controller.go b/controllers/base_controller.go index eed15035..34339bcc 100644 --- a/controllers/base_controller.go +++ b/controllers/base_controller.go @@ -2,6 +2,7 @@ package controllers import ( "context" + "errors" "fmt" "net/http" @@ -344,7 +345,8 @@ func isTransientStatusCode(StatusCode int) bool { func (r *BaseReconciler) handleError(ctx context.Context, operationType smClientTypes.OperationCategory, err error, resource api.SAPBTPResource) (ctrl.Result, error) { log := GetLogger(ctx) - smError, ok := err.(*sm.ServiceManagerError) + var smError *sm.ServiceManagerError + ok := errors.As(err, &smError) if !ok { log.Info("unable to cast error to SM error, will be treated as non transient") return r.markAsNonTransientError(ctx, operationType, err.Error(), resource) diff --git a/controllers/serviceinstance_controller.go b/controllers/serviceinstance_controller.go index 4b551484..fcd3bf28 100644 --- a/controllers/serviceinstance_controller.go +++ b/controllers/serviceinstance_controller.go @@ -80,6 +80,21 @@ func (r *ServiceInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ } } + if meta.IsStatusConditionPresentAndEqual(serviceInstance.GetConditions(), api.ConditionSucceeded, metav1.ConditionFalse) && + serviceInstance.IsIgnoreNonTransientAnnotationExistAndValid(log, r.Config.IgnoreNonTransientTimeout) { + operation := smClientTypes.CREATE + if len(serviceInstance.Status.InstanceID) > 0 { + operation = smClientTypes.UPDATE + } + setInProgressConditions(ctx, operation, "", serviceInstance) + conditions := serviceInstance.GetConditions() + if len(conditions) > 0 { + meta.RemoveStatusCondition(&conditions, api.ConditionFailed) + } + serviceInstance.SetConditions(conditions) + r.Status().Update(ctx, serviceInstance) + } + if isFinalState(ctx, serviceInstance, r.Config.IgnoreNonTransientTimeout) { if len(serviceInstance.Status.HashedSpec) == 0 { updateHashedSpecValue(serviceInstance) @@ -558,10 +573,7 @@ func isFinalState(ctx context.Context, serviceInstance *servicesv1.ServiceInstan log.Info("instance is not in final state, sync operation is in progress") return false } - if isInRetry(serviceInstance, log, nonTransientTimeout) { - log.Info("instance is not in final state, IgnoreNonTransientErrorAnnotation exist") - return false - } + if sharingUpdateRequired(serviceInstance) { log.Info("instance is not in final state, need to sync sharing status") if len(serviceInstance.Status.HashedSpec) == 0 { @@ -593,9 +605,7 @@ func updateRequired(serviceInstance *servicesv1.ServiceInstance, log logr.Logger if cond != nil && cond.Reason == UpdateInProgress { //in case of transient error occurred return true } - if serviceInstance.IsIgnoreNonTransientAnnotationExistAndValid(log, nonTransientTimeout) { - return true - } + return getSpecHash(serviceInstance) != serviceInstance.Status.HashedSpec } diff --git a/controllers/serviceinstance_controller_test.go b/controllers/serviceinstance_controller_test.go index de5d88d1..7c35b169 100644 --- a/controllers/serviceinstance_controller_test.go +++ b/controllers/serviceinstance_controller_test.go @@ -251,18 +251,18 @@ var _ = Describe("ServiceInstance controller", func() { Context("first with 400 status then success", func() { errMessage := "failed to provision instance" BeforeEach(func() { - fakeClient.ProvisionReturns(nil, &sm.ServiceManagerError{ + fakeClient.ProvisionReturnsOnCall(0, nil, &sm.ServiceManagerError{ StatusCode: http.StatusBadRequest, Description: errMessage, }) - fakeClient.ProvisionReturnsOnCall(2, &sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) + fakeClient.ProvisionReturnsOnCall(1, &sm.ProvisionResponse{InstanceID: fakeInstanceID}, nil) }) It("should have failure condition", func() { serviceInstance = createInstance(ctx, instanceSpec, false) waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) }) - It("ignoreNonTransientErrorAnnotation should have failure condition after timeout", func() { + It("ignoreNonTransientErrorAnnotation should remove the annotation once succeeded", func() { serviceInstance = createInstanceWithAnnotation(ctx, instanceSpec, ignoreNonTransientErrorAnnotation, false) waitForResourceCondition(ctx, serviceInstance, api.ConditionSucceeded, metav1.ConditionTrue, Created, "") waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) @@ -280,6 +280,7 @@ var _ = Describe("ServiceInstance controller", func() { serviceInstance = createInstanceWithAnnotation(ctx, instanceSpec, ignoreNonTransientErrorAnnotation, false) waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) waitForResourceAnnotationRemove(ctx, serviceInstance, api.IgnoreNonTransientErrorAnnotation, api.IgnoreNonTransientErrorTimestampAnnotation) + waitForResourceCondition(ctx, serviceInstance, api.ConditionFailed, metav1.ConditionTrue, CreateFailed, errMessage) sinceCreate := time.Since(serviceInstance.GetCreationTimestamp().Time) Expect(sinceCreate > ignoreNonTransientTimeout) }) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 29e48924..3cfdddcc 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -67,7 +67,7 @@ const ( interval = time.Millisecond * 50 syncPeriod = time.Millisecond * 250 pollInterval = time.Millisecond * 250 - ignoreNonTransientTimeout = time.Second * 1 + ignoreNonTransientTimeout = time.Second * 10 fakeBindingID = "fake-binding-id" bindingTestNamespace = "test-namespace"