Repository navigation
vcsim: fix reattach in CNS - #4141
Conversation
🛑 Legal Compliance Check FailedHi @elesueur, thank you for your contribution! To merge this Pull Request, you must sign our DCO. Note: Even if you signed off your commits locally (using 1. Read the Document: Click here to read the DCO ⏳ Processing Schedule: |
|
I have read the DCO Document and I hereby sign the DCO for this and all future contributions. |
9963edf to
12f3269
Compare
|
|
||
| // mu guards attachments. CnsAttachVolume/CnsDetachVolume task bodies run | ||
| // in their own goroutine (see simulator.Task.Run), so concurrent attach | ||
| // calls for different volumes targeting the same VM can race on this map. |
There was a problem hiding this comment.
Did you experience such a race? vcsim has built-in locking that should cover this, including Task.Run:
Line 114 in e53aa9f
Another recent PR originally added locking that wasn't needed: #4109 (comment)
There was a problem hiding this comment.
Hi Doug, yea, you're right - it's not needed. I pushed an update.
When a CNS volume is already attached to a VM, a request to attach it to the same VM fails with ResourceInUse. In a real VC, this is allowed to succeed as an idempotent operation. The ResourceInUse failure is triggered when CSI driver in vanilla k8s cluster restarts after disks are already attached and subsequent attachments continue to fail forever. Signed-off-by: Etienne Le Sueur <etienne.le-sueur@broadcom.com>
12f3269 to
b84acab
Compare
2 participants
Description
When a CNS volume is already attached to a VM, a request to attach it to the same VM fails with ResourceInUse.
In a real VC, this is allowed to succeed as an idempotent operation.
The ResourceInUse failure is triggered when CSI driver in vanilla k8s cluster restarts after disks are already attached and subsequent attachments continue to fail forever.
How Has This Been Tested?
Manual unit tests and testing when vcsim is integrated into our own shim for testing k8s native vsphere CSI driver.