Ensure binding credentials are stored on async bind #3693
Merged
+251
−188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| When("the last operation is in progress", func() { | ||
| BeforeEach(func() { | ||
| brokerClient.GetServiceInstanceLastOperationReturns(osbapi.LastOperationResponse{ |
There was a problem hiding this comment.
This looks like it duplicates the test starting on line 288
There was a problem hiding this comment.
Indeed, the duplicate has been removed. Thanks!
danail-branekov
force-pushed
the
fix-async-bind
branch
from
7afd8f2 to
6c92587
Compare
* When the bind operation state is `succeeded`, requeue the reconcile request. Thus we ensure that the controller would try to `bind` again, and as the bind operation has already succeeded, it would get a response containing the binding credentials * When checking the state of the last operation in asynchronous provision/bind, instance/binding controllers consider the operation to have completed when its state is either `succeeded`, or `failed`. Anything else is considered as if the operation is ongoing. Thus we protect ourselves from brokers returning nonsense operation state * The test for the instance controller is refactored so that the synchronous provisioning is the "default" case and there is a dedicated context for asynchronous provisioning. This aligns test style of instance/binding controller tests
* `Provision`/`Deprovision`/`Bind`/`Unbind` operations
- return `HTTP 202 Accepted` on first invocation for a resource and return operation id
- store the operation in a map and flags it as `in-progress`
* `GetLastOperation`
- returns `in-progress` state upon first invocation for given
operation and flags the operation as `done` in the map above
- on subsequent invocation, `succeeded` state is returned
* Once the last operation succeeds (and is flagged as succeeded in the
map), `Provision`/`Deprovision`/`Bind`/`Unbind` return `HTTP 200 OK`
* Misc:
- last operation handler is common for instances and bindings
- helper functions to implement async responses in a common manner
- improved logging
danail-branekov
force-pushed
the
fix-async-bind
branch
from
6c92587 to
651ff42
Compare
georgethebeatle
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Is there a related GitHub Issue?
No
What is this change about?
Ensure credentials secret is created on async bind
When the bind operation state is
succeeded, requeue the reconcilerequest. Thus we ensure that the controller would try to
bindagain,and as the bind operation has already succeeded, it would get a
response containing the binding credentials
When checking the state of the last operation in asynchronous
provision/bind, instance/binding controllers consider the operation to
have completed when its state is either
succeeded, orfailed.Anything else is considered as if the operation is ongoing. Thus we
protect ourselves from brokers returning nonsense operation state
The test for the instance controller is refactored so that the
synchronous provisioning is the "default" case and there is a
dedicated context for asynchronous provisioning. This aligns test
style of instance/binding controller tests
The sample broker implements the async flow properly
Does this PR introduce a breaking change?
No
Tag your pair, your PM, and/or team
@georgethebeatle @uzabanov