Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

plugin_utils.go:GetVolumeInfo() must compare with fully qualified volume name in refcount map #1511

Closed
govint opened this issue Jun 29, 2017 · 1 comment

Comments

@govint
Copy link
Contributor

govint commented Jun 29, 2017

#1500 (comment) has a description of the issue seen in a CI failure when testing new restart tests in #1500.

The function returns the full name of a volume after checking if its already full, else the volume meta-data is fetched from ESX and the full name is constructed.

The function also checks if there is already a volume of the same name in the refmap from the refcounter.

The proposed fix is to always compare the fully name of a volume (volume@datastore) with the names in the refmap got from refcount module. Comparing only the volume names doesn't help when there are volumes of the same name in different datastores (as tested in the restart tests). A volume already in the refmap (same name, different datastore) gets double counted and prevents the plugin from unmounting it ever.

Changes are like this,

-// JoinVolName - return a full name in format volume@datastore
-func JoinVolName(volName string, datastoreName string) string {
+// makeFullVolName - return a full name in format volume@datastore
+func makeFullVolName(volName string, datastoreName string) string {
        return strings.Join([]string{volName, datastoreName}, "@")
 }

@@ -107,7 +107,6 @@ func GetNameFromRefmap(volName string, d drivers.VolumeDriver) string {
        fullname := ""

        for _, name := range volumeNameList {
-               refVolName := SplitVolName(name)[0]
                if refVolName != volName {
                        continue
                }
@@ -132,12 +131,7 @@ func GetVolumeInfo(name string, datastoreName string, d drivers.VolumeDriver) (*

        // if datastore name is provided, append and return
        if datastoreName != "" {
-               return &VolumeInfo{JoinVolName(name, datastoreName), datastoreName, nil}, nil
-       }
-
-       // find full volume names using refmap if possible
-       if fullVolumeName := GetNameFromRefmap(name, d); fullVolumeName != "" {
-               return &VolumeInfo{fullVolumeName, "", nil}, nil
+               return &VolumeInfo{makeFullVolName(name, datastoreName), datastoreName, nil}, nil
        }

        // Do a get trip to esx and construct full name
@@ -147,5 +141,13 @@ func GetVolumeInfo(name string, datastoreName string, d drivers.VolumeDriver) (*
                return nil, err
        }
        datastoreName = volumeMeta[datastoreKey].(string)
-       return &VolumeInfo{JoinVolName(name, datastoreName), datastoreName, volumeMeta}, nil
+
+       volName := makeFullVolName(name, datastoreName)
+
+       // find full volume names using refmap if possible
+       if fullVolumeName := GetNameFromRefmap(volName, d); fullVolumeName != "" {
+               return &VolumeInfo{fullVolumeName, "", nil}, nil
+       }
+
+       return &VolumeInfo{volName, datastoreName, volumeMeta}, nil
 }
@govint
Copy link
Contributor Author

govint commented Jun 30, 2017

Fixes #1512

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

3 participants