search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.34k stars 373 forks source link

Replace `MAY NOT` in spec with clearer language #343

Open gman0 opened 5 years ago

gman0 commented 5 years ago

As discussed offline, there's an inconsistency in the spec whether name == volume_id is possible:

message CreateVolumeRequest {
  ...
  // 2) Suggested name - Some storage systems allow callers to specify
  //    an identifier by which to refer to the newly provisioned
  //    storage. If a storage system supports this, it can optionally
  //    use this name as the identifier for the new volume.
  string name = 1;
  ...

This paragraph is making it sort-of clear that the CO-chosen name may be used as the volume ID. Also, if this is true maybe it could also mention that the volume name generated by CO has to be unique as well - or perhaps that's implied by saying that it can be used as an identifier?

But then in the RPC Interactions section it says:

It is worth noting that the plugin-generated volume_id is a REQUIRED field for the DeleteVolume RPC, as opposed to the CO-generated volume name that is REQUIRED for the CreateVolume RPC: these fields MAY NOT contain the same value.

Which is inconsistent with the comment in the CreateVolumeRequest.

gman0 commented 5 years ago

cc @saad-ali

jdef commented 5 years ago

"MAY NOT" is RFC language that means "the plugin-generated volume_id may or may not contain the same value as the CO-generated volume name"

On Tue, Dec 11, 2018 at 5:29 AM Róbert Vašek notifications@github.com wrote:

cc @saad-ali https://github.com/saad-ali

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/343#issuecomment-446151901, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLKRLEjIHQshgjRy-BkZ8SmDKJmdnks5u34kKgaJpZM4ZNAOE .

saad-ali commented 5 years ago

"MAY NOT" is RFC language that means "the plugin-generated volume_id may or may not contain the same value as the CO-generated volume name"

That might be part of the confusion. MAY NOT is not one of the keywords defined in RFC 2119:

The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" are to be interpreted as described in RFC 2119 (Bradner, S., "Key words for use in RFCs to Indicate Requirement Levels", BCP 14, RFC 2119, March 1997).

In this case maybe these fields may or may not contain the same value might be clearer?

We should also scrub the spec for other instances of MAY NOT and replace them with an appropriate RFC 2119 keyword or other language to ensure they are clear.

jdef commented 5 years ago

Good catch, Saad. Agreed, we should scrub,

On Fri, Dec 21, 2018 at 5:17 PM Saad Ali notifications@github.com wrote:

"MAY NOT" is RFC language that means "the plugin-generated volume_id may or may not contain the same value as the CO-generated volume name"

That might be part of the confusion. MAY NOT is not one of the keywords defined in RFC 2119 http://tools.ietf.org/html/rfc2119:

The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" are to be interpreted as described in RFC 2119 http://tools.ietf.org/html/rfc2119 (Bradner, S., "Key words for use in RFCs to Indicate Requirement Levels", BCP 14, RFC 2119, March 1997).

In this case maybe these fields may or may not contain the same value might be clearer?

We should also scrub the spec for other instances of MAY NOT and replace them with an appropriate RFC 2119 keyword or other language to ensure they are clear.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/343#issuecomment-449512139, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLNRN0SB7rfPzDrDgn2TIPUFEUxFLks5u7V4EgaJpZM4ZNAOE .