Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeStageVolume/NodeUnstageVolume #169

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 99 additions & 6 deletions csi.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ service Controller {
}

service Node {
rpc NodeStageVolume (NodeStageVolumeRequest)
returns (NodeStageVolumeResponse) {}

rpc NodeUnstageVolume (NodeUnstageVolumeRequest)
returns (NodeUnstageVolumeResponse) {}

rpc NodePublishVolume (NodePublishVolumeRequest)
returns (NodePublishVolumeResponse) {}

Expand Down Expand Up @@ -371,7 +377,8 @@ message ControllerPublishVolumeRequest {

message ControllerPublishVolumeResponse {
// The SP specific information that will be passed to the Plugin in
// the subsequent `NodePublishVolume` call for the given volume.
// the subsequent `NodeStageVolume` or `NodePublishVolume` calls
// for the given volume.
// This information is opaque to the CO. This field is OPTIONAL.
map<string, string> publish_info = 1;
}
Expand Down Expand Up @@ -544,6 +551,82 @@ message ControllerServiceCapability {
}
////////
////////
message NodeStageVolumeRequest {
// The API version assumed by the CO. This is a REQUIRED field.
Version version = 1;

// The ID of the volume to publish. This field is REQUIRED.
string volume_id = 2;

// The CO SHALL set this field to the value returned by
// `ControllerPublishVolume` if the corresponding Controller Plugin
// has `PUBLISH_UNPUBLISH_VOLUME` controller capability, and SHALL be
// left unset if the corresponding Controller Plugin does not have
// this capability. This is an OPTIONAL field.
map<string, string> publish_info = 3;

// The path to which the volume will be published. It MUST be an
// absolute path in the root filesystem of the process serving this
// request. The CO SHALL ensure that there is only one
// staging_target_path per volume.
// This is a REQUIRED field.
string staging_target_path = 4;

// The capability of the volume the CO expects the volume to have.
// This is a REQUIRED field.
VolumeCapability volume_capability = 5;

// Credentials used by Node plugin to authenticate/authorize node
// stage request.
// This field contains credential data, for example username and
// password. Each key must consist of alphanumeric characters, '-',
// '_' or '.'. Each value MUST contain a valid string. An SP MAY
// choose to accept binary (non-string) data by using a binary-to-text
// encoding scheme, like base64. An SP SHALL advertise the
// requirements for credentials in documentation. COs SHALL permit
// passing through the required credentials. This information is
// sensitive and MUST be treated as such (not logged, etc.) by the CO.
// This field is OPTIONAL.
map<string, string> node_stage_credentials = 6;

// Attributes of the volume to publish. This field is OPTIONAL and
// MUST match the attributes of the VolumeInfo identified by
// `volume_id`.
map<string,string> volume_attributes = 7;
}

message NodeStageVolumeResponse {}
////////
////////
message NodeUnstageVolumeRequest {
// The API version assumed by the CO. This is a REQUIRED field.
Version version = 1;

// The ID of the volume. This field is REQUIRED.
string volume_id = 2;

// The path at which the volume was published. It MUST be an absolute
// path in the root filesystem of the process serving this request.
// This is a REQUIRED field.
string staging_target_path = 3;

// Credentials used by Node plugin to authenticate/authorize node
// unstage request.
// This field contains credential data, for example username and
// password. Each key must consist of alphanumeric characters, '-',
// '_' or '.'. Each value MUST contain a valid string. An SP MAY
// choose to accept binary (non-string) data by using a binary-to-text
// encoding scheme, like base64. An SP SHALL advertise the
// requirements for credentials in documentation. COs SHALL permit
// passing through the required credentials. This information is
// sensitive and MUST be treated as such (not logged, etc.) by the CO.
// This field is OPTIONAL.
map<string, string> node_unstage_credentials = 4;
}

message NodeUnstageVolumeResponse {}
////////
////////
message NodePublishVolumeRequest {
// The API version assumed by the CO. This is a REQUIRED field.
Version version = 1;
Expand All @@ -558,21 +641,29 @@ message NodePublishVolumeRequest {
// this capability. This is an OPTIONAL field.
map<string, string> publish_info = 3;

// The path to which the device was mounted by `NodeStageVolume`.
// It MUST be an absolute path in the root filesystem of the process
// serving this request.
// It MUST be set if the Node Plugin implements the
// `STAGE_UNSTAGE_VOLUME` node capability.
// This is an OPTIONAL field.
string staging_target_path = 4;

// The path to which the volume will be published. It MUST be an
// absolute path in the root filesystem of the process serving this
// request. The CO SHALL ensure uniqueness of target_path per volume.
// The CO SHALL ensure that the path exists, and that the process
// serving the request has `read` and `write` permissions to the path.
// This is a REQUIRED field.
string target_path = 4;
string target_path = 5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we avoid renumbering proto fields now that we have a tagged/published version of the spec in the world?

Copy link
Contributor Author

@davidz627 davidz627 Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding that this should be an OK change, it will introduce incompatablity between different CSI spec versions but I think thats OK. Thats why we have a supported_versions field right?

The main issue with changing proto tags is when the protos are actually persisted somewhere and new code with different tags comes along and reads the old protos and gets confused. I don't believe that is an issue here.

@saad-ali may be able to provide more context and a more definitive answer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client/server mismatch of proto field number can cause issues. However, for v0.2, we should be ok making breaking changes, if needed.


// The capability of the volume the CO expects the volume to have.
// This is a REQUIRED field.
VolumeCapability volume_capability = 5;
VolumeCapability volume_capability = 6;

// Whether to publish the volume in readonly mode. This field is
// REQUIRED.
bool readonly = 6;
bool readonly = 7;

// Credentials used by Node plugin to authenticate/authorize node
// publish request.
Expand All @@ -585,12 +676,13 @@ message NodePublishVolumeRequest {
// passing through the required credentials. This information is
// sensitive and MUST be treated as such (not logged, etc.) by the CO.
// This field is OPTIONAL.
map<string, string> node_publish_credentials = 7;
map<string, string> node_publish_credentials = 8;


// Attributes of the volume to publish. This field is OPTIONAL and
// MUST match the attributes of the Volume identified by
// `volume_id`.
map<string,string> volume_attributes = 8;
map<string,string> volume_attributes = 9;
}

message NodePublishVolumeResponse {}
Expand Down Expand Up @@ -662,6 +754,7 @@ message NodeServiceCapability {
message RPC {
enum Type {
UNKNOWN = 0;
STAGE_UNSTAGE_VOLUME = 1;
}

Type type = 1;
Expand Down
Loading