Cascading binding deletion

charts/catalog/templates/controller-manager-deployment.yaml

@@ -95,6 +95,10 @@ spec:
         - --feature-gates
         - NamespacedServiceBroker=false
         {{- end }}
+        {{- if .Values.cascadingDeletionEnabled }}
+        - --feature-gates
+        - CascadingDeletion=true
+        {{- end }}
         ports:
         - containerPort: 8444
         {{- if .Values.controllerManager.healthcheck.enabled }}

charts/catalog/templates/rbac.yaml

@@ -37,8 +37,11 @@ rules:
       resources: ["clusterservicebrokers"]
       verbs:     ["get","list","watch", "update"]
     - apiGroups: ["servicecatalog.k8s.io"]
-      resources: ["serviceinstances","servicebindings"]
+      resources: ["serviceinstances"]
       verbs:     ["get","list","watch", "update"]
+    - apiGroups: ["servicecatalog.k8s.io"]
+      resources: ["servicebindings"]
+      verbs:     ["get","list","watch", "update", "delete"]
     - apiGroups: ["servicecatalog.k8s.io"]
       resources: ["clusterservicebrokers/status","clusterserviceclasses/status","clusterserviceplans/status","serviceinstances/status","servicebindings/status"]
       verbs:     ["update"]

charts/catalog/values.yaml

@@ -110,6 +110,8 @@ asyncBindingOperationsEnabled: false
 namespacedServiceBrokerDisabled: false
 # Whether the ServicePlanDefaults alpha feature should be enabled
 servicePlanDefaultsEnabled: false
+# Whether the CascadingDeletion alpha feature should be enabled
+cascadingDeletionEnabled: false
 ## Security context give the opportunity to run container as nonroot by setting a securityContext
 ## by example :
 ## securityContext: { runAsUser: 1001 }

docs/feature-gates.md

@@ -32,6 +32,7 @@ different Service Catalog.
 | `ResponseSchema` | `false` | Alpha | v0.1.12 | |
 | `ServicePlanDefaults` | `false` | Alpha | v0.1.32 | |
 | `UpdateDashboardURL` | `false` | Alpha | v0.1.13 | |
+| `CascadingDeletion` | ` false` | Alpha | v0.3.0 | |
 
 
 ## Using a Feature
@@ -100,3 +101,5 @@ and bindings
 - `UpdateDashboardURL`:  Enables the update of DashboardURL in response to
 update service instance requests to brokers.
 
+- `CascadingDeletion`: Enables deletion of the existing ServiceBindings when deleting a ServiceInstance.
+

pkg/controller/case_test.go

@@ -680,6 +680,18 @@ func (ct *controllerTest) SetCatalogReactionError() {
 	}
 }
 
