Fix removing finalizer after switching to CRD /status sub-resource

pkg/controller/controller_binding.go

@@ -1516,15 +1516,23 @@ func (c *controller) handleServiceBindingReconciliationError(binding *v1beta1.Se
 // updating of a ServiceBinding that has successfully finished graceful
 // deletion.
 func (c *controller) processServiceBindingGracefulDeletionSuccess(binding *v1beta1.ServiceBinding) error {
-	finalizers := sets.NewString(binding.Finalizers...)
-	finalizers.Delete(v1beta1.FinalizerServiceCatalog)
-	binding.Finalizers = finalizers.List()
+	pcb := pretty.NewBindingContextBuilder(binding)
 
-	if _, err := c.updateServiceBindingStatus(binding); err != nil {
-		return err
+	updatedBinding, err := c.updateServiceBindingStatus(binding)
+	if err != nil {
+		return fmt.Errorf("while updating status: %v", err)
 	}
+	klog.Info(pcb.Message("Status updated"))
 
-	pcb := pretty.NewBindingContextBuilder(binding)
+	toUpdate := updatedBinding.DeepCopy()
+	finalizers := sets.NewString(toUpdate.Finalizers...)
+	finalizers.Delete(v1beta1.FinalizerServiceCatalog)
+	toUpdate.Finalizers = finalizers.List()
+
+	_, err = c.serviceCatalogClient.ServiceBindings(toUpdate.Namespace).Update(toUpdate)
+	if err != nil {
+		return fmt.Errorf("while removing finalizer entry: %v", err)
+	}
 	klog.Info(pcb.Message("Cleared finalizer"))
 
 	return nil

pkg/controller/controller_clusterservicebroker.go

@@ -708,7 +708,7 @@ func (c *controller) updateClusterServiceBrokerFinalizers(
 	logContext := fmt.Sprint(pcb.Messagef("Updating finalizers to %v", finalizers))
 
 	klog.V(4).Info(pcb.Messagef("Updating %v", logContext))
-	_, err = c.serviceCatalogClient.ClusterServiceBrokers().UpdateStatus(toUpdate)
+	_, err = c.serviceCatalogClient.ClusterServiceBrokers().Update(toUpdate)
 	if err != nil {
 		klog.Error(pcb.Messagef("Error updating %v: %v", logContext, err))
 	}

pkg/controller/controller_clusterservicebroker_test.go

@@ -674,7 +674,7 @@ func TestReconcileClusterServiceBrokerDelete(t *testing.T) {
 
 			assertGet(t, catalogActions[5], broker)
 
-			updatedClusterServiceBroker = assertUpdateStatus(t, catalogActions[6], broker)
+			updatedClusterServiceBroker = assertUpdate(t, catalogActions[6], broker)
 			assertEmptyFinalizers(t, updatedClusterServiceBroker)
 
 			events := getRecordedEvents(testController)

pkg/controller/controller_instance.go

@@ -1876,8 +1876,7 @@ func (c *controller) updateServiceInstanceStatus(instance *v1beta1.ServiceInstan
 // version's status with the status on the ServiceInstance passed
 // to it; it also runs the provided postConflictUpdateFunc,
 // allowing the caller to make additional changes to the
-// new version of the instance - to other parts of the object
-// (e.g. finalizers).
+// new version of the instance - to other parts of the object.
 func (c *controller) updateServiceInstanceStatusWithRetries(
 	instance *v1beta1.ServiceInstance,
 	postConflictUpdateFunc func(*v1beta1.ServiceInstance)) (*v1beta1.ServiceInstance, error) {
@@ -2535,24 +2534,28 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta
 // updating of a ServiceInstance that has successfully finished graceful
 // deletion.
 func (c *controller) processServiceInstanceGracefulDeletionSuccess(instance *v1beta1.ServiceInstance) error {
-	c.removeFinalizer(instance)
-	if _, err := c.updateServiceInstanceStatusWithRetries(instance, c.removeFinalizer); err != nil {
+	updatedInstance, err := c.updateServiceInstanceStatusWithRetries(instance, nil)
+	if err != nil {
 		return err
 	}
 
+	toUpdate := updatedInstance.DeepCopy()
+	finalizers := sets.NewString(toUpdate.Finalizers...)
+	finalizers.Delete(v1beta1.FinalizerServiceCatalog)
+	toUpdate.Finalizers = finalizers.List()
+
+	_, err = c.serviceCatalogClient.ServiceInstances(toUpdate.Namespace).Update(toUpdate)
+	if err != nil {
+		return fmt.Errorf("while removing finalizer entry: %v", err)
+	}
+
 	pcb := pretty.NewInstanceContextBuilder(instance)
 	klog.Info(pcb.Message("Cleared finalizer"))
 
 	c.removeInstanceFromRetryMap(instance)
 	return nil
 }
 
-func (c *controller) removeFinalizer(instance *v1beta1.ServiceInstance) {
-	finalizers := sets.NewString(instance.Finalizers...)
-	finalizers.Delete(v1beta1.FinalizerServiceCatalog)
-	instance.Finalizers = finalizers.List()
-}
-
 // handleServiceInstanceReconciliationError is a helper function that handles
 // on error whether the error represents an operation error and should update
 // the ServiceInstance resource.

pkg/controller/controller_instance_ns_test.go

@@ -426,6 +426,10 @@ func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) {
 		return true, instance, nil
 	})
 
+	// simulate real update and return updated object,
+	// without that fake client will return empty ServiceInstances struct
+	fakeCatalogClient.AddReactor(updateObjectReactor("serviceinstances"))
+
 	if err := reconcileServiceInstance(t, testController, instance); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -452,9 +456,10 @@ func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) {
 	assertNumberOfActions(t, kubeActions, 0)
 
 	actions := fakeCatalogClient.Actions()
-	assertNumberOfActions(t, actions, 1)
+	assertNumberOfActions(t, actions, 2)
 
-	updatedServiceInstance := assertUpdateStatus(t, actions[0], instance)
+	assertUpdateStatus(t, actions[0], instance)
+	updatedServiceInstance := assertUpdate(t, actions[1], instance)
 	assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testClusterServicePlanName, testClusterServicePlanGUID, instance)
 
 	events := getRecordedEvents(testController)
@@ -665,6 +670,10 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizerNamespa
 	sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass())
 	sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan())
 
+	// simulate real update and return updated object,
+	// without that fake client will return empty ServiceInstances struct
+	fakeCatalogClient.AddReactor(updateObjectReactor("serviceinstances"))
+
 	instance := getTestServiceInstanceAsyncDeprovisioningWithNamespacedRefs(testOperation)
 	instanceKey := testNamespace + "/" + testServiceInstanceName
 
@@ -697,9 +706,10 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizerNamespa
 	assertNumberOfActions(t, kubeActions, 0)
 
 	actions := fakeCatalogClient.Actions()
-	assertNumberOfActions(t, actions, 1)
+	assertNumberOfActions(t, actions, 2)
 
-	updatedServiceInstance := assertUpdateStatus(t, actions[0], instance)
+	assertUpdateStatus(t, actions[0], instance)
+	updatedServiceInstance := assertUpdate(t, actions[1], instance)
 	assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testServicePlanName, testServicePlanGUID, instance)
 
 	events := getRecordedEvents(testController)