Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

[question][help wanted] How to block ServiceBinding deletion #1954

Closed
mszostok opened this issue Apr 18, 2018 · 7 comments
Closed

[question][help wanted] How to block ServiceBinding deletion #1954

mszostok opened this issue Apr 18, 2018 · 7 comments
Labels
kind/support Categorizes issue or PR as a support question. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@mszostok
Copy link
Contributor

Hi

This is a nutshell of our issue:

In our K8s platform we added a new custom resource (CR) which depends on the ServiceBinding. When such CR is created we want to block the deletion of the ServiceBinding connected with it.

Details

Scenario - creation

  1. Create a ServiceInstance
  2. Create a ServiceBinding
  3. Create a ServiceBindingUsage (our CR)

Scenario – deletions, current state

  1. Delete a ServiceInstance (will be block if the correlated ServiceBinding exists)
  2. Delete a ServiceBinding – deleted without any checks
  3. Delete a ServiceBindingUsage – deleted without any checks

Scenario - deletions, desired state

  1. Delete a ServiceInstance (will be block if the correlated ServiceBinding exists)
  2. Delete a ServiceBinding – (will be block if the correlated ServiceBindingUsages exists)
  3. Delete a ServiceBindingUsage – deleted without any checks (in the future graceful deletion will also be handled by us)

We thought that we can achieve the flow of the last scenario by adding our new finalizer to the ServiceBinding finalizer list, so we did that. Thanks to that the ServiceBinding still exists in etcd but... unfortunately the Service Catalog deletes the Secret and the ServiceBinding has the following status:

Status:
  Async Op In Progress:  false
  Conditions:
    Last Transition Time:         2018-04-18T10:14:59Z
    Message:                      The binding was deleted successfully
    Reason:                       UnboundSuccessfully
    Status:                       False
    Type:                         Ready
  Orphan Mitigation In Progress:  false
  Reconciled Generation:          2
  Unbind Status:                  Succeeded

And that breaks our flow because the Service Catalog always executes the finalizer logic for the ServiceBinding, and we do not know how to block it without using "hacks".

Additionally, we found such comment in the source code for version 0.0.11
https://github.com/kubernetes-incubator/service-catalog/blob/v0.0.13/pkg/controller/controller_binding.go#L286-L292

and the best part of this comment is the following lines:

// (..) we proceed with the soft delete
// only if it's "our turn--" i.e. only if the finalizer we care about is at
// the head of the finalizers list.

In the newest versions this idea was dropped and not implemented.

Could you elaborate on how we can handle the last-described scenario?

Maybe we can extend the Service Catalog with a feature which checks if the list of finalizers equals 1 and that this finalizer is the FinalizerServiceCatalog. Only then the Service Catalog should execute the finalizer logic for the ServiceBinding.

Thank you in advance

@mszostok mszostok changed the title How to block ServiceBinding deletion [question][help wanted] How to block ServiceBinding deletion Apr 18, 2018
@jboyd01
Copy link
Contributor

jboyd01 commented Apr 18, 2018

re In the newest versions this idea was dropped: Service Catalog handles the finalizers as an unsorted set so that comment was removed. See #1093 and #894. I'm not certain if you can get an order list of finalizers from the platform.

Blocking a binding (or any resource) deletion may be a hard issue - we've had numerous scenarios where a namespace is deleted and catalog blocks the deletion of the namespace because it had problems (or bugs) removing the underlying resource. The namespace that can't be deleted is a problem. At initial glance this seems like it will make the potential for this problem larger.

@pmorie might have some thoughts.

@n3wscott n3wscott added the kind/support Categorizes issue or PR as a support question. label Apr 18, 2018
@aszecowka
Copy link

aszecowka commented Apr 19, 2018

@jboyd01 On the other hand, you block removing of ServiceInstance if ServiceBinding for that instance exist.
If we want to follow your pattern and forbid removing of ServiceBinding if ServiceBindingUsage exists, then we need to postpone executing your finalizer. It can be achieved, by changing controller in ServiceCatalog, and perform clean up, only if your finalizer is the last one on the list. In k8s, similar approach is applied to prevent removing of PVC in use.

But in case of Secrets, ConfigMaps used in the pod, there is no such prevention. What you suggest choosing in our scenario? Should we:

  • perform cascade deletion (when ServiceBinding is deleted, we remove ServiceBindingUsage)
  • block deletion of ServiceBinding and Secret
  • do nothing

@jpeeler
Copy link

jpeeler commented Jun 29, 2018

For your ServiceBindingUsage, it sounds like the best option is #1. I'm thinking that would require setting the owner reference to point to the Service Binding and setting a foregroundDeletion finalizer on the binding. Would that work?

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 24, 2019
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 24, 2019
@mszostok
Copy link
Contributor Author

/close

@k8s-ci-robot
Copy link
Contributor

@mszostok: Closing this issue.

In response to this:

/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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/support Categorizes issue or PR as a support question. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

7 participants