Skip to content

Commit

Permalink
Remove watching of the CFApp resource
Browse files Browse the repository at this point in the history
The binding controller does not get any data from the CFApp, therefore
watching CFApps does not make sense. The only reason we had the watch is
to make the binding not ready when the app is gone. In an ideal world
however this is not a valid state, as app finalization [deletes all
bindings for the app](https://github.com/cloudfoundry/korifi/blob/df9925de6b12eeb51cc05d3dd70019250b8e0175/controllers/controllers/workloads/apps/controller.go#L364-L386)

fixes #3527

Co-authored-by: Danail Branekov <[email protected]>
  • Loading branch information
2 people authored and georgethebeatle committed Nov 5, 2024
1 parent 2c85f78 commit 88295c3
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 74 deletions.
36 changes: 0 additions & 36 deletions controllers/controllers/services/bindings/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder {
Watches(
&korifiv1alpha1.CFServiceInstance{},
handler.EnqueueRequestsFromMapFunc(r.serviceInstanceToServiceBindings),
).
Watches(
&korifiv1alpha1.CFApp{},
handler.EnqueueRequestsFromMapFunc(r.appToServiceBindings),
)
}

Expand Down Expand Up @@ -112,31 +108,6 @@ func (r *Reconciler) serviceInstanceToServiceBindings(ctx context.Context, o cli
return requests
}

func (r *Reconciler) appToServiceBindings(ctx context.Context, o client.Object) []reconcile.Request {
cfApp := o.(*korifiv1alpha1.CFApp)

serviceBindings := &korifiv1alpha1.CFServiceBindingList{}

if err := r.k8sClient.List(ctx, serviceBindings,
client.InNamespace(cfApp.Namespace),
client.MatchingFields{shared.IndexServiceBindingAppGUID: cfApp.Name},
); err != nil {
return []reconcile.Request{}
}

requests := []reconcile.Request{}
for _, sb := range serviceBindings.Items {
requests = append(requests, reconcile.Request{
NamespacedName: types.NamespacedName{
Name: sb.Name,
Namespace: sb.Namespace,
},
})
}

return requests
}

//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebindings,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebindings/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebindings/finalizers,verbs=update
Expand Down Expand Up @@ -174,13 +145,6 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBinding *ko
return res, err
}

cfApp := new(korifiv1alpha1.CFApp)
err = r.k8sClient.Get(ctx, types.NamespacedName{Name: cfServiceBinding.Spec.AppRef.Name, Namespace: cfServiceBinding.Namespace}, cfApp)
if err != nil {
log.Info("error when fetching CFApp", "reason", err)
return ctrl.Result{}, err
}

sbServiceBinding, err := r.reconcileSBServiceBinding(ctx, cfServiceBinding)
if err != nil {
log.Info("error creating/updating servicebinding.io servicebinding", "reason", err)
Expand Down
45 changes: 7 additions & 38 deletions controllers/controllers/services/bindings/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
var _ = Describe("CFServiceBinding", func() {
var (
testNamespace string
cfApp *korifiv1alpha1.CFApp
cfAppGUID string
instanceGUID string
binding *korifiv1alpha1.CFServiceBinding
)
Expand All @@ -42,22 +42,7 @@ var _ = Describe("CFServiceBinding", func() {
},
})).To(Succeed())

cfApp = &korifiv1alpha1.CFApp{
ObjectMeta: metav1.ObjectMeta{
Name: uuid.NewString(),
Namespace: testNamespace,
},
Spec: korifiv1alpha1.CFAppSpec{
DisplayName: uuid.NewString(),
DesiredState: korifiv1alpha1.StartedState,
Lifecycle: korifiv1alpha1.Lifecycle{
Type: "docker",
},
},
}
Expect(
adminClient.Create(ctx, cfApp),
).To(Succeed())
cfAppGUID = uuid.NewString()

instanceGUID = uuid.NewString()

Expand All @@ -76,7 +61,7 @@ var _ = Describe("CFServiceBinding", func() {
APIVersion: "korifi.cloudfoundry.org/v1alpha1",
},
AppRef: corev1.LocalObjectReference{
Name: cfApp.Name,
Name: cfAppGUID,
},
},
}
Expand Down Expand Up @@ -214,7 +199,7 @@ var _ = Describe("CFServiceBinding", func() {

g.Expect(sbServiceBinding.Labels).To(SatisfyAll(
HaveKeyWithValue(bindings.ServiceBindingGUIDLabel, binding.Name),
HaveKeyWithValue(korifiv1alpha1.CFAppGUIDLabelKey, cfApp.Name),
HaveKeyWithValue(korifiv1alpha1.CFAppGUIDLabelKey, cfAppGUID),
HaveKeyWithValue(bindings.ServiceCredentialBindingTypeLabel, "app"),
))

Expand All @@ -228,7 +213,7 @@ var _ = Describe("CFServiceBinding", func() {
"Kind": Equal("StatefulSet"),
"Selector": PointTo(Equal(metav1.LabelSelector{
MatchLabels: map[string]string{
korifiv1alpha1.CFAppGUIDLabelKey: cfApp.Name,
korifiv1alpha1.CFAppGUIDLabelKey: cfAppGUID,
},
})),
}))
Expand Down Expand Up @@ -477,22 +462,6 @@ var _ = Describe("CFServiceBinding", func() {
})
})

When("the CFApp is not available", func() {
BeforeEach(func() {
Expect(adminClient.Delete(ctx, cfApp)).To(Succeed())
})

It("sets the Ready condition to false", 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.ConditionFalse)),
)))
}).Should(Succeed())
})
})

When("the binding references a 'legacy' instance credentials secret", func() {
JustBeforeEach(func() {
Expect(k8s.Patch(ctx, adminClient, instance, func() {
Expand Down Expand Up @@ -691,9 +660,9 @@ var _ = Describe("CFServiceBinding", func() {
BindRequest: osbapi.BindRequest{
ServiceId: "service-offering-id",
PlanID: "service-plan-id",
AppGUID: cfApp.Name,
AppGUID: cfAppGUID,
BindResource: osbapi.BindResource{
AppGUID: cfApp.Name,
AppGUID: cfAppGUID,
},
},
}))
Expand Down

0 comments on commit 88295c3

Please sign in to comment.