csi: improve plugin error messages and volume validation #7984
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
|
I just did a quick search over the rest of the code base and it looks like this is the only test file with the |
drewbailey
reviewed
May 15, 2020
drewbailey
reviewed
May 15, 2020
drewbailey
reviewed
May 15, 2020
Some CSI plugins don't return much for errors over the gRPC socket above and beyond the bare minimum error codes. This changeset improves the operator experience by unpacking the error codes when available and wrapping the error with some user-friendly direction. Improving these errors also revealed a bad comparison with `require.Error` when `require.EqualError` should be used in the test code for plugin errors. This defect in turn was hiding a bug in volume validation where we're being overly permissive in allowing mount flags, which is now fixed.
drewbailey
approved these changes
May 15, 2020
langmartin
approved these changes
May 15, 2020
| @@ -469,7 +505,7 @@ func (c *client) NodeStageVolume(ctx context.Context, volumeID string, publishCo | |||
| } | |||
|
|
|||
| // These errors should not be returned during production use but exist as aids | |||
| // during Nomad Development | |||
| // during Nomad development | |||
There was a problem hiding this comment.
def seem less ominous/sarcastic now 😁
tgross
added a commit
that referenced
this pull request
May 27, 2020
Some CSI plugins don't return much for errors over the gRPC socket above and beyond the bare minimum error codes. This changeset improves the operator experience by unpacking the error codes when available and wrapping the error with some user-friendly direction. Improving these errors also revealed a bad comparison with `require.Error` when `require.EqualError` should be used in the test code for plugin errors. This defect in turn was hiding a bug in volume validation where we're being overly permissive in allowing mount flags, which is now fixed.
|
Cherry-picked to 0.11.3 branch as b904bf4 |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #7424
Some CSI plugins don't return much for errors over the gRPC socket above and beyond the bare minimum error codes. This changeset improves the operator experience by unpacking the error codes when available and wrapping the error with some user-friendly direction.
Improving these errors also revealed a bad comparison with
require.Errorwhenrequire.EqualErrorshould be used in the test code for plugin errors. This defect in turn was hiding a bug in volume validation where we're being overly permissive in allowing mount flags, which is now fixed.cc @angrycub as a heads up