Fix to match full volume names in refcount map.

[govint]

The changes are added to the changes submitted for #1500 and being tested there for the docker restart tests.

[govint]

CI failed with govc error,

2017/06/29 11:19:30 Adding VM [photon.vmfs] to a vmgroup [vmgroup_test1] on esx [192.168.31.62]
2017/06/29 11:19:32 Adding VM [govc: no such VM] to a vmgroup [vmgroup_test1] on esx [192.168.31.62]

---

FAIL: vmgroups_test.go:53: VmGroupTest.SetUpSuite

vmgroups_test.go:79:
c.Assert(err, IsNil, Commentf(out))
... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc42032d180), Stderr:[]uint8(nil)} ("exit status 2")
... usage: vmdkops_admin.py [-h] {status,policy,config,volume,vmgroup} ...
vmdkops_admin.py: error: unrecognized arguments: no such VM

2017/06/29 11:19:34 Removing VM [photon.vmfs] from a vmgroup [vmgroup_test1] on esx [192.168.31.62]
2017/06/29 11:19:37 Removing VM [govc: no such VM] from a vmgroup [vmgroup_test1] on esx [192.168.31.62]
2017/06/29 11:19:39 Removing VM [photon.vmfs] from a vmgroup [vmgroup_test2] on esx [192.168.31.62]
2017/06/29 11:19:41 Removing VM [govc: no such VM] from a vmgroup [vmgroup_test2] on esx [192.168.31.62]
2017/06/29 11:19:43 Deleting a vmgroup [vmgroup_test1] on esx [192.168.31.62]
2017/06/29 11:19:46 Deleting a vmgroup [vmgroup_test2] on esx [192.168.31.62]

[pshahzeb]

Thanks for fixing this Govindan.

This fix will work. However, this logic can be optimized a bit.

The idea of introducing GetNameFromRefmap was to avoid a trip to ESX vmdkops service to get volume meta. Hence the call to it was placed before actually doing a GetVolume ( L139 in the earlier change)

With this bug, looks like this optimization is not valid in all cases.

With this change, since we first endup doing a GetVolume, we are bound to construct a full volume name after that. We can just call makeFullVolName and return the VolumeInfo

Call to GetNameFromRefmap won't get us anything useful after that. Infact this entire thing can be cleaned up. I am fine if you think this should be cleaned up in a separate PR.

[govint]

I was wondering if the GetNameFromRefmap() was also for checking duplicates. If that's not a needed check then I'll remove that all together. And the SplitVolume() function too. This fix needs to go in before #1500.

[pshahzeb]

Yes. You can remove. The fix itself looks good.

[pshahzeb]

LGTM

[shaominchen]

LGTM.

[pshahzeb]

The build failed is likely due to the issue fixed by #1519. Let's rebase and get CI pass and we are good to merge.

[msterin]

@govint - please add a few words to description directly explaining what is the problem and what is the fix. As of now, there is a reference to another (long and not super clear) issue so I got lost really fast.

[govint]

@msterin, sure, below is the issue:

The issue is how refcounting handles volumes with the same name on different datastores. When the plugin restarts and begins refcount'ing volumes it does this:

1. Get the list of containers (paused/running)
2. For each container, go thru its mounts and update the refmap.
3. For (2), plugin_utils:GetVolumeInfo() is called to get the full name of the volume, GetVolumeInfo() actually checks back in the refcnt:refmap to see if the volume is already in it and returns the name from the refmap (which keep a map of volume@datastore names to refcounts). Otherwise a Get() is done to the ESX service and the full volume name is created and returned.
4. Now, GetVolumeInfo() checks only the volume name and the first match is returned to its caller and that volume's refcount is incremented.
5. We have two distinct volumes (same name different DS), but GetVolumeInfo() doesn't identify those as separate volumes (as only volume name is compared).
6. Refcount ends up double counting one of the volumes and keeps that mounted and unmounts the other, while the containers using both are still alive.

[govint]

Make of plugin failed,

mkdir -p /tmp/plugin/rootfs
docker export tempContainer | tar -x -C /tmp/plugin/rootfs
sh: 0: getcwd() failed: No such file or directory
cp config.json docker-volume-vsphere /tmp/plugin
cp: cannot stat 'config.json': No such file or directory
cp: cannot stat 'docker-volume-vsphere': No such file or directory
make: *** [plugin] Error 1
Makefile:95: recipe for target 'plugin' failed
scp: /tmp/docker-volume-vsphere/build: No such file or directory
sh: 0: Can't open /tmp/docker-volume-vsphere/build/plugin_sanity_test.sh
bash: line 0: cd: /tmp/docker-volume-vsphere/plugin: No such file or directory
make: *** No rule to make target 'push'. Stop.