+// WaitForServiceBindingToNotExists waits until the ServiceBinding will be removed
+func (ct *controllerTest) WaitForServiceBindingToNotExists() error {
+	return wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) {
+		_, err := ct.scInterface.ServiceBindings(testNamespace).Get(testBindingName, metav1.GetOptions{})
+		if err != nil && apierrors.IsNotFound(err) {
+			return true, nil
+		}
+
+		return false, err
+	})
+}
+
 // WaitForReadyBinding waits until the ServiceBinding is in Ready state
 func (ct *controllerTest) WaitForReadyBinding() error {
 	return ct.waitForBindingStatusCondition(v1beta1.ServiceBindingCondition{

pkg/controller/controller_flow_instance_test.go

@@ -26,8 +26,10 @@ import (
 
 	"github.com/kubernetes-sigs/go-open-service-broker-client/v2"
 	"github.com/kubernetes-sigs/service-catalog/pkg/apis/servicecatalog/v1beta1"
+	"github.com/kubernetes-sigs/service-catalog/pkg/features"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 )
 
 // TestProvisionInstanceWithRetries tests creating a ServiceInstance
@@ -131,6 +133,33 @@ func TestServiceInstanceDeleteWithAsyncProvisionInProgress(t *testing.T) {
 	}
 }
 
+// TestServiceInstanceDeleteWithExistingServiceBindings tests the cascading service binding deletion.
+// When the instance is deleted all existing bindings for that instance must be deleted.
+func TestServiceInstanceDeleteWithExistingServiceBindings(t *testing.T) {
+	// GIVEN
+	utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=true", features.CascadingDeletion))
+	defer utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=false", features.CascadingDeletion))
+	ct := newControllerTest(t)
+	defer ct.TearDown()
+
+	require.NoError(t, ct.CreateSimpleClusterServiceBroker())
+	require.NoError(t, ct.WaitForReadyBroker())
+	ct.AssertClusterServiceClassAndPlan(t)
+	assert.NoError(t, ct.CreateServiceInstance())
+	assert.NoError(t, ct.WaitForReadyInstance())
+	assert.NoError(t, ct.CreateBinding())
+	assert.NoError(t, ct.WaitForReadyBinding())
+
+	// WHEN
+	require.NoError(t, ct.Deprovision())
+
+	//THEN
+	assert.NoError(t, ct.WaitForServiceBindingToNotExists())
+	ct.WaitForClusterServiceClassToNotExists()
+	assert.NoError(t, ct.WaitForDeprovisionStatus(v1beta1.ServiceInstanceDeprovisionStatusSucceeded))
+	assert.NotZero(t, ct.NumberOfOSBDeprovisionCalls())
+}
+
 // TestServiceInstanceDeleteWithAsyncUpdateInProgress tests that you can delete
 // an instance during an async update.  That is, if you request a delete during
 // an instance update, the instance will be deleted when the update completes

pkg/controller/controller_instance.go

@@ -30,8 +30,9 @@ import (
 	"github.com/kubernetes-sigs/service-catalog/pkg/pretty"
 	"github.com/kubernetes-sigs/service-catalog/pkg/util"
 
+	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -86,6 +87,8 @@ const (
 	asyncUpdatingInstanceMessage            string = "The instance is being updated asynchronously"
 	asyncDeprovisioningReason               string = "Deprovisioning"
 	asyncDeprovisioningMessage              string = "The instance is being deprovisioned asynchronously"
+	serviceBindingsDeletionReason           string = "ServiceBindingsDeletion"
+	serviceBindingsDeletionMessage          string = "The instance's service bindings is beaing deleted"
 	provisioningInFlightReason              string = "ProvisionRequestInFlight"
 	provisioningInFlightMessage             string = "Provision request for ServiceInstance in-flight to Broker"
 	instanceUpdatingInFlightReason          string = "UpdateInstanceRequestInFlight"
@@ -290,7 +293,7 @@ func (c *controller) reconcileServiceInstanceKey(key string) error {
 	}
 	pcb := pretty.NewContextBuilder(pretty.ServiceInstance, namespace, name, "")
 	instance, err := c.instanceLister.ServiceInstances(namespace).Get(name)
-	if errors.IsNotFound(err) {
+	if apierrors.IsNotFound(err) {
 		klog.Info(pcb.Messagef("Not doing work for %v because it has been deleted", key))
 		return nil
 	}
@@ -904,6 +907,15 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
 
 	// We don't want to delete the instance if there are any bindings associated.
 	if err := c.checkServiceInstanceHasExistingBindings(instance); err != nil {
+		// if the CascadingDeletion feature flag is set, delete existing bindings instead of update the status with an error
+		if utilfeature.DefaultFeatureGate.Enabled(scfeatures.CascadingDeletion) {
+			err := c.deleteExistingBindings(instance)
+			if err != nil {
+				klog.V(4).Info(pcb.Messagef("unable to delete existing bindings: %s", err.Error()))
+				return c.processDeprovisionError(instance, fmt.Sprintf("Delete existing ServiceBinding failed: %v", err.Error()))
+			}
+			return c.processServiceBindingsDeletion(instance)
+		}
 		return c.handleServiceInstanceReconciliationError(instance, err)
 	}
 
@@ -977,15 +989,7 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
 			msg = fmt.Sprintf("Deprovision call failed; received error response from broker: %v", httpErr)
 		}
 
-		readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionUnknown, errorDeprovisionCallFailedReason, msg)
-
-		if c.reconciliationRetryDurationExceeded(instance.Status.OperationStartTime) {
-			msg := "Stopping reconciliation retries because too much time has elapsed"
-			failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, errorReconciliationRetryTimeoutReason, msg)
-			return c.processDeprovisionFailure(instance, readyCond, failedCond)
-		}
-
-		return c.processServiceInstanceOperationError(instance, readyCond)
+		return c.processDeprovisionError(instance, msg)
 	}
 
 	if response.Async {
@@ -995,6 +999,18 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns
 	return c.processDeprovisionSuccess(instance)
 }
 
+func (c *controller) processDeprovisionError(instance *v1beta1.ServiceInstance, msg string) error {
+	readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionUnknown, errorDeprovisionCallFailedReason, msg)
+
+	if c.reconciliationRetryDurationExceeded(instance.Status.OperationStartTime) {
+		msg := "Stopping reconciliation retries because too much time has elapsed"
+		failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, errorReconciliationRetryTimeoutReason, msg)
+		return c.processDeprovisionFailure(instance, readyCond, failedCond)
+	}
+
+	return c.processServiceInstanceOperationError(instance, readyCond)
+}
+
 func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) error {
 	pcb := pretty.NewInstanceContextBuilder(instance)
 	klog.V(4).Info(pcb.Message("Processing poll event"))
@@ -1888,7 +1904,7 @@ func (c *controller) updateServiceInstanceWithRetries(
 		klog.V(4).Info(pcb.Message("Updating instance"))
 		upd, err := c.serviceCatalogClient.ServiceInstances(instanceToUpdate.Namespace).Update(instanceToUpdate)
 		if err != nil {
-			if !errors.IsConflict(err) {
+			if !apierrors.IsConflict(err) {
 				return false, err
 			}
 			klog.V(4).Info(pcb.Message("Couldn't update instance because the resource was stale"))
@@ -1944,7 +1960,7 @@ func (c *controller) updateServiceInstanceStatusWithRetries(
 		klog.V(4).Info(pcb.Message("Updating status"))
 		upd, err := c.serviceCatalogClient.ServiceInstances(instanceToUpdate.Namespace).UpdateStatus(instanceToUpdate)
 		if err != nil {
-			if !errors.IsConflict(err) {
+			if !apierrors.IsConflict(err) {
 				return false, err
 			}
 			klog.V(4).Info(pcb.Message("Couldn't update status because the resource was stale"))
@@ -2194,26 +2210,55 @@ func clearServiceInstanceCurrentOperation(toUpdate *v1beta1.ServiceInstance) {
 // checkServiceInstanceHasExistingBindings returns true if there are any existing
 // bindings associated with the given ServiceInstance.
 func (c *controller) checkServiceInstanceHasExistingBindings(instance *v1beta1.ServiceInstance) error {
-	bindingLister := c.bindingLister.ServiceBindings(instance.Namespace)
-
-	selector := labels.NewSelector()
-	bindingList, err := bindingLister.List(selector)
+	existingBindings, err := c.listExistingBindings(instance)
 	if err != nil {
 		return err
 	}
+	if len(existingBindings) > 0 {
+		return &operationError{
+			reason:  errorDeprovisionBlockedByCredentialsReason,
+			message: "All associated ServiceBindings must be removed before this ServiceInstance can be deleted",
+		}
+	}
 
+	return nil
+}
+
+func (c *controller) listExistingBindings(instance *v1beta1.ServiceInstance) ([]*v1beta1.ServiceBinding, error) {
+	bindingLister := c.bindingLister.ServiceBindings(instance.Namespace)
+
+	bindingList, err := bindingLister.List(labels.NewSelector())
+	if err != nil {
+		return []*v1beta1.ServiceBinding{}, err
+	}
+	var found []*v1beta1.ServiceBinding
 	for _, binding := range bindingList {
 		// Note that as we are potentially looking at a stale binding resource
 		// and cannot rely on UnbindStatus == ServiceBindingUnbindStatusNotRequired
 		// to filter out binding requests that have yet to be sent to the broker.
 		if instance.Name == binding.Spec.InstanceRef.Name {
-			return &operationError{
-				reason:  errorDeprovisionBlockedByCredentialsReason,
-				message: "All associated ServiceBindings must be removed before this ServiceInstance can be deleted",
-			}
+			found = append(found, binding)
 		}
 	}
 
+	return found, nil
+}
+
+func (c *controller) deleteExistingBindings(instance *v1beta1.ServiceInstance) error {
+	klog.V(4).Infof("Delete existing bindings for the instance %s", instance.Name)
+	bindings, err := c.listExistingBindings(instance)
+	if err != nil {
+		return errors.Wrapf(err, "while listing existing service bindings")
+	}
+	for _, binding := range bindings {
+		err := c.serviceCatalogClient.ServiceBindings(instance.Namespace).Delete(binding.Name, &metav1.DeleteOptions{})
+		switch {
+		case apierrors.IsNotFound(err):
+			continue
+		case err != nil:
+			return errors.Wrap(err, "while deleting existing service binding")
+		}
+	}
 	return nil
 }
 
@@ -2870,6 +2915,17 @@ func (c *controller) processDeprovisionSuccess(instance *v1beta1.ServiceInstance
 	return nil
 }
 
+func (c *controller) processServiceBindingsDeletion(instance *v1beta1.ServiceInstance) error {
+	setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, v1beta1.ConditionFalse, serviceBindingsDeletionReason, serviceBindingsDeletionMessage)
+
+	if _, err := c.updateServiceInstanceStatus(instance); err != nil {
+		return err
+	}
+
+	c.recorder.Event(instance, corev1.EventTypeNormal, serviceBindingsDeletionReason, serviceBindingsDeletionMessage)
+	return c.beginPollingServiceInstance(instance)
+}
+
 // processDeprovisionFailure handles the logging and updating of a
 // ServiceInstance that hit a terminal failure during deprovision
 // reconciliation.

pkg/features/features.go

@@ -78,6 +78,11 @@ const (
 	// owner: @carolynvs
 	// alpha: v0.1.32
 	ServicePlanDefaults utilfeature.Feature = "ServicePlanDefaults"
+
+	// CascadingDeletion enables deletion of the existing ServiceBindings when deleting a ServiceInstance
+	// owner: @piotrmiskiewicz
+	// alpha: v0.3.0
+	CascadingDeletion utilfeature.Feature = "CascadingDeletion"
 )
 
 func init() {
@@ -96,4 +101,5 @@ var defaultServiceCatalogFeatureGates = map[utilfeature.Feature]utilfeature.Feat
 	UpdateDashboardURL:         {Default: false, PreRelease: utilfeature.Alpha},
 	OriginatingIdentityLocking: {Default: true, PreRelease: utilfeature.Alpha},
 	ServicePlanDefaults:        {Default: false, PreRelease: utilfeature.Alpha},
+	CascadingDeletion:          {Default: false, PreRelease: utilfeature.Alpha},
 }