Fix a idempotency issues in CreateVolume #99
Merged
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
WanzenBug
force-pushed
the
fix-retry-in-create-volume
branch
from
a9fd0de
to
30c3396
Compare
In case creating a new volume takes longer than the GRPC timeout, the provisioner could get in a state where volumes are considered ready even if no resource was ever created. This commit fixes this issue by ensuring that volumes are only considered ready after the expected amount of volumes was placed. The bug happened in cases where: * Create() succesfully called saveVolume(), persisting the volume information as annotations, which is interpreted as "ready" resources * Immediatly after, the GRPC timeout cancels the request context, meaning the volume scheduler aborted before any resources could be assigned to nodes A simple reording of "volumeScheduler.Create()" and "saveVolume()" would lead to a different issue, in which continuously more volumes are placed but never marked as ready. To prevent this, volumes that reference at least the number of resources as required by the parameters are considered ready. Note that this is a stopgap solution until volume schedulers are re-written to be idempotent themselves.
WanzenBug
force-pushed
the
fix-retry-in-create-volume
branch
from
30c3396
to
f4eeb37
Compare
|
LGTM. thanks |
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.
In case creating a new volume takes longer than the GRPC timeout,
the provisioner could get in a state where volumes are considered
ready even if no resource was ever created. This commit fixes this
issue by ensuring that volumes are only considered ready after the
expected amount of volumes was placed.
The bug happened in cases where:
Create()succesfully calledsaveVolume(), persisting the volumeinformation as annotations, which is interpreted as "ready" resources
the volume scheduler aborted before any resources could be assigned to
nodes
A simple reording of
volumeScheduler.Create()andsaveVolume()wouldlead to a different issue, in which continuously more volumes are placed
but never marked as ready. To prevent this, volumes that reference at least
the number of resources as required by the parameters are considered ready.
Note that this is a stopgap solution until volume schedulers are re-written
to be idempotent themselves.