openservicebrokerapi / servicebroker

Open Service Broker API Specification
https://openservicebrokerapi.org/
Apache License 2.0
1.19k stars 433 forks source link

SHOULD vs MUST with orphan mitigation #370

Closed kibbles-n-bytes closed 5 years ago

kibbles-n-bytes commented 6 years ago

In the new text about platforms sending a deprovision request for every resource that has attempted to be provisioned, we specify that the platform MUST send a deletion request. However, the existing orphan mitigation section says that the platform SHOULD attempt a deletion for the specific orphan mitigation circumstances. As neither section specifies the timing as to when this deletion request will happen, the new text basically overwrites the entirety of the orphan mitigation section.

We should clarify our intent here. Should orphan mitigation happen immediately upon the platform deciding to mark the resource as failed? If so, we should specify that here. If not, then is orphan mitigation no longer relevant? We should then rework this section entirely to just talk about cleaning up of resources in general.

angarg12 commented 6 years ago

Reading #368 I see this:

the platform MUST cease polling, the operation state MUST be considered failed, and the Platform SHOULD send a deprovision request

And then the current orphan mitigation

To mitigate orphan Service Instances and bindings, the marketplace SHOULD attempt to delete resources it cannot be sure were successfully created, and SHOULD keep trying to delete them until the Service Broker responds with a success.

Both sections seems to be in sync, am I missing something?

duglin commented 6 years ago

I think @kibbles-n-bytes is talking about this line in the provision section:

Responses with any other status code will be interpreted as a failure and a deprovision request MUST be sent to the Service Broker to prevent an orphan being created on the Service Broker.

Note the "MUST".

There might be some inconsistency between the two sections, but I would prefer to solve this by: 1 - moving the orphan mitigation section into the appropriate Provision and Binding sections of the spec. Having the logic of how to deal with each type of response split between two sections of the spec hard to follow and leads to possible conflicts, like @kibbles-n-bytes is suggesting we have. 2 - once we do 1, we can then see if we still have an inconsistency - but I doubt we will.

Note, this is related to: https://github.com/openservicebrokerapi/servicebroker/pull/254 and might end up closing that one.

mattmcneeney commented 5 years ago

The new orphan mitigation section makes this much clearer. Closing.