search

container-storage-interface / spec

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

Volume expansion proposal #334

Closed gnufied closed 5 years ago

gnufied commented 5 years ago

Version of https://github.com/container-storage-interface/spec/pull/222

The original PR was timing out while loading.

jdef commented 5 years ago

@saad-ali @jieyu @julian-hj @ddebroy PTAL

gnufied commented 5 years ago

@jdef I am going to squash these commits while rebasing if everything looks okay. cheers.

saad-ali commented 5 years ago

@julian-hj @ddebroy Will merge after your LGTM

gnufied commented 5 years ago

@julian-hj can you take another look?

julian-hj commented 5 years ago

@gnufied hmm. To me, it would be natural to assume that OFFLINE as a plugin capability means that the volume must be offline in order to be expanded, particularly since OFFLINE is a plugin capability, not a controller capability. So I think the proposed spec is still fairly confusing.

If it is acceptable for a CO to call NodeExpandVolume when a plugin reports OFFLINE capability, then I think we need to say that explicitly in the description of OFFLINE. Or if ONLINE/OFFLINE is really only about ControllerExpandVolume behavior, then probably it should be moved to ControllerGetCapabilities rather than GetPluginCapabilities.

gnufied commented 5 years ago

A OFFLINE plugin capability can't mean that - volume has to be offline on the node for NodeExpandVolume too. That was ruled out because of say - supporting XFS file system on Azure, which requires volume to be mounted for file system expansion but volume must be OFFLINE for controller expansion. So in my view - applying one value of online and offline to both controller and node operation isn't going to work and hence this proposal simplifies the design by making node side operation always online.

It is true to some extent that, may be then we should not use plugin capabilities to define this property and stick this with controller capabilities. But - there are two problems with that:

  1. Controller capabilities currently define which RPC call the volume plugin supports. Not which RPC call can be made depending on the state of the volume. That is not what controller capabilities are used for (but may be I am wrong?)
  2. There is another use case that this proposal covers is, online node only expansion. A plugin may not implement any control plane call at all and as long as it declares ONLINE capability, volume expansion operation could be done in node side. The language of ONLINE capability allows that. This case will not be adequately covered by moving online and offline capabilities to controller capabilities or we will have to come up with another way of finding out volume expansion capabilities.
julian-hj commented 5 years ago

@gnufied @jdef OK, so if I am understanding things, then what you've said isn't consistent. Earlier, you both agreed that OFFLINE and ONLINE only concerns ControllerExpandVolume but just now, you're saying that OFFLINE applies to NodeExpandVolume too. But that isn't called out in the spec, which is what I was trying to say in my first comment.

So...would it be accurate to say that NodeExpandVolume cannot be called unless the plugin advertises ONLINE capability? If so, then I think we should say that somewhere. If not, then I think the relationship between OFFLINE/ONLINE and the node RPC still needs to be clarified.

gnufied commented 5 years ago

Earlier, you both agreed that OFFLINE and ONLINE only concerns ControllerExpandVolume but just now, you're saying that OFFLINE applies to NodeExpandVolume too.

What you mean by "applies"? I think you meant - even if plugin has only OFFLINE capability it still may require a call of NodeExpandVolume on the node? If yes - that is true. But what I meant earlier is - ONLINE and OFFLINE capability only control whether ControllerExpandVolume call can be made with published volume or not. These capabilities as currently defined do not prescribe whether NodeExpandVolume call can be made when volume is online/offline, because NodeExpandVolume always requires the volume to be published(and hence online) to a node.

Or to rephrase - the capabilities don't say anything at all about when NodeExpandVolume call can be made. So these capabilities and NodeExpandVolume call are not related at all.

So...would it be accurate to say that NodeExpandVolume cannot be called unless the plugin advertises ONLINE capability? If so, then I think we should say that somewhere. I

I think that won't be accurate to say. Using example I gave previously. Using XFS file system on a Azure Disk volume. The plugin will declare expansion capabilities as OFFLINE. Which means - ControllerExpandVolume can only be called when volume is not published to a node. But NodeExpandVolume MUST be called when volume is published to the node. Because a XFS file system can only be expanded when mounted on a node. So - OFFLINE capability only applies to RPC ControllerExpandVolume. NodeExpandVolume does not support offline mode of file system expansion at all. Volume must be published/mounted for file system expansion on the node.