Add capability for ControllerPublishVolume.readonly field
#310
Conversation
|
Will change this per #293 (comment) |
saad-ali
force-pushed
the
readonlyOptional
branch
from
bb04428
to
fbebc7a
Compare
saad-ali
changed the title
ControllerPublishVolume.readonly field
|
@julian-hj @jieyu per our discussion, I changed this from a comment to a new capability. |
| // REQUIRED. | ||
| // Whether to publish the volume in readonly mode. This field MUST be | ||
| // specified only if SP has PUBLISH_READONLY controller capability. | ||
| // This is a REQUIRED field. | ||
| bool readonly = 4; |
There was a problem hiding this comment.
Is making readonly flag optional simpler?
There was a problem hiding this comment.
Problem with making it optional is that the CO doesn't know when to set it. If CO sets it to true and a plugin doesn't support it, what happens? Consider the case where the CO uses the same flag for both ControllerPublish and NodePublish readonly -- we don't want to get in to a case where the CO thinks a volume is readonly but it's not.
csi.proto
Outdated
| // Whether to publish the volume in readonly mode. This field is | ||
| // REQUIRED. | ||
| // Whether to publish the volume in readonly mode. This field MUST be | ||
| // specified only if SP has PUBLISH_READONLY controller capability. |
There was a problem hiding this comment.
This field MUST be REQUIRED specified only if conflicts with This is a REQUIRED field.
We should say that if the SP does not have PUBLISH_READONLY capability, the CO will set this field to false.
saad-ali
force-pushed
the
readonlyOptional
branch
2 times, most recently
from
b1efdca
to
1f2ff53
Compare
|
Feedback addressed PTAL @jieyu |
saad-ali
force-pushed
the
readonlyOptional
branch
from
1f2ff53
to
d7df952
Compare
Closes #293