container-storage-interface / spec

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

Clarify NodeStageVolume for block volumes #361

Closed codenrhoden closed 5 years ago

codenrhoden commented 5 years ago

I've hit some contradictions in the spec when implementing a plugin that supports both BlockVolume and MountVolume capabilities, and also implements NodeStageVolume.

According to the spec, if a node service advertises the STAGE_UNSTAGE_VOLUME capabilities, then NodeStageVolume must be called once before NodePublishVolume. The NodeStageVolume RPC has this for staging_target_path:

  // The path to which the volume MAY be staged. It MUST be an
  // absolute path in the root filesystem of the process serving this
  // request, and MUST be a directory. The CO SHALL ensure that there
  // is only one `staging_target_path` per volume. The CO SHALL ensure
  // that the path is directory and that the process serving the
  // request has `read` and `write` permission to that directory. The
  // CO SHALL be responsible for creating the directory if it does not
  // exist.
  // This is a REQUIRED field.
  string staging_target_path = 3;

The path must be a directory. Which doesn't work for block. if we wanted to stage a block device, we could stage it over a file but not to a directory.

I see a few solutions here: (1) staging_target_path is only required when the volume is Mount and not Block, therefore the CO could leave it blank (2) clarify that NodeStageVolume is only required when the volume Mount and not Block allowing the CO to skip it entirely (3) Since the spec actually says that target path is the path where the volume MAY be staged, take advantage of that and consider this call a noop for block volumes.

Is option 3 the intent of the spec? That's how I'm going to have to implement it for now. If that's the intent, I think some clarifying language around that would be helpful.

julian-hj commented 5 years ago

IIRC, we discussed this issue in one of the community meetings a few months back. The idea was that the CO gives the plugin a directory, and the plugin does whatever it likes with that directory.

So option #4 would be that the plugin creates a file within the directory, and stages the block device there, but option #3 is also totally acceptable.

codenrhoden commented 5 years ago

Didn't know I still had this open, but thanks for the feedback @julian-hj, that makes sense.