Merge pull request #120101 from lowang-bh/Hotfix

fix: concurrent map writes in e2e test

test/e2e/storage/vsphere/bootstrap.go

@@ -51,7 +51,7 @@ func bootstrapOnce(f *framework.Framework) {
 	if err != nil {
 		framework.Failf("Failed to get nodes: %v", err)
 	}
-	TestContext = Context{NodeMapper: &NodeMapper{}, VSphereInstances: vsphereInstances}
+	TestContext = Context{NodeMapper: NewNodeMapper(), VSphereInstances: vsphereInstances}
 	// 3. Get Node to VSphere mapping
 	err = TestContext.NodeMapper.GenerateNodeMap(vsphereInstances, *nodeList)
 	if err != nil {

test/e2e/storage/vsphere/nodemapper.go

@@ -36,6 +36,9 @@ import (
 
 // NodeMapper contains information to generate nameToNodeInfo and vcToZoneDatastore maps
 type NodeMapper struct {
+	nodeInfoRWLock        *sync.RWMutex
+	nameToNodeInfo        map[string]*NodeInfo
+	vcToZoneDatastoresMap map[string](map[string][]string)
 }
 
 // NodeInfo contains information about vcenter nodes
@@ -54,10 +57,14 @@ const (
 	hostSystemType             = "HostSystem"
 )
 
-var (
+// NewNodeMapper returns a new NodeMapper
-	nameToNodeInfo        = make(map[string]*NodeInfo)
+func NewNodeMapper() *NodeMapper {
-	vcToZoneDatastoresMap = make(map[string](map[string][]string))
+	return &NodeMapper{
-)
+		nodeInfoRWLock:        &sync.RWMutex{},
+		nameToNodeInfo:        make(map[string]*NodeInfo),
+		vcToZoneDatastoresMap: make(map[string](map[string][]string)),
+	}
+}
 
 // GenerateNodeMap populates node name to node info map
 func (nm *NodeMapper) GenerateNodeMap(vSphereInstances map[string]*VSphere, nodeList v1.NodeList) error {
@@ -108,6 +115,7 @@ func (nm *NodeMapper) GenerateNodeMap(vSphereInstances map[string]*VSphere, node
 
 	for _, node := range nodeList.Items {
 		n := node
+		wg.Add(1)
 		go func() {
 			nodeUUID := getUUIDFromProviderID(n.Spec.ProviderID)
 			framework.Logf("Searching for node with UUID: %s", nodeUUID)
@@ -132,11 +140,10 @@ func (nm *NodeMapper) GenerateNodeMap(vSphereInstances map[string]*VSphere, node
 			}
 			wg.Done()
 		}()
-		wg.Add(1)
 	}
 	wg.Wait()
 
-	if len(nameToNodeInfo) != len(nodeList.Items) {
+	if len(nm.nameToNodeInfo) != len(nodeList.Items) {
 		return errors.New("all nodes not mapped to respective vSphere")
 	}
 	return nil
@@ -220,7 +227,7 @@ func (nm *NodeMapper) GenerateZoneToDatastoreMap() error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	// 3. Populate vcToZoneHostsMap and vcToHostDatastoresMap
-	for _, nodeInfo := range nameToNodeInfo {
+	for _, nodeInfo := range nm.nameToNodeInfo {
 		vc := nodeInfo.VSphere.Config.Hostname
 		host := nodeInfo.HostSystemRef.Value
 		for _, zone := range nodeInfo.Zones {
@@ -247,13 +254,13 @@ func (nm *NodeMapper) GenerateZoneToDatastoreMap() error {
 	for vc, zoneToHostsMap := range vcToZoneHostsMap {
 		for zone, hosts := range zoneToHostsMap {
 			commonDatastores := retrieveCommonDatastoresAmongHosts(hosts, vcToHostDatastoresMap[vc])
-			if vcToZoneDatastoresMap[vc] == nil {
+			if nm.vcToZoneDatastoresMap[vc] == nil {
-				vcToZoneDatastoresMap[vc] = make(map[string][]string)
+				nm.vcToZoneDatastoresMap[vc] = make(map[string][]string)
 			}
-			vcToZoneDatastoresMap[vc][zone] = commonDatastores
+			nm.vcToZoneDatastoresMap[vc][zone] = commonDatastores
 		}
 	}
-	framework.Logf("Zone to datastores map : %+v", vcToZoneDatastoresMap)
+	framework.Logf("Zone to datastores map : %+v", nm.vcToZoneDatastoresMap)
 	return nil
 }
 
@@ -277,15 +284,21 @@ func retrieveCommonDatastoresAmongHosts(hosts []string, hostToDatastoresMap map[
 
 // GetDatastoresInZone returns all the datastores in the specified zone
 func (nm *NodeMapper) GetDatastoresInZone(vc string, zone string) []string {
-	return vcToZoneDatastoresMap[vc][zone]
+	nm.nodeInfoRWLock.RLock()
+	defer nm.nodeInfoRWLock.RUnlock()
+	return nm.vcToZoneDatastoresMap[vc][zone]
 }
 
 // GetNodeInfo returns NodeInfo for given nodeName
 func (nm *NodeMapper) GetNodeInfo(nodeName string) *NodeInfo {
-	return nameToNodeInfo[nodeName]
+	nm.nodeInfoRWLock.RLock()
+	defer nm.nodeInfoRWLock.RUnlock()
+	return nm.nameToNodeInfo[nodeName]
 }
 
 // SetNodeInfo sets NodeInfo for given nodeName. This function is not thread safe. Users need to handle concurrency.
 func (nm *NodeMapper) SetNodeInfo(nodeName string, nodeInfo *NodeInfo) {
-	nameToNodeInfo[nodeName] = nodeInfo
+	nm.nodeInfoRWLock.Lock()
+	defer nm.nodeInfoRWLock.Unlock()
+	nm.nameToNodeInfo[nodeName] = nodeInfo
 }