Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add text around what to do on a deprovision when there are bindings #314

Merged
merged 1 commit into from
Sep 26, 2017
Merged

Add text around what to do on a deprovision when there are bindings #314

merged 1 commit into from
Sep 26, 2017

Conversation

duglin
Copy link
Contributor

@duglin duglin commented Sep 10, 2017

Closes #127

Basic does this:

  • clarify in the spec that it is RECOMMENDED that platforms delete all Bindings prior to deleting an Instance
  • brokers MAY support deleting an Instance that still has bindings (causing a cascading delete), but if they don't then we'll specify the exact error that MUST be returned
  • platforms MAY attempt to delete an Instance that still has Bindings, but if they get the well defined error then they MUST delete the bindings first.

Signed-off-by: Doug Davis dug@us.ibm.com

@cfdreddbot
Copy link

Hey duglin!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@angarg12
Copy link
Contributor

angarg12 commented Sep 11, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

mattmcneeney
mattmcneeney reviewed Sep 11, 2017
View reviewed changes
spec.md Outdated
It is RECOMMENDED that platforms delete all bindings for a service prior
to attempting to deprovision the service. If a broker receives a deprovision
request for a service that has bindings, then the broker MAY attempt
to delete all bindings prior deprovisioning the service. Any error during
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prior to

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@vaikas
Copy link
Contributor

vaikas commented Sep 12, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@duglin
Copy link
Contributor Author

duglin commented Sep 12, 2017

AI: Switch to 422 per today's call

@duglin
Copy link
Contributor Author

duglin commented Sep 18, 2017

Need reviews

@mattmcneeney
Copy link
Contributor

mattmcneeney commented Sep 18, 2017 •
edited
Loading

@duglin Is this supposed to be changed to a 422 as per your AI?

Edit: same question for #300 (I missed last week's call so may have missed something - did you decide on 409 or 422 for both?)

@duglin
Copy link
Contributor Author

duglin commented Sep 18, 2017

@mattmcneeney yep - missed that - done now

@mattmcneeney
Copy link
Contributor

mattmcneeney commented Sep 18, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

1 similar comment
@angarg12
Copy link
Contributor

angarg12 commented Sep 18, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

Add text around what to do on a deprovision when there are bindings
b693a27 
Closes openservicebrokerapi#127

Basic does this:
- clarify in the spec that it is RECOMMENDED that platforms delete all Bindings prior to deleting an Instance
- brokers MAY support deleting an Instance that still has bindings (causing a cascading delete), but if they don't then we'll specify the exact error that MUST be returned
- platforms MAY attempt to delete an Instance that still has Bindings, but if they get the well defined error then they MUST delete the bindings first.

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Contributor Author

duglin commented Sep 25, 2017

Completed AI from previous call:
Proposal:Platforms MUST delete bindings prior to a deprovision request && spec makes no statement about what a broker is to do if a deprovision comes in while there are bindings.

@duglin
Copy link
Contributor Author

duglin commented Sep 26, 2017

another round of reviews needed.

vaikas
vaikas reviewed Sep 26, 2017
View reviewed changes
spec.md
@@ -1021,6 +1021,11 @@ delete any resources it created during the provision.
Usually this means that all resources are immediately reclaimed for future
provisions.

Platforms MUST delete all bindings for a service prior
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we change this to "MUST"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed during the call

@fmui
Copy link
Member

fmui commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

4 similar comments
@avade
Copy link
Contributor

avade commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@mattmcneeney
Copy link
Contributor

mattmcneeney commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@vaikas
Copy link
Contributor

vaikas commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@pmorie
Copy link
Contributor

pmorie commented Sep 26, 2017 •
edited by chipchilders
Loading

LGTM

Approved with PullApprove

@vaikas vaikas merged commit ef51846 into openservicebrokerapi:master Sep 26, 2017
@duglin duglin deleted the Deprovision branch October 26, 2017 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattmcneeney mattmcneeney mattmcneeney left review comments

@vaikas vaikas vaikas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@duglin @cfdreddbot @angarg12 @vaikas @mattmcneeney @fmui @avade @pmorie