diff --git a/api/v1alpha1/servicebinding_webhook.go b/api/v1alpha1/servicebinding_webhook.go index a95e4ca..7b0f239 100644 --- a/api/v1alpha1/servicebinding_webhook.go +++ b/api/v1alpha1/servicebinding_webhook.go @@ -82,7 +82,8 @@ func (r *ServiceBinding) specChanged(old runtime.Object) bool { r.Spec.ServiceInstanceName != oldBinding.Spec.ServiceInstanceName || // TODO + labels //r.Spec.Labels != oldBinding.Spec.Labels || - r.Spec.Parameters.String() != oldBinding.Spec.Parameters.String() + r.Spec.Parameters.String() != oldBinding.Spec.Parameters.String() || + r.Spec.SecretName != oldBinding.Spec.SecretName } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type diff --git a/controllers/servicebinding_controller.go b/controllers/servicebinding_controller.go index e4a91a0..00a2727 100644 --- a/controllers/servicebinding_controller.go +++ b/controllers/servicebinding_controller.go @@ -323,12 +323,17 @@ func (r *ServiceBindingReconciler) poll(ctx context.Context, smClient smclient.C return ctrl.Result{}, statusErr } + if status == nil { + setFailureConditions(serviceBinding.Status.OperationType, "failed to get last operation status", serviceBinding) + return ctrl.Result{Requeue: true}, r.updateStatusWithRetries(ctx, serviceBinding, log) + } switch status.State { case string(smTypes.IN_PROGRESS): fallthrough case string(smTypes.PENDING): return ctrl.Result{Requeue: true, RequeueAfter: r.Config.PollInterval}, nil case string(smTypes.FAILED): + //non transient error - should not retry setFailureConditions(smTypes.OperationCategory(status.Type), status.Description, serviceBinding) if serviceBinding.Status.OperationType == smTypes.DELETE { serviceBinding.Status.OperationURL = "" diff --git a/controllers/servicebinding_controller_test.go b/controllers/servicebinding_controller_test.go index a419092..ecb387e 100644 --- a/controllers/servicebinding_controller_test.go +++ b/controllers/servicebinding_controller_test.go @@ -167,21 +167,6 @@ var _ = Describe("ServiceBinding controller", func() { fakeClient = &smclientfakes.FakeClient{} fakeClient.ProvisionReturns("12345678", "", nil) fakeClient.BindReturns(&smclientTypes.ServiceBinding{ID: fakeBindingID, Credentials: json.RawMessage("{\"secret_key\": \"secret_value\"}")}, "", nil) - fakeClient.ListBindingsReturnsOnCall(0, nil, nil) - fakeClient.ListBindingsReturnsOnCall(1, &smclientTypes.ServiceBindings{ - ServiceBindings: []smclientTypes.ServiceBinding{ - { - ID: fakeBindingID, - Name: "fake-binding-external-name", - Credentials: json.RawMessage("{\"secret_key\": \"secret_value\"}"), - LastOperation: &smTypes.Operation{ - Type: smTypes.CREATE, - State: smTypes.SUCCEEDED, - Description: "fake-description", - }, - }, - }, - }, nil) smInstance := &smclientTypes.ServiceInstance{ID: fakeInstanceID, Ready: true, LastOperation: &smTypes.Operation{State: smTypes.SUCCEEDED, Type: smTypes.UPDATE}} fakeClient.GetInstanceByIDReturns(smInstance, nil) @@ -412,9 +397,10 @@ var _ = Describe("ServiceBinding controller", func() { When("bind polling returns error", func() { JustBeforeEach(func() { - fakeClient.GetBindingByIDReturns(&smclientTypes.ServiceBinding{ID: fakeBindingID, LastOperation: &smTypes.Operation{State: smTypes.SUCCEEDED, Type: smTypes.CREATE}}, nil) - fakeClient.StatusReturns(nil, fmt.Errorf("no polling for you")) fakeClient.BindReturns(nil, "/v1/service_bindings/id/operations/1234", nil) + fakeClient.StatusReturnsOnCall(0, nil, fmt.Errorf("no polling for you")) + fakeClient.StatusReturnsOnCall(1, &smclientTypes.Operation{ResourceID: fakeBindingID, State: string(smTypes.SUCCEEDED), Type: string(smTypes.CREATE)}, nil) + fakeClient.GetBindingByIDReturns(&smclientTypes.ServiceBinding{ID: fakeBindingID, LastOperation: &smTypes.Operation{State: smTypes.SUCCEEDED, Type: smTypes.CREATE}}, nil) }) It("should eventually succeed", func() { createdBinding, err := createBindingWithoutAssertions(context.Background(), bindingName, bindingTestNamespace, instanceName, "") @@ -423,7 +409,7 @@ var _ = Describe("ServiceBinding controller", func() { err := k8sClient.Get(context.Background(), types.NamespacedName{Name: bindingName, Namespace: bindingTestNamespace}, createdBinding) Expect(err).ToNot(HaveOccurred()) return isReady(createdBinding) - }, timeout, interval).Should(BeTrue()) + }, timeout/2, interval).Should(BeTrue()) }) }) })