Skip to content

Commit b433a3f

Browse files
committed
Add new GRPC alternative
1 parent 0602a5f commit b433a3f

File tree

1 file changed

+50
-0
lines changed
  • keps/sig-storage/5381-mutable-pv-affinity

1 file changed

+50
-0
lines changed

keps/sig-storage/5381-mutable-pv-affinity/README.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,22 @@ message ControllerModifyVolumeRequest {
368368
// Indicates whether the CO allows the SP to update the topology
369369
// as a part of the modification.
370370
bool allow_topology_updates = 4;
371+
372+
// Specifies where (regions, zones, racks, etc.) the
373+
// volume MUST be accessible from after modification.
374+
// COs SHALL only specify topological accessibility
375+
// information supported by the SP.
376+
// This field is OPTIONAL.
377+
// This field SHALL NOT be specified unless the SP has the
378+
// VOLUME_ACCESSIBILITY_CONSTRAINTS plugin capability, the
379+
// MODIFY_VOLUME_TOPOLOGY controller capability and
380+
// allow_topology_updates is set to true.
381+
// If this field is not specified and the SP has the
382+
// VOLUME_ACCESSIBILITY_CONSTRAINTS plugin capability, the
383+
// MODIFY_VOLUME_TOPOLOGY controller capability and
384+
// allow_topology_updates is set to true, the SP MAY
385+
// modify the volume topology according to the mutable_parameters.
386+
TopologyRequirement accessibility_requirements = 5;
371387
}
372388
373389
message ControllerModifyVolumeResponse {
@@ -981,6 +997,40 @@ Why should this KEP _not_ be implemented?
981997
-->
982998

983999
## Alternatives
1000+
Instead of adding new fields to CSI GRPC `ControllerModifyVolume`, we could add a new GRPC `ControllerModifyVolumeTopology`:
1001+
1002+
```protobuf
1003+
rpc ControllerModifyVolumeTopology (ControllerModifyVolumeTopologyRequest)
1004+
returns (ControllerModifyVolumeTopologyResponse) {
1005+
option (alpha_method) = true;
1006+
}
1007+
1008+
message ControllerModifyVolumeTopologyRequest {
1009+
option (alpha_message) = true;
1010+
string volume_id = 1;
1011+
map<string, string> secrest = 2 [(csi_secret) = true];
1012+
map<string, string> mutable_parameters = 3;
1013+
TopologyRequirement accessibility_requirements = 4;
1014+
}
1015+
1016+
message ControllerModifyVolumeTopologyResponse {
1017+
option (alpha_message) = true;
1018+
repeated Topology accessible_topology = 1;
1019+
bool in_progress = 2;
1020+
}
1021+
```
1022+
1023+
The workflow of this new GRPC is essentially the same as the current `ControllerModifyVolume` GRPC, but it allows SPs to mutate the accessible
1024+
topologies of volumes by default.
1025+
1026+
SPs with the `MODIFY_VOLUME_TOPOLOGY` controller capability should use this new GRPC instead of `ControllerModifyVolume` when modifying volumes.
1027+
1028+
Comparison between these two approaches:
1029+
| Criteria | PR 592 (Extended GRPC) | PR 593 (New GRPC) |
1030+
| -------- | ---------------------- | ----------------- |
1031+
| Maintenance Difficulty | ✅ Low | ⚠️ High, need to also modify ControllerModifyVolumeTopology when making changes to ControllerModifyVolume |
1032+
| Implementation Complexity | ✅ Low | ⚠️ High, SPs will have to implement a new GRPC if they want to support topology modification even if they have implemented ControllerModifyVolume |
1033+
| Side Effects | ⚠️ Will impede the GA process of K8s VAC | ✅ No influence on other features |
9841034

9851035
<!--
9861036
What other approaches did you consider, and why did you rule them out? These do

0 commit comments

Comments
 (0)