Add the ability to use exclusive RBD locking to prevent inadvertent multi-node access of an RWO image

[ShyamsundarR]

Although CO systems are meant to ensure an RWO volume is attached to a single node, under any (un)availability constraints. It seems prudent to have an additional layer of protection to ensure this is taken care of.

Quoting solutions presented by @dillaman here:
"The best solution is to ensure that k8s will never request the CSI to map a PV to a node when it could already be mapped on an unresponsive node. Barring that, the CSI should enable the exclusive-lock feature on RBD images for RWO volumes at provision time. At attach time, it should break any existing locks prior to mapping via "rbd lock break" (which will blacklist the previous lock owner from the storage cluster), and the map operation should use "rbd device map --exclusive" to ensure that it immediately acquires the lock and will refuse to release it until unmapped / blacklisted."

We would need to analyze what this entails in a CO environment, and what workflows may be needed to remove nodes from a black list (if at all needed).

[dillaman]

I want to retract the possibility of using "rbd device map --exclusive". The issue being that when that option is used, krbd will never release the lock. Normally that is exactly what you'd want, except for the case where you are trying to perform some maintenance operation on the image (snapshot create/remove, flatten, ...).

[Madhu-1]

we need to do below steps
During create

• add --exclusive-lock feature while creating an image if its RWO
  During node stage
• use rbd map --exclusive
  During unstage
• list the lock on image
• remove the lock
• unmap the rbd device

@dillaman @ShyamsundarR correct me if am wrong

@dillaman to remove the lock we need to connect the cluster right, means we need mon endpoints, admin ID and credentials ( is it possible to remove the lock without that)

as we do map --exclusive we cannot take a snapshot, right? is this expected

[dillaman]

we need to do below steps
During create

* add `--exclusive-lock` feature while creating an image if its RWO
  During node stage

It's --image-feature layering,exclusive-lock,...... (there is no --exclusive-lock optional)

* use `rbd map --exclusive`

Negative -- that was my retraction above. If you use this, you will break the ability to have third-party tools (i.e. rbd snap create) function against in-use images.

  During unstage

* list the lock on image

* remove the lock

* unmap the rbd device

Negative -- on a normal unstage, if you were using exclusive-lock, it will automatically clean up the lock. You would only need to break a lock on node stage if it's still flagged as in-use on another node. But, using exclusive-lock in such a way gets back to the point above ...

@dillaman to remove the lock we need to connect the cluster right, means we need mon endpoints, admin ID and credentials ( is it possible to remove the lock without that)

You need to talk to the cluster, so you need all the required details to talk to the cluster. Again, not sure it matters much since (1) you wouldn't be doing this during node unstage, and (2) see above ...

as we do map --exclusive we cannot take a snapshot, right? is this expected

It's a limitation of the fact that krbd doesn't perform any maintenance ops. If you used rbd-nbd --exclusive, you would still be able to create snapshots because it would handle the request from a remote rbd client.

[humblec]

This is an important item for v2.0.0 ( #557 ) and would like to solve it at this release. @ShyamsundarR @Madhu-1 @dillaman any volunteers for this task?

[dillaman]

@humblec There is currently no agreed path forward on this issue. The RBD exclusive-lock feature cannot be used and the CSI RPCs do not provide the necessary credentials to provide a manual workaround.

[humblec]

@humblec There is currently no agreed path forward on this issue. The RBD exclusive-lock feature cannot be used and the CSI RPCs do not provide the necessary credentials to provide a manual workaround.

@dillaman true, I am aware of the conclusion we made with our last discussions on this topic. However, trying to figure out what can be done here to reach to a (new) solution/workaround, be it by adding support from RBD core side or by trying possibilities from other layers. I was told that, we have to bring this extra safety measure to be assured that, we dont end up in multi access and thus its consequences.

@mykaul jfyi ^^

[dillaman]

Agreed it's a great safety feature -- but it will require a CSI spec and/or external-provisioner sidecar change. There is no workaround possible w/o the credentials. The timeline for getting mirroring to work w/ the external-provisioner is several releases out per the downstream team.

[Madhu-1]

@dillaman @humblec if possible can we create an issue in CSI spec for the changes required? what changes did we require in the external provisioner sidecar?

[humblec]

@dillaman sure, we can poke upstream for the changes in external provisioner sidecar and try our best to get it in . But we have to list it out the same.

@Madhu-1 this issue track it in CSI repo, we need to come up with steps we are missing to achieve it here.
Also need a volunteer 👍

[dillaman]

@dillaman sure, we can poke upstream for the changes in external provisioner sidecar and try our best to get it in . But we have to list it out the same.

It seems silly to me to tag a ceph-csi issue against v2 and priority 0 when it will 100% not land in v2 due to upstream dependencies. I think the best you can do is have an issue against the spec and/or CSI external provisioner limitation just to track the the opening of an issue against the other external components. Actually adding support for exclusive locking is something that cannot be scheduled until after the support is merged into the upstream components.

[ShyamsundarR]

The solution to use rbd lock add ... commands is not quite correct, as it provides only an advisory lock, and further is equivalent to setting an annotation on the image that we do not control.

Instead, it is was discussed that we can add our own annotation and ensure this is set, before mapping the image on the node. This will be done in NodeStage (and hence undone in NodeUnstage), to ensure we are not caught in any other kubernetes/CO related bugs (which is what this issue is about).

Performing the above in ControllerPublish/Unpublish defeats the purpose, because if the CO decides its state is correct, i.e there are no nodes that have the image mapped, when there is actually an active image map on a node, the CO would simply invoke ControllerUnpublish to remove the older annotation and call ControllerPublish add a new one causing the issue that led to this problem in the first place.

The order of operations would look like:

• NodeStage:

  1. Check if there is an existing annotation and it is not the current node
  2. If present and not the current node, error out [A]
  3. If not present, add an annotation with the required node details
  4. Read and check if the annotation is what was added (prevent races from parallel Staging operation on other nodes that may have added an annotation at the same time)
     • If the annotation is different (last writer won), error out [A]
  5. If annotation has been successfully set, proceed to map [B]

• NodeUnstage:

  • (simplified) Check and remove the annotation added [C]

[A] If a node goes down without removing an annotation, then the cluster/storage/CSI admin would need to remove the annotation to clear the state, hence this solution does not provide auto recovery or connection fencing.

Connection fencing is not chosen, as all images mapped on a node uses a common connection, for efficiency of time (connection establishment) and resources (memory footprint on the node) reasons, and hence fencing one will fence all on the node, which would be an bigger issue to deal with.

[B] There is a race between steps 1 and 5, when 2 clients can pass the no annotation set check (as they read an empty key), and proceed with steps 3 and 4, ending up with both clients writing their node details and reading it back assuming they are the sole owners. This needs more thought.

[C] Currently NodeUnstage does not pass in secrets and other required volume attributes. There is some reasoning as to why in these issues,

• https://github.com/container-storage-interface/spec/issues/198
• https://github.com/container-storage-interface/spec/issues/354
  but, once we clear up [A] and [B] and other assumptions in here (such as we will not use node nonce based fencing for sure), we may need to revisit [C] with the CSI spec project.