Skip to content

Clarify NodeStageVolume for block volumes #361

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

Closed
codenrhoden opened this issue Mar 21, 2019 · 2 comments
Closed

Clarify NodeStageVolume for block volumes #361

codenrhoden opened this issue Mar 21, 2019 · 2 comments

Comments

@codenrhoden
Copy link

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
Copy link
Contributor

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants