Skip to content

Commit

Permalink
Ensure credentials secret is created on async bind
Browse files Browse the repository at this point in the history
* When the bind operation state is `succeeded`, requeue the reconcile
  request. Thus we ensure that the controller would try to `bind` again,
  and as the bind operation has already succeeded, it would get a
  response containing the binding credentials
* When checking the state of the last operation in asynchronous
  provision/bind, instance/binding controllers consider the operation to
  have completed when its state is either `succeeded`, or `failed`.
  Anything else is considered as if the operation is ongoing. Thus we
  protect ourselves from brokers returning nonsense operation state
* The test for the instance controller is refactored so that the
  synchronous provisioning is the "default" case and there is a
  dedicated context for asynchronous provisioning. This aligns test
  style of instance/binding controller tests
  • Loading branch information
danail-branekov committed Jan 9, 2025
1 parent e47d419 commit 6fce46e
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 144 deletions.
17 changes: 15 additions & 2 deletions controllers/controllers/services/bindings/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ var _ = Describe("CFServiceBinding", func() {
}, nil)

brokerClient.GetServiceBindingLastOperationReturns(osbapi.LastOperationResponse{
State: "in progress",
State: "in-progress-or-whatever",
}, nil)
})

Expand All @@ -809,6 +809,7 @@ var _ = Describe("CFServiceBinding", func() {
g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll(
HasType(Equal(korifiv1alpha1.StatusConditionReady)),
HasStatus(Equal(metav1.ConditionFalse)),
HasReason(Equal("BindingInProgress")),
)))
}).Should(Succeed())
})
Expand Down Expand Up @@ -841,6 +842,7 @@ var _ = Describe("CFServiceBinding", func() {
g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll(
HasType(Equal(korifiv1alpha1.StatusConditionReady)),
HasStatus(Equal(metav1.ConditionFalse)),
HasReason(Equal("GetLastOperationFailed")),
HasMessage(ContainSubstring("get-last-op-failed")),
)))
}).Should(Succeed())
Expand Down Expand Up @@ -880,13 +882,24 @@ var _ = Describe("CFServiceBinding", func() {
State: "succeeded",
}, nil)

brokerClient.BindReturns(osbapi.BindResponse{
brokerClient.BindReturnsOnCall(3, osbapi.BindResponse{
Credentials: map[string]any{
"foo": "bar",
},
}, nil)
})

It("sets the ready condition to true", func() {
Eventually(func(g Gomega) {
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())

g.Expect(binding.Status.Conditions).To(ContainElement(SatisfyAll(
HasType(Equal(korifiv1alpha1.StatusConditionReady)),
HasStatus(Equal(metav1.ConditionTrue)),
)))
}).Should(Succeed())
})

It("creates the credentials secret", func() {
Eventually(func(g Gomega) {
g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(binding), binding)).To(Succeed())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func (r *ManagedBindingsReconciler) processBindOperation(
cfServiceBinding *korifiv1alpha1.CFServiceBinding,
lastOperation osbapi.LastOperationResponse,
) (ctrl.Result, error) {
if lastOperation.State == "in progress" {
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingInProgress").WithRequeue()
if lastOperation.State == "succeeded" {
return ctrl.Result{Requeue: true}, nil
}

if lastOperation.State == "failed" {
Expand All @@ -160,10 +160,10 @@ func (r *ManagedBindingsReconciler) processBindOperation(
Reason: "BindingFailed",
Message: lastOperation.Description,
})
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingFailed")
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingFailed").WithMessage(lastOperation.Description)
}

return ctrl.Result{}, nil
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("BindingInProgress").WithRequeue()
}

func (r *ManagedBindingsReconciler) reconcileCredentials(ctx context.Context, cfServiceBinding *korifiv1alpha1.CFServiceBinding, creds map[string]any) error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ func (r *Reconciler) processProvisionOperation(
serviceInstance *korifiv1alpha1.CFServiceInstance,
lastOpResponse osbapi.LastOperationResponse,
) (ctrl.Result, error) {
if lastOpResponse.State == "in progress" {
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionInProgress").WithRequeue()
if lastOpResponse.State == "succeeded" {
return ctrl.Result{}, nil
}

if lastOpResponse.State == "failed" {
Expand All @@ -263,7 +263,7 @@ func (r *Reconciler) processProvisionOperation(
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionFailed")
}

return ctrl.Result{}, nil
return ctrl.Result{}, k8s.NewNotReadyError().WithReason("ProvisionInProgress").WithRequeue()
}

func (r *Reconciler) finalizeCFServiceInstance(
Expand Down Expand Up @@ -381,7 +381,7 @@ func (r *Reconciler) pollLastOperation(
return osbapi.LastOperationResponse{}, k8s.NewNotReadyError().WithCause(err).WithReason("GetLastOperationFailed")
}

serviceInstance.Status.LastOperation.State = lastOpResponse.State
serviceInstance.Status.LastOperation.State = lastOpResponse.State.Value()
serviceInstance.Status.LastOperation.Description = lastOpResponse.Description
return lastOpResponse, nil
}
Expand Down
Loading

0 comments on commit 6fce46e

Please sign in to comment.