Service Instances cascading delete proposal

[mszostok]

Service Instances cascading delete proposal

This is the umbrella issue for the Service Instances cascading delete operation.
In this issue, we also track the implementation of this functionality from alpha to GA stage.

Motivation

Currently, during the deprovisioning operation, the default case is to fail the deprovisioning if there are bindings against the instance being deprovisioned.
​
Problems:

• Users expect that deleting a Kubernetes API resource results in the total deletion of resources associated with the API resource
• Not going back once deletion timestamp is set
  ​
  OSB API Spec states:

Platforms MUST delete all Service Bindings for a Service Instance prior to attempting to deprovision the Service Instance. This specification does not specify what a Service Broker is to do if it receives a deprovision request while there are still Service Bindings associated with it.

source: https://github.com/openservicebrokerapi/servicebroker/blob/v2.15/spec.md#deprovisioning

Discussions

• Create consensus on deprovision #46

  Issue created on Nov 22, 2016

  remove first the instance then remove the bindings.

• Policy for handling broker deletion and deprovision #481

  F2F meeting in late January, 2017

  On a deprovision operation, we decided that the default case is to fail the deprovision if there are bindings against the instance being deprovisioned. A "force" or "cascade" flag can be explicitly set on the call to make all bindings be unbound, and then the deprovision be done.
  ​

• Determine how to handle deprovision requests to an instance with bindings #820

  F2F meeting on May 9, 2017

  When a user attempts to delete an instance (ie kubectl delete instance) Catalog will check if any bindings are associated with the instance. If there are, the operation will fail with an error indicating that the instance is in use. The user can repeat the operation with a -force flag. In this case, the instance will be marked as soft deleted. Instances that don't have bindings will be deleted immediate with or without the force flag.
  ​

• Implement Matrix of Deleting Things - recursive namespace deletion decision from F2F #2229

  F2F meeting on Jul 23, 2018

  We decided that the object that owns other objects should recursively delete everything that these objects own in case they are deleted.

  Action	Description of Scenario	What we do / What will happen
  Delete Instance	Has bindings, broker allows it	"Depth First Deletion": Cascade delete the bindings from the Broker. Once all bindings are deleted, delete the instance (blockOwnerDeletion, foreground deletion). Instance status says something like "Deprovision will start after bindings are unbound" instead of "Can't deprovision until all bindings are unbound" (current wording)

​

Proposed behaviors

Based on the discussions above, there are four proposed options:

1. Use the kubectl --cascade flag

   --cascade=true: If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a
   ReplicationController). Default true.

   Basically, under the hood, the propagationPolicy is applied on a given resource. In case of cascade=true, it is propagationPolicy=foreground. In case of cascade=false, it is propagationPolicy=background. The propagationPolicy is managed by the Kubernetes Garbage Collection.
   ​
   However, this option cannot be considered because of the expected behavior:

   The ownerReference cannot be used because of a problem when a user sets the --cascade=false option. In such case, Kubernetes requires the propagationPolicy=background.

   In background cascading deletion, Kubernetes deletes the owner object immediately and the garbage collector then deletes the dependents in the background.

   source: https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#background-cascading-deletion

   In our case, the binding must be deleted before the instance. Implementing the background behavior is not an option in the case of Service Catalog.
   ​

2. Global flag for changing the behavior
   Add the disable-cascading-deletion=<true/false> flag for the Service Catalog controller-manager. This flag globally changes the behavior of the controller-manager.

3. ServiceInstance spec property changes​
   We can add the deleteBehavior field under the Instance spec. The field has two possible behaviors: forceCascade or failOnBindings. In the former case, deleting an instance force-deletes all Bindings that reference the instance, without confirming with the person who deletes it. In the latter one, the deletion simply fails if there are bindings that reference the instance. *This field will be optional and it will default to failOnBindings.
   ​

4. Always perform cascade deletion
   When a user deletes a ServiceInstance, the related ServiceBindings are deleted (execute the unbind call to the broker). When all ServiceBindings are deleted successfully, the ServiceInstance is also deleted (execute the deprovision call to the broker).
   ​

Accepted solution

Based on the client feedback and OSB API specification, we know that we want to implement this behavior and to do it safely we decided to go with the 2nd option: Global flag for changing the behavior.
​
Reasons:

1. OSB API Spec:

   Platforms MUST delete all Service Bindings for a Service Instance prior to attempting to deprovision the Service Instance. This specification does not specify what a Service Broker is to do if it receives a deprovision request while there are still Service Bindings associated with it.

   source: https://github.com/openservicebrokerapi/servicebroker/blob/v2.15/spec.md#deprovisioning

2. If Users expect that deleting a Kubernetes API resource results in the total deletion of resources associated with the API resource then they can set the disable-cascading-deletion flag to true and controller-manager will take care about that.

This solution was accepted and the alpha implementation has already been added in this PR: #2711. Starting from the Service Catalog version 0.3.0, you can enable this option by setting the CascadingDeletion feature gate to true. Users, who do not accept cascading deletion, the controller-manager provides a flag disable-cascading-deletion which blocks the feature even if the cascading deletion is enabled by default (in the future).

ACTION REQUIRED

Going with this behavior is backward compatible. It was implemented as alpha.
We are waiting for your feedback and use-cases if this approach happens to break your flow. It's unacceptable for us, so please let us know.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[mszostok]

/remove-lifecycle rotten

[mszostok]

ICYMI:

The Cascading binding deletion feature is officially available in 0.3.0 release. The controller manager deletes all Service Bindings for a Service Instance before attempting to deprovision the Service Instance. This option can be enabled by setting the CascadingDeletion feature gate to true (#2711)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jhvhs]

/remove-lifecycle rotten
/reopen
/lifecycle frozen

[k8s-ci-robot]

@jhvhs: Reopened this issue.

In response to this:

/remove-lifecycle rotten
/reopen
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[mrbobbytables]

This project is being archived, closing open issues and PRs.
Please see this PR for more information: kubernetes/community#6632