From 973dc173314bf7ca5df49f44a3d852804e92f2a7 Mon Sep 17 00:00:00 2001 From: Ivan Sim Date: Fri, 16 Sep 2022 15:25:17 -0700 Subject: [PATCH 1/5] Add CSI RPC spec for VolumeSnapshotDelta changed block tracking feature Signed-off-by: Ivan Sim --- csi.proto | 107 +++++++++++++++++++++++++++++++++++++++++++ spec.md | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 242 insertions(+) diff --git a/csi.proto b/csi.proto index ee73aa7a..b4751c78 100644 --- a/csi.proto +++ b/csi.proto @@ -5,6 +5,7 @@ package csi.v1; import "google/protobuf/descriptor.proto"; import "google/protobuf/timestamp.proto"; import "google/protobuf/wrappers.proto"; +import "google/protobuf/duration.proto"; option go_package = "csi"; @@ -100,6 +101,11 @@ service Controller { returns (ControllerGetVolumeResponse) { option (alpha_method) = true; } + + rpc ListSnapshotDeltas(ListSnapshotDeltasRequest) + returns (ListSnapshotDeltasResponse) { + option (alpha_method) = true; + } } service Node { @@ -1080,6 +1086,10 @@ message ControllerServiceCapability { // SINGLE_NODE_SINGLE_WRITER and/or SINGLE_NODE_MULTI_WRITER are // supported, in order to permit older COs to continue working. SINGLE_NODE_MULTI_WRITER = 13 [(alpha_enum_value) = true]; + + // Indicates the SP can compute and retrieve the deltas between + // two snapshots. + LIST_SNAPSHOT_DELTAS = 14 [(alpha_enum_value) = true]; } Type type = 1; @@ -1227,6 +1237,103 @@ message ListSnapshotsResponse { // An empty string is equal to an unspecified field value. string next_token = 2; } + +// List the deltas between two snapshots on the storage system +// regardless of how they were created +message ListSnapshotDeltasRequest { + option (alpha_message) = true; + + // The ID of the base snapshot handle to use for comparison. If + // not specified, return all changed blocks up to the target + // specified by snapshot_target. This field is OPTIONAL. + string snapshot_base_id = 1; + + // The ID of the target snapshot handle to use for comparison. If + // not specified, an error is returned. This field is REQUIRED. + string snapshot_target_id = 2; + + // Defines the type of storage. Default to "BLOCK". This field is + // REQUIRED. + enum Mode { + option (alpha_enum) = true; + + // BLOCK indicates that the snapshot is of block type. + BLOCK = 0 [(alpha_enum_value) = true]; + + // FILE indicates that the snapshot is of file type. + FILE = 1 [(alpha_enum_value) = true]; + } + + // If specified (non-zero value), the Plugin MUST NOT return more + // entries than this number in the response. If the actual number of + // entries is more than this number, the Plugin MUST set `next_token` + // in the response which can be used to get the next page of entries + // in the subsequent `ListSnapshotDeltas` call. This field is + // OPTIONAL. If not specified (zero value), it means there is no + // restriction on the number of entries that can be returned. + // The value of this field MUST NOT be negative. + int32 max_entries = 4; + + // A token to specify where to start paginating. Set this field to + // `next_token` returned by a previous `ListSnapshotDeltas` call to + // get the next page of entries. This field is OPTIONAL. + // An empty string is equal to an unspecified field value. + string starting_token = 5; +} + +message ListSnapshotDeltasResponse { + option (alpha_message) = true; + + // The volume size in bytes. This field is OPTIONAL. + uint64 volume_size_bytes = 1; + + // This token allows you to get the next page of entries for + // `ListSnapshotDeltas` request. If the number of entries is larger + // than `max_entries`, use the `next_token` as a value for the + // `starting_token` field in the next `ListSnapshotDeltas` request. + // This field is OPTIONAL. + // An empty string is equal to an unspecified field value. + string next_token = 2; + + // Changed block deltas between the source and target snapshots. An + // empty list means there is no difference between the two. Leave + // unspecified if the volume isn't of block type. This field is + // OPTIONAL. + repeated BlockSnapshotChangedBlock changed_blocks = 3; +} + +message BlockSnapshotChangedBlock { + option (alpha_message) = true; + + // The block logical offset on the volume. This field is REQUIRED. + uint64 offset = 1; + + // The size of the block in bytes. This field is REQUIRED. + uint64 block_size_bytes = 2; + + // The token and other information needed to retrieve the actual + // data block at the given offset. If the provider doesn't support + // token-based data blocks retrieval, this should be left + // unspecified. This field is OPTIONAL. + BlockSnapshotChangedBlockToken token = 3; +} + +message BlockSnapshotChangedBlockToken { + option (alpha_message) = true; + + // The token to use to retrieve the actual data block at the given + // offset. This field is REQUIRED. + string token = 1; + + // Timestamp when the token is issued. This field is REQUIRED. + .google.protobuf.Timestamp issuance_time = 2; + + // The TTL of the token in seconds. The expiry time is calculated by + // adding the time of issuance with this value. This field is + // REQUIRED. + .google.protobuf.Duration ttl_seconds = 3; +} + message ControllerExpandVolumeRequest { // The ID of the volume to expand. This field is REQUIRED. string volume_id = 1; diff --git a/spec.md b/spec.md index bd537fee..a1995ee0 100644 --- a/spec.md +++ b/spec.md @@ -381,6 +381,11 @@ service Controller { returns (ControllerGetVolumeResponse) { option (alpha_method) = true; } + + rpc ListSnapshotDeltas(ListSnapshotDeltasRequest) + returns (ListSnapshotDeltasResponse) { + option (alpha_method) = true; + } } service Node { @@ -1740,6 +1745,10 @@ message ControllerServiceCapability { // SINGLE_NODE_SINGLE_WRITER and/or SINGLE_NODE_MULTI_WRITER are // supported, in order to permit older COs to continue working. SINGLE_NODE_MULTI_WRITER = 13 [(alpha_enum_value) = true]; + + // Indicates the SP can compute and retrieve the deltas between + // two snapshots. + LIST_SNAPSHOT_DELTAS = 14 [(alpha_enum_value) = true]; } Type type = 1; @@ -1997,6 +2006,132 @@ The CO MUST implement the specified error recovery behavior when it encounters t | Invalid `starting_token` | 10 ABORTED | Indicates that `starting_token` is not valid. | Caller SHOULD start the `ListSnapshots` operation again with an empty `starting_token`. | +#### `ListSnapshotDeltas` + +**ALPHA FEATURE** + +A Controller Plugin MUST implement this RPC call if it has the +`LIST_SNAPSHOT_DELTAS` capability. The Plugin SHALL return the +information about the deltas between the two provided snapshots on the storage +system within the given parameters regardless of how they were created. +`ListSnapshotDeltas` SHALL NOT list deltas of snapshots that are being created +but have not been committed to the storage system. If snapshots are created +and/or deleted while the CO is concurrently paging through `ListSnapshotDeltas` +results, then it is possible that the CO MAY either witness duplicate results in +the list, not witness existing deltas, or both. The CO SHALL NOT expect a +consistent "view" of all snapshot deltas when paging through the snapshot list +via multiple calls to `ListSnapshotDeltas`. + +```protobuf +message ListSnapshotDeltasRequest { + option (alpha_message) = true; + + // The ID of the base snapshot handle to use for comparison. If + // not specified, return all changed blocks up to the target + // specified by snapshot_target. This field is OPTIONAL. + string snapshot_base_id = 1; + + // The ID of the target snapshot handle to use for comparison. If + // not specified, an error is returned. This field is REQUIRED. + string snapshot_target_id = 2; + + // Defines the type of storage. Default to "BLOCK". This field is + // REQUIRED. + enum Mode { + option (alpha_enum) = true; + + // BLOCK indicates that the snapshot is of block type. + BLOCK = 0 [(alpha_enum_value) = true]; + + // FILE indicates that the snapshot is of file type. + FILE = 1 [(alpha_enum_value) = true]; + } + + // If specified (non-zero value), the Plugin MUST NOT return more + // entries than this number in the response. If the actual number of + // entries is more than this number, the Plugin MUST set `next_token` + // in the response which can be used to get the next page of entries + // in the subsequent `ListSnapshotDeltas` call. This field is + // OPTIONAL. If not specified (zero value), it means there is no + // restriction on the number of entries that can be returned. + // The value of this field MUST NOT be negative. + int32 max_entries = 4; + + // A token to specify where to start paginating. Set this field to + // `next_token` returned by a previous `ListSnapshotDeltas` call to + // get the next page of entries. This field is OPTIONAL. + // An empty string is equal to an unspecified field value. + string starting_token = 5; +} + +message ListSnapshotDeltasResponse { + option (alpha_message) = true; + + // The volume size in bytes. This field is OPTIONAL. + uint64 volume_size_bytes = 1; + + // This token allows you to get the next page of entries for + // `ListSnapshotDeltas` request. If the number of entries is larger + // than `max_entries`, use the `next_token` as a value for the + // `starting_token` field in the next `ListSnapshotDeltas` request. + // This field is OPTIONAL. + // An empty string is equal to an unspecified field value. + string next_token = 2; + + // Changed block deltas between the source and target snapshots. An + // empty list means there is no difference between the two. Leave + // unspecified if the volume isn't of block type. This field is + // OPTIONAL. + repeated BlockSnapshotChangedBlock changed_blocks = 3; +} + +message BlockSnapshotChangedBlock { + option (alpha_message) = true; + + // The block logical offset on the volume. This field is REQUIRED. + uint64 offset = 1; + + // The size of the block in bytes. This field is REQUIRED. + uint64 block_size_bytes = 2; + + // The token and other information needed to retrieve the actual + // data block at the given offset. If the provider doesn't support + // token-based data blocks retrieval, this should be left + // unspecified. This field is OPTIONAL. + BlockSnapshotChangedBlockToken token = 3; +} + +message BlockSnapshotChangedBlockToken { + option (alpha_message) = true; + + // The token to use to retrieve the actual data block at the given + // offset. This field is REQUIRED. + string token = 1; + + // Timestamp when the token is issued. This field is REQUIRED. + .google.protobuf.Timestamp issuance_time = 2; + + // The TTL of the token in seconds. The expiry time is calculated by + // adding the time of issuance with this value. This field is + // REQUIRED. + .google.protobuf.Duration ttl_seconds = 3; +} +``` + +##### ListSnapshotDeltas Errors + +If the plugin is unable to complete the `ListSnapshotDeltas` call successfully, +it MUST return a non-ok gRPC code in the gRPC status. If the conditions defined +below are encountered, the plugin MUST return the specified gRPC error code. The +CO MUST implement the specified error recovery behavior when it encounters the +gRPC error code. + +| Condition | gRPC Code | Description | Recovery Behavior | +|-----------|-----------|-------------|-------------------| +| Target volume snapshot does not exist | 5 NOT_FOUND | Indicates that the target volume snapshot corresponding to the specified `snapshot_target_id` does not exist. | Caller MUST verify that the `snapshot_target_id` is correct and that the volume snapshot is accessible and has not been deleted before retrying with exponential back off. | +| Invalid `starting_token` | 10 ABORTED | Indicates that `starting_token` is not valid. | Caller SHOULD start the `ListVolumeGroupSnapshots` operation again with an empty `starting_token`. | + + #### `ControllerExpandVolume` A Controller plugin MUST implement this RPC call if plugin has `EXPAND_VOLUME` controller capability. From 83dbc101ac344601fed40d93c622f968853faf2f Mon Sep 17 00:00:00 2001 From: Ivan Sim Date: Tue, 20 Sep 2022 11:03:22 -0700 Subject: [PATCH 2/5] Address feedback Signed-off-by: Ivan Sim --- csi.proto | 7 +++---- spec.md | 16 +++++++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/csi.proto b/csi.proto index b4751c78..853734a8 100644 --- a/csi.proto +++ b/csi.proto @@ -5,7 +5,6 @@ package csi.v1; import "google/protobuf/descriptor.proto"; import "google/protobuf/timestamp.proto"; import "google/protobuf/wrappers.proto"; -import "google/protobuf/duration.proto"; option go_package = "csi"; @@ -1246,11 +1245,11 @@ message ListSnapshotDeltasRequest { // The ID of the base snapshot handle to use for comparison. If // not specified, return all changed blocks up to the target // specified by snapshot_target. This field is OPTIONAL. - string snapshot_base_id = 1; + string from_snapshot_id = 1; // The ID of the target snapshot handle to use for comparison. If // not specified, an error is returned. This field is REQUIRED. - string snapshot_target_id = 2; + string to_snapshot_id = 2; // Defines the type of storage. Default to "BLOCK". This field is // REQUIRED. @@ -1331,7 +1330,7 @@ message BlockSnapshotChangedBlockToken { // The TTL of the token in seconds. The expiry time is calculated by // adding the time of issuance with this value. This field is // REQUIRED. - .google.protobuf.Duration ttl_seconds = 3; + int32 ttl_seconds = 3; } message ControllerExpandVolumeRequest { diff --git a/spec.md b/spec.md index a1995ee0..8036af4a 100644 --- a/spec.md +++ b/spec.md @@ -2022,6 +2022,12 @@ the list, not witness existing deltas, or both. The CO SHALL NOT expect a consistent "view" of all snapshot deltas when paging through the snapshot list via multiple calls to `ListSnapshotDeltas`. +A Controller Plugin MUST NOT attempt to include any raw data blocks in the RPC +response to ensure data plane operations remain outside the scope of CSI. The +response payload MUST contain enough block location metadata for the end users +to retrieve the raw data blocks. The end users are responsbile for devising +their own transport and retrieval mechanism to access the raw data blocks. + ```protobuf message ListSnapshotDeltasRequest { option (alpha_message) = true; @@ -2029,11 +2035,11 @@ message ListSnapshotDeltasRequest { // The ID of the base snapshot handle to use for comparison. If // not specified, return all changed blocks up to the target // specified by snapshot_target. This field is OPTIONAL. - string snapshot_base_id = 1; + string from_snapshot_id = 1; // The ID of the target snapshot handle to use for comparison. If // not specified, an error is returned. This field is REQUIRED. - string snapshot_target_id = 2; + string to_snapshot_id = 2; // Defines the type of storage. Default to "BLOCK". This field is // REQUIRED. @@ -2114,7 +2120,7 @@ message BlockSnapshotChangedBlockToken { // The TTL of the token in seconds. The expiry time is calculated by // adding the time of issuance with this value. This field is // REQUIRED. - .google.protobuf.Duration ttl_seconds = 3; + int32 ttl_seconds = 3; } ``` @@ -2128,8 +2134,8 @@ gRPC error code. | Condition | gRPC Code | Description | Recovery Behavior | |-----------|-----------|-------------|-------------------| -| Target volume snapshot does not exist | 5 NOT_FOUND | Indicates that the target volume snapshot corresponding to the specified `snapshot_target_id` does not exist. | Caller MUST verify that the `snapshot_target_id` is correct and that the volume snapshot is accessible and has not been deleted before retrying with exponential back off. | -| Invalid `starting_token` | 10 ABORTED | Indicates that `starting_token` is not valid. | Caller SHOULD start the `ListVolumeGroupSnapshots` operation again with an empty `starting_token`. | +| Target volume snapshot does not exist | 5 NOT_FOUND | Indicates that the target volume snapshot corresponding to the specified `to_snapshot_id` does not exist. | Caller MUST verify that the `to_snapshot_id` is correct and that the volume snapshot is accessible and has not been deleted before retrying with exponential back off. | +| Invalid `starting_token` | 10 ABORTED | Indicates that `starting_token` is not valid. | Caller SHOULD start the `ListSnapshotDeltas` operation again with an empty `starting_token`. | #### `ControllerExpandVolume` From 27755844969e9c4d6d636dd809a6add2895d8992 Mon Sep 17 00:00:00 2001 From: Ivan Sim Date: Wed, 21 Sep 2022 09:43:25 -0700 Subject: [PATCH 3/5] Remove 'mode' enum from CBT RPC Signed-off-by: Ivan Sim --- csi.proto | 12 ------------ spec.md | 12 ------------ 2 files changed, 24 deletions(-) diff --git a/csi.proto b/csi.proto index 853734a8..06e17ea0 100644 --- a/csi.proto +++ b/csi.proto @@ -1251,18 +1251,6 @@ message ListSnapshotDeltasRequest { // not specified, an error is returned. This field is REQUIRED. string to_snapshot_id = 2; - // Defines the type of storage. Default to "BLOCK". This field is - // REQUIRED. - enum Mode { - option (alpha_enum) = true; - - // BLOCK indicates that the snapshot is of block type. - BLOCK = 0 [(alpha_enum_value) = true]; - - // FILE indicates that the snapshot is of file type. - FILE = 1 [(alpha_enum_value) = true]; - } - // If specified (non-zero value), the Plugin MUST NOT return more // entries than this number in the response. If the actual number of // entries is more than this number, the Plugin MUST set `next_token` diff --git a/spec.md b/spec.md index 8036af4a..cefcee19 100644 --- a/spec.md +++ b/spec.md @@ -2041,18 +2041,6 @@ message ListSnapshotDeltasRequest { // not specified, an error is returned. This field is REQUIRED. string to_snapshot_id = 2; - // Defines the type of storage. Default to "BLOCK". This field is - // REQUIRED. - enum Mode { - option (alpha_enum) = true; - - // BLOCK indicates that the snapshot is of block type. - BLOCK = 0 [(alpha_enum_value) = true]; - - // FILE indicates that the snapshot is of file type. - FILE = 1 [(alpha_enum_value) = true]; - } - // If specified (non-zero value), the Plugin MUST NOT return more // entries than this number in the response. If the actual number of // entries is more than this number, the Plugin MUST set `next_token` From e2b4f8d44ce4918bb57dd970e71da5ccb263c214 Mon Sep 17 00:00:00 2001 From: Ivan Sim Date: Wed, 21 Sep 2022 09:57:51 -0700 Subject: [PATCH 4/5] Add secretRefs to ListSnapshotDeltasRequest message Signed-off-by: Ivan Sim --- csi.proto | 6 ++++++ spec.md | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/csi.proto b/csi.proto index 06e17ea0..64e77d42 100644 --- a/csi.proto +++ b/csi.proto @@ -1251,6 +1251,12 @@ message ListSnapshotDeltasRequest { // not specified, an error is returned. This field is REQUIRED. string to_snapshot_id = 2; + // Secrets required by plugin to complete list snapshot deltas + // request. + // This field is OPTIONAL. Refer to the `Secrets Requirements` + // section on how to use this field. + map secrets = 3 [(csi_secret) = true]; + // If specified (non-zero value), the Plugin MUST NOT return more // entries than this number in the response. If the actual number of // entries is more than this number, the Plugin MUST set `next_token` diff --git a/spec.md b/spec.md index cefcee19..80915c52 100644 --- a/spec.md +++ b/spec.md @@ -2041,6 +2041,12 @@ message ListSnapshotDeltasRequest { // not specified, an error is returned. This field is REQUIRED. string to_snapshot_id = 2; + // Secrets required by plugin to complete list snapshot deltas + // request. + // This field is OPTIONAL. Refer to the `Secrets Requirements` + // section on how to use this field. + map secrets = 3 [(csi_secret) = true]; + // If specified (non-zero value), the Plugin MUST NOT return more // entries than this number in the response. If the actual number of // entries is more than this number, the Plugin MUST set `next_token` From 4fea03d0f4c5727ffebc5fe6f91cfb09acc9aed0 Mon Sep 17 00:00:00 2001 From: Ivan Sim Date: Tue, 4 Oct 2022 14:34:56 -0700 Subject: [PATCH 5/5] Specify unit of 'offset' field Signed-off-by: Ivan Sim --- csi.proto | 5 +++-- spec.md | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/csi.proto b/csi.proto index 64e77d42..cc256266 100644 --- a/csi.proto +++ b/csi.proto @@ -1298,8 +1298,9 @@ message ListSnapshotDeltasResponse { message BlockSnapshotChangedBlock { option (alpha_message) = true; - // The block logical offset on the volume. This field is REQUIRED. - uint64 offset = 1; + // The block logical offset (in bytes) on the volume. This field is + // REQUIRED. + uint64 offset_bytes = 1; // The size of the block in bytes. This field is REQUIRED. uint64 block_size_bytes = 2; diff --git a/spec.md b/spec.md index 80915c52..ade8b760 100644 --- a/spec.md +++ b/spec.md @@ -2088,8 +2088,9 @@ message ListSnapshotDeltasResponse { message BlockSnapshotChangedBlock { option (alpha_message) = true; - // The block logical offset on the volume. This field is REQUIRED. - uint64 offset = 1; + // The block logical offset (in bytes) on the volume. This field is + // REQUIRED. + uint64 offset_bytes = 1; // The size of the block in bytes. This field is REQUIRED. uint64 block_size_bytes = 2;