csi: loosen ValidateVolumeCapability requirements #9049
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
tgross
force-pushed
the
b-csi-validate-volume-caps
branch
from
c8315bf to
1fdb3f6
Compare
tgross
commented
Oct 8, 2020
| multierror.Append(&err, fmt.Errorf( | ||
| "requested AccessType Block but got AccessType Mount")) | ||
| "'block-device' access type was not requested but was validated by the controller")) |
There was a problem hiding this comment.
Note to reviewers: I've changed the wording of these error messages so that they match what the operator has submitted in the volume spec or jobspec, rather than matching the CSI internal nomenclature.
tgross
force-pushed
the
b-csi-validate-volume-caps
branch
from
1fdb3f6 to
0f781f7
Compare
The CSI specification for `ValidateVolumeCapability` says that we shall "reconcile successful capability-validation responses by comparing the validated capabilities with those that it had originally requested" but leaves the details of that reconcilation unspecified. This API is not implemented in Kubernetes, so controller plugins don't have a real-world implementation to verify their behavior against. We have found that CSI plugins in the wild may return "successful" but incomplete `VolumeCapability` responses, so we can't require that all capabilities we expect have been validated, only that the ones that have been validated match. This appears to violate the CSI specification but until that's been resolved in upstream we have to loosen our validation requirements. The tradeoff is that we're more likely to have runtime errors during `NodeStageVolume` instead of at the time of volume registration.
tgross
force-pushed
the
b-csi-validate-volume-caps
branch
from
0f781f7 to
ebff972
Compare
tgross
commented
Oct 8, 2020
| @@ -91,7 +91,7 @@ context { | |||
| - `mount_options` - Options for mounting `file-system` volumes that don't | |||
| already have a pre-formatted file system. Consult the documentation for your | |||
| storage provider and CSI plugin as to whether these options are required or | |||
| neccessary. | |||
| necessary. | |||
There was a problem hiding this comment.
Note to reviewers: I dunno why these slipped by the docs PR but triggered the linter here. Maybe we don't check spelling there?
notnoop
approved these changes
Oct 8, 2020
cgbaker
approved these changes
Oct 8, 2020
fredrikhgrelland
pushed a commit
to fredrikhgrelland/nomad
that referenced
this pull request
Oct 22, 2020
The CSI specification for `ValidateVolumeCapability` says that we shall "reconcile successful capability-validation responses by comparing the validated capabilities with those that it had originally requested" but leaves the details of that reconcilation unspecified. This API is not implemented in Kubernetes, so controller plugins don't have a real-world implementation to verify their behavior against. We have found that CSI plugins in the wild may return "successful" but incomplete `VolumeCapability` responses, so we can't require that all capabilities we expect have been validated, only that the ones that have been validated match. This appears to violate the CSI specification but until that's been resolved in upstream we have to loosen our validation requirements. The tradeoff is that we're more likely to have runtime errors during `NodeStageVolume` instead of at the time of volume registration.
|
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 #9021
The CSI specification for
ValidateVolumeCapabilitysays that we shall"reconcile successful capability-validation responses by comparing the
validated capabilities with those that it had originally requested" but leaves
the details of that reconcilation unspecified. This API is not implemented in
Kubernetes, so controller plugins don't have a real-world implementation to
verify their behavior against.
We have found that CSI plugins in the wild may return "successful" but
incomplete
VolumeCapabilityresponses, so we can't require that allcapabilities we expect have been validated, only that the ones that have been
validated match. This appears to violate the CSI specification but until
that's been resolved in upstream we have to loosen our validation
requirements. The tradeoff is that we're more likely to have runtime errors
during
NodeStageVolumeinstead of at the time of volume registration.