Service-catalog: kubectl delete doesn't delete Instance resources when deprovision calls fail

[arschles]

If a kubectl delete of an Instance fails because the broker deprovision call failed, the Instance resource itself is not deleted.

I have reproduced this by going through the steps in my Azure demo repository, but I can boil it down to some basic repro steps:

1. Create a Broker that installs 1 or more ServiceClasses
2. Create an Instance that points to a valid ServiceClass
3. Wait for the broker's provision call to succeed
4. Shut down the broker server itself, but do not remove any Kubernetes resources in this step
5. kubectl delete the instance

After step 5, the controller will try to deprovision and it will fail because there is no broker available to run the deprovision REST call against. This simulates a poorly behaving broker. Notice that the Instance resource has a DeprovisionCallFailed condition on it, but it isn't, and cannot be deleted. This leaves the cluster in a state in which it is impossible to try to re-create the Instance (and reprovision a service).

[vaikas]

Just moving a conversation from the slack channel to here. To summarize. Broker responds with 500 and we leave the resource laying around, which according to the spec is what should happen:
"Responses with any other status code will be interpreted as a failure and the service instance will remain in the marketplace database. Brokers can include a user-facing message in the description field; for details see Broker Errors."

[arschles]

Yep - agree that this is per-spec. As a user, however, I am stuck with an Instance that I can't remove. In my specific case, I know that a backing resource was never created and I want to delete this Instance so I can re-try (in a larger organization, this force-delete operation might be an operator's job). The only thing that I can do at the moment is to restart the service-catalog API server & purge my etcd database to remove the zombie Instance.

Following the spec to the "T" is fine, but we need an escape hatch as well for real world brokers that may not adhere 100% to the OSB API spec. Also, I want to mention that @gabrtv has also seen this behavior, and is on the same page as I am that this is something we need to solve beyond just saying that it's the broker's problem that it doesn't adhere to the spec completely.

[duglin]

I believe that cf allows for a "-f" type of option for cases where the broker is gone - we should do the same.

[arschles]

@duglin looks like kubectl delete --force might do the trick:

      --force=false: Immediate deletion of some resources may result in inconsistency or data loss and requires
confirmation.

[duglin]

yep, that's what I was thinking too

[arschles]

@simonleung8 I know that you are working on kubectl plugins. What are your thoughts here?

[simonleung8]

@arschles Currently we don't have plugin commands for any of the 'delete' action. To improve user's experience, we certainly could have an optional -f to delete the resource forcibly from the core api, and inform the user what the potential complication might be with -f

[arschles]

@simonleung8 something like that would be very helpful. are there any plans to implement a delete action in the plugin?

[arschles]

Changing title to discuss only the force flag, since the deletion behavior is fixed now

[MHBauer]

@jberkhahn @carolynvs Is this something covered by svcat already? Does #1723 completely subsume this ?

[carolynvs]

Does #1723 completely subsume this ?

@MHBauer I believe it does. Closing in favor of #1723. Feel free to re-open if I'm off base.

[MHBauer]

Excellent. Goal is to close things that are done!