Skip to content
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

WIP node server: don't persist vol #25

Merged
merged 1 commit into from
Mar 7, 2018
Merged

WIP node server: don't persist vol #25

merged 1 commit into from
Mar 7, 2018

Conversation

rootfs
Copy link
Member

@rootfs rootfs commented Feb 20, 2018

Signed-off-by: Huamin Chen <hchen@redhat.com>
@sbezverk
Copy link
Contributor

@rootfs Create and Delete Volume gets from CSI volume name, but Node Publish/Unpublish operates with Volume IDs, if we do not persist volume information how you can build relation then? Also CSI spec expects in case of a plugin/kubelet failure to be able to recover previously created volumes, without persistent info, how it can be done? Even in k8s CSI core side, there is a persistent storage. Another thing about CSI RPC LIST_VOLUMES, with the persistent info it is fiarly easy to do.

@rootfs
Copy link
Member Author

rootfs commented Feb 20, 2018

controller still keeps the persistent json, but node doesn't need it. Node can figure out what device to unmap from mountpath.

@sbezverk
Copy link
Contributor

@rootfs still how about the recovery scenario? For un-publish you get target_path and volume id, to get device for rbd unmap you will need to do an expensive search(on prod cluster there might be lots of mounted devices), instead of just a quick search in the persistent storage file.

@rootfs
Copy link
Member Author

rootfs commented Feb 20, 2018

@sbezverk searching device path is what kubernetes rbd/iscsi do in this case too.

@sbezverk
Copy link
Contributor

@rootfs right, I know and I heard negative feedback of doing so, I do not see the optimization of going back to this way. Appreciate if you could provide the benefits.

@rootfs
Copy link
Member Author

rootfs commented Feb 20, 2018

@sbezverk we have to parse /proc/mount in either case - rbd images can only be safely detached if there is no more mountpoint.

@sbezverk
Copy link
Contributor

sbezverk commented Mar 7, 2018

/lgtm

@sbezverk sbezverk merged commit 4b3ebc1 into ceph:master Mar 7, 2018
wilmardo pushed a commit to wilmardo/ceph-csi that referenced this pull request Jul 29, 2019
WIP node server: don't persist vol
nixpanic added a commit to nixpanic/ceph-csi that referenced this pull request Jun 22, 2021
…k-23-to-release-4.7

[release-4.7] ci: add OWNERS file to the repo
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

Successfully merging this pull request may close these issues.

2 participants