Serialize attach/detach ops for the same vm

[brunotm]

This is a simple change that fixes concurrency on the same docker host. It adds a per vm lock on attach/detach ops.

Fixes #809

//CC @msterin

before:

=== RUN   TestSanity
Running tests on  tcp://vsc02:2375 (may take a while)...
Running create/delete parallel tests on tcp://vsc02:2375 and tcp://vsc02:2375 (may take a while)...
--- FAIL: TestSanity (3.66s)
	sanity_test.go:190: Successfully connected to tcp://vsc02:2375
	sanity_test.go:190: Successfully connected to tcp://vsc02:2375
	sanity_test.go:196: Creating vol=DefaultTestVol on client tcp://vsc02:2375.
	sanity_test.go:90: Running cmd=&[touch /mnt/testvol/DefaultTestVol/file_to_touch] with vol=DefaultTestVol on client tcp://vsc02:2375
	sanity_test.go:90: Running cmd=&[stat /mnt/testvol/DefaultTestVol/file_to_touch] with vol=DefaultTestVol on client tcp://vsc02:2375
	sanity_test.go:262: Parallel test failed, err: Error response from daemon: create volTestP10: VolumeDriver.Create: Another task is already in progress.
FAIL
Makefile:319: recipe for target 'test-vm' failed
make[1]: *** [test-vm] Error 1

after:

=== RUN   TestSanity
Running tests on  tcp://vsc02:2375 (may take a while)...
Running create/delete parallel tests on tcp://vsc02:2375 and tcp://vsc02:2375 (may take a while)...
Running clone parallel tests (may take a while)...
--- PASS: TestSanity (17.83s)
	sanity_test.go:190: Successfully connected to tcp://vsc02:2375
	sanity_test.go:190: Successfully connected to tcp://vsc02:2375
	sanity_test.go:196: Creating vol=DefaultTestVol on client tcp://vsc02:2375.
	sanity_test.go:90: Running cmd=&[touch /mnt/testvol/DefaultTestVol/file_to_touch] with vol=DefaultTestVol on client tcp://vsc02:2375
	sanity_test.go:90: Running cmd=&[stat /mnt/testvol/DefaultTestVol/file_to_touch] with vol=DefaultTestVol on client tcp://vsc02:2375
PASS
make[1]: Leaving directory '/usr/share/go-1.6/src/github.com/vmware/docker-volume-vsphere/vmdk_plugin'

[govint]

The client plugin also has a lock (per docker-host), do we need one more lock in the ESX service? The disk level locks are good?

[brunotm]

Disk level locks are good. :)
The error comes from already having a reconfig task running for the same VM. (different volumes)
I didn't investigated why the mutex in the plugin driver is not protecting this situation, but serialization for attach/detach is really needed on the esx part. I might be missing something, but in the plugin we need to protect the refcount part only?

[govint]

The plugin holds the lock for the entire duration of the attach/detach. Two reconfgs can't happen for the same VM in parallel but the host may sequence those I guess.

…

On Thu, Dec 8, 2016 at 5:58 PM, Bruno Moura ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In esx_service/vmdk_ops.py <#808>: > elif cmd == "detach": - response = detachVMDK(vmdk_path, vm_uuid) + with lockManager.get_lock(vm_uuid): + response = detachVMDK(vmdk_path, vm_uuid) Disk level locks are good. :) The error comes from already having a reconfig task running for the same VM. (different volumes) I didn't investigated why the mutex in the plugin driver is not protecting this situation, but serialization for attach/detach is really needed on the esx part. I might be missing something, but in the plugin we need to protect the refcount part only? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#808>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APHseEUp4-DZ0vgXwJP0TydXnznpi2eaks5rF_f7gaJpZM4LHvwr> .

[msterin]

@govint writes

...Two reconfgs can't happen for the same VM in parallel but the host may sequence those I guess.

They can if there are 2 docker requests for different volumes issued in parallel on the same VM - which the test output in the description of this PR demonstrates