From a5c4f29a54a31da288fa4b2607a75795c9a7d0ec Mon Sep 17 00:00:00 2001 From: Jonathan Basseri Date: Tue, 21 Apr 2020 17:15:20 -0700 Subject: [PATCH] Fix race in vsphere cloud provider When a go loop launches goroutines, it is a common mistake to assume that the routines can reference the loop value from their specific loop iteration. Actually, all goroutines share the same reference to the loop veriable, and they typically all see the last value. To correct this, the loop variable must be captured as a parameter to the anonymous func. This fix might eliminate the need for the retry logic below, but I do not have enough information to back that up. Signed-off-by: Jonathan Basseri --- .../src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go index 96ada113059..9caf1d813c3 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go +++ b/staging/src/k8s.io/legacy-cloud-providers/vsphere/vsphere.go @@ -1071,11 +1071,11 @@ func (vs *VSphere) DisksAreAttached(nodeVolumes map[k8stypes.NodeName][]string) dcNodes[VC_DC] = append(dcNodes[VC_DC], nodeName) } - for _, nodes := range dcNodes { + for _, nodeNames := range dcNodes { localAttachedMap := make(map[string]map[string]bool) localAttachedMaps = append(localAttachedMaps, localAttachedMap) // Start go routines per VC-DC to check disks are attached - go func() { + go func(nodes []k8stypes.NodeName) { nodesToRetryLocal, err := vs.checkDiskAttached(ctx, nodes, nodeVolumes, localAttachedMap, retry) if err != nil { if !vclib.IsManagedObjectNotFoundError(err) { @@ -1089,7 +1089,7 @@ func (vs *VSphere) DisksAreAttached(nodeVolumes map[k8stypes.NodeName][]string) nodesToRetry = append(nodesToRetry, nodesToRetryLocal...) nodesToRetryMutex.Unlock() wg.Done() - }() + }(nodeNames) wg.Add(1) } wg.Wait()