driver: handle non existing volumes and nodes #60
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
fatih
force-pushed
the
handle-not-found
branch
from
3036d68 to
023f21a
Compare
According to the spec we should return a `NOT_FOUND` in various method. We always assume the volume or node exist, however this might not the case. For example the user might delete the volume externally from the UI or make a HTTP call to the DigitalOcean API to delete the volume. Actions like this are outside the Kubernetes system, so the plugin needs to handle these edge cases by signaling back that the resources don't exist. We return `NOT_FOUND` for any creation actions, such as creating a volume, attaching a volume, formatting, etc.. Because we really need to return an error as there is no way to recover from this We don't return `NOT_FOUND` for any destroy actions, such as deleting, detaching, unmounting, etc.. Because in all these cases it doesn't matter if the volume doesn't exist. There is not much to do and assuming it's "done" makes the overall system more stable
fatih
force-pushed
the
handle-not-found
branch
from
023f21a to
89c4115
Compare
nanzhong
reviewed
Aug 21, 2018
driver/node.go
Outdated
| // Invalid argument is returned by the `mount` command in this container | ||
| // image when a path is already umounted. We only return an error for | ||
| // anything else | ||
| if err != nil && !strings.Contains(err.Error(), "Invalid argument") { |
There was a problem hiding this comment.
Mhmm this string comparison feels like it’s a little fragile, is there another way to determine the error case?
There was a problem hiding this comment.
Good call, as I said I'll use IsMounted that doesn't rely on the Source so it's a safe bet
|
lgtm, one question about checking the error. |
|
The |
fatih
force-pushed
the
handle-not-found
branch
from
5087aa3 to
1134850
Compare
fatih
force-pushed
the
handle-not-found
branch
from
1134850 to
8a50f61
Compare
|
🚀 |
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.
According to the spec we should return a
NOT_FOUNDin various method.We always assume the volume or node exist, however this might not the
case.
For example the user might delete the volume externally from the
UI or make a HTTP call to the DigitalOcean API to delete the volume.
Actions like this are outside the Kubernetes system, so the plugin needs
to handle these edge cases by signaling back that the resources don't
exist.
We return
NOT_FOUNDfor any creation actions, such as creating avolume, attaching a volume, formatting, etc.. Because we really need to
return an error as there is no way to recover from this
We don't return
NOT_FOUNDfor any destroy actions, such as deleting,detaching, unmounting, etc.. Because in all these cases it doesn't
matter if the volume doesn't exist. There is not much to do and assuming
it's "done" makes the overall system more stable