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

Concurrent updates on Instance #1149

Closed
duglin opened this issue Aug 21, 2017 · 4 comments
Closed

Concurrent updates on Instance #1149

duglin opened this issue Aug 21, 2017 · 4 comments
Milestone

Comments

@duglin
Copy link
Contributor

duglin commented Aug 21, 2017

We need to discuss how we're going to deal with concurrent updates on things like Instances. For example, while we're creating and Instance if one person updates the Plan and one person updates the Params we're going to need to send 2 different requests to the broker because they might be two different users. How are we going to deal with this? Queue requests? Block current updates? others?

This was referenced Aug 21, 2017
@pmorie
Copy link
Contributor

pmorie commented Aug 28, 2017

At the F2F we decided that we will block updates to an instance's spec while the controller is performing an operation on that instance.

This probably won't bear out in the long-term as a valid approach but it is good enough to get us to an initial beta.

@pmorie
Copy link
Contributor

pmorie commented Aug 30, 2017

Proposal

  • Add a CurrentOperation field to ServiceInstanceStatus
  • Add a CurrentOperation field to ServiceInstanceCredentialStatus
  • The controller should set this field when it begins an operation for a resource
  • If setting the field fails due to a conflict, the controller should return the error and do a rate-limited retry
  • The controller should unset the field when it finishes working on the resource
  • Add an admission controller that rejects updates to the spec of ServiceInstance or ServiceInstanceCredential when metadata.Generation != status.ReconciledGeneration

API Changes

type ServiceInstanceStatus struct {
	// other fields omitted
	CurrentOperation *ServiceInstanceOperation `json:"currentOperation,omitempty"`
}

// ServiceInstanceOperation represents a type of operation the controller can
// be performing for a service instance in the OSB API.
type ServiceInstanceOperation string

const (
    ServiceInstanceOperationProvision   ServiceInstanceOperation = "Provision"
    ServiceInstanceOperationUpdate      ServiceInstanceOperation = "Update"
    ServiceInstanceOperationDeprovision ServiceInstanceOperation = "Deprovision"
)

type ServiceInstanceCredentialStatus struct {
	// other fields omitted
	CurrentOperation *ServiceInstanceCredentialOperation `json:"currentOperation,omitempty"`
}

// ServiceInstanceCredentialOperation represents a type of operation
// the controller can be performing for a binding in the OSB API.
type ServiceInstanceCredentialOperation string

const (
    ServiceInstanceCredentialOperationBind   ServiceInstanceCredentialOperation = "Bind"
    ServiceInstanceCredentialOperationUnbind ServiceInstanceCredentialOperation = "Unbind"
)

@duglin
Copy link
Contributor Author

duglin commented Aug 30, 2017

overall seems good but I think there's a race condition. Setting the op should happen immediately update the Spec being changed if the op flag is the blocker for not doing concurrent updates. Or, make the blocker be Generation != ReconciledGeneration. Otherwise 2 updates can happen before the controller sets the op flag.

@pmorie
Copy link
Contributor

pmorie commented Aug 31, 2017

@duglin I had a parse error reading your comment, but as we discussed in the meeting, the criteria for blocking updates can be ReconciledGeneration != meta.Generation, will update comment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants