Implement (un)stage/(un)publish volume according csi spec for mount mode #2195

antonmyagkov · 2024-10-01T18:49:20Z

Problem:

CSI Driver implementation violates CSI specification in terms of stage/publish/unstage/unpublish volumes.
At this moment StageVolume step is completely ignored and start endpoint/mounting volumes happens at PublishVolume step. As a result CSI Driver doesn't support ReadWriteOnce access mode in the correct way and only one pod on the same node can mount the volume, however it should be allowed to mount the same volume into multiple pods on the same node.

According to CSI Driver specification:

NodeStageVolume should mount the volume to the staging path
NodePublishVolume should mount the volume to the target path
NodeUnpublishVolume should unmount the volume from the target path
NodeUnstageVolume should unmount the volume from the staging path

As we already have current implementation of CSI Driver in production clusters we need to handle migration from existing implementation of mounting volumes(only NodePublishVolume/NodeUnpublishVolume is implemented) to the new implementation.

The tricky part here is using different UnixSocketPath/InstanceId/ClientId for already bounded volumes and "new" volumes.
Current format of UnixSocketPath: socketsDir/podId/volumeId
New format of UnixSocketPath: socketsDir/nodeId/volumeId

Current format of InstanceId: podId
New format of InstanceId: nodeId

Current format of ClientId: clientID-podId
New format of ClientId: clientID-nodeId

Possible scenarios:

Volume was staged and published
CSI Driver was updated
Volume was unpublished and unstaged <- here we should handle unpublish with old unix socket path

Volume was staged and published
CSI Driver was updated
Kubelet restart had happened
CSI Driver received stage and publish for the same volume again <- here we should handle stage/publish with old unix socket path

CSI Driver was updated
Volume was staged and published
endpoint should be start with new unix socket path
Volume was unpublished and unstaged
UnstageVolume should stop endpoint with new unix socket path

CSI Driver was updated
Volume was staged and published on the node1 with RWO access mode
Staging volume on the node2
StageVolume on the node2 should return error

Migration is splitted for differnt modes
VM mode: #1982
Mount mode: #2195
Block mode: #2269

After migration of all volumes to the new endpoints we can remove backward compatibility
with old format of endpoints.

External links/documentation

https://github.com/container-storage-interface/spec/blob/master/spec.md#node-service-rpc

github-actions · 2024-10-01T20:09:26Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 2ad8e33.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5856	5844	0	8	4	0

github-actions · 2024-10-02T12:05:08Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 2e70895.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5856	5845	0	7	4	0

cloud/blockstore/tools/csi_driver/internal/driver/node.go

github-actions · 2024-10-02T23:28:05Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit c3f12f1.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5856	5848	0	4	4	0

cloud/blockstore/tools/csi_driver/internal/driver/node.go

github-actions · 2024-10-03T15:01:21Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit e414b69.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5856	5848	0	4	4	0

github-actions · 2024-10-04T12:58:47Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 83eece4.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3296	3294	0	2	0	0

aikuchin

Я вижу потенциальную проблему с рестартом кублета при переезде на новую схему: когда он рестартует то восстанавливает картину мира, полагаясь на идемпотентность запросов - просто повторно посылает NodeStage и NodePublish всем вольюмам, которые по его мнению должны быть замонтированы.
Если диск был подключен со старым драйвером Stage попробует стартовать эндпоинт по новому пути и я предполагаю что возникнет конфликт (если NBS в этом месте не останавливает автоматически старый эндпоинт).
В случае ВМ у нас в качесте признака что эндпоинт стартует на Stage является атрибут instanceId, для инфракубера я не вижу как можно отличить старый диск от нового.

antonmyagkov · 2024-10-07T10:37:20Z

Я вижу потенциальную проблему с рестартом кублета при переезде на новую схему: когда он рестартует то восстанавливает картину мира, полагаясь на идемпотентность запросов - просто повторно посылает NodeStage и NodePublish всем вольюмам, которые по его мнению должны быть замонтированы. Если диск был подключен со старым драйвером Stage попробует стартовать эндпоинт по новому пути и я предполагаю что возникнет конфликт (если NBS в этом месте не останавливает автоматически старый эндпоинт). В случае ВМ у нас в качесте признака что эндпоинт стартует на Stage является атрибут instanceId, для инфракубера я не вижу как можно отличить старый диск от нового.

Можно попробовать так:
Делаем StartEndpoint по новому формату эндпоинта. В случае ошибки mount conflict вернуть ОК и уже на publish примонтироваться по старому формату эндпоинта или вернуть ошибку в случае повторного mount conflict.

github-actions · 2024-10-07T10:48:37Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit d7dafeb.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3303	3301	0	2	0	0

cloud/blockstore/tools/csi_driver/internal/driver/node.go

github-actions · 2024-10-08T17:56:28Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 5496f67.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3307	3300	0	3	4	0

github-actions · 2024-10-09T17:42:23Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 9b3dc89.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3308	3301	0	3	4	0

github-actions · 2024-10-10T13:59:10Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit cb4f860.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3307	3306	0	1	0	0

…fication

github-actions · 2024-10-11T12:01:37Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 57f3167.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

cloud/blockstore/tools/csi_driver/stage-publish-unpublish-unstage-flow.md

cloud/blockstore/tools/csi_driver/internal/driver/node.go

github-actions · 2024-10-11T15:58:27Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 1d88142.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

github-actions · 2024-10-11T16:51:06Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e924066.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

github-actions · 2024-10-13T22:14:24Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 4f7f5c4.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

github-actions · 2024-10-14T12:24:59Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 20b6040.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

github-actions · 2024-10-15T08:47:50Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 93ca36c.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3314	0	1	0	0

github-actions · 2024-10-15T10:42:23Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 520bc78.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3314	0	1	0	0

github-actions · 2024-10-15T11:53:25Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 520bc78.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3314	0	1	0	0

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 520bc78.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

github-actions · 2024-10-16T11:20:14Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 9191b39.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
3315	3315	0	0	0	0

antonmyagkov force-pushed the users/myagkov/csi-fix-stage-publish-steps branch from 62d8f3b to 2e70895 Compare October 2, 2024 10:41

antonmyagkov commented Oct 2, 2024

View reviewed changes

cloud/blockstore/tools/csi_driver/internal/driver/node.go Outdated Show resolved Hide resolved

tpashkin reviewed Oct 3, 2024

View reviewed changes

antonmyagkov added the blockstore Add this label to run only cloud/blockstore build and tests on PR label Oct 4, 2024

aikuchin reviewed Oct 5, 2024

View reviewed changes

antonmyagkov force-pushed the users/myagkov/csi-fix-stage-publish-steps branch from 83eece4 to d7dafeb Compare October 7, 2024 10:15

tpashkin reviewed Oct 8, 2024

View reviewed changes

cloud/blockstore/tools/csi_driver/internal/driver/node.go Show resolved Hide resolved

antonmyagkov force-pushed the users/myagkov/csi-fix-stage-publish-steps branch from d7dafeb to 5496f67 Compare October 8, 2024 17:18

antonmyagkov added 12 commits October 10, 2024 14:22

Implement stage/unstage/publish/unpublish volumes according csi speci…

3ff8d38

…fication

fix: NodeExpandVolume

3e3148d

fix: publish already staged volumes

81e2d58

fix: skip fsType for bind mount

7ff9703

fix: check that StagingTargetPath is mount point

dc46af7

fix tests

46bd097

fix: check access mode on stage step

1e3091a

fix: change ownership for target path

6183ccc

revert changes for block mode

fa7babf

fix: handle kubelet restart

2545942

refactor: update error message

1902e45

fix test

0e4f424

tpashkin reviewed Oct 11, 2024

View reviewed changes

cloud/blockstore/tools/csi_driver/stage-publish-unpublish-unstage-flow.md Outdated Show resolved Hide resolved

tpashkin reviewed Oct 11, 2024

View reviewed changes

cloud/blockstore/tools/csi_driver/stage-publish-unpublish-unstage-flow.md Outdated Show resolved Hide resolved

fix typos

1d88142

antonmyagkov marked this pull request as ready for review October 11, 2024 15:31

antonmyagkov changed the title ~~Implement stage/unstage/publish/unpublish volume according csi spec~~ Implement stage/unstage/publish/unpublish volume according csi spec for mount mode Oct 11, 2024

tpashkin reviewed Oct 11, 2024

View reviewed changes

cloud/blockstore/tools/csi_driver/internal/driver/node.go Outdated Show resolved Hide resolved

fix: format

e924066

antonmyagkov requested review from tpashkin and aikuchin October 11, 2024 16:15

fix: error message

4f7f5c4

antonmyagkov changed the title ~~Implement stage/unstage/publish/unpublish volume according csi spec for mount mode~~ Implement (un)stage/(un)publish volume according csi spec for mount mode Oct 13, 2024

antonmyagkov mentioned this pull request Oct 14, 2024

StartEndpoint should return error if nbd device filename is empty #2270

Open

add link to block mode pr

20b6040

describe test scenario

520bc78

antonmyagkov force-pushed the users/myagkov/csi-fix-stage-publish-steps branch from 93ca36c to 520bc78 Compare October 15, 2024 10:09

antonmyagkov mentioned this pull request Oct 15, 2024

revert NBSNEBIUS-199 #2278

Open

do not use node id in the socket path

9191b39

tpashkin approved these changes Oct 16, 2024

View reviewed changes

EvgeniyKozev approved these changes Oct 16, 2024

View reviewed changes

antonmyagkov merged commit 080c2ce into main Oct 16, 2024
11 of 12 checks passed

antonmyagkov deleted the users/myagkov/csi-fix-stage-publish-steps branch October 16, 2024 12:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement (un)stage/(un)publish volume according csi spec for mount mode #2195

Implement (un)stage/(un)publish volume according csi spec for mount mode #2195

antonmyagkov commented Oct 1, 2024 •

edited

Loading

github-actions bot commented Oct 1, 2024

github-actions bot commented Oct 2, 2024

github-actions bot commented Oct 2, 2024

github-actions bot commented Oct 3, 2024

github-actions bot commented Oct 4, 2024

aikuchin left a comment

antonmyagkov commented Oct 7, 2024 •

edited

Loading

github-actions bot commented Oct 7, 2024

github-actions bot commented Oct 8, 2024

github-actions bot commented Oct 9, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 13, 2024

github-actions bot commented Oct 14, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 16, 2024

Implement (un)stage/(un)publish volume according csi spec for mount mode #2195

Implement (un)stage/(un)publish volume according csi spec for mount mode #2195

Conversation

antonmyagkov commented Oct 1, 2024 • edited Loading

Problem:

Possible scenarios:

External links/documentation

github-actions bot commented Oct 1, 2024

github-actions bot commented Oct 2, 2024

github-actions bot commented Oct 2, 2024

github-actions bot commented Oct 3, 2024

github-actions bot commented Oct 4, 2024

aikuchin left a comment

Choose a reason for hiding this comment

antonmyagkov commented Oct 7, 2024 • edited Loading

github-actions bot commented Oct 7, 2024

github-actions bot commented Oct 8, 2024

github-actions bot commented Oct 9, 2024

github-actions bot commented Oct 10, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 11, 2024

github-actions bot commented Oct 13, 2024

github-actions bot commented Oct 14, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 15, 2024

github-actions bot commented Oct 16, 2024

antonmyagkov commented Oct 1, 2024 •

edited

Loading

antonmyagkov commented Oct 7, 2024 •

edited

Loading