Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@xing-yang
xing-yang committed Apr 11, 2020
1 parent 254fe5d commit a15a86a
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 50 deletions.
40 changes: 24 additions & 16 deletions csi.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,7 +855,7 @@ message ListVolumesResponse {
// reported by the SP. The CO MUST be resilient to that.
repeated string published_node_ids = 1;

// Information about the current health of the volume.
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
Expand Down Expand Up @@ -885,9 +885,8 @@ message ListVolumesResponse {
message ControllerGetVolumeRequest {
option (alpha_message) = true;

// Identity information for a specific volume. This field is
// REQUIRED. ControllerGetVolume will return with current
// volume information.
// The ID of the volume to fetch current volume information for.
// This field is REQUIRED.
string volume_id = 1;
}

Expand All @@ -904,7 +903,7 @@ message ControllerGetVolumeResponse {
// reported by the SP. The CO MUST be resilient to that.
repeated string published_node_ids = 1;

// Information about the current health of the volume.
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
Expand All @@ -914,8 +913,7 @@ message ControllerGetVolumeResponse {
// This field is REQUIRED
Volume volume = 1;

// This field is REQUIRED. This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
// This field is REQUIRED.
VolumeStatus status = 2;
}
message GetCapacityRequest {
Expand Down Expand Up @@ -994,21 +992,20 @@ message ControllerServiceCapability {
LIST_VOLUMES_PUBLISHED_NODES = 10;

// Indicates the SP supports controller level volume condition.
// An SP can implement `VolumeCondition` in only the Controller
// Plugin, only the Node Plugin, or both if possible.
// An SP MAY implement `VolumeCondition` in only the Controller
// Plugin, only the Node Plugin, or both.
// If `VolumeCondition` is implemented in both the Controller and
// Node Plugins, it SHALL report from different perspectives.
// If for some reason Controller and Node Plugins report
// misaligned volume conditions, CO SHALL assume the worst case
// is the truth.
// Note that for for alpha `VolumeCondition` is intended be
// informative for humans only, not for automated machine
// response.
// Note that, for alpha, `VolumeCondition` is intended be
// informative for humans only, not for automation.
VOLUME_CONDITION = 11 [(alpha_enum_value) = true];

// Indicates the SP supports the ControllerGetVolume RPC.
// This enables COs to, for example, fetch per volume
// health after a volume is provisioned.
// condition after a volume is provisioned.
GET_VOLUME = 12 [(alpha_enum_value) = true];
}

Expand Down Expand Up @@ -1338,7 +1335,10 @@ message NodeGetVolumeStatsRequest {
message NodeGetVolumeStatsResponse {
// This field is OPTIONAL.
repeated VolumeUsage usage = 1;
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the VOLUME_CONDITION node
// capability is supported.
VolumeCondition volume_condition = 2 [(alpha_field) = true];
}

Expand Down Expand Up @@ -1399,9 +1399,17 @@ message NodeServiceCapability {
GET_VOLUME_STATS = 2;
// See VolumeExpansion for details.
EXPAND_VOLUME = 3;
// If Plugin implements VOLUME_CONDITION capability
// then it MUST implement NodeGetVolumeStats RPC
// call for fetching volume condition.
// Indicates the SP supports node level volume condition.
// An SP MAY implement `VolumeCondition` in only the Node
// Plugin, only the Controller Plugin, or both.
// If `VolumeCondition` is implemented in both the Node and
// Controller Plugins, it SHALL report from different
// perspectives.
// If for some reason Node and Controller Plugins report
// misaligned volume conditions, CO SHALL assume the worst case
// is the truth.
// Note that, for alpha, `VolumeCondition` is intended to be
// informative for humans only, not for automation.
VOLUME_CONDITION = 4 [(alpha_enum_value) = true];
}

Expand Down
40 changes: 24 additions & 16 deletions lib/go/csi/csi.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 24 additions & 18 deletions spec.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1457,7 +1457,7 @@ message ListVolumesResponse {
// reported by the SP. The CO MUST be resilient to that.
repeated string published_node_ids = 1;
// Information about the current health of the volume.
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
Expand Down Expand Up @@ -1513,9 +1513,8 @@ If the volume does not exist any more, `ControllerGetVolume` should return gRPC
message ControllerGetVolumeRequest {
option (alpha_message) = true;
// Identity information for a specific volume. This field is
// REQUIRED. ControllerGetVolume will return with current
// volume information.
// The ID of the volume to fetch current volume information for.
// This field is REQUIRED.
string volume_id = 1;
}
Expand All @@ -1532,7 +1531,7 @@ message ControllerGetVolumeResponse {
// reported by the SP. The CO MUST be resilient to that.
repeated string published_node_ids = 1;
// Information about the current health of the volume.
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
Expand All @@ -1542,8 +1541,7 @@ message ControllerGetVolumeResponse {
// This field is REQUIRED
Volume volume = 1;
// This field is REQUIRED. This field MUST be specified if the
// VOLUME_CONDITION controller capability is supported.
// This field is REQUIRED.
VolumeStatus status = 2;
}
```
Expand Down Expand Up @@ -1649,21 +1647,20 @@ message ControllerServiceCapability {
LIST_VOLUMES_PUBLISHED_NODES = 10;
// Indicates the SP supports controller level volume condition.
// An SP can implement `VolumeCondition` in only the Controller
// Plugin, only the Node Plugin, or both if possible.
// An SP MAY implement `VolumeCondition` in only the Controller
// Plugin, only the Node Plugin, or both.
// If `VolumeCondition` is implemented in both the Controller and
// Node Plugins, it SHALL report from different perspectives.
// If for some reason Controller and Node Plugins report
// misaligned volume conditions, CO SHALL assume the worst case
// is the truth.
// Note that for for alpha `VolumeCondition` is intended be
// informative for humans only, not for automated machine
// response.
// Note that, for alpha, `VolumeCondition` is intended be
// informative for humans only, not for automation.
VOLUME_CONDITION = 11 [(alpha_enum_value) = true];
// Indicates the SP supports the ControllerGetVolume RPC.
// This enables COs to, for example, fetch per volume
// health after a volume is provisioned.
// condition after a volume is provisioned.
GET_VOLUME = 12 [(alpha_enum_value) = true];
}
Expand Down Expand Up @@ -2316,8 +2313,6 @@ The `staging_target_path` field is not required, for backwards compatibility, bu
Plugins can use this field to determine if `volume_path` is where the volume is published or staged,
and setting this field to non-empty allows plugins to function with less stored state on the node.

A Node plugin MUST implement NodeGetVolumeStats RPC call for fetching volume condition if it has VOLUME_CONDITION node capability.

```protobuf
message NodeGetVolumeStatsRequest {
// The ID of the volume. This field is REQUIRED.
Expand All @@ -2341,7 +2336,10 @@ message NodeGetVolumeStatsRequest {
message NodeGetVolumeStatsResponse {
// This field is OPTIONAL.
repeated VolumeUsage usage = 1;
// Information about the current condition of the volume.
// This field is OPTIONAL.
// This field MUST be specified if the VOLUME_CONDITION node
// capability is supported.
VolumeCondition volume_condition = 2 [(alpha_field) = true];
}
Expand Down Expand Up @@ -2421,9 +2419,17 @@ message NodeServiceCapability {
GET_VOLUME_STATS = 2;
// See VolumeExpansion for details.
EXPAND_VOLUME = 3;
// If Plugin implements VOLUME_CONDITION capability
// then it MUST implement NodeGetVolumeStats RPC
// call for fetching volume condition.
// Indicates the SP supports node level volume condition.
// An SP MAY implement `VolumeCondition` in only the Node
// Plugin, only the Controller Plugin, or both.
// If `VolumeCondition` is implemented in both the Node and
// Controller Plugins, it SHALL report from different
// perspectives.
// If for some reason Node and Controller Plugins report
// misaligned volume conditions, CO SHALL assume the worst case
// is the truth.
// Note that, for alpha, `VolumeCondition` is intended to be
// informative for humans only, not for automation.
VOLUME_CONDITION = 4 [(alpha_enum_value) = true];
}
Expand Down

0 comments on commit a15a86a

Please sign in to comment.