spec: Align volume validation with creation. #271
Conversation
|
TODO: add secrets, storage backend might need them to execute validation for a pre-existing volume |
There was a problem hiding this comment.
I thought about this some more.
My understanding is that ValidateVolumeCapabilities is used to verify a pre-provisioned volume. That means that the volume already exists, and the CO wants to validate its assumptions to verify it has the correct information for how to access it.
If that is the case, the fields in ValidateVolumeCapabilitiesRequest should be similar to the fields returned in the CreateVolumeResponse (a description of a newly created volume) rather then the CreateVolumeRequest (a set of requirements for how to provision a new volume).
For example, the topology field should be repeated Topology accessible_topology = 5; not TopologyRequirement accessibility_requirements. Because we are trying to validate our assumptions about a (presumably) existing volume (it is accessible from all the specified accessible_topology). We are NOT trying to verify that it is possible to create a NEW volume with these topology requirements.
Thoughts?
There was a problem hiding this comment.
Discussed w/ @jieyu and it sounds like we're aligned on this PR being mostly correct. There was some concern about "parameters" missing from somewhere .. but that's not very clear to me since it's part of this API already. I also see the comment about explicitness that I don't understand.
@saad-ali can you elaborate?
jdef
force-pushed
the
align_validation_with_creation
branch
from
d784d24
to
1df9624
Compare
THIS IS A BREAKING CHANGE. * ValidateVolumeCapabilitiesRequest accepts the same parameters and topology requirements as CreateVolumeRequest. * ValidateVolumeCapabilitiesResponse is forward-compatible between newer COs (clients) and older plugins (servers). * fixes container-storage-interface#242
Amended wording in the spec that referred to the field `supported` that no longer exists as of this PR and has been effectively replaced with `confirmed`.
Since `parameters` and `volume_attributes` are complex fields the `Confirmed` message of `ValidateVolumeCapabilitiesResponse` has been extended to report those map entries recognized by the plugin.
Current thinking is that topology of a volume can be discovered via the ListVolumes RPC of the controller and is not strictly required here. We can always add it back later if needed.
jdef
force-pushed
the
align_validation_with_creation
branch
from
d175f5c
to
3664c5c
Compare
|
addressed remaining feedback and rebased to master |
|
LGTM |
THIS IS A BREAKING CHANGE.
ValidateVolumeCapabilitiesRequest accepts the same parameters and
attributes requirements as CreateVolumeRequest.
ValidateVolumeCapabilitiesResponse is forward-compatible between newer
COs (clients) and older plugins (servers).
fixes Consider making ValidateVolumeCapabilitesRequest align with CreateVolumeRequest #242