Fix service instance purging #3616
Conversation
* Remove the finalizers from all related bindings
| }, | ||
| } | ||
|
|
||
| serviceBinding.Finalizers = append(serviceBinding.Finalizers, korifiv1alpha1.CFServiceBindingFinalizerName) |
There was a problem hiding this comment.
This can be in-lined in the literal
| @@ -519,6 +522,28 @@ func (r *ServiceInstanceRepo) GetDeletedAt(ctx context.Context, authInfo authori | |||
| return serviceInstance.DeletedAt, nil | |||
| } | |||
|
|
|||
| func (r *ServiceInstanceRepo) DeleteServiceBindings(ctx context.Context, userClient client.WithWatch, namespace, instanceGUID string) error { | |||
There was a problem hiding this comment.
This method does not delete service bindings but removes their finalizers, so it would be better to call it removeBindingFinalizer or something in this sense. Also it does not need to be public
| @@ -1052,13 +1073,14 @@ var _ = Describe("ServiceInstanceRepository", func() { | |||
| It("purges the service instance", func() { | |||
There was a problem hiding this comment.
This test should check that both the instance and the binding are not found in an Eventually block. Removing the finalizer is an implementation detail.
There was a problem hiding this comment.
@georgethebeatle The service binding wont be deleted since the with envtest we do not have the garbage collector controller. As per: https://book.kubebuilder.io/reference/envtest#testing-considerations
There was a problem hiding this comment.
@georgethebeatle The service binding wont be deleted since the with envtest we do not have the garbage collector controller. As per: book.kubebuilder.io/reference/envtest#testing-considerations
makes sense, indeed
Is there a related GitHub Issue?
#3615
What is this change about?
Previously the service bindings did not have a finalizer, but now they do. So when purging a service instance, we need to remove them from every related service binding.
Does this PR introduce a breaking change?
No
Acceptance Steps
Tag your pair, your PM, and/or team