KEP-5943: Introducing Snapshot Topology#606
Conversation
|
/assign @bswartz |
mdzraf
changed the title
|
Do we need to add some constraints for TopologyRequirement in CreateVolumeRequest if VolumeContentSource is a Snapshot? |
We can discuss this but we will already be enforcing it at provisioning time via the external-provisioner and the custom scheduler plugin. |
| @@ -2037,7 +2070,7 @@ | |||
| |-----------|-----------|-------------|-------------------| | |||
| | Snapshot already exists but is incompatible | 6 ALREADY_EXISTS | Indicates that a snapshot corresponding to the specified snapshot `name` already exists but is incompatible with the specified `volume_id`. | Caller MUST fix the arguments or use a different `name` before retrying. | | |||
There was a problem hiding this comment.
13 Is not the right number for RESOURCE_EXHAUSTED this is fixed in #605, hence why the number change here as well instead of a separate entry.
| This RPC will be called by the CO to create a new snapshot from a source volume on behalf of a user. | ||
|
|
||
| This operation MUST be idempotent. | ||
| If a snapshot corresponding to the specified snapshot `name` is successfully cut and ready to use (meaning it MAY be specified as a `volume_content_source` in a `CreateVolumeRequest`), the Plugin MUST reply `0 OK` with the corresponding `CreateSnapshotResponse`. |
There was a problem hiding this comment.
A reference to accessibility_requirements is needed here, as in CreateVolume
| map<string, string> parameters = 4; | ||
|
|
||
| // Specifies where (regions, zones, racks, etc.) the provisioned | ||
| // snapshot MUST be accessible from. |
There was a problem hiding this comment.
I'm uncomfortable with the word "accessible" here. The word has a specific meaning for volumes which doesn't apply to snapshots. What we are trying to communicate is not accessibility but whether the snapshot can be used to create volumes in a given topology. We don't really have a word that means that anywhere in the spec. I could imagine "located" or "usable from". We could use the awkward phrasing:
Specifies where (regions, zones, racks, etc.) volumes created from the provisioned snapshot MUST be accessible from.
This communicates the actual thing that CSI cares about, which is whether you can create a volume that is accessible in the topology in question, but I'm not sure it's exactly what we want to say.
If we introduced a new word:
Specifies where (regions, zones, racks, etc.) the provisioned snapshot MUST be usable from. A snapshot is usable from a location if volumes created from that snapshot are guaranteed to be accessible.
This is a little bit more clear. What do you think?
4 participants
What type of PR is this?
Introducing Snapshot Topology to CSI Spec.
What this PR does / why we need it:
This PR is part of the changes necessary for kubernetes/enhancements#5944
Special notes for your reviewer:
Does this PR introduce an API-breaking change?: